[Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Prasanna Kumar Kalever]

This patch adds a way to specify multiple backup volfile servers to the gluster
block backend of QEMU with both tcp and rdma transport types.

Problem:

Currenly VM Image on gluster volume is specified like this:

file=gluster[+tcp]://server1:24007/testvol/a.img

Assuming we have have three servers in trustred pool with replica 3 volume
in action and unfortunately server1 (mentioned in the command above) went down
for some reason, since the volume is replica 3 we now have other 2 servers
active from which we can boot the VM.

But currently there is no mechanism to pass the other 2 gluster server
addresses to qemu.

Solution:

New way of specifying VM Image on gluster volume with backup volfile servers:

file=gluster[+transport-type]://server1:24007/testvol/a.img\
     ?backup-volfile-servers=server2&backup-volfile-servers=server3

This patch gives a mechanism to provide all the server addresses which are in
replica set, so in case server1 is down VM can still boot from any of the
active servers.

This is equivalent to the backup-volfile-servers option supported by
mount.glusterfs (FUSE way of mounting gluster volume)

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
---
 block/gluster.c | 118 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 83 insertions(+), 35 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 1eb3a8c..ad2fb94 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -25,17 +25,23 @@ typedef struct BDRVGlusterState {
 } BDRVGlusterState;
 
 typedef struct GlusterConf {
-    char *server;
+    GList *server;
     int port;
     char *volname;
     char *image;
     char *transport;
 } GlusterConf;
 
+typedef struct GlusterOptions {
+    struct glfs *glfs;
+    GlusterConf *gconf;
+} GlusterOptions;
+
+
 static void qemu_gluster_gconf_free(GlusterConf *gconf)
 {
     if (gconf) {
-        g_free(gconf->server);
+        g_list_foreach(gconf->server, (GFunc)g_free, NULL);
         g_free(gconf->volname);
         g_free(gconf->image);
         g_free(gconf->transport);
@@ -43,6 +49,7 @@ static void qemu_gluster_gconf_free(GlusterConf *gconf)
     }
 }
 
+
 static int parse_volume_options(GlusterConf *gconf, char *path)
 {
     char *p, *q;
@@ -68,8 +75,19 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
     return 0;
 }
 
+
+static void parse_volfile_server_options(gpointer data, gpointer user_data)
+{
+    char *server = (char *) data;
+    GlusterOptions *options = (GlusterOptions *) user_data;
+
+    glfs_set_volfile_server(options->glfs, options->gconf->transport,
+            server, options->gconf->port);
+}
+
 /*
- * file=gluster[+transport]://[server[:port]]/volname/image[?socket=...]
+ * file=gluster[+transport]://[server[:port]]/volname/image[?socket=...] \
+ *      [?backup-volfile-servers=server2\&backup-volfile-servers=server3 ...]
  *
  * 'gluster' is the protocol.
  *
@@ -102,15 +120,23 @@ static int parse_volume_options(GlusterConf *gconf, char *path)
  * file=gluster+tcp://[1:2:3:4:5:6:7:8]/testvol/dir/a.img
  * file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir/a.img
  * file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img
+ * file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img\
+ *      ?backup-volfile-servers=server1&backup-volfile-servers=server2
  * file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket
  * file=gluster+rdma://1.2.3.4:24007/testvol/a.img
+ * file=gluster+rdma://1.2.3.4:24007/testvol/a.img\
+ *      ?backup-volfile-servers=4.5.6.7&backup-volfile-servers=8.9.10.11
  */
 static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
 {
-    URI *uri;
-    QueryParams *qp = NULL;
-    bool is_unix = false;
-    int ret = 0;
+    URI         *uri     = NULL;
+    QueryParams *qp      = NULL;
+    bool        is_unix  = false;
+    bool        is_tcp   = false;
+    bool        is_rdma  = false;
+    int         i        = 0;
+    int         ret      = 0;
+    int         nservers = 0;
 
     uri = uri_parse(filename);
     if (!uri) {
@@ -120,13 +146,16 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
     /* transport */
     if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
         gconf->transport = g_strdup("tcp");
+        is_tcp = true;
     } else if (!strcmp(uri->scheme, "gluster+tcp")) {
         gconf->transport = g_strdup("tcp");
+        is_tcp = true;
     } else if (!strcmp(uri->scheme, "gluster+unix")) {
         gconf->transport = g_strdup("unix");
         is_unix = true;
     } else if (!strcmp(uri->scheme, "gluster+rdma")) {
         gconf->transport = g_strdup("rdma");
+        is_rdma = true;
     } else {
         ret = -EINVAL;
         goto out;
@@ -138,23 +167,37 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
     }
 
     qp = query_params_parse(uri->query);
-    if (qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n)) {
-        ret = -EINVAL;
-        goto out;
-    }
-
-    if (is_unix) {
-        if (uri->server || uri->port) {
-            ret = -EINVAL;
-            goto out;
+    if (!(qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n))) {
+        if (is_unix) {
+            if (uri->server || uri->port) {
+                ret = -EINVAL;
+                goto out;
+            }
+            if (strcmp(qp->p[0].name, "socket")) {
+                ret = -EINVAL;
+                goto out;
+            }
+            gconf->server = g_list_append(NULL, g_strdup(qp->p[0].value));
         }
-        if (strcmp(qp->p[0].name, "socket")) {
-            ret = -EINVAL;
-            goto out;
+    } else if (!(!qp->n || ((is_tcp || is_rdma) && !qp->n) ||
+                                        (!(is_tcp || is_rdma) && qp->n))) {
+        nservers = qp->n;
+        if (is_tcp || is_rdma) {
+            for (i = 0; i < nservers; i++) {
+                if (strcmp(qp->p[i].name, "backup-volfile-servers")) {
+                    ret = -EINVAL;
+                    goto out;
+                }
+            }
+            gconf->server = g_list_append(NULL, g_strdup(uri->server));
+            for (i = 0; i < nservers; i++) {
+                gconf->server = g_list_append(gconf->server,
+                                                    g_strdup(qp->p[i].value));
+            }
         }
-        gconf->server = g_strdup(qp->p[0].value);
     } else {
-        gconf->server = g_strdup(uri->server ? uri->server : "localhost");
+        gconf->server = g_list_append(NULL,
+                            g_strdup(uri->server ? uri->server : "localhost"));
         gconf->port = uri->port;
     }
 
@@ -169,28 +212,32 @@ out:
 static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
                                       Error **errp)
 {
-    struct glfs *glfs = NULL;
-    int ret;
-    int old_errno;
+    struct glfs     *glfs   = NULL;
+    GlusterOptions  *params = NULL;
+    int              ret    = -1;
+    int              old_errno;
 
     ret = qemu_gluster_parseuri(gconf, filename);
     if (ret < 0) {
         error_setg(errp, "Usage: file=gluster[+transport]://[server[:port]]/"
-                   "volname/image[?socket=...]");
+                  "volname/image[?socket=...][?backup-volfile-servers=server1"
+                   "&backup-volfile-servers=server2...]");
         errno = -ret;
         goto out;
     }
 
+    params = g_malloc(sizeof(GlusterOptions));
+
     glfs = glfs_new(gconf->volname);
     if (!glfs) {
         goto out;
     }
 
-    ret = glfs_set_volfile_server(glfs, gconf->transport, gconf->server,
-            gconf->port);
-    if (ret < 0) {
-        goto out;
-    }
+    params->glfs  = glfs;
+    params->gconf = gconf;
+
+    g_list_foreach(gconf->server, (GFunc) parse_volfile_server_options,
+                        (GlusterOptions *)params);
 
     /*
      * TODO: Use GF_LOG_ERROR instead of hard code value of 4 here when
@@ -204,17 +251,17 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
     ret = glfs_init(glfs);
     if (ret) {
         error_setg_errno(errp, errno,
-                         "Gluster connection failed for server=%s port=%d "
-                         "volume=%s image=%s transport=%s", gconf->server,
-                         gconf->port, gconf->volname, gconf->image,
-                         gconf->transport);
-
+                         "Gluster connection failed for "
+                         "volume=%s image=%s transport=%s port=%d",
+                         gconf->volname, gconf->image,
+                         gconf->transport, gconf->port);
         /* glfs_init sometimes doesn't set errno although docs suggest that */
         if (errno == 0)
             errno = EINVAL;
 
         goto out;
     }
+    g_free(params);
     return glfs;
 
 out:
@@ -223,6 +270,7 @@ out:
         glfs_fini(glfs);
         errno = old_errno;
     }
+    g_free(params);
     return NULL;
 }
 
-- 
2.1.0

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Peter Krempa]

[-- Attachment #1: Type: text/plain, Size: 1772 bytes --]

On Tue, Sep 08, 2015 at 18:34:09 +0530, Prasanna Kumar Kalever wrote:
> This patch adds a way to specify multiple backup volfile servers to the gluster
> block backend of QEMU with both tcp and rdma transport types.
> 
> Problem:
> 
> Currenly VM Image on gluster volume is specified like this:
> 
> file=gluster[+tcp]://server1:24007/testvol/a.img
> 
> Assuming we have have three servers in trustred pool with replica 3 volume
> in action and unfortunately server1 (mentioned in the command above) went down
> for some reason, since the volume is replica 3 we now have other 2 servers
> active from which we can boot the VM.
> 
> But currently there is no mechanism to pass the other 2 gluster server
> addresses to qemu.
> 
> Solution:
> 
> New way of specifying VM Image on gluster volume with backup volfile servers:
> 
> file=gluster[+transport-type]://server1:24007/testvol/a.img\
>      ?backup-volfile-servers=server2&backup-volfile-servers=server3

This syntax doesn't seem to support different port numbers for the
backup volfile servers since it's possible to specify the port number
for the primary one, the backup ones should allow that too to support
all possible configurations.

> 
> This patch gives a mechanism to provide all the server addresses which are in
> replica set, so in case server1 is down VM can still boot from any of the
> active servers.
> 
> This is equivalent to the backup-volfile-servers option supported by
> mount.glusterfs (FUSE way of mounting gluster volume)
> 
> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> ---
>  block/gluster.c | 118 +++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 83 insertions(+), 35 deletions(-)
> 

Peter

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Daniel P. Berrange]

On Tue, Sep 08, 2015 at 06:34:09PM +0530, Prasanna Kumar Kalever wrote:
> This patch adds a way to specify multiple backup volfile servers to the gluster
> block backend of QEMU with both tcp and rdma transport types.
> 
> Problem:
> 
> Currenly VM Image on gluster volume is specified like this:
> 
> file=gluster[+tcp]://server1:24007/testvol/a.img
> 
> Assuming we have have three servers in trustred pool with replica 3 volume
> in action and unfortunately server1 (mentioned in the command above) went down
> for some reason, since the volume is replica 3 we now have other 2 servers
> active from which we can boot the VM.
> 
> But currently there is no mechanism to pass the other 2 gluster server
> addresses to qemu.
> 
> Solution:
> 
> New way of specifying VM Image on gluster volume with backup volfile servers:
> 
> file=gluster[+transport-type]://server1:24007/testvol/a.img\
>      ?backup-volfile-servers=server2&backup-volfile-servers=server3

Comparison with RBD syntax:

  file=rbd:pool/image:auth_supported=none:\
    mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
    mon3.example.org\:6322,if=virtio,format=raw

As Peter already mentioned, you're missing port numbers.

It is slightly unpleasant to have different ways of specifying the first
vs second, third, etc hosts. I wonder if it would be nicer to keep all
the hostnames in the host part of the URI. eg

 file=gluster[+transport-type]://server1:24007,server2:3553,server3:2423/testvol/a.img\
      ?backup-volfile-servers=server2&backup-volfile-servers=server3

Of course it ceases to be a wellformed URI at that point, so another option
would be to just allow the host part of the URI to be optional, and then
accept mutliple instances ofa  'server' arg, eg

 file=gluster[+transport-type]:///testvol/a.img\
      ?server=server1:2424&server=server2:2423&sever=server3:34222

I think I prefer this last syntax most.

> 
> This patch gives a mechanism to provide all the server addresses which are in
> replica set, so in case server1 is down VM can still boot from any of the
> active servers.
> 
> This is equivalent to the backup-volfile-servers option supported by
> mount.glusterfs (FUSE way of mounting gluster volume)
> 
> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> ---
>  block/gluster.c | 118 +++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 83 insertions(+), 35 deletions(-)


>  static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename)
>  {
> -    URI *uri;
> -    QueryParams *qp = NULL;
> -    bool is_unix = false;
> -    int ret = 0;
> +    URI         *uri     = NULL;
> +    QueryParams *qp      = NULL;
> +    bool        is_unix  = false;
> +    bool        is_tcp   = false;
> +    bool        is_rdma  = false;
> +    int         i        = 0;
> +    int         ret      = 0;
> +    int         nservers = 0;

Aligning indentation like this is really not desirable, as it results
in huge whitespace diffs for existing code anytime someone adds/removes
a variable which changes the indent depth, so please don't do this.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Deepak Shetty]

[-- Attachment #1: Type: text/plain, Size: 4170 bytes --]

On Tue, Sep 8, 2015 at 8:32 PM, Daniel P. Berrange <berrange@redhat.com>
wrote:

> On Tue, Sep 08, 2015 at 06:34:09PM +0530, Prasanna Kumar Kalever wrote:
> > This patch adds a way to specify multiple backup volfile servers to the
> gluster
> > block backend of QEMU with both tcp and rdma transport types.
> >
> > Problem:
> >
> > Currenly VM Image on gluster volume is specified like this:
> >
> > file=gluster[+tcp]://server1:24007/testvol/a.img
> >
> > Assuming we have have three servers in trustred pool with replica 3
> volume
> > in action and unfortunately server1 (mentioned in the command above)
> went down
> > for some reason, since the volume is replica 3 we now have other 2
> servers
> > active from which we can boot the VM.
> >
> > But currently there is no mechanism to pass the other 2 gluster server
> > addresses to qemu.
> >
> > Solution:
> >
> > New way of specifying VM Image on gluster volume with backup volfile
> servers:
> >
> > file=gluster[+transport-type]://server1:24007/testvol/a.img\
> >      ?backup-volfile-servers=server2&backup-volfile-servers=server3
>
> Comparison with RBD syntax:
>
>   file=rbd:pool/image:auth_supported=none:\
>     mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
>     mon3.example.org\:6322,if=virtio,format=raw
>
> As Peter already mentioned, you're missing port numbers.
>
> It is slightly unpleasant to have different ways of specifying the first
> vs second, third, etc hosts. I wonder if it would be nicer to keep all
> the hostnames in the host part of the URI. eg
>
>
>  file=gluster[+transport-type]://server1:24007,server2:3553,server3:2423/testvol/a.img\
>       ?backup-volfile-servers=server2&backup-volfile-servers=server3
>
> Of course it ceases to be a wellformed URI at that point, so another option
> would be to just allow the host part of the URI to be optional, and then
> accept mutliple instances ofa  'server' arg, eg
>
>  file=gluster[+transport-type]:///testvol/a.img\
>       ?server=server1:2424&server=server2:2423&sever=server3:34222
>
>
Is it allowed to have this syntax and be a valid URI ? I admit i haven't
looked at the
URI rfc for a long time now, hence the Q. Also looking at rbd syntax, it
looks
to follow this model already is it ? Whats the difference between using ':'
to
separate key=value pairs Vs using '?" query syntax ? Should we look at
having
a uniform way of specifying URI be it rbd or gluster or sheepdog ... ? If
yes
what that uniform syntax be using ':" or '?" ?

thanx,
deepak



> I think I prefer this last syntax most.
>
> >
> > This patch gives a mechanism to provide all the server addresses which
> are in
> > replica set, so in case server1 is down VM can still boot from any of the
> > active servers.
> >
> > This is equivalent to the backup-volfile-servers option supported by
> > mount.glusterfs (FUSE way of mounting gluster volume)
> >
> > Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> > ---
> >  block/gluster.c | 118
> +++++++++++++++++++++++++++++++++++++++-----------------
> >  1 file changed, 83 insertions(+), 35 deletions(-)
>
>
> >  static int qemu_gluster_parseuri(GlusterConf *gconf, const char
> *filename)
> >  {
> > -    URI *uri;
> > -    QueryParams *qp = NULL;
> > -    bool is_unix = false;
> > -    int ret = 0;
> > +    URI         *uri     = NULL;
> > +    QueryParams *qp      = NULL;
> > +    bool        is_unix  = false;
> > +    bool        is_tcp   = false;
> > +    bool        is_rdma  = false;
> > +    int         i        = 0;
> > +    int         ret      = 0;
> > +    int         nservers = 0;
>
> Aligning indentation like this is really not desirable, as it results
> in huge whitespace diffs for existing code anytime someone adds/removes
> a variable which changes the indent depth, so please don't do this.
>
> Regards,
> Daniel
> --
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/
> :|
> |: http://libvirt.org              -o-             http://virt-manager.org
> :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/
> :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc
> :|
>
>

[-- Attachment #2: Type: text/html, Size: 6323 bytes --]

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Deepak C Shetty]

[-- Attachment #1: Type: text/plain, Size: 3275 bytes --]



On 09/08/2015 08:40 PM, Deepak Shetty wrote:
>
>
> On Tue, Sep 8, 2015 at 8:32 PM, Daniel P. Berrange 
> <berrange@redhat.com <mailto:berrange@redhat.com>>wrote:
>
>     On Tue, Sep 08, 2015 at 06:34:09PM +0530, Prasanna Kumar Kalever
>     wrote:
>     > This patch adds a way to specify multiple backup volfile servers
>     to the gluster
>     > block backend of QEMU with both tcp and rdma transport types.
>     >
>     > Problem:
>     >
>     > Currenly VM Image on gluster volume is specified like this:
>     >
>     > file=gluster[+tcp]://server1:24007/testvol/a.img
>     >
>     > Assuming we have have three servers in trustred pool with
>     replica 3 volume
>     > in action and unfortunately server1 (mentioned in the command
>     above) went down
>     > for some reason, since the volume is replica 3 we now have other
>     2 servers
>     > active from which we can boot the VM.
>     >
>     > But currently there is no mechanism to pass the other 2 gluster
>     server
>     > addresses to qemu.
>     >
>     > Solution:
>     >
>     > New way of specifying VM Image on gluster volume with backup
>     volfile servers:
>     >
>     > file=gluster[+transport-type]://server1:24007/testvol/a.img\
>     > ?backup-volfile-servers=server2&backup-volfile-servers=server3
>
>     Comparison with RBD syntax:
>
>       file=rbd:pool/image:auth_supported=none:\
>         mon_host=mon1.example.org
>     <http://mon1.example.org>\:6321\;mon2.example.org
>     <http://mon2.example.org>\:6322\;\
>     mon3.example.org <http://mon3.example.org>\:6322,if=virtio,format=raw
>
>     As Peter already mentioned, you're missing port numbers.
>
>     It is slightly unpleasant to have different ways of specifying the
>     first
>     vs second, third, etc hosts. I wonder if it would be nicer to keep all
>     the hostnames in the host part of the URI. eg
>
>      file=gluster[+transport-type]://server1:24007,server2:3553,server3:2423/testvol/a.img\
>     ?backup-volfile-servers=server2&backup-volfile-servers=server3
>
>     Of course it ceases to be a wellformed URI at that point, so
>     another option
>     would be to just allow the host part of the URI to be optional,
>     and then
>     accept mutliple instances ofa  'server' arg, eg
>
>      file=gluster[+transport-type]:///testvol/a.img\
>     ?server=server1:2424&server=server2:2423&sever=server3:34222
>
>
> Is it allowed to have this syntax and be a valid URI ? I admit i 
> haven't looked at the
> URI rfc for a long time now, hence the Q. Also looking at rbd syntax, 
> it looks
> to follow this model already is it ? Whats the difference between 
> using ':' to
> separate key=value pairs Vs using '?" query syntax ? Should we look at 
> having
> a uniform way of specifying URI be it rbd or gluster or sheepdog ... ? 
> If yes
> what that uniform syntax be using ':" or '?" ?

Answering myself, based on what I figured .... :)
Looks like rbd syntax is a propertiary one and not adhering to URI rfc, 
while gluster syntax
is URI compliant so the 2nd option suggested by danpb seems good

Also need to ensure that old syntax of providing server:port in the 
authority field should
be honoured so that older clients/apps generating the old syntax won't 
be broken

thanx,
deepak


[-- Attachment #2: Type: text/html, Size: 7150 bytes --]

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Stefan Hajnoczi]

On Tue, Sep 08, 2015 at 08:40:58PM +0530, Deepak Shetty wrote:
> On Tue, Sep 8, 2015 at 8:32 PM, Daniel P. Berrange <berrange@redhat.com>
> wrote:
> 
> > On Tue, Sep 08, 2015 at 06:34:09PM +0530, Prasanna Kumar Kalever wrote:
> > > This patch adds a way to specify multiple backup volfile servers to the
> > gluster
> > > block backend of QEMU with both tcp and rdma transport types.
> > >
> > > Problem:
> > >
> > > Currenly VM Image on gluster volume is specified like this:
> > >
> > > file=gluster[+tcp]://server1:24007/testvol/a.img
> > >
> > > Assuming we have have three servers in trustred pool with replica 3
> > volume
> > > in action and unfortunately server1 (mentioned in the command above)
> > went down
> > > for some reason, since the volume is replica 3 we now have other 2
> > servers
> > > active from which we can boot the VM.
> > >
> > > But currently there is no mechanism to pass the other 2 gluster server
> > > addresses to qemu.
> > >
> > > Solution:
> > >
> > > New way of specifying VM Image on gluster volume with backup volfile
> > servers:
> > >
> > > file=gluster[+transport-type]://server1:24007/testvol/a.img\
> > >      ?backup-volfile-servers=server2&backup-volfile-servers=server3
> >
> > Comparison with RBD syntax:
> >
> >   file=rbd:pool/image:auth_supported=none:\
> >     mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
> >     mon3.example.org\:6322,if=virtio,format=raw
> >
> > As Peter already mentioned, you're missing port numbers.
> >
> > It is slightly unpleasant to have different ways of specifying the first
> > vs second, third, etc hosts. I wonder if it would be nicer to keep all
> > the hostnames in the host part of the URI. eg
> >
> >
> >  file=gluster[+transport-type]://server1:24007,server2:3553,server3:2423/testvol/a.img\
> >       ?backup-volfile-servers=server2&backup-volfile-servers=server3
> >
> > Of course it ceases to be a wellformed URI at that point, so another option
> > would be to just allow the host part of the URI to be optional, and then
> > accept mutliple instances ofa  'server' arg, eg
> >
> >  file=gluster[+transport-type]:///testvol/a.img\
> >       ?server=server1:2424&server=server2:2423&sever=server3:34222
> >
> >
> Is it allowed to have this syntax and be a valid URI ? I admit i haven't
> looked at the
> URI rfc for a long time now, hence the Q. Also looking at rbd syntax, it
> looks
> to follow this model already is it ? Whats the difference between using ':'
> to
> separate key=value pairs Vs using '?" query syntax ? Should we look at
> having
> a uniform way of specifying URI be it rbd or gluster or sheepdog ... ? If
> yes
> what that uniform syntax be using ':" or '?" ?

Instead of trying to make a gluster:// URI that accommodates multiple
volfile servers, perhaps the block driver can take a list of URIs.
Something like:

  -drive driver=gluster,uri[0]=gluster[+transport-type]://server1:24007/testvol/a.img,
                        uri[1]=gluster[+transport-type]://server2:24008/testvol/a.img,
                        uri[2]=gluster[+transport-type]://server3:24009/testvol/a.img

This approach allows full flexibility.

I have CCed Kevin in case he has comments.

Stefan

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Deepak C Shetty]

On 09/09/2015 03:16 PM, Stefan Hajnoczi wrote:
> On Tue, Sep 08, 2015 at 08:40:58PM +0530, Deepak Shetty wrote:
>> On Tue, Sep 8, 2015 at 8:32 PM, Daniel P. Berrange <berrange@redhat.com>
>> wrote:
>>
>>> On Tue, Sep 08, 2015 at 06:34:09PM +0530, Prasanna Kumar Kalever wrote:
>>>> This patch adds a way to specify multiple backup volfile servers to the
>>> gluster
>>>> block backend of QEMU with both tcp and rdma transport types.
>>>>
>>>> Problem:
>>>>
>>>> Currenly VM Image on gluster volume is specified like this:
>>>>
>>>> file=gluster[+tcp]://server1:24007/testvol/a.img
>>>>
>>>> Assuming we have have three servers in trustred pool with replica 3
>>> volume
>>>> in action and unfortunately server1 (mentioned in the command above)
>>> went down
>>>> for some reason, since the volume is replica 3 we now have other 2
>>> servers
>>>> active from which we can boot the VM.
>>>>
>>>> But currently there is no mechanism to pass the other 2 gluster server
>>>> addresses to qemu.
>>>>
>>>> Solution:
>>>>
>>>> New way of specifying VM Image on gluster volume with backup volfile
>>> servers:
>>>> file=gluster[+transport-type]://server1:24007/testvol/a.img\
>>>>       ?backup-volfile-servers=server2&backup-volfile-servers=server3
>>> Comparison with RBD syntax:
>>>
>>>    file=rbd:pool/image:auth_supported=none:\
>>>      mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
>>>      mon3.example.org\:6322,if=virtio,format=raw
>>>
>>> As Peter already mentioned, you're missing port numbers.
>>>
>>> It is slightly unpleasant to have different ways of specifying the first
>>> vs second, third, etc hosts. I wonder if it would be nicer to keep all
>>> the hostnames in the host part of the URI. eg
>>>
>>>
>>>   file=gluster[+transport-type]://server1:24007,server2:3553,server3:2423/testvol/a.img\
>>>        ?backup-volfile-servers=server2&backup-volfile-servers=server3
>>>
>>> Of course it ceases to be a wellformed URI at that point, so another option
>>> would be to just allow the host part of the URI to be optional, and then
>>> accept mutliple instances ofa  'server' arg, eg
>>>
>>>   file=gluster[+transport-type]:///testvol/a.img\
>>>        ?server=server1:2424&server=server2:2423&sever=server3:34222
>>>
>>>
>> Is it allowed to have this syntax and be a valid URI ? I admit i haven't
>> looked at the
>> URI rfc for a long time now, hence the Q. Also looking at rbd syntax, it
>> looks
>> to follow this model already is it ? Whats the difference between using ':'
>> to
>> separate key=value pairs Vs using '?" query syntax ? Should we look at
>> having
>> a uniform way of specifying URI be it rbd or gluster or sheepdog ... ? If
>> yes
>> what that uniform syntax be using ':" or '?" ?
> Instead of trying to make a gluster:// URI that accommodates multiple
> volfile servers, perhaps the block driver can take a list of URIs.
> Something like:
>
>    -drive driver=gluster,uri[0]=gluster[+transport-type]://server1:24007/testvol/a.img,
>                          uri[1]=gluster[+transport-type]://server2:24008/testvol/a.img,
>                          uri[2]=gluster[+transport-type]://server3:24009/testvol/a.img

Whats the adv of this Vs extending existing gluster URI ?
This actually sounds like a lot more typing, and making qemu cmd line a 
lot more lengthier :)

Is -drive file=gluster://.... same as -drive driver=gluster, 
uri[0]=gluster..... ?

Also this would mean we need to change libvirt support for gluster based 
network disk completely
and also maintain the old code for backward compat, right ?

thanx,
deepak

>
> This approach allows full flexibility.
>
> I have CCed Kevin in case he has comments.
>
> Stefan

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Stefan Hajnoczi]

On Wed, Sep 9, 2015 at 11:19 AM, Deepak C Shetty <deepakcs@redhat.com> wrote:
> On 09/09/2015 03:16 PM, Stefan Hajnoczi wrote:
>> On Tue, Sep 08, 2015 at 08:40:58PM +0530, Deepak Shetty wrote:
>>> On Tue, Sep 8, 2015 at 8:32 PM, Daniel P. Berrange <berrange@redhat.com>
>>> wrote:
>>>> On Tue, Sep 08, 2015 at 06:34:09PM +0530, Prasanna Kumar Kalever wrote:
>>>>> This patch adds a way to specify multiple backup volfile servers to the
>>>>
>>>> gluster
>>>>>
>>>>> block backend of QEMU with both tcp and rdma transport types.
>>>>>
>>>>> Problem:
>>>>>
>>>>> Currenly VM Image on gluster volume is specified like this:
>>>>>
>>>>> file=gluster[+tcp]://server1:24007/testvol/a.img
>>>>>
>>>>> Assuming we have have three servers in trustred pool with replica 3
>>>>
>>>> volume
>>>>>
>>>>> in action and unfortunately server1 (mentioned in the command above)
>>>>
>>>> went down
>>>>>
>>>>> for some reason, since the volume is replica 3 we now have other 2
>>>>
>>>> servers
>>>>>
>>>>> active from which we can boot the VM.
>>>>>
>>>>> But currently there is no mechanism to pass the other 2 gluster server
>>>>> addresses to qemu.
>>>>>
>>>>> Solution:
>>>>>
>>>>> New way of specifying VM Image on gluster volume with backup volfile
>>>>
>>>> servers:
>>>>>
>>>>> file=gluster[+transport-type]://server1:24007/testvol/a.img\
>>>>>       ?backup-volfile-servers=server2&backup-volfile-servers=server3
>>>>
>>>> Comparison with RBD syntax:
>>>>
>>>>    file=rbd:pool/image:auth_supported=none:\
>>>>      mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
>>>>      mon3.example.org\:6322,if=virtio,format=raw
>>>>
>>>> As Peter already mentioned, you're missing port numbers.
>>>>
>>>> It is slightly unpleasant to have different ways of specifying the first
>>>> vs second, third, etc hosts. I wonder if it would be nicer to keep all
>>>> the hostnames in the host part of the URI. eg
>>>>
>>>>
>>>>
>>>> file=gluster[+transport-type]://server1:24007,server2:3553,server3:2423/testvol/a.img\
>>>>        ?backup-volfile-servers=server2&backup-volfile-servers=server3
>>>>
>>>> Of course it ceases to be a wellformed URI at that point, so another
>>>> option
>>>> would be to just allow the host part of the URI to be optional, and then
>>>> accept mutliple instances ofa  'server' arg, eg
>>>>
>>>>   file=gluster[+transport-type]:///testvol/a.img\
>>>>        ?server=server1:2424&server=server2:2423&sever=server3:34222
>>>>
>>>>
>>> Is it allowed to have this syntax and be a valid URI ? I admit i haven't
>>> looked at the
>>> URI rfc for a long time now, hence the Q. Also looking at rbd syntax, it
>>> looks
>>> to follow this model already is it ? Whats the difference between using
>>> ':'
>>> to
>>> separate key=value pairs Vs using '?" query syntax ? Should we look at
>>> having
>>> a uniform way of specifying URI be it rbd or gluster or sheepdog ... ? If
>>> yes
>>> what that uniform syntax be using ':" or '?" ?
>>
>> Instead of trying to make a gluster:// URI that accommodates multiple
>> volfile servers, perhaps the block driver can take a list of URIs.
>> Something like:
>>
>>    -drive
>> driver=gluster,uri[0]=gluster[+transport-type]://server1:24007/testvol/a.img,
>>
>> uri[1]=gluster[+transport-type]://server2:24008/testvol/a.img,
>>
>> uri[2]=gluster[+transport-type]://server3:24009/testvol/a.img
>
>
> Whats the adv of this Vs extending existing gluster URI ?

>From QEMU's perspective, it would be better to use separate fields
(that have type information) than to encode everything in an opaque
URI string.  Fields can do input validation in common code so that
block drivers don't need to check whether something is a valid number,
for example.  Fields can also be listed and their type information can
be displayed so the user knows the expected range of inputs
(self-documenting).

The other problem with URIs is that it becomes hard to encode
arbitrary data fields into them once you hit escaping.  It becomes
cumbersome for humans to construct these URIs.

> This actually sounds like a lot more typing, and making qemu cmd line a lot
> more lengthier :)

If you are typing a list of volfile backup servers each time then it's
already too long.  Anyone doing this would either use libvirt or a
shell script, so I'm not concerned about the length of the
command-line in this use case.

> Is -drive file=gluster://.... same as -drive driver=gluster,
> uri[0]=gluster..... ?

Yes, the code to make this possible is already in place today.  Block
drivers don't have to parse a single filename string, they can use a
dictionary of options passed to them.

> Also this would mean we need to change libvirt support for gluster based
> network disk completely
> and also maintain the old code for backward compat, right ?

Yes, libvirt would need to emit the new syntax.  The old syntax would
continue to work.

Stefan

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Raghavendra Talur]

[-- Attachment #1: Type: text/plain, Size: 6081 bytes --]

On Wed, Sep 9, 2015 at 5:59 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Wed, Sep 9, 2015 at 11:19 AM, Deepak C Shetty <deepakcs@redhat.com>
> wrote:
> > On 09/09/2015 03:16 PM, Stefan Hajnoczi wrote:
> >> On Tue, Sep 08, 2015 at 08:40:58PM +0530, Deepak Shetty wrote:
> >>> On Tue, Sep 8, 2015 at 8:32 PM, Daniel P. Berrange <
> berrange@redhat.com>
> >>> wrote:
> >>>> On Tue, Sep 08, 2015 at 06:34:09PM +0530, Prasanna Kumar Kalever
> wrote:
> >>>>> This patch adds a way to specify multiple backup volfile servers to
> the
> >>>>
> >>>> gluster
> >>>>>
> >>>>> block backend of QEMU with both tcp and rdma transport types.
> >>>>>
> >>>>> Problem:
> >>>>>
> >>>>> Currenly VM Image on gluster volume is specified like this:
> >>>>>
> >>>>> file=gluster[+tcp]://server1:24007/testvol/a.img
> >>>>>
> >>>>> Assuming we have have three servers in trustred pool with replica 3
> >>>>
> >>>> volume
> >>>>>
> >>>>> in action and unfortunately server1 (mentioned in the command above)
> >>>>
> >>>> went down
> >>>>>
> >>>>> for some reason, since the volume is replica 3 we now have other 2
> >>>>
> >>>> servers
> >>>>>
> >>>>> active from which we can boot the VM.
> >>>>>
> >>>>> But currently there is no mechanism to pass the other 2 gluster
> server
> >>>>> addresses to qemu.
> >>>>>
> >>>>> Solution:
> >>>>>
> >>>>> New way of specifying VM Image on gluster volume with backup volfile
> >>>>
> >>>> servers:
> >>>>>
> >>>>> file=gluster[+transport-type]://server1:24007/testvol/a.img\
> >>>>>       ?backup-volfile-servers=server2&backup-volfile-servers=server3
> >>>>
> >>>> Comparison with RBD syntax:
> >>>>
> >>>>    file=rbd:pool/image:auth_supported=none:\
> >>>>      mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
> >>>>      mon3.example.org\:6322,if=virtio,format=raw
> >>>>
> >>>> As Peter already mentioned, you're missing port numbers.
> >>>>
> >>>> It is slightly unpleasant to have different ways of specifying the
> first
> >>>> vs second, third, etc hosts. I wonder if it would be nicer to keep all
> >>>> the hostnames in the host part of the URI. eg
> >>>>
> >>>>
> >>>>
> >>>>
> file=gluster[+transport-type]://server1:24007,server2:3553,server3:2423/testvol/a.img\
> >>>>        ?backup-volfile-servers=server2&backup-volfile-servers=server3
> >>>>
> >>>> Of course it ceases to be a wellformed URI at that point, so another
> >>>> option
> >>>> would be to just allow the host part of the URI to be optional, and
> then
> >>>> accept mutliple instances ofa  'server' arg, eg
> >>>>
> >>>>   file=gluster[+transport-type]:///testvol/a.img\
> >>>>        ?server=server1:2424&server=server2:2423&sever=server3:34222
> >>>>
> >>>>
> >>> Is it allowed to have this syntax and be a valid URI ? I admit i
> haven't
> >>> looked at the
> >>> URI rfc for a long time now, hence the Q. Also looking at rbd syntax,
> it
> >>> looks
> >>> to follow this model already is it ? Whats the difference between using
> >>> ':'
> >>> to
> >>> separate key=value pairs Vs using '?" query syntax ? Should we look at
> >>> having
> >>> a uniform way of specifying URI be it rbd or gluster or sheepdog ... ?
> If
> >>> yes
> >>> what that uniform syntax be using ':" or '?" ?
> >>
> >> Instead of trying to make a gluster:// URI that accommodates multiple
> >> volfile servers, perhaps the block driver can take a list of URIs.
> >> Something like:
> >>
> >>    -drive
> >>
> driver=gluster,uri[0]=gluster[+transport-type]://server1:24007/testvol/a.img,
> >>
> >> uri[1]=gluster[+transport-type]://server2:24008/testvol/a.img,
> >>
> >> uri[2]=gluster[+transport-type]://server3:24009/testvol/a.img
> >
> >
> > Whats the adv of this Vs extending existing gluster URI ?
>
> From QEMU's perspective, it would be better to use separate fields
> (that have type information) than to encode everything in an opaque
> URI string.  Fields can do input validation in common code so that
> block drivers don't need to check whether something is a valid number,
> for example.  Fields can also be listed and their type information can
> be displayed so the user knows the expected range of inputs
> (self-documenting).
>

Coming from Gluster side of things,  the variable/option here is tuple of
three
    transport-type
    server
    port

volname and file name should be the same in all the URIs. Just pointing
out here so that implementation can ensure that all URIs have the same
volname and filename;
which are testvol and a.img in the above example.

By fields if you mean something like
-drive
file=gluster[+transport]://[server[:port]]/volname/image[?socket=...],\
               file.backup-volfile-server=[tcp:|rdma:|unix:]server2[:port],
               file.backup-volfile-server=[tcp:|rdma:|unix:]server2[:port]

then +1 from me on that. Is this idea of a simple URI followed by fields
which
are optional allowed in QEMU?

If not, I prefer URI appended with options in
&backup-volfile-server=[tcp:|rdma:|unix:]server2[:port] format.

Thanks,
Raghavendra Talur





>
> The other problem with URIs is that it becomes hard to encode
> arbitrary data fields into them once you hit escaping.  It becomes
> cumbersome for humans to construct these URIs.


> > This actually sounds like a lot more typing, and making qemu cmd line a
> lot
> > more lengthier :)
>
> If you are typing a list of volfile backup servers each time then it's
> already too long.  Anyone doing this would either use libvirt or a
> shell script, so I'm not concerned about the length of the
> command-line in this use case.
>
> > Is -drive file=gluster://.... same as -drive driver=gluster,
> > uri[0]=gluster..... ?
>
> Yes, the code to make this possible is already in place today.  Block
> drivers don't have to parse a single filename string, they can use a
> dictionary of options passed to them.
>
> > Also this would mean we need to change libvirt support for gluster based
> > network disk completely
> > and also maintain the old code for backward compat, right ?
>
> Yes, libvirt would need to emit the new syntax.  The old syntax would
> continue to work.
>
> Stefan
>

[-- Attachment #2: Type: text/html, Size: 9167 bytes --]

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Deepak Shetty]

[-- Attachment #1: Type: text/plain, Size: 3443 bytes --]

[snip]

On Wed, Sep 9, 2015 at 10:37 PM, Raghavendra Talur <rtalur@redhat.com>
wrote:

>
>
> From QEMU's perspective, it would be better to use separate fields
>
>> (that have type information) than to encode everything in an opaque
>> URI string.  Fields can do input validation in common code so that
>> block drivers don't need to check whether something is a valid number,
>> for example.  Fields can also be listed and their type information can
>> be displayed so the user knows the expected range of inputs
>> (self-documenting).
>>
>
> Coming from Gluster side of things,  the variable/option here is tuple of
> three
>     transport-type
>     server
>     port
>
> volname and file name should be the same in all the URIs. Just pointing
> out here so that implementation can ensure that all URIs have the same
> volname and filename;
> which are testvol and a.img in the above example.
>
> By fields if you mean something like
> -drive
> file=gluster[+transport]://[server[:port]]/volname/image[?socket=...],\
>                file.backup-volfile-server=[tcp:|rdma:|unix:]server2[:port],
>                file.backup-volfile-server=[tcp:|rdma:|unix:]server2[:port]
>

Raghavendra,
  Thanks for pitching in.

So are you saying that its possible to have different transport types (tcp,
rdma etc)
for different gluster server nodes, all of which are part of the same
cluster ?

If that is true then in the above suggestion of yours, gluster
[+transport]://
doesn't make sense, since it gives a feeling that the transport mentioned
before :// applies to whole URI, only to be overridden by the later
file.backup-volfile-server= option

Maybe then as kwolf mentioned in prev thread of this mail ...

  -drive driver=gluster,uri[0]=gluster[+transport-type]://server1:24007/testvol/a.img,

uri[1]=gluster[+transport-type]://server2:24008/testvol/a.img,

uri[2]=gluster[+transport-type]://server3:24009/testvol/a.img

seems like a better way of representing things, as then we can

change transport:server:port for each server

thanx,

deepak




>
> then +1 from me on that. Is this idea of a simple URI followed by fields
> which
> are optional allowed in QEMU?
>
> If not, I prefer URI appended with options in
> &backup-volfile-server=[tcp:|rdma:|unix:]server2[:port] format.
>
> Thanks,
> Raghavendra Talur
>
>
>
>
>
>>
>> The other problem with URIs is that it becomes hard to encode
>> arbitrary data fields into them once you hit escaping.  It becomes
>> cumbersome for humans to construct these URIs.
>
>
>> > This actually sounds like a lot more typing, and making qemu cmd line a
>> lot
>> > more lengthier :)
>>
>> If you are typing a list of volfile backup servers each time then it's
>> already too long.  Anyone doing this would either use libvirt or a
>> shell script, so I'm not concerned about the length of the
>> command-line in this use case.
>>
>> > Is -drive file=gluster://.... same as -drive driver=gluster,
>> > uri[0]=gluster..... ?
>>
>> Yes, the code to make this possible is already in place today.  Block
>> drivers don't have to parse a single filename string, they can use a
>> dictionary of options passed to them.
>>
>> > Also this would mean we need to change libvirt support for gluster based
>> > network disk completely
>> > and also maintain the old code for backward compat, right ?
>>
>> Yes, libvirt would need to emit the new syntax.  The old syntax would
>> continue to work.
>>
>> Stefan
>>
>
>

[-- Attachment #2: Type: text/html, Size: 5285 bytes --]

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Daniel P. Berrange]

On Thu, Sep 10, 2015 at 11:12:46AM +0530, Deepak Shetty wrote:
> [snip]
> 
> On Wed, Sep 9, 2015 at 10:37 PM, Raghavendra Talur <rtalur@redhat.com>
> wrote:
> 
> >
> >
> > From QEMU's perspective, it would be better to use separate fields
> >
> >> (that have type information) than to encode everything in an opaque
> >> URI string.  Fields can do input validation in common code so that
> >> block drivers don't need to check whether something is a valid number,
> >> for example.  Fields can also be listed and their type information can
> >> be displayed so the user knows the expected range of inputs
> >> (self-documenting).
> >>
> >
> > Coming from Gluster side of things,  the variable/option here is tuple of
> > three
> >     transport-type
> >     server
> >     port
> >
> > volname and file name should be the same in all the URIs. Just pointing
> > out here so that implementation can ensure that all URIs have the same
> > volname and filename;
> > which are testvol and a.img in the above example.
> >
> > By fields if you mean something like
> > -drive
> > file=gluster[+transport]://[server[:port]]/volname/image[?socket=...],\
> >                file.backup-volfile-server=[tcp:|rdma:|unix:]server2[:port],
> >                file.backup-volfile-server=[tcp:|rdma:|unix:]server2[:port]
> >
> 
> Raghavendra,
>   Thanks for pitching in.
> 
> So are you saying that its possible to have different transport types (tcp,
> rdma etc)
> for different gluster server nodes, all of which are part of the same
> cluster ?
> 
> If that is true then in the above suggestion of yours, gluster
> [+transport]://
> doesn't make sense, since it gives a feeling that the transport mentioned
> before :// applies to whole URI, only to be overridden by the later
> file.backup-volfile-server= option
> 
> Maybe then as kwolf mentioned in prev thread of this mail ...
> 
>   -drive driver=gluster,uri[0]=gluster[+transport-type]://server1:24007/testvol/a.img,
> 
> uri[1]=gluster[+transport-type]://server2:24008/testvol/a.img,
> 
> uri[2]=gluster[+transport-type]://server3:24009/testvol/a.img
> 
> seems like a better way of representing things, as then we can
> 
> change transport:server:port for each server

Yep, if you want to be able to control transport, server & port for
each, then that really suggests using a single URI is no longer
appropriate. So I'd concur, that something like Kevin's / Stefan's
suggestions are better bets. This example you give above looks fine
for example.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Kevin Wolf]

Am 10.09.2015 um 07:42 hat Deepak Shetty geschrieben:
> [snip]
> 
> On Wed, Sep 9, 2015 at 10:37 PM, Raghavendra Talur <rtalur@redhat.com> wrote:
> 
> 
> 
>     From QEMU's perspective, it would be better to use separate fields
> 
>         (that have type information) than to encode everything in an opaque
>         URI string.  Fields can do input validation in common code so that
>         block drivers don't need to check whether something is a valid number,
>         for example.  Fields can also be listed and their type information can
>         be displayed so the user knows the expected range of inputs
>         (self-documenting).
> 
> 
>     Coming from Gluster side of things,  the variable/option here is tuple of
>     three
>         transport-type
>         server
>         port 
> 
>     volname and file name should be the same in all the URIs. Just pointing
>     out here so that implementation can ensure that all URIs have the same
>     volname and filename;
>     which are testvol and a.img in the above example.
> 
>     By fields if you mean something like 
>     -drive file=gluster[+transport]://[server[:port]]/volname/image[?socket
>     =...],\
>                    file.backup-volfile-server=[tcp:|rdma:|unix:]server2[:port],
>                    file.backup-volfile-server=[tcp:|rdma:|unix:]server2[:port]
> 
> 
> Raghavendra,
>   Thanks for pitching in.
> 
> So are you saying that its possible to have different transport types (tcp,
> rdma etc)
> for different gluster server nodes, all of which are part of the same cluster ?
> 
> If that is true then in the above suggestion of yours, gluster [+transport]://
> doesn't make sense, since it gives a feeling that the transport mentioned
> before :// applies to whole URI, only to be overridden by the later
> file.backup-volfile-server= option
> 
> Maybe then as kwolf mentioned in prev thread of this mail ...
> 
> 
>   -drive driver=gluster,uri[0]=gluster[+transport-type]://server1:24007/testvol/a.img,
>                         uri[1]=gluster[+transport-type]://server2:24008/testvol/a.img,
>                         uri[2]=gluster[+transport-type]://server3:24009/testvol/a.img

To be clear, I did _not_ say that you should use URIs. I think URIs are
a convenient shortcut syntax for simple setups. More complex setups
should use structured options, which should be defined in the QAPI
schema. The command line options are directly derived from that then.

URIs would still be supported as a shortcut syntax on the command line,
but internally they would be converted into structured options before
.bdrv_open() would be called. You can check the nbd or ssh drivers for
comparison, they already work like that.

Kevin

Re: [Qemu-devel] [PATCH 1/1] block/gluster: add support for multiple gluster backup volfile servers

[Kevin Wolf]

Am 09.09.2015 um 11:46 hat Stefan Hajnoczi geschrieben:
> On Tue, Sep 08, 2015 at 08:40:58PM +0530, Deepak Shetty wrote:
> > On Tue, Sep 8, 2015 at 8:32 PM, Daniel P. Berrange <berrange@redhat.com>
> > wrote:
> > 
> > > On Tue, Sep 08, 2015 at 06:34:09PM +0530, Prasanna Kumar Kalever wrote:
> > > > This patch adds a way to specify multiple backup volfile servers to the
> > > gluster
> > > > block backend of QEMU with both tcp and rdma transport types.
> > > >
> > > > Problem:
> > > >
> > > > Currenly VM Image on gluster volume is specified like this:
> > > >
> > > > file=gluster[+tcp]://server1:24007/testvol/a.img
> > > >
> > > > Assuming we have have three servers in trustred pool with replica 3
> > > volume
> > > > in action and unfortunately server1 (mentioned in the command above)
> > > went down
> > > > for some reason, since the volume is replica 3 we now have other 2
> > > servers
> > > > active from which we can boot the VM.
> > > >
> > > > But currently there is no mechanism to pass the other 2 gluster server
> > > > addresses to qemu.
> > > >
> > > > Solution:
> > > >
> > > > New way of specifying VM Image on gluster volume with backup volfile
> > > servers:
> > > >
> > > > file=gluster[+transport-type]://server1:24007/testvol/a.img\
> > > >      ?backup-volfile-servers=server2&backup-volfile-servers=server3
> > >
> > > Comparison with RBD syntax:
> > >
> > >   file=rbd:pool/image:auth_supported=none:\
> > >     mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
> > >     mon3.example.org\:6322,if=virtio,format=raw
> > >
> > > As Peter already mentioned, you're missing port numbers.
> > >
> > > It is slightly unpleasant to have different ways of specifying the first
> > > vs second, third, etc hosts. I wonder if it would be nicer to keep all
> > > the hostnames in the host part of the URI. eg
> > >
> > >
> > >  file=gluster[+transport-type]://server1:24007,server2:3553,server3:2423/testvol/a.img\
> > >       ?backup-volfile-servers=server2&backup-volfile-servers=server3
> > >
> > > Of course it ceases to be a wellformed URI at that point, so another option
> > > would be to just allow the host part of the URI to be optional, and then
> > > accept mutliple instances ofa  'server' arg, eg
> > >
> > >  file=gluster[+transport-type]:///testvol/a.img\
> > >       ?server=server1:2424&server=server2:2423&sever=server3:34222
> > >
> > >
> > Is it allowed to have this syntax and be a valid URI ? I admit i haven't
> > looked at the
> > URI rfc for a long time now, hence the Q. Also looking at rbd syntax, it
> > looks
> > to follow this model already is it ? Whats the difference between using ':'
> > to
> > separate key=value pairs Vs using '?" query syntax ? Should we look at
> > having
> > a uniform way of specifying URI be it rbd or gluster or sheepdog ... ? If
> > yes
> > what that uniform syntax be using ':" or '?" ?
> 
> Instead of trying to make a gluster:// URI that accommodates multiple
> volfile servers, perhaps the block driver can take a list of URIs.
> Something like:
> 
>   -drive driver=gluster,uri[0]=gluster[+transport-type]://server1:24007/testvol/a.img,
>                         uri[1]=gluster[+transport-type]://server2:24008/testvol/a.img,
>                         uri[2]=gluster[+transport-type]://server3:24009/testvol/a.img
> 
> This approach allows full flexibility.
> 
> I have CCed Kevin in case he has comments.

In fact, I think for more complex setups like this one it might be
appropriate to expect full structured blockdev-add style options instead
of URLs:

{ "driver": "gluster",
  "servers": [
    { "transport": "tcp", "host": "server1", "port": 24007,
       "volume": "testvol", "image": "a.img" },
    ...
  ] }

And on the command line either use the json: pseudo-protocol or the
verbose version with every option containing the full "path":

-drive driver=gluster,servers.0.transport=tcp,servers.0.host=server1,...

Kevin