Re: [libvirt] [PATCH v4 1/3] Add support for Veritas HyperScale (VxHS) block device protocol

From: Ashish Mittal <ashish.mittal(a)veritas.com&gt; Sample XML for a VxHS disk: <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> 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. v4 changelog: (1) Fixes per review comments from v3. (2) Had to remove a test from the previous version that checked for error when multiple hosts are specified for VxHS device. This started failing virschematest with the error "XML document failed to validate against schema" as the docs/schemas/domain.rng specifies only a single host. docs/formatdomain.html.in | 15 ++++- docs/schemas/domaincommon.rng | 13 ++++ src/libxl/libxl_conf.c | 1 + src/qemu/qemu_command.c | 70 ++++++++++++++++++++++ src/qemu/qemu_driver.c | 3 + src/qemu/qemu_parse_command.c | 25 ++++++++ src/util/virstoragefile.c | 64 +++++++++++++++++++- src/util/virstoragefile.h | 1 + src/xenconfig/xen_xl.c | 1 + .../qemuargv2xml-disk-drive-network-vxhs-fail.args | 24 ++++++++ tests/qemuargv2xmltest.c | 17 +++++- .../qemuxml2argv-disk-drive-network-vxhs.args | 25 ++++++++ .../qemuxml2argv-disk-drive-network-vxhs.xml | 34 +++++++++++ tests/qemuxml2argvtest.c | 1 + tests/virstoragetest.c | 19 ++++++ 15 files changed, 308 insertions(+), 5 deletions(-) create mode 100644 tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-vxhs-fail.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 36bea67..62d67f4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in

@@ -2511,6 +2511,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 3.3.0</span>), thea

+ <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>

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index bdf7103..7525a2a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1613,6 +1613,18 @@ </element> </define> + <define name="diskSourceNetworkProtocolVxHS"> + <element name="source"> + <attribute name="protocol"> + <choice> + <value>vxhs</value> + </choice> + </attribute> + <attribute name="name"/> + <ref name="diskSourceNetworkHost"/>

+ </element> + </define> + <define name="diskSourceNetwork"> <attribute name="type"> <value>network</value>

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c53ab97..8e00782 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -524,6 +524,7 @@ qemuNetworkDriveGetPort(int protocol, return 0; case VIR_STORAGE_NET_PROTOCOL_RBD: + case VIR_STORAGE_NET_PROTOCOL_VXHS: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: /* not applicable */

@@ -931,6 +932,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, "%s", + _("protocol VxHS accepts only one host")); + 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; +} + +

@@ -1136,6 +1196,11 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, ret = virBufferContentAndReset(&buf); break; + case VIR_STORAGE_NET_PROTOCOL_VXHS: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'VxHS' protocol does not support URI syntax")); + goto cleanup;

+ case VIR_STORAGE_NET_PROTOCOL_SSH: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'ssh' protocol is not yet supported"));

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cdb727b..d43de69 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c

@@ -13746,6 +13747,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 "

diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index af9063c..aa15225 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,11 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, if (VIR_STRDUP(def->src->path, vdi) < 0) goto error; } + } else if (STRPREFIX(def->src->path, "vxhs:")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("VxHS protocol does not support URI syntax '%s'"), + def->src->path);

+ goto error; } else { def->src->type = VIR_STORAGE_TYPE_FILE; }

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index f0ed5c6..eb36694 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",

@@ -3219,6 +3221,65 @@ virStorageSourceParseBackingJSONRaw(virStorageSourcePtr src, return virStorageSourceParseBackingJSONInternal(src, json); } +#define QEMU_DEFAULT_VXHS_PORT "9999"

+ +static int +virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *uri = virJSONValueObjectGetString(json, "filename"); + const char *vdisk_id = virJSONValueObjectGetString(json, "vdisk-id"); + virJSONValuePtr server = virJSONValueObjectGetObject(json, "server"); + const char *hostname; + const char *port; + + /* Check for legacy URI based syntax passed via 'filename' option */ + if (uri) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'VxHS' protocol does not support URI syntax")); + return -1; + }

+ + if (!vdisk_id || !server) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'vdisk-id' or 'server' attribute in " + "JSON backing definition for VxHS volume")); + return -1; + } + + hostname = virJSONValueObjectGetString(server, "host"); + port = virJSONValueObjectGetString(server, "port");

+ + if (!hostname) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing hostname for tcp backing server in " + "JSON backing definition for VxHS volume")); + return -1; + } + + if (!port) + port = QEMU_DEFAULT_VXHS_PORT;

diff --git a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-vxhs-fail.args b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-vxhs-fail.args new file mode 100644 index 0000000..f6e3e37 --- /dev/null +++ b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-vxhs-fail.args @@ -0,0 +1,24 @@ +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=vxhs://192.168.0.1:9999/eb90327c-8302-4725-9e1b-4e85ed4dc251,\

+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/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 1adbcfe..fc15714 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -50,6 +50,7 @@ static int testSanitizeDef(virDomainDefPtr vmdef)

typedef enum { FLAG_EXPECT_WARNING = 1 << 0, + FLAG_EXPECT_FAIL = 1 << 1, } virQemuXML2ArgvTestFlags; static int testCompareXMLToArgvFiles(const char *xmlfile, @@ -67,7 +68,16 @@ static int testCompareXMLToArgvFiles(const char *xmlfile, if (!(vmdef = qemuParseCommandLineString(driver.caps, driver.xmlopt, cmd, NULL, NULL, NULL))) - goto fail; + { + if (flags & FLAG_EXPECT_FAIL) { + if (virTestLogContentAndReset() == NULL)

+ goto fail; + + VIR_TEST_DEBUG("Got expected error from " + "qemuParseCommandLineString:\n"); + goto out; + } + } if (!virTestOOMActive()) { if ((log = virTestLogContentAndReset()) == NULL) @@ -106,6 +116,7 @@ static int testCompareXMLToArgvFiles(const char *xmlfile, if (virTestCompareToFile(actualxml, xmlfile) < 0) goto fail; + out: ret = 0; fail:

@@ -166,6 +177,9 @@ mymain(void) # define DO_TEST(name) \ DO_TEST_FULL(name, 0) +# define DO_TEST_FAIL(name) \ + DO_TEST_FULL(name, FLAG_EXPECT_FAIL) + setenv("PATH", "/bin", 1); setenv("USER", "test", 1); setenv("LOGNAME", "test", 1); @@ -220,6 +234,7 @@ mymain(void) /* older format using CEPH_ARGS env var */ DO_TEST("disk-drive-network-rbd-ceph-env"); DO_TEST("disk-drive-network-sheepdog"); + DO_TEST_FAIL("disk-drive-network-vxhs-fail"); DO_TEST("disk-usb"); DO_TEST("graphics-vnc"); DO_TEST("graphics-vnc-socket");

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index f344083..3a4e03b 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1575,6 +1575,25 @@ mymain(void) "<source protocol='sheepdog' name='test'>\n" " <host name='example.com' port='321'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{ \"file\": { " + "\"driver\": \"raw\"," + "\"file\": {" + "\"driver\": \"file\"," + "\"filename\": \"/path/to/file\" } } }", + "<source file='/path/to/file'/>\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"); + TEST_BACKING_PARSE("json:{\"file.driver\":\"vxhs\"," + "\"file.filename\":\"vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0\"" + "}", NULL);