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. > This is certainly where things didn't exactly match up the way I thought you have desired the virstoragefile and virstoragetest code to work. In particular, it's the "user" and "password-secret" options that cause the disconnect since they're a @disk private object and not a @disk->src object. I considered a number of different ways to cheat, but came up empty on each, so I just followed the existing RBD code. One cannot reconstruct the <auth> element properly given that all the arguments have is a username and an alias, but would need to have either a "usage" or "uuid" string. The secret object doesn't contain that either, so it'd need to be stored somehow. Perhaps if the @secinfo moved from private into source that would help, although perhaps not following the RNG. Still there'd also have to be a way to save the string used in the original <auth> element used to look up the secret (either by uuid or by usage). Having the password-secret alias is OK, but really not useful. Maybe "some future" adjustment could modify the password-secret alias to be (for example) "virtio-disk0-%s-secret0" where the %s is either a UUID or a Usage string of the secret used to generate the object. Perhaps even the unsigned char UUID too so as to not have too many extra "-"'s to parse/read. That'd be an awful alias, but serve a purpose as well. A similar problem exists for RBD, but the RBD code in virstoragefile and virstoragetest totally ignore the authentication pieces... That syntax is a bit more hairly because it's only the RBD password-secret field that needs the "file." (or not if -drive driver=rbd,... was supported). John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
