[libvirt] [PATCH v3 UPDATED 0/2] Qemu/Gluster support in Libvirt
Changelog: 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 (2): Qemu/Gluster: Add Gluster protocol as supported network disk formats. tests: Add tests for gluster protocol based network disks support docs/formatdomain.html.in | 24 ++- docs/schemas/domaincommon.rng | 35 ++++- src/conf/domain_conf.c | 72 +++++++-- src/conf/domain_conf.h | 12 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 168 ++++++++++++++++++++- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-gluster.args | 1 + .../qemuxml2argv-disk-drive-network-gluster.xml | 35 +++++ tests/qemuxml2argvtest.c | 2 + 10 files changed, 323 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
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> In the <host> element above, transport is a new optional attribute. Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed. If transport type is unix, another new optional attribute socket specifies path to unix socket: <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> --- docs/formatdomain.html.in | 24 ++++-- docs/schemas/domaincommon.rng | 35 +++++++-- src/conf/domain_conf.c | 72 ++++++++++++++---- src/conf/domain_conf.h | 12 +++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 168 +++++++++++++++++++++++++++++++++++++++++- 6 files changed, 284 insertions(+), 29 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..ea6bc54 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,15 +1056,35 @@ </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/> + <empty/> </element> </optional> <ref name="diskspec"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e1e7f..52a965d 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,43 @@ 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_INTERNAL_ERROR, + _("unknown protocol transport type '%s'"), + protocol_transport); + goto error; + } } - hosts[nhosts - 1].port = virXMLPropString(child, "port"); - if (!hosts[nhosts - 1].port) { + 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_INTERNAL_ERROR, - "%s", _("missing port for host")); - goto error; + "%s", _("missing socket for unix 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_INTERNAL_ERROR, + "%s", _("missing name for host")); + 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; + } } } child = child->next; @@ -11754,10 +11786,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 >= 0) { + 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; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 20730a9..f4fdd67 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 = -1; + disk->hosts[disk->nhosts-1].socket = NULL; + return 0; no_memory: @@ -2021,6 +2025,127 @@ no_memory: return -1; } +static int qemuParseGlusterString(virDomainDiskDefPtr def) +{ + char *uritoparse = strdup(def->src); + char *transp = NULL; + char *sock = NULL; + virURIPtr uri = NULL; + uri = virURIParse(uritoparse); + + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + return -1; + } + + if (STREQ(uri->scheme, "gluster")) { + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + } else if (strstr(uri->scheme, "+")) { + transp = strstr(uri->scheme, "+"); + transp++; + def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp); + if (def->hosts->transport < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid gluster transport type '%s'"), transp); + return -1; + + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid transport/scheme '%s'"), uri->scheme); + } + + def->nhosts = 1; + def->hosts->name = strdup(uri->server); + if (!def->hosts->name) { + virReportOOMError(); + return -1; + } + + if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) { + virReportOOMError(); + return -1; + } + if(STRPREFIX(uri->query, "socket=")) { + sock = strstr(uri->query, "="); + sock++; + def->hosts->socket = strdup(sock); + if (!def->hosts->socket) { + virReportOOMError(); + return -1; + } + + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid query parameter '%s'"), uri->query); + + } + + def->src = strdup(uri->path); + if (!def->src) { + virReportOOMError(); + return -1; + } + + VIR_FREE(uritoparse); + + return 0; +} + +static int +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) +{ + int ret = 0, port = 0; + char *tmpscheme = NULL; + char *volimg = NULL; + char *sock = NULL; + char *builturi = NULL; + + virBufferAddLit(opt, "file="); + if (disk->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster accepts only one host")); + ret = -1; + } else { + + if (virAsprintf(&tmpscheme, "gluster+%s", + virDomainDiskProtocolTransportTypeToString(disk->hosts->transport)) < 0) { + virReportOOMError(); + return -1; + } + if (virAsprintf(&volimg, "/%s", disk->src) < 0) { + virReportOOMError(); + return -1; + } + + if (disk->hosts->port) { + port = atoi(disk->hosts->port); + } + + if (disk->hosts->socket) { + if (virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) { + virReportOOMError(); + return -1; + } + } + virURI uri = { + .scheme = tmpscheme, /* gluster+<transport> */ + .server = disk->hosts->name, + .port = port, + .path = volimg, + .query = sock, + }; + + builturi = virURIFormat(&uri); + virBufferAsprintf(opt, "%s", builturi); + + VIR_FREE(tmpscheme); + VIR_FREE(volimg); + VIR_FREE(sock); + } + return ret; +} + char * qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, @@ -2162,6 +2287,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 +5373,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 +7062,8 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } - + def->hosts->transport = -1; + def->hosts->socket = NULL; VIR_FREE(def->src); def->src = NULL; } else if (STRPREFIX(def->src, "rbd:")) { @@ -6937,6 +7081,14 @@ 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 +7124,9 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } + def->hosts->transport = -1; + def->hosts->socket = NULL; + def->src = strdup(vdi); if (!def->src) { virReportOOMError(); @@ -8126,6 +8281,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 +8369,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, goto no_memory; } break; + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + if (qemuParseGlusterString(disk) < 0) + goto error; + + break; } } @@ -8666,6 +8829,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, VIR_FREE(hosts); goto no_memory; } + first_rbd_disk->hosts[first_rbd_disk->nhosts].transport = -1; + first_rbd_disk->hosts[first_rbd_disk->nhosts].socket = NULL; + first_rbd_disk->nhosts++; token = strtok_r(NULL, ",", &saveptr); } -- 1.7.11.7
On Fri, Oct 26, 2012 at 10:27 PM, Harsh Prateek Bora <harsh@linux.vnet.ibm.com> wrote:
@@ -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>
Its actually the server which contains the volume file specification of the gluster volume Regards, Bharata.
On 10/27/2012 11:29 AM, Bharata B Rao wrote:
On Fri, Oct 26, 2012 at 10:27 PM, Harsh Prateek Bora <harsh@linux.vnet.ibm.com> wrote:
@@ -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>
Its actually the server which contains the volume file specification of the gluster volume
If a server is part of storage pool (ie, gluster peer probe is successful), the server will have 'volume file' in it, even if it is not contributing any exports from that server. So, I think the above definition still holds good. Regards, Amar
On Fri, Oct 26, 2012 at 22:27:35 +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>
In the <host> element above, transport is a new optional attribute. Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed. If transport type is unix, another new optional attribute socket specifies path to unix socket:
<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>
We agreed with Daniel that this XML schema is just good enough. ...
docs/formatdomain.html.in | 24 ++++-- docs/schemas/domaincommon.rng | 35 +++++++-- src/conf/domain_conf.c | 72 ++++++++++++++---- src/conf/domain_conf.h | 12 +++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 168 +++++++++++++++++++++++++++++++++++++++++-
Separate all changes in src/qemu/qemu_command.c into a new patch that just implements gluster support in qemu driver. Unless doing so would break the build in between of course (but I believe it should not break it). ...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f47fdad..ea6bc54 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng ... @@ -1055,15 +1056,35 @@ </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/> + <empty/>
Looks like an accidental whitespace change.
</element> </optional> <ref name="diskspec"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e1e7f..52a965d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
...
@@ -3566,19 +3574,43 @@ 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_INTERNAL_ERROR,
I know we use VIR_ERR_INTERNAL_ERROR for this kind of error in most of our current code but that's not right and we should not be adding more of them. The correct error code for this is 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) { + 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_INTERNAL_ERROR,
VIR_ERR_XML_ERROR
- "%s", _("missing port for host")); - goto error; + "%s", _("missing socket for unix transport")); + }
Since the RNG schema forbids socket attribute with transport != unix, we should forbid that in the code as well.
+ + if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { +
We don't need this empty line.
+ hosts[nhosts - 1].name = virXMLPropString(child, "name"); + if (!hosts[nhosts - 1].name) { + virReportError(VIR_ERR_INTERNAL_ERROR,
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_INTERNAL_ERROR,
VIR_ERR_XML_ERROR
+ "%s", _("missing port for host")); + goto error; + } } } child = child->next; @@ -11754,10 +11786,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 >= 0) {
Just change this to ``if (def->hosts[i].transport)'' to avoid the need for using magic -1 value for initializing transport further in the code.
+ 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/qemu/qemu_command.c b/src/qemu/qemu_command.c index 20730a9..f4fdd67 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 = -1;
Just set it to TRANS_TCP. It uses tcp transport, after all.
+ disk->hosts[disk->nhosts-1].socket = NULL; + return 0;
no_memory: @@ -2021,6 +2025,127 @@ no_memory: return -1; }
+static int qemuParseGlusterString(virDomainDiskDefPtr def)
New line after "static int".
+{ + char *uritoparse = strdup(def->src); + char *transp = NULL; + char *sock = NULL; + virURIPtr uri = NULL; + uri = virURIParse(uritoparse);
This code does not check for strdup or virURIParse failures. And the function leaks uritoparse and def->hosts on error and leaks uri in all cases. It should be rewritten to use cleanup label. And why do you need to strdup(def->src) in the first place? Just using it directly in virURIParse should work or am I missing something?
+ + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + return -1; + } + + if (STREQ(uri->scheme, "gluster")) { + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + } else if (strstr(uri->scheme, "+")) {
This should rather check for STRPREFIX(uri->scheme, "gluster+")
+ transp = strstr(uri->scheme, "+"); + transp++;
You could even squash the increment into the previous line :-)
+ def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp); + if (def->hosts->transport < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid gluster transport type '%s'"), transp); + return -1; +
Unneeded empty line.
+ } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid transport/scheme '%s'"), uri->scheme);
Four spaces of indentation would have been enough :-)
+ } + + def->nhosts = 1;
I'd move this line just before ret = 0 (after you modify the code to use cleanup label) so that you can always free def->hosts on error path without confusing the caller with hosts == NULL and nhosts == 1.
+ def->hosts->name = strdup(uri->server); + if (!def->hosts->name) { + virReportOOMError(); + return -1; + } + + if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) { + virReportOOMError(); + return -1; + }
I'd move one of the extra empty lines here :-)
+ if(STRPREFIX(uri->query, "socket=")) { + sock = strstr(uri->query, "="); + sock++; + def->hosts->socket = strdup(sock); + if (!def->hosts->socket) { + virReportOOMError(); + return -1; + } +
Extra empty line.
+ } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid query parameter '%s'"), uri->query); +
Extra empty line.
+ } + + def->src = strdup(uri->path); + if (!def->src) { + virReportOOMError(); + return -1; + } + + VIR_FREE(uritoparse); + + return 0; +} + +static int +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) +{ + int ret = 0, port = 0; + char *tmpscheme = NULL; + char *volimg = NULL; + char *sock = NULL; + char *builturi = NULL; + + virBufferAddLit(opt, "file="); + if (disk->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster accepts only one host"));
Extra space, "_" should be aligned with "V" in VIR_ERR_INTERNAL_ERROR.
+ ret = -1;
Heh, why don't you just return -1 to avoid the need for else branch and additional indentation level? Also it would make more sense to do this check before adding "file=" to the buffer although it does not really matter.
+ } else { +
Extra empty line.
+ if (virAsprintf(&tmpscheme, "gluster+%s", + virDomainDiskProtocolTransportTypeToString(disk->hosts->transport)) < 0) {
One space is missing, "virDomainDisk..." should be aligned with "&". But anyway, I'd use a temporary variable for transport to make this long line shorter.
+ virReportOOMError(); + return -1; + } + if (virAsprintf(&volimg, "/%s", disk->src) < 0) { + virReportOOMError(); + return -1; + } + + if (disk->hosts->port) { + port = atoi(disk->hosts->port); + } + + if (disk->hosts->socket) { + if (virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) { + virReportOOMError(); + return -1; + } + } + virURI uri = { + .scheme = tmpscheme, /* gluster+<transport> */ + .server = disk->hosts->name, + .port = port, + .path = volimg, + .query = sock, + }; + + builturi = virURIFormat(&uri); + virBufferAsprintf(opt, "%s", builturi);
I think this should rather call virBufferEscape(opt, ',', ",", "%s", builturi); so that a possible ',' in volume name does not confuse qemu.
+ + VIR_FREE(tmpscheme); + VIR_FREE(volimg); + VIR_FREE(sock); + } + return ret;
Again, the function may leak several strings in error case and always leaks builturi.
+} + char * qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, ... @@ -6919,7 +7062,8 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } - + def->hosts->transport = -1;
Initialize to TRANSP_TCP.
+ def->hosts->socket = NULL; VIR_FREE(def->src); def->src = NULL; } else if (STRPREFIX(def->src, "rbd:")) { @@ -6937,6 +7081,14 @@ qemuParseCommandLineDisk(virCapsPtr caps, goto cleanup;
VIR_FREE(p); + } else if (STRPREFIX(def->src, "gluster")) { +
Extra empty line.
+ def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; + + if (qemuParseGlusterString(def) < 0) + goto cleanup; +
Extra empty line.
} else if (STRPREFIX(def->src, "sheepdog:")) { char *p = def->src; char *port, *vdi; @@ -6972,6 +7124,9 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } + def->hosts->transport = -1;
TRANSP_TCP
+ def->hosts->socket = NULL; + def->src = strdup(vdi); if (!def->src) { virReportOOMError(); ... @@ -8666,6 +8829,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, VIR_FREE(hosts); goto no_memory; } + first_rbd_disk->hosts[first_rbd_disk->nhosts].transport = -1;
TRANSP_TCP
+ first_rbd_disk->hosts[first_rbd_disk->nhosts].socket = NULL; + first_rbd_disk->nhosts++; token = strtok_r(NULL, ",", &saveptr); }
OK, quite a lot of non-trivial changes make me request a new version. Jirka
On 11/16/2012 03:53 AM, Jiri Denemark wrote:
On Fri, Oct 26, 2012 at 22:27:35 +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>
In the <host> element above, transport is a new optional attribute. Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed. If transport type is unix, another new optional attribute socket specifies path to unix socket:
<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>
We agreed with Daniel that this XML schema is just good enough.
...
docs/formatdomain.html.in | 24 ++++-- docs/schemas/domaincommon.rng | 35 +++++++-- src/conf/domain_conf.c | 72 ++++++++++++++---- src/conf/domain_conf.h | 12 +++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 168 +++++++++++++++++++++++++++++++++++++++++-
Separate all changes in src/qemu/qemu_command.c into a new patch that just implements gluster support in qemu driver. Unless doing so would break the build in between of course (but I believe it should not break it).
...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f47fdad..ea6bc54 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng ... @@ -1055,15 +1056,35 @@ </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/> + <empty/>
Looks like an accidental whitespace change.
</element> </optional> <ref name="diskspec"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e1e7f..52a965d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
...
@@ -3566,19 +3574,43 @@ 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_INTERNAL_ERROR,
I know we use VIR_ERR_INTERNAL_ERROR for this kind of error in most of our current code but that's not right and we should not be adding more of them. The correct error code for this is 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) { + 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_INTERNAL_ERROR,
VIR_ERR_XML_ERROR
- "%s", _("missing port for host")); - goto error; + "%s", _("missing socket for unix transport")); + }
Since the RNG schema forbids socket attribute with transport != unix, we should forbid that in the code as well.
+ + if (hosts[nhosts - 1].transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { +
We don't need this empty line.
+ hosts[nhosts - 1].name = virXMLPropString(child, "name"); + if (!hosts[nhosts - 1].name) { + virReportError(VIR_ERR_INTERNAL_ERROR,
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_INTERNAL_ERROR,
VIR_ERR_XML_ERROR
+ "%s", _("missing port for host")); + goto error; + } } } child = child->next; @@ -11754,10 +11786,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 >= 0) {
Just change this to ``if (def->hosts[i].transport)'' to avoid the need for using magic -1 value for initializing transport further in the code.
+ 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/qemu/qemu_command.c b/src/qemu/qemu_command.c index 20730a9..f4fdd67 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 = -1;
Just set it to TRANS_TCP. It uses tcp transport, after all.
+ disk->hosts[disk->nhosts-1].socket = NULL; + return 0;
no_memory: @@ -2021,6 +2025,127 @@ no_memory: return -1; }
+static int qemuParseGlusterString(virDomainDiskDefPtr def)
New line after "static int".
+{ + char *uritoparse = strdup(def->src); + char *transp = NULL; + char *sock = NULL; + virURIPtr uri = NULL; + uri = virURIParse(uritoparse);
This code does not check for strdup or virURIParse failures. And the function leaks uritoparse and def->hosts on error and leaks uri in all cases. It should be rewritten to use cleanup label. And why do you need to strdup(def->src) in the first place? Just using it directly in virURIParse should work or am I missing something?
+ + if (VIR_ALLOC(def->hosts) < 0) { + virReportOOMError(); + return -1; + } + + if (STREQ(uri->scheme, "gluster")) { + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + } else if (strstr(uri->scheme, "+")) {
This should rather check for STRPREFIX(uri->scheme, "gluster+")
+ transp = strstr(uri->scheme, "+"); + transp++;
You could even squash the increment into the previous line :-)
That would give me: qemu/qemu_command.c: In function 'qemuParseGlusterString': qemu/qemu_command.c:2048:18: error: lvalue required as left operand of assignment. I have updated patches for the rest of comments and shall be posting soon. Harsh
On 11/22/2012 01:49 AM, Harsh Bora wrote:
+ transp = strstr(uri->scheme, "+");
strstr() for a single-byte needle is overkill.
+ transp++;
You could even squash the increment into the previous line :-)
That would give me:
qemu/qemu_command.c: In function 'qemuParseGlusterString': qemu/qemu_command.c:2048:18: error: lvalue required as left operand of assignment.
Not if you do: transp = strchr(uri->scheme, '+') + 1; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/22/2012 07:05 PM, Eric Blake wrote:
On 11/22/2012 01:49 AM, Harsh Bora wrote:
+ transp = strstr(uri->scheme, "+");
strstr() for a single-byte needle is overkill.
+ transp++;
You could even squash the increment into the previous line :-)
That would give me:
qemu/qemu_command.c: In function 'qemuParseGlusterString': qemu/qemu_command.c:2048:18: error: lvalue required as left operand of assignment.
Not if you do:
transp = strchr(uri->scheme, '+') + 1;
Oh yes, thanks.
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 | 35 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 39 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..e11a9d2 --- /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 -no-kqemu -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..50619e7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.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/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' 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> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0757e37..20ed7cb 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", false, + 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 Fri, Oct 26, 2012 at 22:27:36 +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 | 35 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 39 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..e11a9d2 --- /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 -no-kqemu -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..50619e7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.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/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' 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> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0757e37..20ed7cb 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", false, + 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",
ACK after removing the "transport='tcp'" attribute after implementing the changes I suggested in 1/2. Jirka
Hi Daniel, Does this patchset look good to you now ? Jiri and Paolo, Would you like to formally ack the patchset if there are no further changes required ? regards, Harsh On 10/26/2012 10:27 PM, Harsh Prateek Bora wrote:
Changelog:
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 (2): Qemu/Gluster: Add Gluster protocol as supported network disk formats. tests: Add tests for gluster protocol based network disks support
docs/formatdomain.html.in | 24 ++- docs/schemas/domaincommon.rng | 35 ++++- src/conf/domain_conf.c | 72 +++++++-- src/conf/domain_conf.h | 12 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 168 ++++++++++++++++++++- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-gluster.args | 1 + .../qemuxml2argv-disk-drive-network-gluster.xml | 35 +++++ tests/qemuxml2argvtest.c | 2 + 10 files changed, 323 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
participants (6)
-
Amar Tumballi -
Bharata B Rao -
Eric Blake -
Harsh Bora -
Harsh Prateek Bora
-
Jiri Denemark