Re: [libvirt] [PATCH v2] Add support for Veritas HyperScale (VxHS) block device protocol

On 01/16/2017 04:07 AM, Peter Krempa wrote: > On Sat, Jan 14, 2017 at 10:31:05 -0500, John Ferlan wrote: >> [...] >> >>>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args >>>> new file mode 100644 >>>> index 0000000..f6e3e37 >>>> --- /dev/null >>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args > > [...] > >>>> +-drive file=vxhs://192.168.0.1:9999/eb90327c-8302-4725-9e1b-4e85ed4dc251,\ >>>> +format=raw,if=none,id=drive-virtio-disk0,cache=none \ >>> >>> Please use the modern syntax for disk specification, not the >>> legacy URI syntax ie >>> >>> -drive driver=vxhs,vdisk-id=eb90327c-8302-4725-4e85ed4dc251,\ >>> server.host=example.com,server.port=12345,format=raw, >>> if=none,id=driver-virtio-disk0,cache=0 >>> >> >> FWIW: qemuBuildDriveStr doesn't support that yet for any driver - >> although it probably should be updated... Timing wise whether that work >> could get done before this work is ready - I'm not sure. While I agree >> the code "should" use the new syntax - I do think we'd have to make >> adjustments to all drive strings. That would probably mean knowing >> whether the new syntax was "in place" before the current libvirt qemu >> minimum version... > > That's not true. The new syntax is used for multi-host gluster disks > (qemuGetDriveSourceProps). Also we have QMP introspection code in place > which can detect whether given configuration option is supported by the > given qemu (virQEMUCapsQMPSchemaQueries[]). hmm.. missed that path - it's not exactly well described how to get there though. Suffice to say, well obfuscated. I see that the syntax built for the gluster is "-drive file.driver=gluster,..." though rather than just plain "-drive driver=gluster,...". Whether vxhs would need to follow that model or the one Daniel pointed out - I'm not quite sure. In any case, to help "find" that for the Veritas team - see commit id '7b7da9e28'... If you use gitk - the commits just before that are where Peter has modified qemuBuildDriveSourceStr in order to accept a json object to parse through in order to generate the newer syntax. In those changes there is also the adjustment to the gluster backend storage code to support the newer model - something that could be a model once/if a vxhs storage backend is created.

> > Also we should not fully update the command line generator in cases > where it's not necessary as it would just make the code more complex > without any real benefit. The new protocols can use the new syntax > though. > Right - otherwise it'd already be done... Still would be nice for a non json object path that could generate the newer syntax especially since there's nothing else within the vxhs command line syntax that would be "unique" such that the command line generator had to be changed in order to support it (unlike the multi-host gluster disks syntax). I do know the syntax difference has come into play for the rbd driver decision point to add the password-secret and I'm guessing once secret support is added for vxhs it'll have a similar issue. John