[Gluster-devel] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
Deepak C Shetty
deepakcs at linux.vnet.ibm.com
Thu Oct 4 15:47:08 UTC 2012
On 10/04/2012 07:01 PM, 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='volume/image'>
> <host name='example.org' port='6000' transport='tcp'/>
> </source>
> <target dev='vda' bus='virtio'/>
> </disk>
>
> Note: In the <host> element above, transport is an optional attribute.
> Valid transport values are tcp, unix or rdma. If none specified, tcp is assumed.
> If transport type is unix, host name specifies path to unix socket.
>
> Signed-off-by: Harsh Prateek Bora <harsh at linux.vnet.ibm.com>
> ---
> docs/schemas/domaincommon.rng | 8 ++
> src/conf/domain_conf.c | 28 +++++-
> src/conf/domain_conf.h | 11 +++
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_command.c | 204 ++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 251 insertions(+), 2 deletions(-)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f47fdad..89d9b9f 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>
> @@ -1061,6 +1062,13 @@
> <attribute name="port">
> <ref name="unsignedInt"/>
> </attribute>
> + <attribute name="transport">
> + <choice>
> + <value>tcp</value>
> + <value>unix</value>
> + <value>rdma</value>
> + </choice>
> + </attribute>
'transport' attribute is optional, so it should be placed inside
<optional> </optional> ?
> </element>
> </zeroOrMore>
> <empty/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 33e1e7f..838f079 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",
> @@ -3460,6 +3466,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,6 +3573,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> }
> hosts[nhosts].name = NULL;
> hosts[nhosts].port = NULL;
> + hosts[nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
> nhosts++;
>
> hosts[nhosts - 1].name = virXMLPropString(child, "name");
> @@ -3580,6 +3588,17 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> "%s", _("missing port 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;
> + }
> + }
> }
> child = child->next;
> }
> @@ -11756,8 +11775,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
> for (i = 0; i < def->nhosts; i++) {
> virBufferEscapeString(buf, " <host name='%s'",
> def->hosts[i].name);
> - virBufferEscapeString(buf, " port='%s'/>\n",
> + virBufferEscapeString(buf, " port='%s'",
> def->hosts[i].port);
> + if (def->hosts[i].transport) {
> + virBufferAsprintf(buf, " transport='%s'",
> + virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport));
> + }
> + virBufferAddLit(buf, "/>\n");
> }
> virBufferAddLit(buf, " </source>\n");
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 14dead3..7ba7e9b 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,7 @@ typedef virDomainDiskHostDef *virDomainDiskHostDefPtr;
> struct _virDomainDiskHostDef {
> char *name;
> char *port;
> + int transport; /* enum virDomainDiskProtocolTransport */
> };
>
> enum virDomainDiskIo {
> @@ -2166,6 +2176,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..0bed2bc 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2021,6 +2021,168 @@ no_memory:
> return -1;
> }
>
> +static int qemuParseGlusterString(virDomainDiskDefPtr def)
> +{
> + char *host = NULL, *port = NULL, *volimg, *transp = NULL, *marker;
> +
> + if (STRPREFIX(def->src, "+")) {
> + transp = def->src;
> + transp++;
> + marker = strstr(def->src, "://");
> + *marker++ = '\0';
> + marker += 2;
> + } else {
> + /* transport type not specified, use tcp as default */
> + def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
> + marker = strstr(def->src, "://");
> + marker += 3;
> + }
> +
> + if (transp) {
> + def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp);
> + if (def->hosts->transport < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid gluster transport type '%s'"), transp);
> + return -1;
> + }
> + }
> +
> + /* now marker points to string which can start with one of the following:
> + * IPv6 address in square brackets followed by port (optional)
> + * <hostname> or <IPv4 address> followed by port (optional)
> + * '/' if its a unix socket followed by path to gluster disk volume/image
> + */
> +
> + /* parse server, port */
> + if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (STRPREFIX(marker, "[")) {
> + /* IPv6 addr */
> + host = ++marker;
> + marker = strchr(host, ']');
> + if (!marker) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse IPv6 addr for gluster host %s "), host);
> + return -1;
> + }
> + *marker++ = '\0';
> + /* parse port if specified */
> + if (STRPREFIX(marker, ":")) {
> + port = ++marker;
> + marker = strchr(port, '/');
> + if (!marker) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse filename for gluster disk %s "), port);
> + return -1;
> + }
> + *marker++ = '\0';
> + } else {
> + marker++; /* port not specified, skip slash */
> + }
> + } else {
> + /* IPv4 address / hostname followed by port (optional) */
> + host = marker;
> + marker = strchr(host, '/'); /* skip to path to gluster disk vol/img */
> + if (!marker) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse filename for gluster disk %s "), port);
> + return -1;
> + }
> + *marker++ = '\0';
> + port = strchr(host, ':');
> + if (port) {
> + /* port was specified with host, separate both */
> + *port++ = '\0';
> + }
> + }
> +
> + /* host points to hostname / IPv4 / IPv6 addr
> + * port points to port or NULL is port was not specified
> + */
> + } else {
> + /* transport type is unix, expect one more slash
> + * followed by path to gluster disk vol/img */
> + if (STRPREFIX(marker, "/")) {
> + marker++;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Gluster unix transport url starts with 3 slashes i.e. gluster+unix:///"));
> + return -1;
> + }
> + }
> +
> + /* marker now points to path to gluster disk vol/img */
> + volimg = marker;
> +
> + /* if transport type = unix, path to gluster disk vol/img
> + * is followed by ?socket=<path/to/socket> */
> + if (def->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (strstr(marker, "?socket=")) {
> + /* In libvirt xml, socket path is to be provided as
> + * <host name='/path/to/socket' port='0'>
> + */
> + host = strchr(marker, '=');
> + *host++ = '\0';
> + }
> + }
> +
> + if (VIR_ALLOC(def->hosts) < 0) {
> + virReportOOMError();
> + return -1;
> + }
> + def->nhosts = 1;
> + def->hosts->name = host;
> + if (port) {
> + def->hosts->port = port;
> + } else {
> + def->hosts->port = strdup("0");
> + }
> + if (!def->hosts->port) {
> + virReportOOMError();
> + return -1;
> + }
> + def->src = strdup(volimg);
> + if (!def->src) {
> + virReportOOMError();
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
> +{
> + int ret = 0;
> + virBufferAddLit(opt, "file=");
> + if (disk->nhosts != 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("gluster accepts only one host"));
> + ret = -1;
> + } else {
> + virBufferAsprintf(opt, "gluster+%s://",
> + virDomainDiskProtocolTransportTypeToString(disk->hosts->transport));
> +
> + /* if transport type is not unix, specify server:port */
> + if (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + if (strstr(disk->hosts->name, ":")) {
Wouldn't it be better to check for ':' and check for absence of "." (and
vice versa in the else) so that if someone specified a.b.c:d
or a:b:c:d:e.f we can throw error rite away, instead of qemu erroring
out later in time ? Its a very primtive check but
can help to catch user input error early enuf.
> + /* if IPv6 addr, use square brackets to enclose it */
> + virBufferAsprintf(opt, "[%s]:%s", disk->hosts->name, disk->hosts->port);
> + } else {
> + virBufferAsprintf(opt, "%s:%s", disk->hosts->name, disk->hosts->port);
> + }
> + }
> +
> + /* append source path to gluster disk image */
> + virBufferAsprintf(opt, "/%s", disk->src);
> +
> + /* if transport type is unix, server name is path to unix socket, ignore port */
> + if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> + virBufferAsprintf(opt, "?socket=%s", disk->hosts->name);
> + }
This can be clubbed as part of else clause to the above if condn (if
(disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that ways
we avoid an un-necessary check of transport here.
It also means that disk->src needs to be pulled inside the if & else
clauses, which I feel should be ok.
More information about the Gluster-devel
mailing list