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>
---
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.