Re: [libvirt] [PATCH v4 3/3] Add TLS support for Veritas HyperScale (VxHS) block device protocol

On Fri, Jun 30, 2017 at 2:21 PM, John Ferlan <jferlan(a)redhat.com&gt; wrote: Hi John, Peter, The problem statement - Alias for the TLS certificate can't be a constant. Two TLS objects cannot have the same ID/alias. This was pointed out by both of you as something that needs to be fixed. To that end, I have made some changes to use the block device domain alias (e.g. virtio-disk0) as a unique identifier for each TLS object. This is similar to how char devices create their TLS aliases. I was hoping to get some feedback on whether this diff would be acceptable to fix the said issue. I will reply to/fix all the other comments. Just thought I'd tackle this one first as this appears to be one of the bigger ones... diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 99bc94f..fc58236 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -952,13 +952,19 @@ qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd, int ret = 0; if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) { - disk->src->addTLS = true; - ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir, - false, - true, - false, - "vxhs", - qemuCaps); + if (virAsprintf(&disk->src->aliasTLS, + "vxhs_%s", disk->info.alias) < 0) { + ret = -1; + goto cleanup; + } + + disk->src->addTLS = true; + ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir, + false, + true, + false, + disk->src->aliasTLS, + qemuCaps); } else if (cfg->vxhsTLS == false && disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -968,6 +974,7 @@ qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd, ret = -1; } + cleanup: return ret; } @@ -1040,7 +1047,7 @@ qemuBuildVxHSDriveJSON(virStorageSourcePtr src) if (src->addTLS == true) { char *objalias = NULL; - if (!(objalias = qemuAliasTLSObjFromSrcAlias("vxhs"))) + if (!(objalias = qemuAliasTLSObjFromSrcAlias(src->aliasTLS))) goto cleanup; if (virJSONValueObjectCreate(&ret, diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 449ace4..61cd54a 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2057,7 +2057,8 @@ virStorageSourceCopy(const virStorageSource *src, VIR_STRDUP(ret->configFile, src->configFile) < 0 || VIR_STRDUP(ret->nodeformat, src->nodeformat) < 0 || VIR_STRDUP(ret->nodebacking, src->nodebacking) < 0 || - VIR_STRDUP(ret->compat, src->compat) < 0) + VIR_STRDUP(ret->compat, src->compat) < 0 || + VIR_STRDUP(ret->aliasTLS, src->aliasTLS) < 0) goto error; if (src->nhosts) { @@ -2273,6 +2274,7 @@ virStorageSourceClear(virStorageSourcePtr def) virStorageSourceSeclabelsClear(def); virStoragePermsFree(def->perms); VIR_FREE(def->timestamps); + VIR_FREE(def->aliasTLS); virStorageNetHostDefFree(def->nhosts, def->hosts); virStorageAuthDefFree(def->auth); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index e586170..c1d36bf 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -290,6 +290,9 @@ struct _virStorageSource { * the device. For e.g. this could be based on a combination of * global conf setting + domain specific setting */ bool addTLS; + + /* The alias/ID of the TLS object */ + char *aliasTLS; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args index 4cacb61..b474ce3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args @@ -18,17 +18,19 @@ QEMU_AUDIO_DRV=none \ -no-acpi \ -boot c \ -usb \ --object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\ +-object tls-creds-x509,id=objvxhs_virtio-disk0_tls0,\ +dir=/usr/local/etc/pki/qemu,\ endpoint=client,verify-peer=yes \ --drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\ +-drive file.driver=vxhs,file.tls-creds=objvxhs_virtio-disk0_tls0,\ file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\ 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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\ +-object tls-creds-x509,id=objvxhs_virtio-disk1_tls0,\ +dir=/usr/local/etc/pki/qemu,\ endpoint=client,verify-peer=yes \ --drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\ +-drive file.driver=vxhs,file.tls-creds=objvxhs_virtio-disk1_tls0,\ file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\ file.server.host=192.168.0.2,file.server.port=9999,format=raw,\ if=none,id=drive-virtio-disk1,cache=none \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args index e1ad36e..ad78405 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args @@ -18,9 +18,10 @@ QEMU_AUDIO_DRV=none \ -no-acpi \ -boot c \ -usb \ --object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\ +-object tls-creds-x509,id=objvxhs_virtio-disk0_tls0,\ +dir=/usr/local/etc/pki/qemu,\ endpoint=client,verify-peer=yes \ --drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\ +-drive file.driver=vxhs,file.tls-creds=objvxhs_virtio-disk0_tls0,\ file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\ file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,\ id=drive-virtio-disk0,cache=none \ Thanks, Ashish