[Qemu-devel] [PULL 0/6] QMP queue

[Qemu-devel] [PULL 0/6] QMP queue

[Luiz Capitulino]

The following changes since commit 51455c59ddc370612f6e070d8eb0e594aaa7ef24:

  Merge remote-tracking branch 'afaerber/tags/qom-cpu-for-anthony' into staging (2013-07-10 10:54:16 -0500)

are available in the git repository at:


  git://repo.or.cz/qemu/qmp-unstable.git queue/qmp

for you to fetch changes up to 5e2ac5191772dea782ff78e95edd395985273019:

  add timestamp to error_report() (2013-07-10 13:42:09 -0400)

----------------------------------------------------------------
Kevin Wolf (4):
      qapi.py: Avoid code duplication
      qapi.py: Allow top-level type reference for command definitions
      qapi-schema: Use BlockdevSnapshot type for blockdev-snapshot-sync
      qapi-schema: Use existing type for drive-backup arguments

Markus Armbruster (1):
      qemu-char: Fix ringbuf option size

Seiji Aguchi (1):
      add timestamp to error_report()

 include/qemu/error-report.h |  2 ++
 qapi-schema.json            | 46 ++++-----------------------------------------
 qemu-char.c                 |  2 +-
 qemu-options.hx             | 11 +++++++++++
 scripts/qapi.py             | 37 +++++++++++++++++++++++++++---------
 util/qemu-error.c           | 10 ++++++++++
 vl.c                        | 26 +++++++++++++++++++++++++
 7 files changed, 82 insertions(+), 52 deletions(-)

[Qemu-devel] [PULL 1/6] qemu-char: Fix ringbuf option size

[Luiz Capitulino]

From: Markus Armbruster <armbru@redhat.com>

Any attempt to use it trips an "opt->desc->type == QEMU_OPT_NUMBER"
assertion.  Broken in commit 1da48c65.

Cc: qemu-stable@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qemu-char.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-char.c b/qemu-char.c
index 18c42a3..800d6a6 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3115,7 +3115,7 @@ static void qemu_chr_parse_memory(QemuOpts *opts, ChardevBackend *backend,
 
     backend->memory = g_new0(ChardevMemory, 1);
 
-    val = qemu_opt_get_number(opts, "size", 0);
+    val = qemu_opt_get_size(opts, "size", 0);
     if (val != 0) {
         backend->memory->has_size = true;
         backend->memory->size = val;
-- 
1.8.1.4

[Qemu-devel] [PULL 2/6] qapi.py: Avoid code duplication

[Luiz Capitulino]

From: Kevin Wolf <kwolf@redhat.com>

The code that interprets the read JSON expression and appends types to
the respective global variables was duplicated. We can avoid that by
splitting off the part that reads from the file.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 02ad668..3139994 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -78,10 +78,8 @@ def parse(tokens):
 def evaluate(string):
     return parse(map(lambda x: x, tokenize(string)))[0]
 
-def parse_schema(fp):
-    exprs = []
+def get_expr(fp):
     expr = ''
-    expr_eval = None
 
     for line in fp:
         if line.startswith('#') or line == '\n':
@@ -90,18 +88,20 @@ def parse_schema(fp):
         if line.startswith(' '):
             expr += line
         elif expr:
-            expr_eval = evaluate(expr)
-            if expr_eval.has_key('enum'):
-                add_enum(expr_eval['enum'])
-            elif expr_eval.has_key('union'):
-                add_enum('%sKind' % expr_eval['union'])
-            exprs.append(expr_eval)
+            yield expr
             expr = line
         else:
             expr += line
 
     if expr:
+        yield expr
+
+def parse_schema(fp):
+    exprs = []
+
+    for expr in get_expr(fp):
         expr_eval = evaluate(expr)
+
         if expr_eval.has_key('enum'):
             add_enum(expr_eval['enum'])
         elif expr_eval.has_key('union'):
-- 
1.8.1.4

[Qemu-devel] [PULL 3/6] qapi.py: Allow top-level type reference for command definitions

[Luiz Capitulino]

From: Kevin Wolf <kwolf@redhat.com>

If 'data' for a command definition isn't a dict, but a string, it is
taken as a (struct) type name and the fields of this struct are directly
used as parameters.

This is useful for transactionable commands that can use the same type
definition for both the transaction action and the arguments of the
standalone command.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi.py | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3139994..baf1321 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -106,11 +106,18 @@ def parse_schema(fp):
             add_enum(expr_eval['enum'])
         elif expr_eval.has_key('union'):
             add_enum('%sKind' % expr_eval['union'])
+        elif expr_eval.has_key('type'):
+            add_struct(expr_eval)
         exprs.append(expr_eval)
 
     return exprs
 
 def parse_args(typeinfo):
+    if isinstance(typeinfo, basestring):
+        struct = find_struct(typeinfo)
+        assert struct != None
+        typeinfo = struct['data']
+
     for member in typeinfo:
         argname = member
         argentry = typeinfo[member]
@@ -180,6 +187,18 @@ def type_name(name):
     return name
 
 enum_types = []
+struct_types = []
+
+def add_struct(definition):
+    global struct_types
+    struct_types.append(definition)
+
+def find_struct(name):
+    global struct_types
+    for struct in struct_types:
+        if struct['type'] == name:
+            return struct
+    return None
 
 def add_enum(name):
     global enum_types
-- 
1.8.1.4

[Qemu-devel] [PULL 4/6] qapi-schema: Use BlockdevSnapshot type for blockdev-snapshot-sync

[Luiz Capitulino]

From: Kevin Wolf <kwolf@redhat.com>

We don't have to duplicate the definition any more now that we may refer
to a type instead.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qapi-schema.json | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 5c32528..a90aeb1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1709,16 +1709,7 @@
 #
 # Generates a synchronous snapshot of a block device.
 #
-# @device:  the name of the device to generate the snapshot from.
-#
-# @snapshot-file: the target of the new image. If the file exists, or if it
-#                 is a device, the snapshot will be created in the existing
-#                 file/device. If does not exist, a new file will be created.
-#
-# @format: #optional the format of the snapshot image, default is 'qcow2'.
-#
-# @mode: #optional whether and how QEMU should create a new image, default is
-#        'absolute-paths'.
+# For the arguments, see the documentation of BlockdevSnapshot.
 #
 # Returns: nothing on success
 #          If @device is not a valid block device, DeviceNotFound
@@ -1726,8 +1717,7 @@
 # Since 0.14.0
 ##
 { 'command': 'blockdev-snapshot-sync',
-  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
-            '*mode': 'NewImageMode'} }
+  'data': 'BlockdevSnapshot' }
 
 ##
 # @human-monitor-command:
-- 
1.8.1.4

[Qemu-devel] [PULL 5/6] qapi-schema: Use existing type for drive-backup arguments

[Luiz Capitulino]

From: Kevin Wolf <kwolf@redhat.com>

This removes duplicated definitions and documentation by reusing the
existing data type.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qapi-schema.json | 32 ++------------------------------
 1 file changed, 2 insertions(+), 30 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index a90aeb1..b251d28 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1791,42 +1791,14 @@
 # The operation can be stopped before it has completed using the
 # block-job-cancel command.
 #
-# @device: the name of the device which should be copied.
-#
-# @target: the target of the new image. If the file exists, or if it
-#          is a device, the existing file/device will be used as the new
-#          destination.  If it does not exist, a new file will be created.
-#
-# @format: #optional the format of the new destination, default is to
-#          probe if @mode is 'existing', else the format of the source
-#
-# @mode: #optional whether and how QEMU should create a new image, default is
-#        'absolute-paths'.
-#
-# @speed: #optional the maximum speed, in bytes per second
-#
-# @on-source-error: #optional the action to take on an error on the source,
-#                   default 'report'.  'stop' and 'enospc' can only be used
-#                   if the block device supports io-status (see BlockInfo).
-#
-# @on-target-error: #optional the action to take on an error on the target,
-#                   default 'report' (no limitations, since this applies to
-#                   a different block device than @device).
-#
-# Note that @on-source-error and @on-target-error only affect background I/O.
-# If an error occurs during a guest write request, the device's rerror/werror
-# actions will be used.
+# For the arguments, see the documentation of DriveBackup.
 #
 # Returns: nothing on success
 #          If @device is not a valid block device, DeviceNotFound
 #
 # Since 1.6
 ##
-{ 'command': 'drive-backup',
-  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
-            '*mode': 'NewImageMode', '*speed': 'int',
-            '*on-source-error': 'BlockdevOnError',
-            '*on-target-error': 'BlockdevOnError' } }
+{ 'command': 'drive-backup', 'data': 'DriveBackup' }
 
 ##
 # @drive-mirror
-- 
1.8.1.4

[Qemu-devel] [PULL 6/6] add timestamp to error_report()

[Luiz Capitulino]

From: Seiji Aguchi <seiji.aguchi@hds.com>

[Issue]
When we offer a customer support service and a problem happens
in a customer's system, we try to understand the problem by
comparing what the customer reports with message logs of the
customer's system.

In this case, we often need to know when the problem happens.

But, currently, there is no timestamp in qemu's error messages.
Therefore, we may not be able to understand the problem based on
error messages.

[Solution]
Add a timestamp to qemu's error message logged by
error_report() with g_time_val_to_iso8601().

Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 include/qemu/error-report.h |  2 ++
 qemu-options.hx             | 11 +++++++++++
 util/qemu-error.c           | 10 ++++++++++
 vl.c                        | 26 ++++++++++++++++++++++++++
 4 files changed, 49 insertions(+)

diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index 14c1719..3b098a9 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -14,6 +14,7 @@
 #define QEMU_ERROR_H
 
 #include <stdarg.h>
+#include <stdbool.h>
 #include "qemu/compiler.h"
 
 typedef struct Location {
@@ -40,5 +41,6 @@ void error_print_loc(void);
 void error_set_progname(const char *argv0);
 void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 const char *error_get_progname(void);
+extern bool enable_timestamp_msg;
 
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 7cc4d8e..4e98b4f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3100,6 +3100,17 @@ property must be set.  These objects are placed in the
 '/objects' path.
 ETEXI
 
+DEF("msg", HAS_ARG, QEMU_OPTION_msg,
+    "-msg timestamp[=on|off]\n"
+    "                change the format of messages\n"
+    "                on|off controls leading timestamps (default:on)\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -msg timestamp[=on|off]
+@findex -msg
+prepend a timestamp to each log message.(default:on)
+ETEXI
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/util/qemu-error.c b/util/qemu-error.c
index 08a36f4..fec02c6 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -196,6 +196,7 @@ void error_print_loc(void)
     }
 }
 
+bool enable_timestamp_msg;
 /*
  * Print an error message to current monitor if we have one, else to stderr.
  * Format arguments like sprintf().  The result should not contain
@@ -206,6 +207,15 @@ void error_print_loc(void)
 void error_report(const char *fmt, ...)
 {
     va_list ap;
+    GTimeVal tv;
+    gchar *timestr;
+
+    if (enable_timestamp_msg) {
+        g_get_current_time(&tv);
+        timestr = g_time_val_to_iso8601(&tv);
+        error_printf("%s ", timestr);
+        g_free(timestr);
+    }
 
     error_print_loc();
     va_start(ap, fmt);
diff --git a/vl.c b/vl.c
index bea1a10..25b8f2f 100644
--- a/vl.c
+++ b/vl.c
@@ -516,6 +516,18 @@ static QemuOptsList qemu_realtime_opts = {
     },
 };
 
+static QemuOptsList qemu_msg_opts = {
+    .name = "msg",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_msg_opts.head),
+    .desc = {
+        {
+            .name = "timestamp",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end of list */ }
+    },
+};
+
 /**
  * Get machine options
  *
@@ -1503,6 +1515,12 @@ static void configure_realtime(QemuOpts *opts)
     }
 }
 
+
+static void configure_msg(QemuOpts *opts)
+{
+    enable_timestamp_msg = qemu_opt_get_bool(opts, "timestamp", true);
+}
+
 /***********************************************************/
 /* USB devices */
 
@@ -2942,6 +2960,7 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_object_opts);
     qemu_add_opts(&qemu_tpmdev_opts);
     qemu_add_opts(&qemu_realtime_opts);
+    qemu_add_opts(&qemu_msg_opts);
 
     runstate_init();
 
@@ -3838,6 +3857,13 @@ int main(int argc, char **argv, char **envp)
                 }
                 configure_realtime(opts);
                 break;
+            case QEMU_OPTION_msg:
+                opts = qemu_opts_parse(qemu_find_opts("msg"), optarg, 0);
+                if (!opts) {
+                    exit(1);
+                }
+                configure_msg(opts);
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
-- 
1.8.1.4

Re: [Qemu-devel] [PULL 6/6] add timestamp to error_report()

[Paolo Bonzini]

Il 10/07/2013 19:52, Luiz Capitulino ha scritto:
> From: Seiji Aguchi <seiji.aguchi@hds.com>
> 
> [Issue]
> When we offer a customer support service and a problem happens
> in a customer's system, we try to understand the problem by
> comparing what the customer reports with message logs of the
> customer's system.
> 
> In this case, we often need to know when the problem happens.
> 
> But, currently, there is no timestamp in qemu's error messages.
> Therefore, we may not be able to understand the problem based on
> error messages.
> 
> [Solution]
> Add a timestamp to qemu's error message logged by
> error_report() with g_time_val_to_iso8601().
> 
> Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>

What about removing the option, and instead:

- logging no timestamp until the virtual machine has started

- always logging timestamps after the virtual machine has started

?

Paolo

Re: [Qemu-devel] [PULL 6/6] add timestamp to error_report()

[Luiz Capitulino]

On Fri, 12 Jul 2013 08:58:03 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 10/07/2013 19:52, Luiz Capitulino ha scritto:
> > From: Seiji Aguchi <seiji.aguchi@hds.com>
> > 
> > [Issue]
> > When we offer a customer support service and a problem happens
> > in a customer's system, we try to understand the problem by
> > comparing what the customer reports with message logs of the
> > customer's system.
> > 
> > In this case, we often need to know when the problem happens.
> > 
> > But, currently, there is no timestamp in qemu's error messages.
> > Therefore, we may not be able to understand the problem based on
> > error messages.
> > 
> > [Solution]
> > Add a timestamp to qemu's error message logged by
> > error_report() with g_time_val_to_iso8601().
> > 
> > Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> 
> What about removing the option, and instead:
> 
> - logging no timestamp until the virtual machine has started

You mean, the guest has started? Why? What about if an error
happens before the guest has started?

> - always logging timestamps after the virtual machine has started

I don't like having this enabled by default, because it makes
error messages pretty verbose and also because it's not human
readable at a first glance. So, IMO, only people who really
want this should enable it.

Re: [Qemu-devel] [PULL 6/6] add timestamp to error_report()

[Seiji Aguchi]

> -----Original Message-----
> From: qemu-devel-bounces+seiji.aguchi=hds.com@nongnu.org [mailto:qemu-devel-bounces+seiji.aguchi=hds.com@nongnu.org]
> On Behalf Of Luiz Capitulino
> Sent: Friday, July 12, 2013 8:47 AM
> To: Paolo Bonzini
> Cc: aliguori@us.ibm.com; qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PULL 6/6] add timestamp to error_report()
> 
> On Fri, 12 Jul 2013 08:58:03 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > Il 10/07/2013 19:52, Luiz Capitulino ha scritto:
> > > From: Seiji Aguchi <seiji.aguchi@hds.com>
> > >
> > > [Issue]
> > > When we offer a customer support service and a problem happens
> > > in a customer's system, we try to understand the problem by
> > > comparing what the customer reports with message logs of the
> > > customer's system.
> > >
> > > In this case, we often need to know when the problem happens.
> > >
> > > But, currently, there is no timestamp in qemu's error messages.
> > > Therefore, we may not be able to understand the problem based on
> > > error messages.
> > >
> > > [Solution]
> > > Add a timestamp to qemu's error message logged by
> > > error_report() with g_time_val_to_iso8601().
> > >
> > > Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>
> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> >
> > What about removing the option, and instead:
> >
> > - logging no timestamp until the virtual machine has started
> 
> You mean, the guest has started? Why? What about if an error
> happens before the guest has started?

I agree with Luiz.
We need the timestamp to investigate an error before the guest has started.

> 
> > - always logging timestamps after the virtual machine has started
> 
> I don't like having this enabled by default, because it makes
> error messages pretty verbose and also because it's not human
> readable at a first glance. So, IMO, only people who really
> want this should enable it.

We have already discussed this.
There is an use case that timestamp is not needed.

http://marc.info/?l=qemu-devel&m=135997005926563&w=2

Seiji