Re: [libvirt] [REPOST PATCH v6 6/8] qemu: Use secret objects to pass iSCSI passwords

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(a)redhat.com&gt; > --- > 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.