[libvirt] [PATCH v5 0/4] Qemu/Gluster support in Libvirt

Changelog: v5: - Separated initialization of new host members for nbd, rbd, sheepdog in a separate patch [Patch 3/4] - Fixed tests for appropriate args to DO_TEST - Code cleanups for squashing pointer incrememnts into single line. v4: - Addressed review comment by Jiri on v3 updated series. v3 updated: - Fix other network backends (nbd, rbd, sheepdog) to initialize new members (transport, socket) of _virDomainDiskHostDef appropriately so that garbage values doesnt break the argv2xml tests. v3: - RNG schema updated as required for unix transport [Paolo] - introduced another new attribute 'socket' for unix transport [Paolo] - Uses virURIFormat and virURIParse for URI parsing. [danpb] - updated documentation as required. [Jirka] v2: - Addressed review comments by Jiri - Updated patcheset as per new URI spec Ref: http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg05199.html v1: - Initial prototype Harsh Prateek Bora (4): Add Gluster protocol as supported network disk backend qemu: Add support for gluster protocol based network storage backend. qemu: Fix nbd, rbd, sheepdog to initialize defaults for new host members. tests: Add tests for gluster protocol based network disks support docs/formatdomain.html.in | 24 ++- docs/schemas/domaincommon.rng | 33 +++- src/conf/domain_conf.c | 77 ++++++-- src/conf/domain_conf.h | 12 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 195 ++++++++++++++++++++- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-gluster.args | 1 + .../qemuxml2argv-disk-drive-network-gluster.xml | 34 ++++ tests/qemuxml2argvtest.c | 2 + 10 files changed, 352 insertions(+), 29 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml -- 1.7.11.7

This patch introduces the RNG schema and updates necessary data strucutures to allow various hypervisors to make use of Gluster protocol as one of the supported network disk backend. Next patch will add support to make use of this feature in Qemu since it now supports Gluster protocol as one of the network based storage backend. Two new optional attributes for <host> element are introduced - 'transport' and 'socket'. Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed. If transport is unix, socket specifies path to unix socket. Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 24 ++++++++++---- docs/schemas/domaincommon.rng | 33 +++++++++++++++---- src/conf/domain_conf.c | 77 ++++++++++++++++++++++++++++++++++--------- src/conf/domain_conf.h | 12 +++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 120 insertions(+), 28 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index bc4cc4a..be2ba55 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1369,10 +1369,10 @@ to the directory to use as the disk. If the disk <code>type</code> is "network", then the <code>protocol</code> attribute specifies the protocol to access to the requested image; possible values - are "nbd", "rbd", and "sheepdog". If the <code>protocol</code> - attribute is "rbd" or "sheepdog", an additional - attribute <code>name</code> is mandatory to specify which - image will be used. When the disk <code>type</code> is + are "nbd", "rbd", "sheepdog" or "gluster". If the + <code>protocol</code> attribute is "rbd", "sheepdog" or "gluster", an + additional attribute <code>name</code> is mandatory to specify which + volume/image will be used. When the disk <code>type</code> is "network", the <code>source</code> may have zero or more <code>host</code> sub-elements used to specify the hosts to connect. @@ -1600,9 +1600,11 @@ <span class='since'>Since 0.10.1</span> </dd> <dt><code>host</code></dt> - <dd>The <code>host</code> element has two attributes "name" and "port", - which specify the hostname and the port number. The meaning of this - element and the number of the elements depend on the protocol attribute. + <dd>The <code>host</code> element supports 4 attributes, viz. "name", + "port", "transport" and "socket", which specify the hostname, the port + number, transport type and path to socket, respectively. The meaning + of this element and the number of the elements depend on the protocol + attribute. <table class="top_table"> <tr> <th> Protocol </th> @@ -1624,7 +1626,15 @@ <td> one of the sheepdog servers (default is localhost:7000) </td> <td> zero or one </td> </tr> + <tr> + <td> gluster </td> + <td> a server running glusterd daemon </td> + <td> only one </td> + </tr> </table> + In case of gluster, valid values for transport attribute are tcp, rdma + or unix. If nothing is specified, tcp is assumed. If transport is unix, + socket attribute specifies path to unix socket. </dd> <dt><code>address</code></dt> <dd>If present, the <code>address</code> element ties the disk diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f47fdad..e7cfcb6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1048,6 +1048,7 @@ <value>nbd</value> <value>rbd</value> <value>sheepdog</value> + <value>gluster</value> </choice> </attribute> <optional> @@ -1055,12 +1056,32 @@ </optional> <zeroOrMore> <element name="host"> - <attribute name="name"> - <ref name="dnsName"/> - </attribute> - <attribute name="port"> - <ref name="unsignedInt"/> - </attribute> + <choice> + <group> + <optional> + <attribute name="transport"> + <choice> + <value>tcp</value> + <value>rdma</value> + </choice> + </attribute> + </optional> + <attribute name="name"> + <ref name="dnsName"/> + </attribute> + <attribute name="port"> + <ref name="unsignedInt"/> + </attribute> + </group> + <group> + <attribute name="transport"> + <value>unix</value> + </attribute> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </group> + </choice> </element> </zeroOrMore> <empty/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e1e7f..56986d6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -214,7 +214,13 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST, VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST, "nbd", "rbd", - "sheepdog") + "sheepdog", + "gluster") + +VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST, + "tcp", + "unix", + "rdma") VIR_ENUM_IMPL(virDomainDiskSecretType, VIR_DOMAIN_DISK_SECRET_TYPE_LAST, "none", @@ -994,6 +1000,7 @@ void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def) VIR_FREE(def->name); VIR_FREE(def->port); + VIR_FREE(def->socket); } void virDomainControllerDefFree(virDomainControllerDefPtr def) @@ -3460,6 +3467,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *source = NULL; char *target = NULL; char *protocol = NULL; + char *protocol_transport; char *trans = NULL; virDomainDiskHostDefPtr hosts = NULL; int nhosts = 0; @@ -3566,19 +3574,46 @@ virDomainDiskDefParseXML(virCapsPtr caps, } hosts[nhosts].name = NULL; hosts[nhosts].port = NULL; + hosts[nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + hosts[nhosts].socket = NULL; nhosts++; - hosts[nhosts - 1].name = virXMLPropString(child, "name"); - if (!hosts[nhosts - 1].name) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing name for host")); - goto error; + /* transport can be tcp (default), unix or rdma. */ + protocol_transport = virXMLPropString(child, "transport"); + if (protocol_transport != NULL) { + hosts[nhosts - 1].transport = virDomainDiskProtocolTransportTypeFromString(protocol_transport); + if (hosts[nhosts - 1].transport < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown protocol transport type '%s'"), + protocol_transport); + goto error; + } } - hosts[nhosts - 1].port = virXMLPropString(child, "port"); - if (!hosts[nhosts - 1].port) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing port for host")); - goto error; + hosts[nhosts - 1].socket = virXMLPropString(child, "socket"); + if (hosts[nhosts - 1].transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && + hosts[nhosts - 1].socket == NULL) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing socket for unix transport")); + } + if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && + hosts[nhosts - 1].socket != NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("transport %s do not support socket attribute"), + protocol_transport); + } + if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { + hosts[nhosts - 1].name = virXMLPropString(child, "name"); + if (!hosts[nhosts - 1].name) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing name for host")); + goto error; + } + hosts[nhosts - 1].port = virXMLPropString(child, "port"); + if (!hosts[nhosts - 1].port) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing port for host")); + goto error; + } } } child = child->next; @@ -11754,10 +11789,22 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); for (i = 0; i < def->nhosts; i++) { - virBufferEscapeString(buf, " <host name='%s'", - def->hosts[i].name); - virBufferEscapeString(buf, " port='%s'/>\n", - def->hosts[i].port); + virBufferAddLit(buf, " <host"); + if (def->hosts[i].name) { + virBufferEscapeString(buf, " name='%s'", def->hosts[i].name); + } + if (def->hosts[i].port) { + virBufferEscapeString(buf, " port='%s'", + def->hosts[i].port); + } + if (def->hosts[i].transport) { + virBufferAsprintf(buf, " transport='%s'", + virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport)); + } + if (def->hosts[i].socket) { + virBufferEscapeString(buf, " socket='%s'", def->hosts[i].socket); + } + virBufferAddLit(buf, "/>\n"); } virBufferAddLit(buf, " </source>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 14dead3..32cab91 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -456,10 +456,19 @@ enum virDomainDiskProtocol { VIR_DOMAIN_DISK_PROTOCOL_NBD, VIR_DOMAIN_DISK_PROTOCOL_RBD, VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG, + VIR_DOMAIN_DISK_PROTOCOL_GLUSTER, VIR_DOMAIN_DISK_PROTOCOL_LAST }; +enum virDomainDiskProtocolTransport { + VIR_DOMAIN_DISK_PROTO_TRANS_TCP, + VIR_DOMAIN_DISK_PROTO_TRANS_UNIX, + VIR_DOMAIN_DISK_PROTO_TRANS_RDMA, + + VIR_DOMAIN_DISK_PROTO_TRANS_LAST +}; + enum virDomainDiskTray { VIR_DOMAIN_DISK_TRAY_CLOSED, VIR_DOMAIN_DISK_TRAY_OPEN, @@ -481,6 +490,8 @@ typedef virDomainDiskHostDef *virDomainDiskHostDefPtr; struct _virDomainDiskHostDef { char *name; char *port; + int transport; /* enum virDomainDiskProtocolTransport */ + char *socket; /* path to unix socket */ }; enum virDomainDiskIo { @@ -2166,6 +2177,7 @@ VIR_ENUM_DECL(virDomainDiskBus) VIR_ENUM_DECL(virDomainDiskCache) VIR_ENUM_DECL(virDomainDiskErrorPolicy) VIR_ENUM_DECL(virDomainDiskProtocol) +VIR_ENUM_DECL(virDomainDiskProtocolTransport) VIR_ENUM_DECL(virDomainDiskIo) VIR_ENUM_DECL(virDomainDiskSecretType) VIR_ENUM_DECL(virDomainDiskTray) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dab607a..8e70837 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -350,6 +350,8 @@ virDomainDiskInsertPreAlloced; virDomainDiskIoTypeFromString; virDomainDiskIoTypeToString; virDomainDiskPathByName; +virDomainDiskProtocolTransportTypeFromString; +virDomainDiskProtocolTransportTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; virDomainDiskTypeFromString; -- 1.7.11.7

On Thu, Nov 22, 2012 at 23:40:38 +0530, Harsh Prateek Bora wrote:
This patch introduces the RNG schema and updates necessary data strucutures to allow various hypervisors to make use of Gluster protocol as one of the supported network disk backend. Next patch will add support to make use of this feature in Qemu since it now supports Gluster protocol as one of the network based storage backend.
Two new optional attributes for <host> element are introduced - 'transport' and 'socket'. Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed. If transport is unix, socket specifies path to unix socket.
I think the XML examples from 2/4 should be moved here.
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 24 ++++++++++---- docs/schemas/domaincommon.rng | 33 +++++++++++++++---- src/conf/domain_conf.c | 77 ++++++++++++++++++++++++++++++++++--------- src/conf/domain_conf.h | 12 +++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 120 insertions(+), 28 deletions(-) ... diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e1e7f..56986d6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c ... @@ -3460,6 +3467,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *source = NULL; char *target = NULL; char *protocol = NULL; + char *protocol_transport; char *trans = NULL; virDomainDiskHostDefPtr hosts = NULL; int nhosts = 0; @@ -3566,19 +3574,46 @@ virDomainDiskDefParseXML(virCapsPtr caps, } hosts[nhosts].name = NULL; hosts[nhosts].port = NULL; + hosts[nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + hosts[nhosts].socket = NULL; nhosts++;
- hosts[nhosts - 1].name = virXMLPropString(child, "name"); - if (!hosts[nhosts - 1].name) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing name for host")); - goto error; + /* transport can be tcp (default), unix or rdma. */ + protocol_transport = virXMLPropString(child, "transport"); + if (protocol_transport != NULL) { + hosts[nhosts - 1].transport = virDomainDiskProtocolTransportTypeFromString(protocol_transport); + if (hosts[nhosts - 1].transport < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown protocol transport type '%s'"), + protocol_transport); + goto error; + } }
This code would leak protocol_transport.
- hosts[nhosts - 1].port = virXMLPropString(child, "port"); - if (!hosts[nhosts - 1].port) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing port for host")); - goto error; + hosts[nhosts - 1].socket = virXMLPropString(child, "socket"); + if (hosts[nhosts - 1].transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && + hosts[nhosts - 1].socket == NULL) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing socket for unix transport")); + } + if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && + hosts[nhosts - 1].socket != NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("transport %s do not support socket attribute"),
s/do/does/
+ protocol_transport); + }
goto error is missing in the two error cases above.
+ if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { + hosts[nhosts - 1].name = virXMLPropString(child, "name"); + if (!hosts[nhosts - 1].name) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing name for host")); + goto error; + } + hosts[nhosts - 1].port = virXMLPropString(child, "port"); + if (!hosts[nhosts - 1].port) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing port for host")); + goto error; + } } } child = child->next; ...
I'm intentionally ignoring long lines in your patch since it would be quite hard to get rid of them due to the horrible code structure in virDomainDiskDefParseXML. The function should rather be refactored but that's a separate low-priority thing. ACK with the following patch squashed in: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 57c167e..2ca608f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3527,7 +3527,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *source = NULL; char *target = NULL; char *protocol = NULL; - char *protocol_transport; + char *protocol_transport = NULL; char *trans = NULL; virDomainDiskHostDefPtr hosts = NULL; int nhosts = 0; @@ -3654,13 +3654,16 @@ virDomainDiskDefParseXML(virCapsPtr caps, hosts[nhosts - 1].socket == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing socket for unix transport")); + goto error; } if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && hosts[nhosts - 1].socket != NULL) { virReportError(VIR_ERR_XML_ERROR, - _("transport %s do not support socket attribute"), + _("transport %s does not support socket attribute"), protocol_transport); + goto error; } + VIR_FREE(protocol_transport); if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { hosts[nhosts - 1].name = virXMLPropString(child, "name"); if (!hosts[nhosts - 1].name) { @@ -4265,6 +4268,7 @@ cleanup: } VIR_FREE(hosts); VIR_FREE(protocol); + VIR_FREE(protocol_transport); VIR_FREE(device); VIR_FREE(authUsername); VIR_FREE(usageType);

Qemu accepts gluster protocol as supported storage backend beside others. This patch allows users to specify disks on gluster backends like this: <disk type='network' device='disk'> <driver name='qemu' type='raw'/> <source protocol='gluster' name='Volume1/image'> <host name='example.org' port='6000' transport='tcp'/> </source> <target dev='vda' bus='virtio'/> </disk> <disk type='network' device='disk'> <driver name='qemu' type='raw'/> <source protocol='gluster' name='Volume2/image'> <host transport='unix' socket='/path/to/sock'/> </source> <target dev='vdb' bus='virtio'/> </disk> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 183 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 20730a9..a377744 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2021,6 +2021,156 @@ no_memory: return -1; } +static int +qemuParseGlusterString(virDomainDiskDefPtr def) +{ + int ret = 0; + char *transp = NULL; + char *sock = NULL; + char *volimg = NULL; + virURIPtr uri = NULL; + if (!(uri = virURIParse(def->src))) { + return -1; + } + + if (VIR_ALLOC(def->hosts) < 0) { + ret = -1; + goto no_memory; + } + + if (STREQ(uri->scheme, "gluster")) { + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + } else if (STRPREFIX(uri->scheme, "gluster+")) { + transp = strstr(uri->scheme, "+") + 1; + def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp); + if (def->hosts->transport < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid gluster transport type '%s'"), transp); + ret = -1; + goto cleanup; + + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid transport/scheme '%s'"), uri->scheme); + ret = -1; + goto cleanup; + } + def->nhosts = 0; /* set to 1 once everything succeeds */ + + if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { + def->hosts->name = strdup(uri->server); + if (!def->hosts->name) { + ret = -1; + goto no_memory; + } + + if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) { + ret = -1; + goto no_memory; + } + } else { + def->hosts->name = NULL; + def->hosts->port = 0; + if(uri->query) { + if(STRPREFIX(uri->query, "socket=")) { + sock = strstr(uri->query, "=") + 1; + def->hosts->socket = strdup(sock); + if (!def->hosts->socket) { + ret = -1; + goto no_memory; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid query parameter '%s'"), uri->query); + goto cleanup; + } + } + } + volimg = uri->path + 1; /* skip the prefix slash */ + def->src = strdup(volimg); + if (!def->src) { + ret = -1; + goto no_memory; + } + + def->nhosts = 1; + VIR_FREE(uri); + return ret; + +no_memory: + virReportOOMError(); +cleanup: + VIR_FREE(def->hosts); + VIR_FREE(uri); + + return ret; +} + +static int +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) +{ + int ret = 0, port = 0; + char *tmpscheme = NULL; + char *volimg = NULL; + char *sock = NULL; + char *builturi = NULL; + const char *transp = NULL; + virURI uri = { + .port = port /* just to clear rest of bits */ + }; + + if (disk->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster accepts only one host")); + return -1; + } + + virBufferAddLit(opt, "file="); + transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport); + + if (virAsprintf(&tmpscheme, "gluster+%s", transp) < 0) { + ret = -1; + goto no_memory; + } + + if (virAsprintf(&volimg, "/%s", disk->src) < 0) { + ret = -1; + goto no_memory; + } + + if (disk->hosts->port) { + port = atoi(disk->hosts->port); + } + + if (disk->hosts->socket) { + if (virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) { + ret = -1; + goto no_memory; + } + } + + uri.scheme = tmpscheme; /* gluster+<transport> */ + uri.server = disk->hosts->name; + uri.port = port; + uri.path = volimg; + uri.query = sock; + + builturi = virURIFormat(&uri); + virBufferEscape(opt, ',', ",", "%s", builturi); + goto cleanup; + +no_memory: + virReportOOMError(); +cleanup: + VIR_FREE(builturi); + VIR_FREE(tmpscheme); + VIR_FREE(volimg); + VIR_FREE(sock); + + return ret; +} + char * qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, @@ -2162,6 +2312,12 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, goto error; virBufferAddChar(&opt, ','); break; + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + if (qemuBuildGlusterString(disk, &opt) < 0) + goto error; + virBufferAddChar(&opt, ','); + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: if (disk->nhosts == 0) { virBufferEscape(&opt, ',', ",", "file=sheepdog:%s,", @@ -5242,6 +5398,18 @@ qemuBuildCommandLine(virConnectPtr conn, file = virBufferContentAndReset(&opt); } break; + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + { + virBuffer opt = VIR_BUFFER_INITIALIZER; + if (qemuBuildGlusterString(disk, &opt) < 0) + goto error; + if (virBufferError(&opt)) { + virReportOOMError(); + goto error; + } + file = virBufferContentAndReset(&opt); + } + break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: if (disk->nhosts == 0) { if (virAsprintf(&file, "sheepdog:%s,", disk->src) < 0) { @@ -6919,7 +7087,6 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } - VIR_FREE(def->src); def->src = NULL; } else if (STRPREFIX(def->src, "rbd:")) { @@ -6937,6 +7104,12 @@ qemuParseCommandLineDisk(virCapsPtr caps, goto cleanup; VIR_FREE(p); + } else if (STRPREFIX(def->src, "gluster")) { + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; + + if (qemuParseGlusterString(def) < 0) + goto cleanup; } else if (STRPREFIX(def->src, "sheepdog:")) { char *p = def->src; char *port, *vdi; @@ -6972,6 +7145,7 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } + def->src = strdup(vdi); if (!def->src) { virReportOOMError(); @@ -8126,6 +8300,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; val += strlen("rbd:"); + } else if (STRPREFIX(val, "gluster")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; } else if (STRPREFIX(val, "sheepdog:")) { disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; @@ -8211,6 +8388,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, goto no_memory; } break; + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + if (qemuParseGlusterString(disk) < 0) + goto error; + + break; } } -- 1.7.11.7

On Thu, Nov 22, 2012 at 23:40:39 +0530, Harsh Prateek Bora wrote:
Qemu accepts gluster protocol as supported storage backend beside others. This patch allows users to specify disks on gluster backends like this:
<disk type='network' device='disk'> <driver name='qemu' type='raw'/> <source protocol='gluster' name='Volume1/image'> <host name='example.org' port='6000' transport='tcp'/> </source> <target dev='vda' bus='virtio'/> </disk>
<disk type='network' device='disk'> <driver name='qemu' type='raw'/> <source protocol='gluster' name='Volume2/image'> <host transport='unix' socket='/path/to/sock'/> </source> <target dev='vdb' bus='virtio'/> </disk>
Move these XML examples to 1/4.
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 183 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 20730a9..a377744 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2021,6 +2021,156 @@ no_memory: return -1; }
+static int +qemuParseGlusterString(virDomainDiskDefPtr def) +{ + int ret = 0; + char *transp = NULL; + char *sock = NULL; + char *volimg = NULL; + virURIPtr uri = NULL; + if (!(uri = virURIParse(def->src))) { + return -1; + } + + if (VIR_ALLOC(def->hosts) < 0) { + ret = -1;
In libvirt, we rather initialize ret = -1 and set it to 0 at the end of the function when we know it succeeded. That way you don't have to remember to set ret = -1 on every error.
+ goto no_memory; + } + + if (STREQ(uri->scheme, "gluster")) { + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + } else if (STRPREFIX(uri->scheme, "gluster+")) { + transp = strstr(uri->scheme, "+") + 1; + def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp); + if (def->hosts->transport < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid gluster transport type '%s'"), transp); + ret = -1; + goto cleanup; + + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid transport/scheme '%s'"), uri->scheme); + ret = -1; + goto cleanup; + } + def->nhosts = 0; /* set to 1 once everything succeeds */ + + if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { + def->hosts->name = strdup(uri->server); + if (!def->hosts->name) { + ret = -1; + goto no_memory; + } + + if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) { + ret = -1; + goto no_memory; + } + } else { + def->hosts->name = NULL; + def->hosts->port = 0; + if(uri->query) { + if(STRPREFIX(uri->query, "socket=")) {
make syntax-check requires space between "if" and "(".
+ sock = strstr(uri->query, "=") + 1; + def->hosts->socket = strdup(sock); + if (!def->hosts->socket) { + ret = -1; + goto no_memory; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid query parameter '%s'"), uri->query);
And this is the evidence how easy it is to forget ret = -1 :-)
+ goto cleanup; + } + } + } + volimg = uri->path + 1; /* skip the prefix slash */ + def->src = strdup(volimg); + if (!def->src) { + ret = -1; + goto no_memory; + } + + def->nhosts = 1; + VIR_FREE(uri); + return ret; + +no_memory: + virReportOOMError(); +cleanup:
"cleanup" label is generally used for code which is common to both success and error paths. Use "error" label for error-path-only code.
+ VIR_FREE(def->hosts);
Before freeing def->hosts, all strings referenced from it need to be freed by calling virDomainDiskHostDefFree.
+ VIR_FREE(uri); + + return ret; +} + +static int +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) +{ + int ret = 0, port = 0;
Initialize ret to -1.
+ char *tmpscheme = NULL; + char *volimg = NULL; + char *sock = NULL; + char *builturi = NULL; + const char *transp = NULL; + virURI uri = { + .port = port /* just to clear rest of bits */ + }; + + if (disk->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster accepts only one host")); + return -1; + } + + virBufferAddLit(opt, "file="); + transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport); + + if (virAsprintf(&tmpscheme, "gluster+%s", transp) < 0) { + ret = -1; + goto no_memory; + } + + if (virAsprintf(&volimg, "/%s", disk->src) < 0) { + ret = -1; + goto no_memory; + } + + if (disk->hosts->port) { + port = atoi(disk->hosts->port); + } + + if (disk->hosts->socket) { + if (virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) { + ret = -1; + goto no_memory; + } + } + + uri.scheme = tmpscheme; /* gluster+<transport> */ + uri.server = disk->hosts->name; + uri.port = port; + uri.path = volimg; + uri.query = sock; + + builturi = virURIFormat(&uri); + virBufferEscape(opt, ',', ",", "%s", builturi); + goto cleanup; + +no_memory: + virReportOOMError(); +cleanup: + VIR_FREE(builturi); + VIR_FREE(tmpscheme); + VIR_FREE(volimg); + VIR_FREE(sock); + + return ret;
We like to keep the success path linear...
+} + char * qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, @@ -2162,6 +2312,12 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, goto error; virBufferAddChar(&opt, ','); break; + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + if (qemuBuildGlusterString(disk, &opt) < 0) + goto error; + virBufferAddChar(&opt, ','); + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: if (disk->nhosts == 0) { virBufferEscape(&opt, ',', ",", "file=sheepdog:%s,", @@ -5242,6 +5398,18 @@ qemuBuildCommandLine(virConnectPtr conn, file = virBufferContentAndReset(&opt); } break; + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + { + virBuffer opt = VIR_BUFFER_INITIALIZER; + if (qemuBuildGlusterString(disk, &opt) < 0) + goto error; + if (virBufferError(&opt)) { + virReportOOMError(); + goto error; + } + file = virBufferContentAndReset(&opt); + } + break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: if (disk->nhosts == 0) { if (virAsprintf(&file, "sheepdog:%s,", disk->src) < 0) { @@ -6919,7 +7087,6 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } - VIR_FREE(def->src); def->src = NULL; } else if (STRPREFIX(def->src, "rbd:")) {
Unrelated whitespace change.
@@ -6937,6 +7104,12 @@ qemuParseCommandLineDisk(virCapsPtr caps, goto cleanup;
VIR_FREE(p); + } else if (STRPREFIX(def->src, "gluster")) { + def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; + + if (qemuParseGlusterString(def) < 0) + goto cleanup; } else if (STRPREFIX(def->src, "sheepdog:")) { char *p = def->src; char *port, *vdi; @@ -6972,6 +7145,7 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } + def->src = strdup(vdi); if (!def->src) { virReportOOMError();
Unrelated.
@@ -8126,6 +8300,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; val += strlen("rbd:"); + } else if (STRPREFIX(val, "gluster")) { + disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; } else if (STRPREFIX(val, "sheepdog:")) { disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; @@ -8211,6 +8388,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, goto no_memory; } break; + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + if (qemuParseGlusterString(disk) < 0) + goto error; + + break; } }
ACK with the following patch squashed in: diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 886347c..63e187a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -346,6 +346,7 @@ virDomainDiskErrorPolicyTypeToString; virDomainDiskFindControllerModel; virDomainDiskGeometryTransTypeFromString; virDomainDiskGeometryTransTypeToString; +virDomainDiskHostDefFree; virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 50693a9..e946336 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2043,93 +2043,85 @@ no_memory: static int qemuParseGlusterString(virDomainDiskDefPtr def) { - int ret = 0; + int ret = -1; char *transp = NULL; char *sock = NULL; char *volimg = NULL; virURIPtr uri = NULL; + if (!(uri = virURIParse(def->src))) { return -1; } - if (VIR_ALLOC(def->hosts) < 0) { - ret = -1; + if (VIR_ALLOC(def->hosts) < 0) goto no_memory; - } if (STREQ(uri->scheme, "gluster")) { def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; } else if (STRPREFIX(uri->scheme, "gluster+")) { - transp = strstr(uri->scheme, "+") + 1; + transp = strchr(uri->scheme, '+') + 1; def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp); if (def->hosts->transport < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid gluster transport type '%s'"), transp); - ret = -1; - goto cleanup; - + goto error; } } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid transport/scheme '%s'"), uri->scheme); - ret = -1; - goto cleanup; + goto error; } def->nhosts = 0; /* set to 1 once everything succeeds */ if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { def->hosts->name = strdup(uri->server); - if (!def->hosts->name) { - ret = -1; + if (!def->hosts->name) goto no_memory; - } - if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) { - ret = -1; + if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) goto no_memory; - } } else { def->hosts->name = NULL; def->hosts->port = 0; - if(uri->query) { - if(STRPREFIX(uri->query, "socket=")) { - sock = strstr(uri->query, "=") + 1; + if (uri->query) { + if (STRPREFIX(uri->query, "socket=")) { + sock = strchr(uri->query, '=') + 1; def->hosts->socket = strdup(sock); - if (!def->hosts->socket) { - ret = -1; + if (!def->hosts->socket) goto no_memory; - } } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid query parameter '%s'"), uri->query); - goto cleanup; + goto error; } } } volimg = uri->path + 1; /* skip the prefix slash */ def->src = strdup(volimg); - if (!def->src) { - ret = -1; + if (!def->src) goto no_memory; - } def->nhosts = 1; - VIR_FREE(uri); + ret = 0; + +cleanup: + virURIFree(uri); + return ret; no_memory: virReportOOMError(); -cleanup: +error: + virDomainDiskHostDefFree(def->hosts); VIR_FREE(def->hosts); - VIR_FREE(uri); - - return ret; + goto cleanup; } static int qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) { - int ret = 0, port = 0; + int ret = -1; + int port = 0; char *tmpscheme = NULL; char *volimg = NULL; char *sock = NULL; @@ -2148,26 +2140,19 @@ qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) virBufferAddLit(opt, "file="); transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport); - if (virAsprintf(&tmpscheme, "gluster+%s", transp) < 0) { - ret = -1; + if (virAsprintf(&tmpscheme, "gluster+%s", transp) < 0) goto no_memory; - } - if (virAsprintf(&volimg, "/%s", disk->src) < 0) { - ret = -1; + if (virAsprintf(&volimg, "/%s", disk->src) < 0) goto no_memory; - } if (disk->hosts->port) { port = atoi(disk->hosts->port); } - if (disk->hosts->socket) { - if (virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) { - ret = -1; - goto no_memory; - } - } + if (disk->hosts->socket && + virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) + goto no_memory; uri.scheme = tmpscheme; /* gluster+<transport> */ uri.server = disk->hosts->name; @@ -2177,10 +2162,9 @@ qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) builturi = virURIFormat(&uri); virBufferEscape(opt, ',', ",", "%s", builturi); - goto cleanup; -no_memory: - virReportOOMError(); + ret = 0; + cleanup: VIR_FREE(builturi); VIR_FREE(tmpscheme); @@ -2188,6 +2172,10 @@ cleanup: VIR_FREE(sock); return ret; + +no_memory: + virReportOOMError(); + goto cleanup; } char *

struct _virDomainDiskHostDef recently got new members to support transport and socket attributes. Consumers of this struct therefore needs to initialize default values for them. This patch initializes default values for transport and socket for nbd, rbd and sheepdog storage backends. Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a377744..af2fcd5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1940,6 +1940,10 @@ static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport) disk->hosts[disk->nhosts-1].name = strdup(hostport); if (!disk->hosts[disk->nhosts-1].name) goto no_memory; + + disk->hosts[disk->nhosts-1].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + disk->hosts[disk->nhosts-1].socket = NULL; + return 0; no_memory: @@ -7087,6 +7091,8 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + def->hosts->socket = NULL; VIR_FREE(def->src); def->src = NULL; } else if (STRPREFIX(def->src, "rbd:")) { @@ -7145,6 +7151,8 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + def->hosts->socket = NULL; def->src = strdup(vdi); if (!def->src) { @@ -8848,6 +8856,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, VIR_FREE(hosts); goto no_memory; } + first_rbd_disk->hosts[first_rbd_disk->nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + first_rbd_disk->hosts[first_rbd_disk->nhosts].socket = NULL; + first_rbd_disk->nhosts++; token = strtok_r(NULL, ",", &saveptr); } -- 1.7.11.7

On Thu, Nov 22, 2012 at 23:40:40 +0530, Harsh Prateek Bora wrote:
struct _virDomainDiskHostDef recently got new members to support transport and socket attributes. Consumers of this struct therefore needs to initialize default values for them. This patch initializes default values for transport and socket for nbd, rbd and sheepdog storage backends.
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
This patch needs to be squashed into 1/4. Jirka

Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-gluster.args | 1 + .../qemuxml2argv-disk-drive-network-gluster.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 38 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 5e51d32..bcea8fe 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -183,6 +183,7 @@ mymain(void) DO_TEST("disk-drive-cache-directsync"); DO_TEST("disk-drive-cache-unsafe"); DO_TEST("disk-drive-network-nbd"); + DO_TEST("disk-drive-network-gluster"); DO_TEST("disk-drive-network-rbd"); /* older format using CEPH_ARGS env var */ DO_TEST("disk-drive-network-rbd-ceph-env"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args new file mode 100644 index 0000000..c07c8d0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=gluster+tcp://example.org:6000/Volume1/Image,if=virtio,format=raw -drive 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,if=virtio,format=raw' -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml new file mode 100644 index 0000000..e509b1b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.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/bin/qemu</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='gluster' name='Volume1/Image'> + <host name='example.org' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='gluster' name='Volume2/Image'> + <host transport='unix' socket='/path/to/sock'/> + </source> + <target dev='vdb' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0757e37..ad36758 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -469,6 +469,8 @@ mymain(void) QEMU_CAPS_DRIVE_CACHE_UNSAFE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-nbd", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); + DO_TEST("disk-drive-network-gluster", + QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-rbd", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-sheepdog", -- 1.7.11.7

On Thu, Nov 22, 2012 at 23:40:41 +0530, Harsh Prateek Bora wrote:
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-gluster.args | 1 + .../qemuxml2argv-disk-drive-network-gluster.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 38 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 5e51d32..bcea8fe 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -183,6 +183,7 @@ mymain(void) DO_TEST("disk-drive-cache-directsync"); DO_TEST("disk-drive-cache-unsafe"); DO_TEST("disk-drive-network-nbd"); + DO_TEST("disk-drive-network-gluster"); DO_TEST("disk-drive-network-rbd"); /* older format using CEPH_ARGS env var */ DO_TEST("disk-drive-network-rbd-ceph-env"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args new file mode 100644 index 0000000..c07c8d0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=gluster+tcp://example.org:6000/Volume1/Image,if=virtio,format=raw -drive 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,if=virtio,format=raw' -net none -serial none -parallel none -usb
One of the recent patches moved -usb earlier in the command line (just after "-boot c" in this case)... ACK with the small tweak. Jirka

On Thu, Nov 22, 2012 at 23:40:37 +0530, Harsh Prateek Bora wrote:
Changelog:
v5: - Separated initialization of new host members for nbd, rbd, sheepdog in a separate patch [Patch 3/4] - Fixed tests for appropriate args to DO_TEST - Code cleanups for squashing pointer incrememnts into single line.
v4: - Addressed review comment by Jiri on v3 updated series.
v3 updated: - Fix other network backends (nbd, rbd, sheepdog) to initialize new members (transport, socket) of _virDomainDiskHostDef appropriately so that garbage values doesnt break the argv2xml tests.
v3: - RNG schema updated as required for unix transport [Paolo] - introduced another new attribute 'socket' for unix transport [Paolo] - Uses virURIFormat and virURIParse for URI parsing. [danpb] - updated documentation as required. [Jirka]
v2: - Addressed review comments by Jiri - Updated patcheset as per new URI spec Ref: http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg05199.html
v1: - Initial prototype
OK, I fixed the remaining small issues and pushed this series. Thanks for your work. Jirka

On 11/27/2012 03:01 PM, Jiri Denemark wrote:
On Thu, Nov 22, 2012 at 23:40:37 +0530, Harsh Prateek Bora wrote:
Changelog:
v5: - Separated initialization of new host members for nbd, rbd, sheepdog in a separate patch [Patch 3/4] - Fixed tests for appropriate args to DO_TEST - Code cleanups for squashing pointer incrememnts into single line.
v4: - Addressed review comment by Jiri on v3 updated series.
v3 updated: - Fix other network backends (nbd, rbd, sheepdog) to initialize new members (transport, socket) of _virDomainDiskHostDef appropriately so that garbage values doesnt break the argv2xml tests.
v3: - RNG schema updated as required for unix transport [Paolo] - introduced another new attribute 'socket' for unix transport [Paolo] - Uses virURIFormat and virURIParse for URI parsing. [danpb] - updated documentation as required. [Jirka]
v2: - Addressed review comments by Jiri - Updated patcheset as per new URI spec Ref: http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg05199.html
v1: - Initial prototype
OK, I fixed the remaining small issues and pushed this series. Thanks for your work.
Hi Jirka, Thanks for the detailed reviews and pushing the series with required fixes. It was a true pleasure to work with you and everyone involved. regards, Harsh
Jirka
participants (3)
-
Harsh Bora
-
Harsh Prateek Bora
-
Jiri Denemark