On 09/12/2017 09:36 AM, Peter Krempa wrote: > On Tue, Sep 05, 2017 at 15:09:35 -0400, 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 | 19 ++++++++- >> src/qemu/qemu_domain.c | 4 ++ >> ...xml2argv-disk-drive-network-iscsi-auth-AES.args | 39 ++++++++++++++++++ >> ...uxml2argv-disk-drive-network-iscsi-auth-AES.xml | 43 +++++++++++++++++++ >> ...ml2argv-hostdev-scsi-virtio-iscsi-auth-AES.args | 35 ++++++++++++++++ >> ...xml2argv-hostdev-scsi-virtio-iscsi-auth-AES.xml | 48 ++++++++++++++++++++++ >> tests/qemuxml2argvtest.c | 10 +++++ >> 7 files changed, 196 insertions(+), 2 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 > > Most of the stuff here looks reasonable but I don't think we should mix > the URI syntax with the file.param= syntax generated from the JSON > objects. Since there's a capability when this is supported, the command > line generator should use the new syntax. > > You can mark it in qemuDiskSourceNeedsProps so that it uses the new > generator if it's needed and supported and implement the JSON generator. > > The rest should then work as expected. > One more thing I forgot in the first reply... There's also iSCSI on <hostdev> that uses this same code path, but isn't processed for qemuDiskSourceNeedsProps. I think a significant reworking of the code could be necessary - whether that outweighs the security aspects of providing the passphrase on the command line like is done now is up for "debate" I suppose. John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
