[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