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

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(a)veritas.com&gt; > --- > 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,\ 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).

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

Similar to how NBD (see commit id 'a1674fd9') and SSH (see commit id 'bc225b1b5') are handled. > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index f7bef51..2a071c9 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -2319,9 +2319,9 @@ > <dd> > The <code>protocol</code> attribute specifies the protocol to > access to the requested image. Possible values are "nbd", > - "iscsi", "rbd", "sheepdog" or "gluster". If the > - <code>protocol</code> attribute is "rbd", "sheepdog" or > - "gluster", an additional attribute <code>name</code> is > + "iscsi", "rbd", "sheepdog", "gluster" or "vxhs". If the > + <code>protocol</code> attribute is "rbd", "sheepdog", "gluster" > + or "vxhs", an additional attribute <code>name</code> is > mandatory to specify which volume/image will be used. For "nbd", > the <code>name</code> attribute is optional. For "iscsi" > (<span class="since">since 1.0.4</span>), the <code>name</code> > @@ -2329,6 +2329,9 @@ > target's name by a slash (e.g., > <code>iqn.2013-07.com.example:iscsi-pool/1</code>). If not > specified, the default LUN is zero. > + For "vxhs" (<span class="since">since 2.5.0</span>), the Next release is 3.1.0 > + <code>name</code> is the UUID of the volume, assigned by the > + HyperScale sever. > <span class="since">Since 0.8.7</span> > </dd> > <dt><code>volume</code></dt> > @@ -2431,6 +2434,12 @@ > <td> one or more (<span class="since">Since 2.1.0</span>), just one prior to that </td> > <td> 24007 </td> > </tr> > + <tr> > + <td> vxhs </td> > + <td> a server running Veritas HyperScale daemon </td> > + <td> only one </td> > + <td> 9999 </td> > + </tr> > </table> > <p> > gluster supports "tcp", "rdma", "unix" as valid values for the > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 4d76315..cdc39ca 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -1470,6 +1470,7 @@ > <value>ftp</value> > <value>ftps</value> > <value>tftp</value> > + <value>vxhs</value> > </choice> > </attribute> > <optional> > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index b569dda..7e12d32 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -604,6 +604,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src, > case VIR_STORAGE_NET_PROTOCOL_GLUSTER: > case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: > case VIR_STORAGE_NET_PROTOCOL_SSH: > + case VIR_STORAGE_NET_PROTOCOL_VXHS: > case VIR_STORAGE_NET_PROTOCOL_LAST: > case VIR_STORAGE_NET_PROTOCOL_NONE: > virReportError(VIR_ERR_NO_SUPPORT, > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index f8e48d2..08bad8e 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -491,6 +491,9 @@ qemuNetworkDriveGetPort(int protocol, > /* no default port specified */ > return 0; > > + case VIR_STORAGE_NET_PROTOCOL_VXHS: > + return 9999; > + > case VIR_STORAGE_NET_PROTOCOL_RBD: > case VIR_STORAGE_NET_PROTOCOL_LAST: > case VIR_STORAGE_NET_PROTOCOL_NONE: > @@ -899,6 +902,65 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src) > } > > > +#define QEMU_DEFAULT_VXHS_PORT "9999" > + > +/* Build the VxHS host object */ > +static virJSONValuePtr > +qemuBuildVxHSDriveJSONHost(virStorageSourcePtr src) > +{ > + virJSONValuePtr server = NULL; > + virStorageNetHostDefPtr host; > + const char *portstr; > + > + if (src->nhosts != 1) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("VxHS supports only one server")); make syntax-check failure here - need a format, e.g. virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + goto cleanup; > + } > + > + host = src->hosts; > + portstr = host->port; > + > + if (!portstr) > + portstr = QEMU_DEFAULT_VXHS_PORT; > + > + if (virJSONValueObjectCreate(&server, > + "s:host", host->name, > + "s:port", portstr, > + NULL) < 0) > + server = NULL; > + > + cleanup: > + return server; > +} > + > + > +static virJSONValuePtr > +qemuBuildVxHSDriveJSON(virStorageSourcePtr src) > +{ > + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); > + virJSONValuePtr server = NULL; > + virJSONValuePtr ret = NULL; > + > + if (!(server = qemuBuildVxHSDriveJSONHost(src))) > + return NULL; > + > + /* VxHS disk sepecification example: "specification" > + * { driver:"vxhs", > + * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251", > + * server.host:"1.2.3.4", > + * server.port:1234} > + */ > + if (virJSONValueObjectCreate(&ret, > + "s:driver", protocol, > + "s:vdisk-id", src->path, > + "a:server", server, NULL) < 0) If you're going with the 1 host only model, then I believe this isn't necessary. For the Gluster code there's a difference based solely on whether >1 hosts are present whether the "server:" syntax is added or the legacy syntax is used. If you're not ever going to allow multiple hosts, the "server" option would seem to be unnecessary... Rather I would think you should just be able to add "s:host" and "s:port" type options. > + virJSONValueFree(server); > + > + return ret; > +} > + > + > static char * > qemuBuildNetworkDriveURI(virStorageSourcePtr src, > qemuDomainSecretInfoPtr secinfo) > @@ -1034,6 +1096,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, > case VIR_STORAGE_NET_PROTOCOL_TFTP: > case VIR_STORAGE_NET_PROTOCOL_ISCSI: > case VIR_STORAGE_NET_PROTOCOL_GLUSTER: > + case VIR_STORAGE_NET_PROTOCOL_VXHS: > ret = qemuBuildNetworkDriveURI(src, secinfo); > break; > > @@ -1148,6 +1211,11 @@ qemuGetDriveSourceProps(virStorageSourcePtr src, > if (!(fileprops = qemuBuildGlusterDriveJSON(src))) > return -1; > } > + > + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) { > + if (!(fileprops = qemuBuildVxHSDriveJSON(src))) > + return -1; > + } > break; > } > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index b359e77..c76e9d4 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -13788,6 +13788,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) > case VIR_STORAGE_NET_PROTOCOL_FTPS: > case VIR_STORAGE_NET_PROTOCOL_TFTP: > case VIR_STORAGE_NET_PROTOCOL_SSH: > + case VIR_STORAGE_NET_PROTOCOL_VXHS: > case VIR_STORAGE_NET_PROTOCOL_LAST: > virReportError(VIR_ERR_INTERNAL_ERROR, > _("external inactive snapshots are not supported on " > @@ -13851,6 +13852,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d > case VIR_STORAGE_NET_PROTOCOL_FTPS: > case VIR_STORAGE_NET_PROTOCOL_TFTP: > case VIR_STORAGE_NET_PROTOCOL_SSH: > + case VIR_STORAGE_NET_PROTOCOL_VXHS: > case VIR_STORAGE_NET_PROTOCOL_LAST: > virReportError(VIR_ERR_INTERNAL_ERROR, > _("external active snapshots are not supported on " > @@ -13996,6 +13998,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, > case VIR_STORAGE_NET_PROTOCOL_FTPS: > case VIR_STORAGE_NET_PROTOCOL_TFTP: > case VIR_STORAGE_NET_PROTOCOL_SSH: > + case VIR_STORAGE_NET_PROTOCOL_VXHS: > case VIR_STORAGE_NET_PROTOCOL_LAST: > virReportError(VIR_ERR_INTERNAL_ERROR, > _("internal inactive snapshots are not supported on " > diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c > index 405e655..aab287a 100644 > --- a/src/qemu/qemu_parse_command.c > +++ b/src/qemu/qemu_parse_command.c > @@ -263,6 +263,17 @@ qemuParseNBDString(virDomainDiskDefPtr disk) > return -1; > } > > +static int > +qemuParseVxHSString(virDomainDiskDefPtr def) > +{ > + virURIPtr uri = NULL; > + > + if (!(uri = virURIParse(def->src->path))) > + return -1; > + > + return qemuParseDriveURIString(def, uri, "vxhs"); > +} > + > > /* > * This method takes a string representing a QEMU command line ARGV set > @@ -737,6 +748,12 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, > if (VIR_STRDUP(def->src->path, vdi) < 0) > goto error; > } > + } else if (STRPREFIX(def->src->path, "vxhs:")) { > + def->src->type = VIR_STORAGE_TYPE_NETWORK; > + def->src->protocol = VIR_STORAGE_NET_PROTOCOL_VXHS; > + > + if (qemuParseVxHSString(def) < 0) > + goto error; > } else { > def->src->type = VIR_STORAGE_TYPE_FILE; > } > @@ -1947,6 +1964,10 @@ qemuParseCommandLine(virCapsPtr caps, > disk->src->type = VIR_STORAGE_TYPE_NETWORK; > disk->src->protocol = VIR_STORAGE_NET_PROTOCOL_SHEEPDOG; > val += strlen("sheepdog:"); > + } else if (STRPREFIX(val, "vxhs:")) { > + disk->src->type = VIR_STORAGE_TYPE_NETWORK; > + disk->src->protocol = VIR_STORAGE_NET_PROTOCOL_VXHS; > + val += strlen("vxhs:"); > } else { > disk->src->type = VIR_STORAGE_TYPE_FILE; > } > @@ -2023,6 +2044,11 @@ qemuParseCommandLine(virCapsPtr caps, > goto error; > > break; > + case VIR_STORAGE_NET_PROTOCOL_VXHS: > + if (qemuParseVxHSString(disk) < 0) > + goto error; > + > + break; > case VIR_STORAGE_NET_PROTOCOL_HTTP: > case VIR_STORAGE_NET_PROTOCOL_HTTPS: > case VIR_STORAGE_NET_PROTOCOL_FTP: > diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c > index ce6d213..e62f4e6 100644 > --- a/src/util/virstoragefile.c > +++ b/src/util/virstoragefile.c > @@ -85,7 +85,8 @@ VIR_ENUM_IMPL(virStorageNetProtocol, VIR_STORAGE_NET_PROTOCOL_LAST, > "ftp", > "ftps", > "tftp", > - "ssh") > + "ssh", > + "vxhs") > > VIR_ENUM_IMPL(virStorageNetHostTransport, VIR_STORAGE_NET_HOST_TRANS_LAST, > "tcp", > @@ -2633,6 +2634,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, > case VIR_STORAGE_NET_PROTOCOL_ISCSI: > case VIR_STORAGE_NET_PROTOCOL_GLUSTER: > case VIR_STORAGE_NET_PROTOCOL_SSH: > + case VIR_STORAGE_NET_PROTOCOL_VXHS: > virReportError(VIR_ERR_INTERNAL_ERROR, > _("malformed backing store path for protocol %s"), > protocol); > @@ -2964,6 +2966,64 @@ virStorageSourceParseBackingJSONRBD(virStorageSourcePtr src, > } > > > +static int > +virStorageSourceParseBackingJSONVXHS(virStorageSourcePtr src, s/VXHS/VxHS (to be consistent) > + virJSONValuePtr json, > + int opaque ATTRIBUTE_UNUSED) These two arguments need to be aligned under the 'v' instead of under the '(' > +{ > + virJSONValuePtr server; > + const char *hostname, *port; Single lines for each and follow existing model - e.g. move the lines below setting these to here. > + const char *uri = virJSONValueObjectGetString(json, "filename"); > + const char *vdisk_id = virJSONValueObjectGetString(json, "vdisk-id"); > + > + src->type = VIR_STORAGE_TYPE_NETWORK; > + src->protocol = VIR_STORAGE_NET_PROTOCOL_VXHS; > + > + /* legacy URI based syntax passed via 'filename' option */ > + if (uri) > + return virStorageSourceParseBackingJSONUriStr(src, uri, > + VIR_STORAGE_NET_PROTOCOL_VXHS); Since we don't want to support the legacy option - this becomes a failure path with some sort of error indicating unsupported format. For the other protocols, the legacy exists because 'filename' existed prior to when the "new" syntax support was added so both need to be supported. > + > + server = virJSONValueObjectGetObject(json, "server"); > + hostname = virJSONValueObjectGetString(server, "host"); > + port = virJSONValueObjectGetString(server, "port"); Each of these should be set above similar to how each of the other helpers do this. Although whether "server" is necessary depends on the multiple hosts decision. The *JSONGluster uses it because the legacy syntax supports only one "host", while the non legacy syntax supports having multiple hosts (although it's really not well described in/from commit id '7b7da9e2833"). > + > + if (!vdisk_id) { > + virReportError(VIR_ERR_INVALID_ARG, "%s", > + _("missing 'vdisk-id' attribute in " > + "JSON backing definition for VxHS volume")); > + return -1; > + } > + if (!server) { > + virReportError(VIR_ERR_INVALID_ARG, "%s", > + _("missing 'server' attribute in " > + "JSON backing definition for VxHS volume")); > + return -1; > + } > + if (!hostname) { > + virReportError(VIR_ERR_INVALID_ARG, "%s", > + _("missing hostname for tcp backing server in " > + "JSON backing definition for VxHS volume")); > + return -1; > + } You could combine these to be like others and just have one error message. > + > + > + if (VIR_STRDUP(src->path, vdisk_id) < 0) > + return -1; > + > + if (VIR_ALLOC_N(src->hosts, 1) < 0) > + return -1; > + src->nhosts = 1; > + > + src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; > + > + if (VIR_STRDUP(src->hosts[0].name, hostname) < 0 || > + VIR_STRDUP(src->hosts[0].port, port) < 0) > + return -1; > + > + return 0; > +} > + > struct virStorageSourceJSONDriverParser { > const char *drvname; > int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); > @@ -2985,6 +3045,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { > {"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0}, > {"ssh", virStorageSourceParseBackingJSONSSH, 0}, > {"rbd", virStorageSourceParseBackingJSONRBD, 0}, > + {"vxhs", virStorageSourceParseBackingJSONVXHS, 0}, > }; > > > diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h > index 1f62244..e60b6e9 100644 > --- a/src/util/virstoragefile.h > +++ b/src/util/virstoragefile.h > @@ -134,6 +134,7 @@ typedef enum { > VIR_STORAGE_NET_PROTOCOL_FTPS, > VIR_STORAGE_NET_PROTOCOL_TFTP, > VIR_STORAGE_NET_PROTOCOL_SSH, > + VIR_STORAGE_NET_PROTOCOL_VXHS, > > VIR_STORAGE_NET_PROTOCOL_LAST > } virStorageNetProtocol; > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c > index 18d9fe3..9fe24d6 100644 > --- a/src/xenconfig/xen_xl.c > +++ b/src/xenconfig/xen_xl.c > @@ -942,6 +942,7 @@ xenFormatXLDiskSrcNet(virStorageSourcePtr src) > case VIR_STORAGE_NET_PROTOCOL_GLUSTER: > case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: > case VIR_STORAGE_NET_PROTOCOL_SSH: > + case VIR_STORAGE_NET_PROTOCOL_VXHS: > case VIR_STORAGE_NET_PROTOCOL_LAST: > case VIR_STORAGE_NET_PROTOCOL_NONE: > virReportError(VIR_ERR_NO_SUPPORT, > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs-multi-host.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs-multi-host.xml > new file mode 100644 > index 0000000..660bcde > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs-multi-host.xml > @@ -0,0 +1,35 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219136</memory> > + <currentMemory unit='KiB'>219136</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/libexec/qemu-kvm</emulator> > + <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'/> > + <host name='192.168.0.2' 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> > + <controller type='usb' index='0'/> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <memballoon model='none'/> > + </devices> > +</domain> > 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..23045e1 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args > @@ -0,0 +1,25 @@ > +LC_ALL=C \ > +PATH=/bin \ > +HOME=/home/test \ > +USER=test \ > +LOGNAME=test \ > +QEMU_AUDIO_DRV=none \ > +/usr/libexec/qemu-kvm \ > +-name QEMUGuest1 \ > +-S \ > +-M pc \ > +-cpu qemu32 \ > +-m 214 \ > +-smp 1,sockets=1,cores=1,threads=1 \ > +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ > +-nographic \ > +-nodefaults \ > +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ > +-no-acpi \ > +-boot c \ > +-usb \ > +-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 \ > +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ > +id=virtio-disk0 > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml > new file mode 100644 > index 0000000..45c807f > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml > @@ -0,0 +1,34 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219136</memory> > + <currentMemory unit='KiB'>219136</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/libexec/qemu-kvm</emulator> > + <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> > + <controller type='usb' index='0'/> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <memballoon model='none'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index ab3ad08..f751ec9 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -892,6 +892,8 @@ mymain(void) > # endif > DO_TEST("disk-drive-network-rbd-ipv6", NONE); > DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE); > + DO_TEST("disk-drive-network-vxhs", NONE); > + DO_TEST_FAILURE("disk-drive-network-vxhs-multi-host", NONE); > DO_TEST("disk-drive-no-boot", > QEMU_CAPS_BOOTINDEX); > DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid", > diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c > index f766df1..a28fbd9 100644 > --- a/tests/virstoragetest.c > +++ b/tests/virstoragetest.c > @@ -1492,6 +1492,22 @@ mymain(void) > "<source protocol='rbd' name='testshare'>\n" > " <host name='example.com'/>\n" > "</source>\n"); > + TEST_BACKING_PARSE("json:{\"file.driver\":\"vxhs\"," > + "\"file.filename\":\"vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0\"" > + "}", > + "<source protocol='vxhs' name='c6718f6b-0401-441d-a8c3-1f0064d75ee0'>\n" > + " <host name='192.168.0.1' port='9999'/>\n" > + "</source>\n"); > + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"vxhs\"," > + "\"vdisk-id\":\"c6718f6b-0401-441d-a8c3-1f0064d75ee0\"," > + "\"server\": { \"host\":\"example.com\"," > + "\"port\":\"1234\"" > + "}" > + "}" > + "}", > + "<source protocol='vxhs' name='c6718f6b-0401-441d-a8c3-1f0064d75ee0'>\n" > + " <host name='example.com' port='1234'/>\n" > + "</source>\n"); Since the desire is to not support the older "driver=vxhs,..." syntax that means this second example would result in an error. If you wanted to "check" that it does error, then create a TEST_BACKING_PARSE_ERROR macro that *expects* virTestRun to fail and use the older syntax with that. John > > cleanup: > /* Final cleanup */ >