diff mbox

[PULL,06/11] icount: Add QemuOpts for icount

Message ID 1407341555-13173-7-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Aug. 6, 2014, 4:12 p.m. UTC
From: Sebastian Tanase <sebastian.tanase@openwide.fr>

Make icount parameter use QemuOpts style options in order
to easily add other suboptions.

Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
Tested-by: Camille Bégué <camille.begue@openwide.fr>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c                | 10 +++++++++-
 include/qemu-common.h |  3 ++-
 qemu-options.hx       |  4 ++--
 qtest.c               | 13 +++++++++++--
 vl.c                  | 35 ++++++++++++++++++++++++++++-------
 5 files changed, 52 insertions(+), 13 deletions(-)

Comments

Peter Maydell June 25, 2015, 10:33 a.m. UTC | #1
On 6 August 2014 at 17:12, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Sebastian Tanase <sebastian.tanase@openwide.fr>
>
> Make icount parameter use QemuOpts style options in order
> to easily add other suboptions.
>
> Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> Tested-by: Camille Bégué <camille.begue@openwide.fr>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Hi. I've just noticed that this change means we will
abort and dump core for user command line errors with icount:

$ qemu-system-x86_64 -icount bang
qemu-system-x86_64: icount: Invalid shift value
Aborted (core dumped)

> +        configure_icount(icount_opts, &error_abort);

...we shouldn't be using error_abort for situations where
the errors could be caused by user input, only where we know
that any error must be a bug in QEMU itself.

thanks
-- PMM
diff mbox

Patch

diff --git a/cpus.c b/cpus.c
index bbb8d4e..8291044 100644
--- a/cpus.c
+++ b/cpus.c
@@ -473,13 +473,21 @@  static const VMStateDescription vmstate_timers = {
     }
 };
 
-void configure_icount(const char *option)
+void configure_icount(QemuOpts *opts, Error **errp)
 {
+    const char *option;
+
     seqlock_init(&timers_state.vm_clock_seqlock, NULL);
     vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
+    option = qemu_opt_get(opts, "shift");
     if (!option) {
         return;
     }
+    /* When using -icount shift, the shift option will be
+       misinterpreted as a boolean */
+    if (strcmp(option, "on") == 0 || strcmp(option, "off") == 0) {
+        error_setg(errp, "The shift option must be a number or auto");
+    }
 
     icount_warp_timer = timer_new_ns(QEMU_CLOCK_REALTIME,
                                           icount_warp_rt, NULL);
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 6ef8282..04b0769 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -41,6 +41,7 @@ 
 #include <assert.h>
 #include <signal.h>
 #include "glib-compat.h"
+#include "qemu/option.h"
 
 #ifdef _WIN32
 #include "sysemu/os-win32.h"
@@ -105,7 +106,7 @@  static inline char *realpath(const char *path, char *resolved_path)
 #endif
 
 /* icount */
-void configure_icount(const char *option);
+void configure_icount(QemuOpts *opts, Error **errp);
 extern int use_icount;
 
 #include "qemu/osdep.h"
diff --git a/qemu-options.hx b/qemu-options.hx
index 1549625..5a1b001 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3011,11 +3011,11 @@  re-inject them.
 ETEXI
 
 DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
-    "-icount [N|auto]\n" \
+    "-icount [shift=N|auto]\n" \
     "                enable virtual instruction counter with 2^N clock ticks per\n" \
     "                instruction\n", QEMU_ARCH_ALL)
 STEXI
-@item -icount [@var{N}|auto]
+@item -icount [shift=@var{N}|auto]
 @findex -icount
 Enable virtual instruction counter.  The virtual cpu will execute one
 instruction every 2^@var{N} ns of virtual time.  If @code{auto} is specified
diff --git a/qtest.c b/qtest.c
index 04a6dc1..ef0d991 100644
--- a/qtest.c
+++ b/qtest.c
@@ -19,6 +19,9 @@ 
 #include "hw/irq.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/cpus.h"
+#include "qemu/config-file.h"
+#include "qemu/option.h"
+#include "qemu/error-report.h"
 
 #define MAX_IRQ 256
 
@@ -509,10 +512,16 @@  static void qtest_event(void *opaque, int event)
     }
 }
 
-int qtest_init_accel(MachineClass *mc)
+static void configure_qtest_icount(const char *options)
 {
-    configure_icount("0");
+    QemuOpts *opts  = qemu_opts_parse(qemu_find_opts("icount"), options, 1);
+    configure_icount(opts, &error_abort);
+    qemu_opts_del(opts);
+}
 
+int qtest_init_accel(MachineClass *mc)
+{
+    configure_qtest_icount("0");
     return 0;
 }
 
diff --git a/vl.c b/vl.c
index fe451aa..f2621a5 100644
--- a/vl.c
+++ b/vl.c
@@ -537,6 +537,20 @@  static QemuOptsList qemu_mem_opts = {
     },
 };
 
+static QemuOptsList qemu_icount_opts = {
+    .name = "icount",
+    .implied_opt_name = "shift",
+    .merge_lists = true,
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_icount_opts.head),
+    .desc = {
+        {
+            .name = "shift",
+            .type = QEMU_OPT_STRING,
+        },
+        { /* end of list */ }
+    },
+};
+
 /**
  * Get machine options
  *
@@ -2908,13 +2922,12 @@  int main(int argc, char **argv, char **envp)
 {
     int i;
     int snapshot, linux_boot;
-    const char *icount_option = NULL;
     const char *initrd_filename;
     const char *kernel_filename, *kernel_cmdline;
     const char *boot_order;
     DisplayState *ds;
     int cyls, heads, secs, translation;
-    QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+    QemuOpts *hda_opts = NULL, *opts, *machine_opts, *icount_opts = NULL;
     QemuOptsList *olist;
     int optind;
     const char *optarg;
@@ -2979,6 +2992,7 @@  int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_msg_opts);
     qemu_add_opts(&qemu_name_opts);
     qemu_add_opts(&qemu_numa_opts);
+    qemu_add_opts(&qemu_icount_opts);
 
     runstate_init();
 
@@ -3830,7 +3844,11 @@  int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_icount:
-                icount_option = optarg;
+                icount_opts = qemu_opts_parse(qemu_find_opts("icount"),
+                                              optarg, 1);
+                if (!icount_opts) {
+                    exit(1);
+                }
                 break;
             case QEMU_OPTION_incoming:
                 incoming = optarg;
@@ -4306,11 +4324,14 @@  int main(int argc, char **argv, char **envp)
     qemu_spice_init();
 #endif
 
-    if (icount_option && (kvm_enabled() || xen_enabled())) {
-        fprintf(stderr, "-icount is not allowed with kvm or xen\n");
-        exit(1);
+    if (icount_opts) {
+        if (kvm_enabled() || xen_enabled()) {
+            fprintf(stderr, "-icount is not allowed with kvm or xen\n");
+            exit(1);
+        }
+        configure_icount(icount_opts, &error_abort);
+        qemu_opts_del(icount_opts);
     }
-    configure_icount(icount_option);
 
     /* clean up network at qemu process termination */
     atexit(&net_cleanup);