[Qemu-devel] [PULL for-2.8 0/3] Block patches for -rc3

[Qemu-devel] [PULL for-2.8 0/3] Block patches for -rc3

[Jeff Cody]

The following changes since commit bc66cedb4141fb7588f2462c74310d8fb5dd4cf1:

  Merge remote-tracking branch 'yongbok/tags/mips-20161204' into staging (2016-12-05 10:56:45 +0000)

are available in the git repository at:

  https://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to 76b5550f709b975a7b04fb4c887f300b7bb731c2:

  qemu-doc: update gluster protocol usage guide (2016-12-05 16:30:29 -0500)

----------------------------------------------------------------
Gluster block patches for -rc3
----------------------------------------------------------------

Prasanna Kumar Kalever (3):
  block/gluster: fix QMP to match debug option
  block/nfs: fix QMP to match debug option
  qemu-doc: update gluster protocol usage guide

 block/gluster.c      | 38 ++++++++++++++++-----------------
 block/nfs.c          |  4 ++--
 qapi/block-core.json |  8 +++----
 qemu-doc.texi        | 59 +++++++++++++++++++++++++++++++++++++++-------------
 qemu-options.hx      | 25 ++++++++++++++++++++--
 5 files changed, 93 insertions(+), 41 deletions(-)

-- 
2.7.4

[Qemu-devel] [PULL for-2.8 1/3] block/gluster: fix QMP to match debug option

[Jeff Cody]

From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

The QMP definition of BlockdevOptionsGluster:
{ 'struct': 'BlockdevOptionsGluster',
  'data': { 'volume': 'str',
            'path': 'str',
            'server': ['GlusterServer'],
            '*debug-level': 'int',
            '*logfile': 'str' } }

But instead of 'debug-level we have exported 'debug' as the option for choosing
debug level of gluster protocol driver.

This patch fix QMP definition BlockdevOptionsGluster
s/debug-level/debug/

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/gluster.c      | 38 +++++++++++++++++++-------------------
 qapi/block-core.json |  4 ++--
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 891c13b..a0a74e4 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -48,7 +48,7 @@ typedef struct BDRVGlusterState {
     struct glfs_fd *fd;
     char *logfile;
     bool supports_seek_data;
-    int debug_level;
+    int debug;
 } BDRVGlusterState;
 
 typedef struct BDRVGlusterReopenState {
@@ -434,7 +434,7 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
         }
     }
 
-    ret = glfs_set_logging(glfs, gconf->logfile, gconf->debug_level);
+    ret = glfs_set_logging(glfs, gconf->logfile, gconf->debug);
     if (ret < 0) {
         goto out;
     }
@@ -788,17 +788,17 @@ static int qemu_gluster_open(BlockDriverState *bs,  QDict *options,
 
     filename = qemu_opt_get(opts, GLUSTER_OPT_FILENAME);
 
-    s->debug_level = qemu_opt_get_number(opts, GLUSTER_OPT_DEBUG,
-                                         GLUSTER_DEBUG_DEFAULT);
-    if (s->debug_level < 0) {
-        s->debug_level = 0;
-    } else if (s->debug_level > GLUSTER_DEBUG_MAX) {
-        s->debug_level = GLUSTER_DEBUG_MAX;
+    s->debug = qemu_opt_get_number(opts, GLUSTER_OPT_DEBUG,
+                                   GLUSTER_DEBUG_DEFAULT);
+    if (s->debug < 0) {
+        s->debug = 0;
+    } else if (s->debug > GLUSTER_DEBUG_MAX) {
+        s->debug = GLUSTER_DEBUG_MAX;
     }
 
     gconf = g_new0(BlockdevOptionsGluster, 1);
-    gconf->debug_level = s->debug_level;
-    gconf->has_debug_level = true;
+    gconf->debug = s->debug;
+    gconf->has_debug = true;
 
     logfile = qemu_opt_get(opts, GLUSTER_OPT_LOGFILE);
     s->logfile = g_strdup(logfile ? logfile : GLUSTER_LOGFILE_DEFAULT);
@@ -874,8 +874,8 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
     qemu_gluster_parse_flags(state->flags, &open_flags);
 
     gconf = g_new0(BlockdevOptionsGluster, 1);
-    gconf->debug_level = s->debug_level;
-    gconf->has_debug_level = true;
+    gconf->debug = s->debug;
+    gconf->has_debug = true;
     gconf->logfile = g_strdup(s->logfile);
     gconf->has_logfile = true;
     reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
@@ -1011,14 +1011,14 @@ static int qemu_gluster_create(const char *filename,
     char *tmp = NULL;
 
     gconf = g_new0(BlockdevOptionsGluster, 1);
-    gconf->debug_level = qemu_opt_get_number_del(opts, GLUSTER_OPT_DEBUG,
-                                                 GLUSTER_DEBUG_DEFAULT);
-    if (gconf->debug_level < 0) {
-        gconf->debug_level = 0;
-    } else if (gconf->debug_level > GLUSTER_DEBUG_MAX) {
-        gconf->debug_level = GLUSTER_DEBUG_MAX;
+    gconf->debug = qemu_opt_get_number_del(opts, GLUSTER_OPT_DEBUG,
+                                           GLUSTER_DEBUG_DEFAULT);
+    if (gconf->debug < 0) {
+        gconf->debug = 0;
+    } else if (gconf->debug > GLUSTER_DEBUG_MAX) {
+        gconf->debug = GLUSTER_DEBUG_MAX;
     }
-    gconf->has_debug_level = true;
+    gconf->has_debug = true;
 
     gconf->logfile = qemu_opt_get_del(opts, GLUSTER_OPT_LOGFILE);
     if (!gconf->logfile) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c29bef7..03b19f1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2195,7 +2195,7 @@
 #
 # @server:      gluster servers description
 #
-# @debug-level: #optional libgfapi log level (default '4' which is Error)
+# @debug:       #optional libgfapi log level (default '4' which is Error)
 #
 # @logfile:     #optional libgfapi log file (default /dev/stderr) (Since 2.8)
 #
@@ -2205,7 +2205,7 @@
   'data': { 'volume': 'str',
             'path': 'str',
             'server': ['GlusterServer'],
-            '*debug-level': 'int',
+            '*debug': 'int',
             '*logfile': 'str' } }
 
 ##
-- 
2.7.4

[Qemu-devel] [PULL for-2.8 2/3] block/nfs: fix QMP to match debug option

[Jeff Cody]

From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

The QMP definition of BlockdevOptionsNfs:
{ 'struct': 'BlockdevOptionsNfs',
  'data': { 'server': 'NFSServer',
            'path': 'str',
            '*user': 'int',
            '*group': 'int',
            '*tcp-syn-count': 'int',
            '*readahead-size': 'int',
            '*page-cache-size': 'int',
            '*debug-level': 'int' } }

To make this consistent with other block protocols like gluster, lets
change s/debug-level/debug/

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/nfs.c          | 4 ++--
 qapi/block-core.json | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index d082783..a490660 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -134,7 +134,7 @@ static int nfs_parse_uri(const char *filename, QDict *options, Error **errp)
             qdict_put(options, "page-cache-size",
                       qstring_from_str(qp->p[i].value));
         } else if (!strcmp(qp->p[i].name, "debug")) {
-            qdict_put(options, "debug-level",
+            qdict_put(options, "debug",
                       qstring_from_str(qp->p[i].value));
         } else {
             error_setg(errp, "Unknown NFS parameter name: %s",
@@ -165,7 +165,7 @@ static bool nfs_has_filename_options_conflict(QDict *options, Error **errp)
             !strcmp(qe->key, "tcp-syn-count") ||
             !strcmp(qe->key, "readahead-size") ||
             !strcmp(qe->key, "page-cache-size") ||
-            !strcmp(qe->key, "debug-level") ||
+            !strcmp(qe->key, "debug") ||
             strstart(qe->key, "server.", NULL))
         {
             error_setg(errp, "Option %s cannot be used with a filename",
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 03b19f1..f22ed2a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2292,7 +2292,7 @@
 # @page-cache-size:         #optional set the pagecache size in bytes (defaults
 #                           to libnfs default)
 #
-# @debug-level:             #optional set the NFS debug level (max 2) (defaults
+# @debug:                   #optional set the NFS debug level (max 2) (defaults
 #                           to libnfs default)
 #
 # Since 2.8
@@ -2305,7 +2305,7 @@
             '*tcp-syn-count': 'int',
             '*readahead-size': 'int',
             '*page-cache-size': 'int',
-            '*debug-level': 'int' } }
+            '*debug': 'int' } }
 
 ##
 # @BlockdevOptionsCurl
-- 
2.7.4

[Qemu-devel] [PULL for-2.8 3/3] qemu-doc: update gluster protocol usage guide

[Jeff Cody]

From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

Document:
1. The new debug and logfile options with their usages
2. New json format and its usage and
3. update "GlusterFS, Device URL Syntax" section in "Invocation"

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 qemu-doc.texi   | 59 +++++++++++++++++++++++++++++++++++++++++++--------------
 qemu-options.hx | 25 ++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 16 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index 023c140..02cb39d 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1041,35 +1041,55 @@ GlusterFS is an user space distributed file system.
 
 You can boot from the GlusterFS disk image with the command:
 @example
-qemu-system-x86_64 -drive file=gluster[+@var{transport}]://[@var{server}[:@var{port}]]/@var{volname}/@var{image}[?socket=...]
+URI:
+qemu-system-x86_64 -drive file=gluster[+@var{type}]://[@var{host}[:@var{port}]]/@var{volume}/@var{path}
+                               [?socket=...][,file.debug=9][,file.logfile=...]
+
+JSON:
+qemu-system-x86_64 'json:@{"driver":"qcow2",
+                           "file":@{"driver":"gluster",
+                                    "volume":"testvol","path":"a.img","debug":9,"logfile":"...",
+                                    "server":[@{"type":"tcp","host":"...","port":"..."@},
+                                              @{"type":"unix","socket":"..."@}]@}@}'
 @end example
 
 @var{gluster} is the protocol.
 
-@var{transport} specifies the transport type used to connect to gluster
+@var{type} specifies the transport type used to connect to gluster
 management daemon (glusterd). Valid transport types are
-tcp, unix and rdma. If a transport type isn't specified, then tcp
-type is assumed.
+tcp and unix. In the URI form, if a transport type isn't specified,
+then tcp type is assumed.
 
-@var{server} specifies the server where the volume file specification for
-the given volume resides. This can be either hostname, ipv4 address
-or ipv6 address. ipv6 address needs to be within square brackets [ ].
-If transport type is unix, then @var{server} field should not be specified.
+@var{host} specifies the server where the volume file specification for
+the given volume resides. This can be either a hostname or an ipv4 address.
+If transport type is unix, then @var{host} field should not be specified.
 Instead @var{socket} field needs to be populated with the path to unix domain
 socket.
 
 @var{port} is the port number on which glusterd is listening. This is optional
-and if not specified, QEMU will send 0 which will make gluster to use the
-default port. If the transport type is unix, then @var{port} should not be
-specified.
+and if not specified, it defaults to port 24007. If the transport type is unix,
+then @var{port} should not be specified.
+
+@var{volume} is the name of the gluster volume which contains the disk image.
+
+@var{path} is the path to the actual disk image that resides on gluster volume.
+
+@var{debug} is the logging level of the gluster protocol driver. Debug levels
+are 0-9, with 9 being the most verbose, and 0 representing no debugging output.
+The default level is 4. The current logging levels defined in the gluster source
+are 0 - None, 1 - Emergency, 2 - Alert, 3 - Critical, 4 - Error, 5 - Warning,
+6 - Notice, 7 - Info, 8 - Debug, 9 - Trace
+
+@var{logfile} is a commandline option to mention log file path which helps in
+logging to the specified file and also help in persisting the gfapi logs. The
+default is stderr.
+
 
-@var{volname} is the name of the gluster volume which contains the disk image.
 
-@var{image} is the path to the actual disk image that resides on gluster volume.
 
 You can create a GlusterFS disk image with the command:
 @example
-qemu-img create gluster://@var{server}/@var{volname}/@var{image} @var{size}
+qemu-img create gluster://@var{host}/@var{volume}/@var{path} @var{size}
 @end example
 
 Examples
@@ -1082,6 +1102,17 @@ qemu-system-x86_64 -drive file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir
 qemu-system-x86_64 -drive file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img
 qemu-system-x86_64 -drive file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket
 qemu-system-x86_64 -drive file=gluster+rdma://1.2.3.4:24007/testvol/a.img
+qemu-system-x86_64 -drive file=gluster://1.2.3.4/testvol/a.img,file.debug=9,file.logfile=/var/log/qemu-gluster.log
+qemu-system-x86_64 'json:@{"driver":"qcow2",
+                           "file":@{"driver":"gluster",
+                                    "volume":"testvol","path":"a.img",
+                                    "debug":9,"logfile":"/var/log/qemu-gluster.log",
+                                    "server":[@{"type":"tcp","host":"1.2.3.4","port":24007@},
+                                              @{"type":"unix","socket":"/var/run/glusterd.socket"@}]@}@}'
+qemu-system-x86_64 -drive driver=qcow2,file.driver=gluster,file.volume=testvol,file.path=/path/a.img,
+                                       file.debug=9,file.logfile=/var/log/qemu-gluster.log,
+                                       file.server.0.type=tcp,file.server.0.host=1.2.3.4,file.server.0.port=24007,
+                                       file.server.1.type=unix,file.server.1.socket=/var/run/glusterd.socket
 @end example
 
 @node disk_images_ssh
diff --git a/qemu-options.hx b/qemu-options.hx
index 4a5b29f..c534a2f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2595,13 +2595,34 @@ TCP, Unix Domain Sockets and RDMA transport protocols.
 
 Syntax for specifying a VM disk image on GlusterFS volume is
 @example
-gluster[+transport]://[server[:port]]/volname/image[?socket=...]
+
+URI:
+gluster[+type]://[host[:port]]/volume/path[?socket=...][,debug=N][,logfile=...]
+
+JSON:
+'json:@{"driver":"qcow2","file":@{"driver":"gluster","volume":"testvol","path":"a.img","debug":N,"logfile":"...",
+@                                 "server":[@{"type":"tcp","host":"...","port":"..."@},
+@                                           @{"type":"unix","socket":"..."@}]@}@}'
 @end example
 
 
 Example
 @example
-qemu-system-x86_64 --drive file=gluster://192.0.2.1/testvol/a.img
+URI:
+qemu-system-x86_64 --drive file=gluster://192.0.2.1/testvol/a.img,
+@                               file.debug=9,file.logfile=/var/log/qemu-gluster.log
+
+JSON:
+qemu-system-x86_64 'json:@{"driver":"qcow2",
+@                          "file":@{"driver":"gluster",
+@                                   "volume":"testvol","path":"a.img",
+@                                   "debug":9,"logfile":"/var/log/qemu-gluster.log",
+@                                   "server":[@{"type":"tcp","host":"1.2.3.4","port":24007@},
+@                                             @{"type":"unix","socket":"/var/run/glusterd.socket"@}]@}@}'
+qemu-system-x86_64 -drive driver=qcow2,file.driver=gluster,file.volume=testvol,file.path=/path/a.img,
+@                                      file.debug=9,file.logfile=/var/log/qemu-gluster.log,
+@                                      file.server.0.type=tcp,file.server.0.host=1.2.3.4,file.server.0.port=24007,
+@                                      file.server.1.type=unix,file.server.1.socket=/var/run/glusterd.socket
 @end example
 
 See also @url{http://www.gluster.org}.
-- 
2.7.4

Re: [Qemu-devel] [Qemu-block] [PULL for-2.8 0/3] Block patches for -rc3

[Stefan Hajnoczi]

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

On Mon, Dec 05, 2016 at 04:31:58PM -0500, Jeff Cody wrote:
> The following changes since commit bc66cedb4141fb7588f2462c74310d8fb5dd4cf1:
> 
>   Merge remote-tracking branch 'yongbok/tags/mips-20161204' into staging (2016-12-05 10:56:45 +0000)
> 
> are available in the git repository at:
> 
>   https://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
> 
> for you to fetch changes up to 76b5550f709b975a7b04fb4c887f300b7bb731c2:
> 
>   qemu-doc: update gluster protocol usage guide (2016-12-05 16:30:29 -0500)
> 
> ----------------------------------------------------------------
> Gluster block patches for -rc3
> ----------------------------------------------------------------
> 
> Prasanna Kumar Kalever (3):
>   block/gluster: fix QMP to match debug option
>   block/nfs: fix QMP to match debug option
>   qemu-doc: update gluster protocol usage guide
> 
>  block/gluster.c      | 38 ++++++++++++++++-----------------
>  block/nfs.c          |  4 ++--
>  qapi/block-core.json |  8 +++----
>  qemu-doc.texi        | 59 +++++++++++++++++++++++++++++++++++++++-------------
>  qemu-options.hx      | 25 ++++++++++++++++++++--
>  5 files changed, 93 insertions(+), 41 deletions(-)

BlockdevOptionsGluster.debug(-level) does not have "Added in 2.8" so I
had to dig through git-blame(1) to verify that it was indeed added in
the current release cycle.

In the future please make sure all QAPI changes are marked by version.
If there tricky changes you can include a statement showing you are
aware of QAPI backwards compatibility ("These new options were added in
the 2.8 release cycle and can therefore still be changed without
breaking backward compatibility").  This will make me confident that
you've checked the QAPI changes.

Thanks, applied to my staging tree:
https://github.com/stefanha/qemu/commits/staging

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [Qemu-block] [PULL for-2.8 0/3] Block patches for -rc3

[Eric Blake]

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

On 12/06/2016 04:04 AM, Stefan Hajnoczi wrote:

>>
>> Prasanna Kumar Kalever (3):
>>   block/gluster: fix QMP to match debug option
>>   block/nfs: fix QMP to match debug option
>>   qemu-doc: update gluster protocol usage guide
>>
>>  block/gluster.c      | 38 ++++++++++++++++-----------------
>>  block/nfs.c          |  4 ++--
>>  qapi/block-core.json |  8 +++----
>>  qemu-doc.texi        | 59 +++++++++++++++++++++++++++++++++++++++-------------
>>  qemu-options.hx      | 25 ++++++++++++++++++++--
>>  5 files changed, 93 insertions(+), 41 deletions(-)
> 
> BlockdevOptionsGluster.debug(-level) does not have "Added in 2.8" so I
> had to dig through git-blame(1) to verify that it was indeed added in
> the current release cycle.

Then that implies we should add yet one more patch that adds the
appropriate versioning information to all the gluster fields added for
2.8.  My reviewed-by was given on the assumption that debug was in 2.7
and that this was a break from 2.7 behavior, but that we already KNOW
we're breaking blockdev-add between 2.7 and 2.8; while your argument is
that there is no backwards incompatibility because it was not in 2.7 to
begin with.  I think both reasons are indeed acceptable, but it also
means that my reason was flawed because of the incomplete documentation.

> 
> In the future please make sure all QAPI changes are marked by version.

Indeed, and I try to flag it in my reviews as often as I notice it.

> If there tricky changes you can include a statement showing you are
> aware of QAPI backwards compatibility ("These new options were added in
> the 2.8 release cycle and can therefore still be changed without
> breaking backward compatibility").  This will make me confident that
> you've checked the QAPI changes.
> 
> Thanks, applied to my staging tree:
> https://github.com/stefanha/qemu/commits/staging
> 
> Stefan
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [Qemu-block] [PULL for-2.8 0/3] Block patches for -rc3

[Eric Blake]

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

On 12/06/2016 09:11 AM, Eric Blake wrote:

>> BlockdevOptionsGluster.debug(-level) does not have "Added in 2.8" so I
>> had to dig through git-blame(1) to verify that it was indeed added in
>> the current release cycle.
> 
> Then that implies we should add yet one more patch that adds the
> appropriate versioning information to all the gluster fields added for
> 2.8.  My reviewed-by was given on the assumption that debug was in 2.7
> and that this was a break from 2.7 behavior, but that we already KNOW
> we're breaking blockdev-add between 2.7 and 2.8; while your argument is
> that there is no backwards incompatibility because it was not in 2.7 to
> begin with.  I think both reasons are indeed acceptable, but it also
> means that my reason was flawed because of the incomplete documentation.

I checked again.  'git diff v2.7.0..origin qapi/*.json' shows:

 ##
-# @BlockdevOptionsGluster
+# @BlockdevOptionsGluster:
 #
 # Driver specific block device options for Gluster
 #
@@ -2140,7 +2195,9 @@
 #
 # @server:      gluster servers description
 #
-# @debug-level: #optional libgfapi log level (default '4' which is Error)
+# @debug:       #optional libgfapi log level (default '4' which is Error)
+#
+# @logfile:     #optional libgfapi log file (default /dev/stderr)
(Since 2.8)
 #
 # Since: 2.7
 ##
@@ -2148,25 +2205,163 @@
   'data': { 'volume': 'str',
             'path': 'str',
             'server': ['GlusterServer'],
-            '*debug-level': 'int' } }
+            '*debug': 'int',
+            '*logfile': 'str' } }

So I was right after all - this IS a backwards-incompatible change (and
NOT something that was introduced only in 2.8) - but I still argue that
the change is appropriate NOW (but not later) because blockdev-add is
the only client of this type in QMP, and that command changed
backwards-incompatibly at the same time.

> 
>>
>> In the future please make sure all QAPI changes are marked by version.
> 
> Indeed, and I try to flag it in my reviews as often as I notice it.
> 
>> If there tricky changes you can include a statement showing you are
>> aware of QAPI backwards compatibility ("These new options were added in
>> the 2.8 release cycle and can therefore still be changed without
>> breaking backward compatibility").  This will make me confident that
>> you've checked the QAPI changes.
>>
>> Thanks, applied to my staging tree:
>> https://github.com/stefanha/qemu/commits/staging

In my audit of all differences between 2.7 and current state of qapi
.json files, I only found one addition that was lacking versioning
information:

event DEVICE_TRAY_MOVED gained 'id' parameter

I'll submit a patch for that shortly.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [Qemu-block] [PULL for-2.8 0/3] Block patches for -rc3

[Kevin Wolf]

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

Am 06.12.2016 um 16:43 hat Eric Blake geschrieben:
> On 12/06/2016 09:11 AM, Eric Blake wrote:
> 
> >> BlockdevOptionsGluster.debug(-level) does not have "Added in 2.8" so I
> >> had to dig through git-blame(1) to verify that it was indeed added in
> >> the current release cycle.
> > 
> > Then that implies we should add yet one more patch that adds the
> > appropriate versioning information to all the gluster fields added for
> > 2.8.  My reviewed-by was given on the assumption that debug was in 2.7
> > and that this was a break from 2.7 behavior, but that we already KNOW
> > we're breaking blockdev-add between 2.7 and 2.8; while your argument is
> > that there is no backwards incompatibility because it was not in 2.7 to
> > begin with.  I think both reasons are indeed acceptable, but it also
> > means that my reason was flawed because of the incomplete documentation.
> 
> I checked again.  'git diff v2.7.0..origin qapi/*.json' shows:
> 
>  ##
> -# @BlockdevOptionsGluster
> +# @BlockdevOptionsGluster:
>  #
>  # Driver specific block device options for Gluster
>  #
> @@ -2140,7 +2195,9 @@
>  #
>  # @server:      gluster servers description
>  #
> -# @debug-level: #optional libgfapi log level (default '4' which is Error)
> +# @debug:       #optional libgfapi log level (default '4' which is Error)
> +#
> +# @logfile:     #optional libgfapi log file (default /dev/stderr)
> (Since 2.8)
>  #
>  # Since: 2.7
>  ##
> @@ -2148,25 +2205,163 @@
>    'data': { 'volume': 'str',
>              'path': 'str',
>              'server': ['GlusterServer'],
> -            '*debug-level': 'int' } }
> +            '*debug': 'int',
> +            '*logfile': 'str' } }
> 
> So I was right after all - this IS a backwards-incompatible change (and
> NOT something that was introduced only in 2.8) - but I still argue that
> the change is appropriate NOW (but not later) because blockdev-add is
> the only client of this type in QMP, and that command changed
> backwards-incompatibly at the same time.

I don't completely understand the change anyway. 'debug-level' is the
better name, and if the command line doesn't agree, change that one or
grudgingly accept both. But I think we can just change the command line,
it's only a debug option.

Kevin

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

Re: [Qemu-devel] [Qemu-block] [PULL for-2.8 0/3] Block patches for -rc3

[Eric Blake]

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

On 12/07/2016 04:08 AM, Kevin Wolf wrote:

>>  # Since: 2.7
>>  ##
>> @@ -2148,25 +2205,163 @@
>>    'data': { 'volume': 'str',
>>              'path': 'str',
>>              'server': ['GlusterServer'],
>> -            '*debug-level': 'int' } }
>> +            '*debug': 'int',
>> +            '*logfile': 'str' } }
>>
>> So I was right after all - this IS a backwards-incompatible change (and
>> NOT something that was introduced only in 2.8) - but I still argue that
>> the change is appropriate NOW (but not later) because blockdev-add is
>> the only client of this type in QMP, and that command changed
>> backwards-incompatibly at the same time.
> 
> I don't completely understand the change anyway. 'debug-level' is the
> better name,

Perhaps so, but 2.7 already used 'debug' on the command line, and
released libvirt already targets the command line. Changing the command
line to accept 'debug-level' in addition to 'debug' is the only way to
not break existing libvirt usage.  What's more, we have several
instances of 'debug' in other spots of blockdev-add; either they should
all be consistently named 'debug-level', or none of them; we made the
switch to none of them, as it was easier.

> and if the command line doesn't agree, change that one or
> grudgingly accept both. But I think we can just change the command line,
> it's only a debug option.

It may only be a debug option, but for good or bad, libvirt is already
targetting it by its current spelling of 'debug'.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]