[Qemu-devel] [PULL 1/9] gluster: Fix blockdev-add with server.N.type=unix

Kevin Wolf posted 9 patches 7 years, 10 months ago
[Qemu-devel] [PULL 1/9] gluster: Fix blockdev-add with server.N.type=unix
Posted by Kevin Wolf 7 years, 10 months ago
The legacy command line interface gets the socket path from an option
called 'socket'. QAPI in contract uses SocketAddress, where the
corresponding option is called 'path'.

Fix the gluster block driver to accept both 'socket' and 'path', with
'path' being the preferred syntax.

https://bugzilla.redhat.com/show_bug.cgi?id=1545155

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/gluster.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 296e036b3d..4adc1a875b 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -167,7 +167,12 @@ static QemuOptsList runtime_unix_opts = {
         {
             .name = GLUSTER_OPT_SOCKET,
             .type = QEMU_OPT_STRING,
-            .help = "socket file path)",
+            .help = "socket file path (legacy)",
+        },
+        {
+            .name = GLUSTER_OPT_PATH,
+            .type = QEMU_OPT_STRING,
+            .help = "socket file path (QAPI)",
         },
         { /* end of list */ }
     },
@@ -615,10 +620,18 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
                 goto out;
             }
 
-            ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
+            ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
+            if (!ptr) {
+                ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
+            } else if (qemu_opt_get(opts, GLUSTER_OPT_SOCKET)) {
+                error_setg(&local_err,
+                           "Conflicting parameters 'path' and 'socket'");
+                error_append_hint(&local_err, GERR_INDEX_HINT, i);
+                goto out;
+            }
             if (!ptr) {
                 error_setg(&local_err, QERR_MISSING_PARAMETER,
-                           GLUSTER_OPT_SOCKET);
+                           GLUSTER_OPT_PATH);
                 error_append_hint(&local_err, GERR_INDEX_HINT, i);
                 goto out;
             }
@@ -684,7 +697,7 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
                              "file.server.0.host=1.2.3.4,"
                              "file.server.0.port=24007,"
                              "file.server.1.transport=unix,"
-                             "file.server.1.socket=/var/run/glusterd.socket ..."
+                             "file.server.1.path=/var/run/glusterd.socket ..."
                              "\n");
             return ret;
         }
-- 
2.13.6


Re: [Qemu-devel] [PULL 1/9] gluster: Fix blockdev-add with server.N.type=unix
Posted by Jeff Cody 7 years, 10 months ago
On Tue, Apr 03, 2018 at 06:33:52PM +0200, Kevin Wolf wrote:
> The legacy command line interface gets the socket path from an option
> called 'socket'. QAPI in contract uses SocketAddress, where the
> corresponding option is called 'path'.
> 
> Fix the gluster block driver to accept both 'socket' and 'path', with
> 'path' being the preferred syntax.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1545155
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Oops, you and I had a pull request collision on this patch (it is in both of
our PRs); Sorry, my fault, I didn't know you were pulling it in through your
tree.

> ---
>  block/gluster.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/block/gluster.c b/block/gluster.c
> index 296e036b3d..4adc1a875b 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -167,7 +167,12 @@ static QemuOptsList runtime_unix_opts = {
>          {
>              .name = GLUSTER_OPT_SOCKET,
>              .type = QEMU_OPT_STRING,
> -            .help = "socket file path)",
> +            .help = "socket file path (legacy)",
> +        },
> +        {
> +            .name = GLUSTER_OPT_PATH,
> +            .type = QEMU_OPT_STRING,
> +            .help = "socket file path (QAPI)",
>          },
>          { /* end of list */ }
>      },
> @@ -615,10 +620,18 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>                  goto out;
>              }
>  
> -            ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
> +            ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
> +            if (!ptr) {
> +                ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
> +            } else if (qemu_opt_get(opts, GLUSTER_OPT_SOCKET)) {
> +                error_setg(&local_err,
> +                           "Conflicting parameters 'path' and 'socket'");
> +                error_append_hint(&local_err, GERR_INDEX_HINT, i);
> +                goto out;
> +            }
>              if (!ptr) {
>                  error_setg(&local_err, QERR_MISSING_PARAMETER,
> -                           GLUSTER_OPT_SOCKET);
> +                           GLUSTER_OPT_PATH);
>                  error_append_hint(&local_err, GERR_INDEX_HINT, i);
>                  goto out;
>              }
> @@ -684,7 +697,7 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
>                               "file.server.0.host=1.2.3.4,"
>                               "file.server.0.port=24007,"
>                               "file.server.1.transport=unix,"
> -                             "file.server.1.socket=/var/run/glusterd.socket ..."
> +                             "file.server.1.path=/var/run/glusterd.socket ..."
>                               "\n");
>              return ret;
>          }
> -- 
> 2.13.6
> 
> 

Re: [Qemu-devel] [PULL 1/9] gluster: Fix blockdev-add with server.N.type=unix
Posted by Kevin Wolf 7 years, 10 months ago
Am 03.04.2018 um 23:03 hat Jeff Cody geschrieben:
> On Tue, Apr 03, 2018 at 06:33:52PM +0200, Kevin Wolf wrote:
> > The legacy command line interface gets the socket path from an option
> > called 'socket'. QAPI in contract uses SocketAddress, where the
> > corresponding option is called 'path'.
> > 
> > Fix the gluster block driver to accept both 'socket' and 'path', with
> > 'path' being the preferred syntax.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1545155
> > 
> > Cc: qemu-stable@nongnu.org
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> Oops, you and I had a pull request collision on this patch (it is in both of
> our PRs); Sorry, my fault, I didn't know you were pulling it in through your
> tree.

Or my fault for applying a patch to something that is maintained by you.
Anyway, I think git should sort it out without a problem.

Kevin