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@xxxxxxxxxx> > --- > 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.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
