[libvirt] [PATCH v3] Add support for Veritas HyperScale (VxHS) block device protocol

[John Ferlan]

[...]
Pressed send too soon, sigh.


>>>
>>> #1. Based on Peter's v2 comments, we don't want to support the
>>> older/legacy syntax for VxHS, so it's something that should be removed -
>>> although we should check for it being present and fail if found.
>>>
>>
>> I am testing with changed code to return error if legacy syntax is
>> found for VxHS. Also added a test case to check for failure on legacy
>> syntax and it seems to pass (test #41 below).
>>
>> Then I added a pass test case to check conversion from new native
>> syntax to XML (test #40 below). That test fails with error
>> 'qemuParseCommandLineDisk:901 : internal error: missing file parameter
>> in drive 'file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b...'
> 
> The qemu_parse_command.c changes while nice to have weren't even updated
> when multiple gluster servers were added (e.g. commit id '' or '7b7da9e28')
> Check the changes to add the new s
> 
> IOW: This code knows how to parse something like:
> 
> -drive
> 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,format=raw,if=none,id=drive-virtio-disk1'
> 
> but it's clueless for:
> 
> -drive file.driver=gluster,file.volume=Volume3,file.path=/Image.qcow2,\
> file.server.0.type=tcp,file.server.0.host=example.org,file.server.0.port=6000,\
> file.server.1.type=tcp,file.server.1.host=example.org,file.server.1.port=24007,\
> file.server.2.type=unix,file.server.2.socket=/path/to/sock,format=qcow2,\
> if=none,id=drive-virtio-disk2 \
> -device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,\
> id=virtio-disk2
> 
> See
>>
>> Looks like none of the existing tests in qemuargv2xmltest test for the
>> parsing of new syntax, and qemuParseCommandLineDisk() expects to find
>> 'file=' for a drive or it errors out. If this is true, will it be able
>> to parse the new syntax? Some help here please!

So I wouldn't expect the VxHS code to be able to do that unless you
wanted to be adventurous.  The good news is that this code is primarily
for developers that need to take a qemu command line to generate the
libvirt syntax. It has not really been kept up to date with all the most
recent command line changes. I started to try over a year ago, but got
very side tracked.

>>
>> Output from the newly added test cases (40 should pass and 41 checks
>> for error) :
>>
>> 40) QEMU ARGV-2-XML disk-drive-network-vxhs
>> ... Got unexpected warning from qemuParseCommandLineString:
>> 2017-01-28 00:57:30.814+0000: 10391: info : libvirt version: 3.0.0
>> 2017-01-28 00:57:30.814+0000: 10391: info : hostname: localhost.localdomain
>> 2017-01-28 00:57:30.814+0000: 10391: error :
>> qemuParseCommandLineDisk:901 : internal error: missing file parameter
>> in drive 'file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,id=drive-virtio-disk0,cache=none'
>> libvirt: QEMU Driver error : internal error: missing file parameter in
>> drive 'file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,id=drive-virtio-disk0,cache=none'
>> FAILED
>>
>> 41) QEMU ARGV-2-XML disk-drive-network-vxhs-fail
>> ... Got expected error from qemuParseCommandLineString:
>> libvirt: QEMU Driver error : internal error: VxHS protocol does not
>> support URI syntax
>> 'vxhs://192.168.0.1:9999/eb90327c-8302-4725-9e1b-4e85ed4dc251'
>> OK
>> 42) QEMU ARGV-2-XML disk-usb                                          ... OK
>>
>>
>>
>>> #2. Is the desire to ever support more than 1 host? If not, then is the
>>> "server" syntax you've borrowed from the Gluster code necessary? Could
>>> you just go with the single "host" like NBD and SSH. As it relates to
>>> the qemu command line - I'm not quite as clear. From the example I see
>>> in commit id '7b7da9e28', the gluster syntax would have:
>>>
>>
>> Present understanding is to have only one host. You are right, the
>> "server" part is not necessary. Will have to check with the qemu
>> community on this change.
>>
>>> +file.server.0.type=tcp,file.server.0.host=example.org,file.server.0.port=6000,\
>>> +file.server.1.type=tcp,file.server.1.host=example.org,file.server.1.port=24007,\
>>> +file.server.2.type=unix,file.server.2.socket=/path/to/sock,format=qcow2,\
>>>
>>> whereas, the VxHS syntax is:
>>>  +file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,\
>>>
>>> FWIW: I also note there is no ".type=tcp" in your output - so perhaps
>>> the "default" is tcp unless otherwise specified, but I'm sure of the
>>> qemu syntax requirements in this area. I assume that since there's only
>>> 1 server, the ".0, .1, .2" become unnecessary (something added by commit
>>> id 'f1bbc7df4' for multiple gluster hosts).
>>>
>>
>> That's correct. TCP is the default.
>>
>>> I haven't closedly followed the qemu syntax discussion, but it would it
>>> would be possible to use:
>>>
>>> +file.host=192.168.0.1,file.port=9999
>>>
>>
>> That is correct. Above syntax would also work for us. I will pose this
>> suggestion to the qemu community and update with their response.
>>

It's not that important... I was looking for a simplification and
generation of only what's required. You can continue using the server
syntax - perhaps just leave a note/comment in the code indicating the
decision point and move on.

[...]

John