Thanks for the review! My inputs on some of the comments - On Wed, Jan 25, 2017 at 7:59 AM, John Ferlan <jferlan@xxxxxxxxxx> 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. > 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...' 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! 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. > 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 */ >> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
