On Wed, Jan 25, 2017 at 10:59:59 -0500, John Ferlan wrote: > > > On 01/19/2017 09:21 PM, Ashish Mittal wrote: > > Sample XML for a vxhs vdisk is as follows: > > > > <disk type='network' device='disk'> > > <driver name='qemu' type='raw' cache='none'/> > > <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251'> > > <host name='192.168.0.1' port='9999'/> > > </source> > > <backingStore/> > > <target dev='vda' bus='virtio'/> > > <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial> > > <alias name='virtio-disk0'/> > > <address type='pci' domain='0x0000' bus='0x00' slot='0x04' > > function='0x0'/> > > </disk> > > It's still not really clear how someone knows to use the name string. > IOW: How would someone know what to supply. Perhaps the libvirt wiki > (http://wiki.libvirt.org/page/Main_Page) as opposed to the libvirt docs. > > I assume that someone using VxHS knows how to get that, but still I find > it a "good thing" to be able to have that description somewhere as I can > only assume some day it'll come up. > > But since we don't "require" this for iSCSI, Ceph/RBD, Gluster, etc. > it's not something 'required' for this patch. Still something for > someone's todo list to get a description on the libvirt wiki. Once you > have wiki write access, then you can always keep that data up to date > without requiring libvirt patches. > > > > > Signed-off-by: Ashish Mittal <Ashish.Mittal@xxxxxxxxxxx> > > --- > > v2 changelog: > > (1) Added code for JSON parsing of a VxHS vdisk. > > (2) Added test case to verify JSON parsing. > > (3) Added missing switch-case checks for VIR_STORAGE_NET_PROTOCOL_VXHS. > > (4) Fixed line wrap in qemuxml2argv-disk-drive-network-vxhs.args. > > > > v3 changelog: > > (1) Implemented the modern syntax for VxHS disk specification. > > (2) Changed qemuxml2argvdata VxHS test case to verify the new syntax. > > (3) Added a negative test case to check failure when multiple hosts > > are specified for a VxHS disk. > > > > docs/formatdomain.html.in | 15 ++++- > > docs/schemas/domaincommon.rng | 1 + > > src/libxl/libxl_conf.c | 1 + > > src/qemu/qemu_command.c | 68 ++++++++++++++++++++++ > > src/qemu/qemu_driver.c | 3 + > > src/qemu/qemu_parse_command.c | 26 +++++++++ > > src/util/virstoragefile.c | 63 +++++++++++++++++++- > > src/util/virstoragefile.h | 1 + > > src/xenconfig/xen_xl.c | 1 + > > ...xml2argv-disk-drive-network-vxhs-multi-host.xml | 35 +++++++++++ > > .../qemuxml2argv-disk-drive-network-vxhs.args | 25 ++++++++ > > .../qemuxml2argv-disk-drive-network-vxhs.xml | 34 +++++++++++ > > tests/qemuxml2argvtest.c | 2 + > > tests/virstoragetest.c | 16 +++++ > > 14 files changed, 287 insertions(+), 4 deletions(-) > > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs-multi-host.xml > > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args > > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml > > > > In general things are looking much better - a couple of nits below and 2 > things to consider/rework... > > #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. > > #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: > > +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,\ I think that all protocols while using the JSON or JSON-converted-to-commandline syntax should use exactly the same syntax so that we don't have to do custom generators for every single storage transport somebody invents.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
