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@redhat.com> --- 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