gluster-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Gluster-devel] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as


From: Deepak C Shetty
Subject: Re: [Gluster-devel] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
Date: Thu, 04 Oct 2012 21:17:08 +0530
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1

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 <address@hidden>
---
  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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]