[Qemu-devel] [PULL 0/5] Error reporting patches for 2016-02-19

[Qemu-devel] [PULL 0/5] Error reporting patches for 2016-02-19

[Markus Armbruster]

The following changes since commit dd5e38b19d7cb07d317e1285941d8245c01da540:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20160218-1' into staging (2016-02-18 15:20:35 +0000)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-error-2016-02-19

for you to fetch changes up to 7580f231cf3f1710951416ec85df7b3f79dbaf86:

  vl: Clean up machine selection in main(). (2016-02-19 13:46:44 +0100)

----------------------------------------------------------------
Error reporting patches for 2016-02-19

----------------------------------------------------------------
Eduardo Habkost (3):
      vl: Reset location after handling command-line arguments
      replay: Set error location properly when parsing options
      vl: Set error location when parsing memory options

Marcel Apfelbaum (1):
      vl.c: Fix regression in machine error message

Markus Armbruster (1):
      vl: Clean up machine selection in main().

 replay/replay.c | 10 ++++++++++
 vl.c            | 53 +++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 49 insertions(+), 14 deletions(-)

Eduardo Habkost (3):
  vl: Reset location after handling command-line arguments
  replay: Set error location properly when parsing options
  vl: Set error location when parsing memory options

Marcel Apfelbaum (1):
  vl.c: Fix regression in machine error message

Markus Armbruster (1):
  vl: Clean up machine selection in main().

 replay/replay.c | 10 ++++++++++
 vl.c            | 53 +++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 49 insertions(+), 14 deletions(-)

-- 
2.4.3

[Qemu-devel] [PULL 1/5] vl.c: Fix regression in machine error message

[Markus Armbruster]

From: Marcel Apfelbaum <marcel@redhat.com>

Commit e1ce0c3cb (vl.c: fix regression when reading machine type
from config file) fixed the error message when the machine type
was supplied inside the config file. However now the option name
is not displayed correctly if the error happens when the machine
is specified at command line.

Running
    ./x86_64-softmmu/qemu-system-x86_64 -M q35-1.5 -redir tcp:8022::22
will result in the error message:
    qemu-system-x86_64: -redir tcp:8022::22: unsupported machine type
    Use -machine help to list supported machines

Fixed it by restoring the error location and also extracted the code
dealing with machine options into a separate function.

Reported-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <1455303747-19776-2-git-send-email-ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 vl.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/vl.c b/vl.c
index 18e6086..226bdeb 100644
--- a/vl.c
+++ b/vl.c
@@ -2762,6 +2762,31 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
     return popt;
 }
 
+static void set_machine_options(MachineClass **machine_class)
+{
+    const char *optarg;
+    QemuOpts *opts;
+    Location loc;
+
+    loc_push_none(&loc);
+
+    opts = qemu_get_machine_opts();
+    qemu_opts_loc_restore(opts);
+
+    optarg = qemu_opt_get(opts, "type");
+    if (optarg) {
+        *machine_class = machine_parse(optarg);
+    }
+
+    if (*machine_class == NULL) {
+        error_report("No machine specified, and there is no default");
+        error_printf("Use -machine help to list supported machines\n");
+        exit(1);
+    }
+
+    loc_pop(&loc);
+}
+
 static int machine_set_property(void *opaque,
                                 const char *name, const char *value,
                                 Error **errp)
@@ -3986,17 +4011,7 @@ int main(int argc, char **argv, char **envp)
 
     replay_configure(icount_opts);
 
-    opts = qemu_get_machine_opts();
-    optarg = qemu_opt_get(opts, "type");
-    if (optarg) {
-        machine_class = machine_parse(optarg);
-    }
-
-    if (machine_class == NULL) {
-        error_report("No machine specified, and there is no default");
-        error_printf("Use -machine help to list supported machines\n");
-        exit(1);
-    }
+    set_machine_options(&machine_class);
 
     set_memory_options(&ram_slots, &maxram_size, machine_class);
 
-- 
2.4.3

[Qemu-devel] [PULL 2/5] vl: Reset location after handling command-line arguments

[Markus Armbruster]

From: Eduardo Habkost <ehabkost@redhat.com>

After looping through all command-line arguments, error location
info becomes obsolete, and any function calling error_report()
will print misleading information. This breaks error reporting
for some option handling, like:

  $ qemu-system-x86_64 -icount rr=x -vnc :0
  qemu-system-x86_64: -vnc :0: Invalid icount rr option: x

  $ qemu-system-x86_64 -m size= -vnc :0
  qemu-system-x86_64: -vnc :0: missing 'size' option value

Fix this by resetting location info as soon as we exit the
command-line handling loop.

With this, replay_configure() and set_memory_options() won't
print any location info yet, but at least they won't print
incorrect information.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <1455303747-19776-3-git-send-email-ehabkost@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
["Do not insert code here" comment added to prevent regressions]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 vl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 226bdeb..bf0ef90 100644
--- a/vl.c
+++ b/vl.c
@@ -4008,6 +4008,11 @@ int main(int argc, char **argv, char **envp)
             }
         }
     }
+    /*
+     * Clear error location left behind by the loop.
+     * Best done right after the loop.  Do not insert code here!
+     */
+    loc_set_none();
 
     replay_configure(icount_opts);
 
@@ -4015,8 +4020,6 @@ int main(int argc, char **argv, char **envp)
 
     set_memory_options(&ram_slots, &maxram_size, machine_class);
 
-    loc_set_none();
-
     os_daemonize();
 
     if (qemu_init_main_loop(&main_loop_err)) {
-- 
2.4.3

[Qemu-devel] [PULL 3/5] replay: Set error location properly when parsing options

[Markus Armbruster]

From: Eduardo Habkost <ehabkost@redhat.com>

Set error location so the error_report() calls will show
appropriate command-line argument or config file info.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <1455303747-19776-4-git-send-email-ehabkost@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 replay/replay.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/replay/replay.c b/replay/replay.c
index 9cac178..f8739c2 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -262,6 +262,14 @@ void replay_configure(QemuOpts *opts)
     const char *fname;
     const char *rr;
     ReplayMode mode = REPLAY_MODE_NONE;
+    Location loc;
+
+    if (!opts) {
+        return;
+    }
+
+    loc_push_none(&loc);
+    qemu_opts_loc_restore(opts);
 
     rr = qemu_opt_get(opts, "rr");
     if (!rr) {
@@ -283,6 +291,8 @@ void replay_configure(QemuOpts *opts)
     }
 
     replay_enable(fname, mode);
+
+    loc_pop(&loc);
 }
 
 void replay_start(void)
-- 
2.4.3

[Qemu-devel] [PULL 4/5] vl: Set error location when parsing memory options

[Markus Armbruster]

From: Eduardo Habkost <ehabkost@redhat.com>

Set error location so the error_report() calls will show
appropriate command-line argument or config file info.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <1455303747-19776-5-git-send-email-ehabkost@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 vl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/vl.c b/vl.c
index bf0ef90..8c1a1ff 100644
--- a/vl.c
+++ b/vl.c
@@ -2863,6 +2863,10 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
     const char *maxmem_str, *slots_str;
     const ram_addr_t default_ram_size = mc->default_ram_size;
     QemuOpts *opts = qemu_find_opts_singleton("memory");
+    Location loc;
+
+    loc_push_none(&loc);
+    qemu_opts_loc_restore(opts);
 
     sz = 0;
     mem_str = qemu_opt_get(opts, "size");
@@ -2937,6 +2941,8 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
                 "'%s' option", slots_str ? "maxmem" : "slots");
         exit(EXIT_FAILURE);
     }
+
+    loc_pop(&loc);
 }
 
 int main(int argc, char **argv, char **envp)
-- 
2.4.3

[Qemu-devel] [PULL 5/5] vl: Clean up machine selection in main().

[Markus Armbruster]

We set machine_class to the default first, and update it to the real
one later.  Any use of machine_class in between is almost certainly
wrong (there are no such uses right now).  Set it once and for all
instead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 vl.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/vl.c b/vl.c
index 8c1a1ff..b87e292 100644
--- a/vl.c
+++ b/vl.c
@@ -2762,8 +2762,9 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
     return popt;
 }
 
-static void set_machine_options(MachineClass **machine_class)
+static MachineClass *select_machine(void)
 {
+    MachineClass *machine_class = find_default_machine();
     const char *optarg;
     QemuOpts *opts;
     Location loc;
@@ -2775,16 +2776,17 @@ static void set_machine_options(MachineClass **machine_class)
 
     optarg = qemu_opt_get(opts, "type");
     if (optarg) {
-        *machine_class = machine_parse(optarg);
+        machine_class = machine_parse(optarg);
     }
 
-    if (*machine_class == NULL) {
+    if (!machine_class) {
         error_report("No machine specified, and there is no default");
         error_printf("Use -machine help to list supported machines\n");
         exit(1);
     }
 
     loc_pop(&loc);
+    return machine_class;
 }
 
 static int machine_set_property(void *opaque,
@@ -3031,7 +3033,6 @@ int main(int argc, char **argv, char **envp)
     os_setup_early_signal_handling();
 
     module_call_init(MODULE_INIT_MACHINE);
-    machine_class = find_default_machine();
     cpu_model = NULL;
     snapshot = 0;
     cyls = heads = secs = 0;
@@ -4022,7 +4023,7 @@ int main(int argc, char **argv, char **envp)
 
     replay_configure(icount_opts);
 
-    set_machine_options(&machine_class);
+    machine_class = select_machine();
 
     set_memory_options(&ram_slots, &maxram_size, machine_class);
 
-- 
2.4.3

Re: [Qemu-devel] [PULL 0/5] Error reporting patches for 2016-02-19

[Peter Maydell]

On 19 February 2016 at 13:27, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit dd5e38b19d7cb07d317e1285941d8245c01da540:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20160218-1' into staging (2016-02-18 15:20:35 +0000)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-error-2016-02-19
>
> for you to fetch changes up to 7580f231cf3f1710951416ec85df7b3f79dbaf86:
>
>   vl: Clean up machine selection in main(). (2016-02-19 13:46:44 +0100)
>
> ----------------------------------------------------------------
> Error reporting patches for 2016-02-19


Applied, thanks.

-- PMM