From: Ashish Mittal <Ashish.Mittal@veritas.com>
Alter qemu command line generation in order to possibly add TLS for
a suitably configured domain.
Sample TLS args generated by libvirt -
-object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/qemu,\
endpoint=client,verify-peer=yes \
-drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\
file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
file.server.type=tcp,file.server.host=192.168.0.1,\
file.server.port=9999,format=raw,if=none,\
id=drive-virtio-disk0,cache=none \
-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
id=virtio-disk0
Update the qemuxml2argvtest with a couple of examples. One for a
simple case and the other a bit more complex where multiple VxHS disks
are added where at least one uses a VxHS that doesn't require TLS
credentials and thus sets the domain disk source attribute "tls = 'no'".
Update the hotplug to be able to handle processing the tlsAlias whether
it's to add the TLS object when hotplugging a disk or to remove the TLS
object when hot unplugging a disk. The hot plug/unplug code is largely
generic, but the addition code does make the VXHS specific checks only
because it needs to grab the correct config directory and generate the
object as the command line would do.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
src/qemu/qemu_block.c | 8 +++
src/qemu/qemu_command.c | 29 +++++++++
src/qemu/qemu_hotplug.c | 73 ++++++++++++++++++++++
...-disk-drive-network-tlsx509-multidisk-vxhs.args | 43 +++++++++++++
...v-disk-drive-network-tlsx509-multidisk-vxhs.xml | 50 +++++++++++++++
...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 30 +++++++++
tests/qemuxml2argvtest.c | 7 +++
7 files changed, 240 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index ca6e213b4..458b90d8e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -529,16 +529,24 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
return NULL;
}
+ if (src->haveTLS == VIR_TRISTATE_BOOL_YES && !src->tlsAlias) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("VxHS disk does not have TLS alias set"));
+ return NULL;
+ }
+
if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, true)))
return NULL;
/* VxHS disk specification example:
* { driver:"vxhs",
+ * [tls-creds:"objvirtio-disk0_tls0",]
* vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
* server:[{type:"tcp", host:"1.2.3.4", port:9999}]}
*/
if (virJSONValueObjectCreate(&ret,
"s:driver", protocol,
+ "S:tls-creds", src->tlsAlias,
"s:vdisk-id", src->path,
"a:server", server, NULL) < 0)
virJSONValueFree(server);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0a3278510..7b98e1947 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -794,6 +794,32 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
}
+/* qemuBuildDiskTLSx509CommandLine:
+ *
+ * Add TLS object if the disk uses a secure communication channel
+ *
+ * Returns 0 on success, -1 w/ error on some sort of failure.
+ */
+static int
+qemuBuildDiskTLSx509CommandLine(virCommandPtr cmd,
+ virQEMUDriverConfigPtr cfg,
+ virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps)
+{
+ virStorageSourcePtr src = disk->src;
+
+ /* other protocols may be added later */
+ if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
+ disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
+ return qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
+ false, true, false,
+ disk->info.alias, qemuCaps);
+ }
+
+ return 0;
+}
+
+
static char *
qemuBuildNetworkDriveURI(virStorageSourcePtr src,
qemuDomainSecretInfoPtr secinfo)
@@ -2221,6 +2247,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0)
return -1;
+ if (qemuBuildDiskTLSx509CommandLine(cmd, cfg, disk, qemuCaps) < 0)
+ return -1;
+
virCommandAddArg(cmd, "-drive");
if (!(optstr = qemuBuildDriveStr(disk, cfg, driveBoot, qemuCaps)))
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b365078ec..e4174af35 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -152,6 +152,55 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
static int
+qemuDomainAddDiskTLSObject(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ char **tlsAlias)
+{
+ int ret = -1;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virStorageSourcePtr src = disk->src;
+ virJSONValuePtr tlsProps = NULL;
+
+ /* NB: This may alter haveTLS based on cfg */
+ qemuDomainPrepareDiskSourceTLS(src, disk->info.alias, cfg);
+
+ if (src->haveTLS != VIR_TRISTATE_BOOL_YES) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ /* Initial implementation doesn't require/use a secret to decrypt
+ * a server certificate, so there's no need to manage a tlsSecAlias
+ * and tlsSecProps. See qemuDomainAddChardevTLSObjects for the
+ * methodology required to add a secret object. */
+
+ /* For a VxHS environment, create a TLS object for the client to
+ * connect to the VxHS server. */
+ if (src->type == VIR_STORAGE_TYPE_NETWORK &&
+ src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
+ qemuDomainGetTLSObjects(priv->qemuCaps, NULL,
+ cfg->vxhsTLSx509certdir, false, true,
+ disk->info.alias, &tlsProps, tlsAlias,
+ NULL, NULL) < 0)
+ goto cleanup;
+
+ if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
+ NULL, NULL, *tlsAlias, &tlsProps) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virJSONValueFree(tlsProps);
+ virObjectUnref(cfg);
+
+ return ret;
+}
+
+
+static int
qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
@@ -315,6 +364,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
char *devstr = NULL;
char *drivestr = NULL;
char *drivealias = NULL;
+ char *tlsAlias = NULL;
bool releaseaddr = false;
bool driveAdded = false;
bool secobjAdded = false;
@@ -372,6 +422,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0)
goto error;
+ if (qemuDomainAddDiskTLSObject(driver, vm, disk, &tlsAlias) < 0)
+ goto error;
+
if (!(drivestr = qemuBuildDriveStr(disk, cfg, false, priv->qemuCaps)))
goto error;
@@ -422,6 +475,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
ret = 0;
cleanup:
+ VIR_FREE(tlsAlias);
virJSONValueFree(secobjProps);
virJSONValueFree(encobjProps);
qemuDomainSecretDiskDestroy(disk);
@@ -453,6 +507,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
error:
+ qemuDomainDelTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE, NULL, tlsAlias);
+
if (releaseaddr)
qemuDomainReleaseDeviceAddress(vm, &disk->info, src);
@@ -611,6 +667,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
virErrorPtr orig_err;
char *drivestr = NULL;
char *devstr = NULL;
+ char *tlsAlias = NULL;
bool driveAdded = false;
bool encobjAdded = false;
bool secobjAdded = false;
@@ -667,6 +724,9 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
goto error;
+ if (qemuDomainAddDiskTLSObject(driver, vm, disk, &tlsAlias) < 0)
+ goto error;
+
if (!(drivestr = qemuBuildDriveStr(disk, cfg, false, priv->qemuCaps)))
goto error;
@@ -712,6 +772,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
ret = 0;
cleanup:
+ VIR_FREE(tlsAlias);
virJSONValueFree(secobjProps);
virJSONValueFree(encobjProps);
qemuDomainSecretDiskDestroy(disk);
@@ -740,6 +801,8 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
error:
+ qemuDomainDelTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE, NULL, tlsAlias);
+
ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
goto cleanup;
}
@@ -756,6 +819,7 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
char *drivealias = NULL;
char *drivestr = NULL;
char *devstr = NULL;
+ char *tlsAlias = NULL;
bool driveAdded = false;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *src = virDomainDiskGetSource(disk);
@@ -780,6 +844,9 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
+ if (qemuDomainAddDiskTLSObject(driver, vm, disk, &tlsAlias) < 0)
+ goto error;
+
if (!(drivestr = qemuBuildDriveStr(disk, cfg, false, priv->qemuCaps)))
goto error;
@@ -810,6 +877,7 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
ret = 0;
cleanup:
+ VIR_FREE(tlsAlias);
if (ret < 0 && releaseaddr)
virDomainUSBAddressRelease(priv->usbaddrs, &disk->info);
VIR_FREE(devstr);
@@ -833,6 +901,8 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
error:
+ qemuDomainDelTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE, NULL, tlsAlias);
+
ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
goto cleanup;
}
@@ -3710,6 +3780,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
ignore_value(qemuMonitorDelObject(priv->mon, encAlias));
VIR_FREE(encAlias);
+ if (disk->src->tlsAlias)
+ ignore_value(qemuMonitorDelObject(priv->mon, disk->src->tlsAlias));
+
if (qemuDomainObjExitMonitor(driver, vm) < 0)
return -1;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
new file mode 100644
index 000000000..572c9f36c
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
@@ -0,0 +1,43 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-cpu qemu32 \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/qemu,\
+endpoint=client,verify-peer=yes \
+-drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\
+file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,file.server.type=tcp,\
+file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,\
+id=drive-virtio-disk0,cache=none \
+-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
+id=virtio-disk0 \
+-object tls-creds-x509,id=objvirtio-disk1_tls0,dir=/etc/pki/qemu,\
+endpoint=client,verify-peer=yes \
+-drive file.driver=vxhs,file.tls-creds=objvirtio-disk1_tls0,\
+file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,file.server.type=tcp,\
+file.server.host=192.168.0.2,file.server.port=9999,format=raw,if=none,\
+id=drive-virtio-disk1,cache=none \
+-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk1,\
+id=virtio-disk1 \
+-drive file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc253,\
+file.server.type=tcp,file.server.host=192.168.0.3,file.server.port=9999,\
+format=raw,if=none,id=drive-virtio-disk2,cache=none \
+-device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk2,\
+id=virtio-disk2
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
new file mode 100644
index 000000000..a66e81f06
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
@@ -0,0 +1,50 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <disk type='network' device='disk'>
+ <driver name='qemu' type='raw' cache='none'/>
+ <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251'>
+ <host name='192.168.0.1' port='9999'/>
+ </source>
+ <target dev='vda' bus='virtio'/>
+ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
+ </disk>
+ <disk type='network' device='disk'>
+ <driver name='qemu' type='raw' cache='none'/>
+ <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc252'>
+ <host name='192.168.0.2' port='9999'/>
+ </source>
+ <target dev='vdb' bus='virtio'/>
+ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
+ </disk>
+ <disk type='network' device='disk'>
+ <driver name='qemu' type='raw' cache='none'/>
+ <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc253' tls='no'>
+ <host name='192.168.0.3' port='9999'/>
+ </source>
+ <target dev='vdc' bus='virtio'/>
+ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
new file mode 100644
index 000000000..aaf88635b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
@@ -0,0 +1,30 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-cpu qemu32 \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/qemu,\
+endpoint=client,verify-peer=yes \
+-drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\
+file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,file.server.type=tcp,\
+file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,\
+id=drive-virtio-disk0,cache=none \
+-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
+id=virtio-disk0
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 01a518eff..5cdc1a726 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -932,6 +932,13 @@ mymain(void)
DO_TEST("disk-drive-network-rbd-ipv6", NONE);
DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE);
DO_TEST("disk-drive-network-vxhs", QEMU_CAPS_VXHS);
+ driver.config->vxhsTLS = 1;
+ DO_TEST("disk-drive-network-tlsx509-vxhs", QEMU_CAPS_VXHS,
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+ DO_TEST("disk-drive-network-tlsx509-multidisk-vxhs", QEMU_CAPS_VXHS,
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+ driver.config->vxhsTLS = 0;
+ VIR_FREE(driver.config->vxhsTLSx509certdir);
DO_TEST("disk-drive-no-boot",
QEMU_CAPS_BOOTINDEX);
DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid",
--
2.13.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Sep 14, 2017 at 08:51:56 -0400, John Ferlan wrote:
> From: Ashish Mittal <Ashish.Mittal@veritas.com>
>
> Alter qemu command line generation in order to possibly add TLS for
> a suitably configured domain.
>
> Sample TLS args generated by libvirt -
>
> -object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/qemu,\
> endpoint=client,verify-peer=yes \
> -drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\
> file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
> file.server.type=tcp,file.server.host=192.168.0.1,\
> file.server.port=9999,format=raw,if=none,\
> id=drive-virtio-disk0,cache=none \
> -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
> id=virtio-disk0
>
> Update the qemuxml2argvtest with a couple of examples. One for a
> simple case and the other a bit more complex where multiple VxHS disks
> are added where at least one uses a VxHS that doesn't require TLS
> credentials and thus sets the domain disk source attribute "tls = 'no'".
>
> Update the hotplug to be able to handle processing the tlsAlias whether
> it's to add the TLS object when hotplugging a disk or to remove the TLS
> object when hot unplugging a disk. The hot plug/unplug code is largely
> generic, but the addition code does make the VXHS specific checks only
> because it needs to grab the correct config directory and generate the
> object as the command line would do.
>
> Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> src/qemu/qemu_block.c | 8 +++
> src/qemu/qemu_command.c | 29 +++++++++
> src/qemu/qemu_hotplug.c | 73 ++++++++++++++++++++++
> ...-disk-drive-network-tlsx509-multidisk-vxhs.args | 43 +++++++++++++
> ...v-disk-drive-network-tlsx509-multidisk-vxhs.xml | 50 +++++++++++++++
> ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 30 +++++++++
> tests/qemuxml2argvtest.c | 7 +++
> 7 files changed, 240 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
>
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index ca6e213b4..458b90d8e 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -529,16 +529,24 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
> return NULL;
> }
>
> + if (src->haveTLS == VIR_TRISTATE_BOOL_YES && !src->tlsAlias) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("VxHS disk does not have TLS alias set"));
> + return NULL;
> + }
> +
> if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, true)))
> return NULL;
>
> /* VxHS disk specification example:
> * { driver:"vxhs",
> + * [tls-creds:"objvirtio-disk0_tls0",]
Be careful with square brackets in JSON, they denote an array.
> * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
> * server:[{type:"tcp", host:"1.2.3.4", port:9999}]}
> */
> if (virJSONValueObjectCreate(&ret,
> "s:driver", protocol,
> + "S:tls-creds", src->tlsAlias,
> "s:vdisk-id", src->path,
> "a:server", server, NULL) < 0)
> virJSONValueFree(server);
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0a3278510..7b98e1947 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -794,6 +794,32 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
> }
>
>
> +/* qemuBuildDiskTLSx509CommandLine:
> + *
> + * Add TLS object if the disk uses a secure communication channel
> + *
> + * Returns 0 on success, -1 w/ error on some sort of failure.
> + */
> +static int
> +qemuBuildDiskTLSx509CommandLine(virCommandPtr cmd,
> + virQEMUDriverConfigPtr cfg,
> + virDomainDiskDefPtr disk,
> + virQEMUCapsPtr qemuCaps)
> +{
> + virStorageSourcePtr src = disk->src;
> +
> + /* other protocols may be added later */
> + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
> + disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
> + return qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
> + false, true, false,
> + disk->info.alias, qemuCaps);
> + }
> +
> + return 0;
> +}
> +
> +
> static char *
> qemuBuildNetworkDriveURI(virStorageSourcePtr src,
> qemuDomainSecretInfoPtr secinfo)
[...]
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index b365078ec..e4174af35 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -152,6 +152,55 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
>
>
> static int
> +qemuDomainAddDiskTLSObject(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainDiskDefPtr disk,
> + char **tlsAlias)
I don't particularly like this. You set it into the disk object, so
there should be a function which rolls this back for a disk (or
preferably a virStorageSource).
> +{
> + int ret = -1;
> + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + virStorageSourcePtr src = disk->src;
> + virJSONValuePtr tlsProps = NULL;
> +
> + /* NB: This may alter haveTLS based on cfg */
> + qemuDomainPrepareDiskSourceTLS(src, disk->info.alias, cfg);
This should be executed in the hotplug API function and not in a helper.
> +
> + if (src->haveTLS != VIR_TRISTATE_BOOL_YES) {
> + ret = 0;
> + goto cleanup;
> + }
> +
> + /* Initial implementation doesn't require/use a secret to decrypt
> + * a server certificate, so there's no need to manage a tlsSecAlias
> + * and tlsSecProps. See qemuDomainAddChardevTLSObjects for the
> + * methodology required to add a secret object. */
> +
> + /* For a VxHS environment, create a TLS object for the client to
> + * connect to the VxHS server. */
> + if (src->type == VIR_STORAGE_TYPE_NETWORK &&
> + src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
> + qemuDomainGetTLSObjects(priv->qemuCaps, NULL,
> + cfg->vxhsTLSx509certdir, false, true,
> + disk->info.alias, &tlsProps, tlsAlias,
> + NULL, NULL) < 0)
This looks like another place that extracts qemu-specific stuff, which
need to be duplicated for blockdev add. Can't we make the cert dir part
of virStorageSource?
> + goto cleanup;
> +
> + if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
> + NULL, NULL, *tlsAlias, &tlsProps) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + virJSONValueFree(tlsProps);
> + virObjectUnref(cfg);
> +
> + return ret;
> +}
> +
> +
> +static int
> qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainDiskDefPtr disk,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/19/2017 10:02 AM, Peter Krempa wrote:
> On Thu, Sep 14, 2017 at 08:51:56 -0400, John Ferlan wrote:
>> From: Ashish Mittal <Ashish.Mittal@veritas.com>
>>
>> Alter qemu command line generation in order to possibly add TLS for
>> a suitably configured domain.
>>
>> Sample TLS args generated by libvirt -
>>
>> -object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/qemu,\
>> endpoint=client,verify-peer=yes \
>> -drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\
>> file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
>> file.server.type=tcp,file.server.host=192.168.0.1,\
>> file.server.port=9999,format=raw,if=none,\
>> id=drive-virtio-disk0,cache=none \
>> -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
>> id=virtio-disk0
>>
>> Update the qemuxml2argvtest with a couple of examples. One for a
>> simple case and the other a bit more complex where multiple VxHS disks
>> are added where at least one uses a VxHS that doesn't require TLS
>> credentials and thus sets the domain disk source attribute "tls = 'no'".
>>
>> Update the hotplug to be able to handle processing the tlsAlias whether
>> it's to add the TLS object when hotplugging a disk or to remove the TLS
>> object when hot unplugging a disk. The hot plug/unplug code is largely
>> generic, but the addition code does make the VXHS specific checks only
>> because it needs to grab the correct config directory and generate the
>> object as the command line would do.
>>
>> Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> src/qemu/qemu_block.c | 8 +++
>> src/qemu/qemu_command.c | 29 +++++++++
>> src/qemu/qemu_hotplug.c | 73 ++++++++++++++++++++++
>> ...-disk-drive-network-tlsx509-multidisk-vxhs.args | 43 +++++++++++++
>> ...v-disk-drive-network-tlsx509-multidisk-vxhs.xml | 50 +++++++++++++++
>> ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 30 +++++++++
>> tests/qemuxml2argvtest.c | 7 +++
>> 7 files changed, 240 insertions(+)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
>>
>> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
>> index ca6e213b4..458b90d8e 100644
>> --- a/src/qemu/qemu_block.c
>> +++ b/src/qemu/qemu_block.c
>> @@ -529,16 +529,24 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
>> return NULL;
>> }
>>
>> + if (src->haveTLS == VIR_TRISTATE_BOOL_YES && !src->tlsAlias) {
>> + virReportError(VIR_ERR_INVALID_ARG, "%s",
>> + _("VxHS disk does not have TLS alias set"));
>> + return NULL;
>> + }
>> +
>> if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, true)))
>> return NULL;
>>
>> /* VxHS disk specification example:
>> * { driver:"vxhs",
>> + * [tls-creds:"objvirtio-disk0_tls0",]
>
> Be careful with square brackets in JSON, they denote an array.
>
OK - I can remove... Obviously it's the "optional" marking that I'm after...
>> * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
>> * server:[{type:"tcp", host:"1.2.3.4", port:9999}]}
>> */
>> if (virJSONValueObjectCreate(&ret,
>> "s:driver", protocol,
>> + "S:tls-creds", src->tlsAlias,
>> "s:vdisk-id", src->path,
>> "a:server", server, NULL) < 0)
>> virJSONValueFree(server);
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 0a3278510..7b98e1947 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -794,6 +794,32 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
>> }
>>
>>
>> +/* qemuBuildDiskTLSx509CommandLine:
>> + *
>> + * Add TLS object if the disk uses a secure communication channel
>> + *
>> + * Returns 0 on success, -1 w/ error on some sort of failure.
>> + */
>> +static int
>> +qemuBuildDiskTLSx509CommandLine(virCommandPtr cmd,
>> + virQEMUDriverConfigPtr cfg,
>> + virDomainDiskDefPtr disk,
>> + virQEMUCapsPtr qemuCaps)
>> +{
>> + virStorageSourcePtr src = disk->src;
>> +
>> + /* other protocols may be added later */
>> + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
>> + disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
>> + return qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
>> + false, true, false,
>> + disk->info.alias, qemuCaps);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +
>> static char *
>> qemuBuildNetworkDriveURI(virStorageSourcePtr src,
>> qemuDomainSecretInfoPtr secinfo)
>
> [...]
>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index b365078ec..e4174af35 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -152,6 +152,55 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
>>
>>
>> static int
>> +(virQEMUDriverPtr driver,
>> + virDomainObjPtr vm,
>> + virDomainDiskDefPtr disk,
>> + char **tlsAlias)
>
> I don't particularly like this. You set it into the disk object, so
> there should be a function which rolls this back for a disk (or
> preferably a virStorageSource).
>
Sure that's fine - the rollback code is so ugly anyway - this just adds
to its misery.
>> +{
>> + int ret = -1;
>> + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>> + qemuDomainObjPrivatePtr priv = vm->privateData;
>> + virStorageSourcePtr src = disk->src;
>> + virJSONValuePtr tlsProps = NULL;
>> +
>> + /* NB: This may alter haveTLS based on cfg */
>> + qemuDomainPrepareDiskSourceTLS(src, disk->info.alias, cfg);
>
> This should be executed in the hotplug API function and not in a helper.
>
I was trying to avoid replication of code for each of the 3 disk types
in the event that at some future point more than one has TLS. I actually
prefer it here.
>> +
>> + if (src->haveTLS != VIR_TRISTATE_BOOL_YES) {
>> + ret = 0;
>> + goto cleanup;
>> + }
>> +
>> + /* Initial implementation doesn't require/use a secret to decrypt
>> + * a server certificate, so there's no need to manage a tlsSecAlias
>> + * and tlsSecProps. See qemuDomainAddChardevTLSObjects for the
>> + * methodology required to add a secret object. */
>> +
>> + /* For a VxHS environment, create a TLS object for the client to
>> + * connect to the VxHS server. */
>> + if (src->type == VIR_STORAGE_TYPE_NETWORK &&
>> + src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
>> + qemuDomainGetTLSObjects(priv->qemuCaps, NULL,
>> + cfg->vxhsTLSx509certdir, false, true,
>> + disk->info.alias, &tlsProps, tlsAlias,
>> + NULL, NULL) < 0)
>
> This looks like another place that extracts qemu-specific stuff, which
> need to be duplicated for blockdev add. Can't we make the cert dir part
> of virStorageSource?
>
Hmmmmm... I think we can do that... The qemuDomainPrepareDiskSourceTLS
would change to fill in some sort of src->TLSx509certdir based on
src->type and protocol which then would be used by this code.
And the only reason why @disk was passed was to get src and srcalias. I
can adjust that too.
Tks -
John
>
>> + goto cleanup;
>> +
>> + if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
>> + NULL, NULL, *tlsAlias, &tlsProps) < 0)
>> + goto cleanup;
>> +
>> + ret = 0;
>> +
>> + cleanup:
>> + virJSONValueFree(tlsProps);
>> + virObjectUnref(cfg);
>> +
>> + return ret;
>> +}
>> +
>> +
>> +static int
>> qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
>> virDomainObjPtr vm,
>> virDomainDiskDefPtr disk,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.