On Wed, Nov 08, 2017 at 08:15:59 -0500, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1425757
The blockdev-add code provides a mechanism to sanely provide user and password-secret arguments for iscsi without placing them on the command line to be viewable by a 'ps -ef' type command or needing to create separate -iscsi devices for each disk/volume found.
So modify the iSCSI command line building to check for the presence of the capability in order properly setup and use the domain master secret object to encrypt the password in a secret object and alter the parameters for the command line to utilize.
Modify the xml2argvtest to exhibit the syntax for both disk and hostdev configurations.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 65 +++++++++++++++++----- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_domain.c | 4 ++ src/qemu/qemu_hotplug.c | 50 ++++++++++++++++- ...xml2argv-disk-drive-network-iscsi-auth-AES.args | 41 ++++++++++++++ ...uxml2argv-disk-drive-network-iscsi-auth-AES.xml | 43 ++++++++++++++ ...ml2argv-hostdev-scsi-virtio-iscsi-auth-AES.args | 45 +++++++++++++++ ...xml2argv-hostdev-scsi-virtio-iscsi-auth-AES.xml | 48 ++++++++++++++++ tests/qemuxml2argvtest.c | 10 ++++ 9 files changed, 292 insertions(+), 17 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-AES.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-AES.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-AES.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-AES.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 577c76b44b..f0724223f2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c
[...]
@@ -1573,7 +1579,9 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, virBufferAsprintf(buf, "file.debug=%d,", cfg->glusterDebugLevel); }
- if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES && + disk->src->type == VIR_STORAGE_TYPE_NETWORK && + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
This hunk is misplaced. if 'srcprops' is present no additional parameters should be added via this syntax. The same applies also to the gluster hunk above. I'll post a patch to move them and then you can commit this patch without this hunk.
/* NB: If libvirt starts using the more modern option based * syntax to build the command line (e.g., "-drive driver=rbd, * filename=%s,...") instead of the legacy model (e.g."-drive @@ -4892,22 +4900,36 @@ qemuBuildSCSIHostHostdevDrvStr(virDomainHostdevDefPtr dev) }
static char * -qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) +qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps) { char *source = NULL; char *netsource = NULL; + virJSONValuePtr srcprops = NULL; virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
- /* Rather than pull what we think we want - use the network disk code */ - netsource = qemuBuildNetworkDriveStr(iscsisrc->src, srcPriv ? - srcPriv->secinfo : NULL); - if (!netsource) - goto cleanup; - if (virAsprintf(&source, "file=%s,if=none,format=raw", netsource) < 0) - goto cleanup; + if (qemuDiskSourceNeedsProps(iscsisrc->src, qemuCaps)) { + if (!(srcprops = qemuDiskSourceGetProps(iscsisrc->src))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to build the backend props")); + goto cleanup; + } + + if (!(netsource = virQEMUBuildDriveCommandlineFromJSON(srcprops))) + goto cleanup; + if (virAsprintf(&source, "%s,if=none,format=raw", netsource) < 0) + goto cleanup; + } else { + /* Rather than pull what we think we want - use the network disk code */ + if (!(netsource = qemuBuildNetworkDriveStr(iscsisrc->src, srcPriv ? + srcPriv->secinfo : NULL))) + goto cleanup; + if (virAsprintf(&source, "file=%s,if=none,format=raw", netsource) < 0) + goto cleanup; + }
cleanup: VIR_FREE(netsource);
This does not clean up srcprops. [...]
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-AES.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-AES.args
I think the test cases for hostdevs and disks should be merged into one XML testing the new syntax. I don't see a need to have specific ones for hostdevs and drives. In general ACK once the test case is sanitized after the drive syntax formatter is fixed.