* [Qemu-devel] [PULL 01/11] Convert stderr message calling error_get_pretty() to error_report()
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 02/11] OptsVisitor: introduce basic list modes Luiz Capitulino
` (9 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Seiji Aguchi <seiji.aguchi@hds.com>
Convert stderr messages calling error_get_pretty()
to error_report().
Timestamp is prepended by -msg timstamp option with it.
Per Markus's comment below, A conversion from fprintf() to
error_report() is always an improvement, regardless of
error_get_pretty().
http://marc.info/?l=qemu-devel&m=137513283408601&w=2
But, it is not reasonable to convert them at one time
because fprintf() is used everwhere in qemu.
So, it should be done step by step with avoiding regression.
Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
arch_init.c | 4 ++--
hw/char/serial.c | 5 +++--
hw/i386/pc.c | 6 +++---
qemu-char.c | 2 +-
target-i386/cpu.c | 2 +-
target-ppc/translate_init.c | 3 ++-
vl.c | 9 +++++----
7 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 68a7ab7..94d45e1 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1125,8 +1125,8 @@ void do_acpitable_option(const QemuOpts *opts)
acpi_table_add(opts, &err);
if (err) {
- fprintf(stderr, "Wrong acpi table provided: %s\n",
- error_get_pretty(err));
+ error_report("Wrong acpi table provided: %s",
+ error_get_pretty(err));
error_free(err);
exit(1);
}
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 6025592..a31eb57 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -27,6 +27,7 @@
#include "sysemu/char.h"
#include "qemu/timer.h"
#include "exec/address-spaces.h"
+#include "qemu/error-report.h"
//#define DEBUG_SERIAL
@@ -696,7 +697,7 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
s->chr = chr;
serial_realize_core(s, &err);
if (err != NULL) {
- fprintf(stderr, "%s\n", error_get_pretty(err));
+ error_report("%s", error_get_pretty(err));
error_free(err);
exit(1);
}
@@ -760,7 +761,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
serial_realize_core(s, &err);
if (err != NULL) {
- fprintf(stderr, "%s\n", error_get_pretty(err));
+ error_report("%s", error_get_pretty(err));
error_free(err);
exit(1);
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e8bc8ce..3a620a1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -978,7 +978,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
icc_bridge, &error);
if (error) {
- fprintf(stderr, "%s\n", error_get_pretty(error));
+ error_report("%s", error_get_pretty(error));
error_free(error);
exit(1);
}
@@ -1096,8 +1096,8 @@ void pc_acpi_init(const char *default_dsdt)
acpi_table_add(opts, &err);
if (err) {
- fprintf(stderr, "WARNING: failed to load %s: %s\n", filename,
- error_get_pretty(err));
+ error_report("WARNING: failed to load %s: %s", filename,
+ error_get_pretty(err));
error_free(err);
}
g_free(arg);
diff --git a/qemu-char.c b/qemu-char.c
index 1be1cf6..5446b88 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3344,7 +3344,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
chr = qemu_chr_new_from_opts(opts, init, &err);
if (error_is_set(&err)) {
- fprintf(stderr, "%s\n", error_get_pretty(err));
+ error_report("%s", error_get_pretty(err));
error_free(err);
}
if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6e38252..42c5de0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1852,7 +1852,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
out:
if (error) {
- fprintf(stderr, "%s\n", error_get_pretty(error));
+ error_report("%s", error_get_pretty(error));
error_free(error);
if (cpu != NULL) {
object_unref(OBJECT(cpu));
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 13b290c..609f797 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -27,6 +27,7 @@
#include "cpu-models.h"
#include "mmu-hash32.h"
#include "mmu-hash64.h"
+#include "qemu/error-report.h"
//#define PPC_DUMP_CPU
//#define PPC_DEBUG_SPR
@@ -8281,7 +8282,7 @@ PowerPCCPU *cpu_ppc_init(const char *cpu_model)
object_property_set_bool(OBJECT(cpu), true, "realized", &err);
if (err != NULL) {
- fprintf(stderr, "%s\n", error_get_pretty(err));
+ error_report("%s", error_get_pretty(err));
error_free(err);
object_unref(OBJECT(cpu));
return NULL;
diff --git a/vl.c b/vl.c
index f422a1c..1c283c9 100644
--- a/vl.c
+++ b/vl.c
@@ -2393,7 +2393,7 @@ static int chardev_init_func(QemuOpts *opts, void *opaque)
qemu_chr_new_from_opts(opts, NULL, &local_err);
if (error_is_set(&local_err)) {
- fprintf(stderr, "%s\n", error_get_pretty(local_err));
+ error_report("%s", error_get_pretty(local_err));
error_free(local_err);
return -1;
}
@@ -4375,8 +4375,8 @@ int main(int argc, char **argv, char **envp)
vnc_display_init(ds);
vnc_display_open(ds, vnc_display, &local_err);
if (local_err != NULL) {
- fprintf(stderr, "Failed to start VNC server on `%s': %s\n",
- vnc_display, error_get_pretty(local_err));
+ error_report("Failed to start VNC server on `%s': %s",
+ vnc_display, error_get_pretty(local_err));
error_free(local_err);
exit(1);
}
@@ -4419,7 +4419,8 @@ int main(int argc, char **argv, char **envp)
Error *local_err = NULL;
qemu_start_incoming_migration(incoming, &local_err);
if (local_err) {
- fprintf(stderr, "-incoming %s: %s\n", incoming, error_get_pretty(local_err));
+ error_report("-incoming %s: %s", incoming,
+ error_get_pretty(local_err));
error_free(local_err);
exit(1);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PULL 02/11] OptsVisitor: introduce basic list modes
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 01/11] Convert stderr message calling error_get_pretty() to error_report() Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 03/11] OptsVisitor: introduce list modes for interval flattening Luiz Capitulino
` (8 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Laszlo Ersek <lersek@redhat.com>
We're going to need more state while processing a list of repeated
options. This change eliminates "repeated_opts_first" and adds a new state
variable:
list_mode repeated_opts repeated_opts_first
-------------- ------------- -------------------
LM_NONE NULL false
LM_STARTED non-NULL true
LM_IN_PROGRESS non-NULL false
Additionally, it is documented that lookup_scalar() and processed(), both
called by opts_type_XXX(), are invalid in LM_STARTED -- generated qapi
code calls opts_next_list() to allocate the very first link before trying
to parse a scalar into it. List mode restrictions are expressed in
positive / inclusive form.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/opts-visitor.c | 45 +++++++++++++++++++++++++++++++++++----------
1 file changed, 35 insertions(+), 10 deletions(-)
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 174bd8b..29fd1ab 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -1,7 +1,7 @@
/*
* Options Visitor
*
- * Copyright Red Hat, Inc. 2012
+ * Copyright Red Hat, Inc. 2012, 2013
*
* Author: Laszlo Ersek <lersek@redhat.com>
*
@@ -18,6 +18,15 @@
#include "qapi/visitor-impl.h"
+enum ListMode
+{
+ LM_NONE, /* not traversing a list of repeated options */
+ LM_STARTED, /* opts_start_list() succeeded */
+ LM_IN_PROGRESS /* opts_next_list() has been called */
+};
+
+typedef enum ListMode ListMode;
+
struct OptsVisitor
{
Visitor visitor;
@@ -35,8 +44,8 @@ struct OptsVisitor
/* The list currently being traversed with opts_start_list() /
* opts_next_list(). The list must have a struct element type in the
* schema, with a single mandatory scalar member. */
+ ListMode list_mode;
GQueue *repeated_opts;
- bool repeated_opts_first;
/* If "opts_root->id" is set, reinstantiate it as a fake QemuOpt for
* uniformity. Only its "name" and "str" fields are set. "fake_id_opt" does
@@ -156,9 +165,11 @@ opts_start_list(Visitor *v, const char *name, Error **errp)
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
/* we can't traverse a list in a list */
- assert(ov->repeated_opts == NULL);
+ assert(ov->list_mode == LM_NONE);
ov->repeated_opts = lookup_distinct(ov, name, errp);
- ov->repeated_opts_first = (ov->repeated_opts != NULL);
+ if (ov->repeated_opts != NULL) {
+ ov->list_mode = LM_STARTED;
+ }
}
@@ -168,10 +179,13 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
GenericList **link;
- if (ov->repeated_opts_first) {
- ov->repeated_opts_first = false;
+ switch (ov->list_mode) {
+ case LM_STARTED:
+ ov->list_mode = LM_IN_PROGRESS;
link = list;
- } else {
+ break;
+
+ case LM_IN_PROGRESS: {
const QemuOpt *opt;
opt = g_queue_pop_head(ov->repeated_opts);
@@ -180,6 +194,11 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
return NULL;
}
link = &(*list)->next;
+ break;
+ }
+
+ default:
+ abort();
}
*link = g_malloc0(sizeof **link);
@@ -192,14 +211,16 @@ opts_end_list(Visitor *v, Error **errp)
{
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
+ assert(ov->list_mode == LM_STARTED || ov->list_mode == LM_IN_PROGRESS);
ov->repeated_opts = NULL;
+ ov->list_mode = LM_NONE;
}
static const QemuOpt *
lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
{
- if (ov->repeated_opts == NULL) {
+ if (ov->list_mode == LM_NONE) {
GQueue *list;
/* the last occurrence of any QemuOpt takes effect when queried by name
@@ -207,6 +228,7 @@ lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
list = lookup_distinct(ov, name, errp);
return list ? g_queue_peek_tail(list) : NULL;
}
+ assert(ov->list_mode == LM_IN_PROGRESS);
return g_queue_peek_head(ov->repeated_opts);
}
@@ -214,9 +236,12 @@ lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
static void
processed(OptsVisitor *ov, const char *name)
{
- if (ov->repeated_opts == NULL) {
+ if (ov->list_mode == LM_NONE) {
g_hash_table_remove(ov->unprocessed_opts, name);
+ return;
}
+ assert(ov->list_mode == LM_IN_PROGRESS);
+ /* do nothing */
}
@@ -365,7 +390,7 @@ opts_start_optional(Visitor *v, bool *present, const char *name,
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
/* we only support a single mandatory scalar field in a list node */
- assert(ov->repeated_opts == NULL);
+ assert(ov->list_mode == LM_NONE);
*present = (lookup_distinct(ov, name, NULL) != NULL);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PULL 03/11] OptsVisitor: introduce list modes for interval flattening
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 01/11] Convert stderr message calling error_get_pretty() to error_report() Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 02/11] OptsVisitor: introduce basic list modes Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 04/11] OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS Luiz Capitulino
` (7 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Laszlo Ersek <lersek@redhat.com>
The new modes are equal-rank, exclusive alternatives of LM_IN_PROGRESS.
Teach opts_next_list(), opts_type_int() and opts_type_uint64() to handle
them.
Also enumerate explicitly what functions are valid to call in what modes:
- opts_next_list() is valid to call while flattening a range,
- opts_end_list(): ditto,
- lookup_scalar() is invalid to call during flattening; generated qapi
traversal code must continue asking for the same kind of signed/unsigned
list element until the interval is fully flattened,
- processed(): ditto.
List mode restrictions are always formulated in positive / inclusive
sense. The restrictions for lookup_scalar() and processed() are
automatically satisfied by current qapi traversals if the schema to build
is compatible with OptsVisitor.
The new list modes are not entered yet.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/opts-visitor.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 65 insertions(+), 2 deletions(-)
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 29fd1ab..c2a57bd 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -22,7 +22,32 @@ enum ListMode
{
LM_NONE, /* not traversing a list of repeated options */
LM_STARTED, /* opts_start_list() succeeded */
- LM_IN_PROGRESS /* opts_next_list() has been called */
+
+ LM_IN_PROGRESS, /* opts_next_list() has been called.
+ *
+ * Generating the next list link will consume the most
+ * recently parsed QemuOpt instance of the repeated
+ * option.
+ *
+ * Parsing a value into the list link will examine the
+ * next QemuOpt instance of the repeated option, and
+ * possibly enter LM_SIGNED_INTERVAL or
+ * LM_UNSIGNED_INTERVAL.
+ */
+
+ LM_SIGNED_INTERVAL, /* opts_next_list() has been called.
+ *
+ * Generating the next list link will consume the most
+ * recently stored element from the signed interval,
+ * parsed from the most recent QemuOpt instance of the
+ * repeated option. This may consume QemuOpt itself
+ * and return to LM_IN_PROGRESS.
+ *
+ * Parsing a value into the list link will store the
+ * next element of the signed interval.
+ */
+
+ LM_UNSIGNED_INTERVAL /* Same as above, only for an unsigned interval. */
};
typedef enum ListMode ListMode;
@@ -47,6 +72,15 @@ struct OptsVisitor
ListMode list_mode;
GQueue *repeated_opts;
+ /* When parsing a list of repeating options as integers, values of the form
+ * "a-b", representing a closed interval, are allowed. Elements in the
+ * range are generated individually.
+ */
+ union {
+ int64_t s;
+ uint64_t u;
+ } range_next, range_limit;
+
/* If "opts_root->id" is set, reinstantiate it as a fake QemuOpt for
* uniformity. Only its "name" and "str" fields are set. "fake_id_opt" does
* not survive or escape the OptsVisitor object.
@@ -185,6 +219,22 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
link = list;
break;
+ case LM_SIGNED_INTERVAL:
+ case LM_UNSIGNED_INTERVAL:
+ link = &(*list)->next;
+
+ if (ov->list_mode == LM_SIGNED_INTERVAL) {
+ if (ov->range_next.s < ov->range_limit.s) {
+ ++ov->range_next.s;
+ break;
+ }
+ } else if (ov->range_next.u < ov->range_limit.u) {
+ ++ov->range_next.u;
+ break;
+ }
+ ov->list_mode = LM_IN_PROGRESS;
+ /* range has been completed, fall through in order to pop option */
+
case LM_IN_PROGRESS: {
const QemuOpt *opt;
@@ -211,7 +261,10 @@ opts_end_list(Visitor *v, Error **errp)
{
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
- assert(ov->list_mode == LM_STARTED || ov->list_mode == LM_IN_PROGRESS);
+ assert(ov->list_mode == LM_STARTED ||
+ ov->list_mode == LM_IN_PROGRESS ||
+ ov->list_mode == LM_SIGNED_INTERVAL ||
+ ov->list_mode == LM_UNSIGNED_INTERVAL);
ov->repeated_opts = NULL;
ov->list_mode = LM_NONE;
}
@@ -303,6 +356,11 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
long long val;
char *endptr;
+ if (ov->list_mode == LM_SIGNED_INTERVAL) {
+ *obj = ov->range_next.s;
+ return;
+ }
+
opt = lookup_scalar(ov, name, errp);
if (!opt) {
return;
@@ -328,6 +386,11 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
const QemuOpt *opt;
const char *str;
+ if (ov->list_mode == LM_UNSIGNED_INTERVAL) {
+ *obj = ov->range_next.u;
+ return;
+ }
+
opt = lookup_scalar(ov, name, errp);
if (!opt) {
return;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PULL 04/11] OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
` (2 preceding siblings ...)
2013-08-20 16:10 ` [Qemu-devel] [PULL 03/11] OptsVisitor: introduce list modes for interval flattening Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 05/11] OptsVisitor: rebase opts_type_uint64() to parse_uint_full() Luiz Capitulino
` (6 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Laszlo Ersek <lersek@redhat.com>
When a well-formed range value, bounded by signed integers, is encountered
while processing a repeated option, enter LM_SIGNED_INTERVAL and return
the low bound.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/opts-visitor.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index c2a57bd..90be583 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -367,15 +367,37 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
}
str = opt->str ? opt->str : "";
+ /* we've gotten past lookup_scalar() */
+ assert(ov->list_mode == LM_NONE || ov->list_mode == LM_IN_PROGRESS);
+
errno = 0;
val = strtoll(str, &endptr, 0);
- if (*str != '\0' && *endptr == '\0' && errno == 0 && INT64_MIN <= val &&
- val <= INT64_MAX) {
- *obj = val;
- processed(ov, name);
- return;
+ if (errno == 0 && endptr > str && INT64_MIN <= val && val <= INT64_MAX) {
+ if (*endptr == '\0') {
+ *obj = val;
+ processed(ov, name);
+ return;
+ }
+ if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) {
+ long long val2;
+
+ str = endptr + 1;
+ val2 = strtoll(str, &endptr, 0);
+ if (errno == 0 && endptr > str && *endptr == '\0' &&
+ INT64_MIN <= val2 && val2 <= INT64_MAX && val <= val2) {
+ ov->range_next.s = val;
+ ov->range_limit.s = val2;
+ ov->list_mode = LM_SIGNED_INTERVAL;
+
+ /* as if entering on the top */
+ *obj = ov->range_next.s;
+ return;
+ }
+ }
}
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, "an int64 value");
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
+ (ov->list_mode == LM_NONE) ? "an int64 value" :
+ "an int64 value or range");
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PULL 05/11] OptsVisitor: rebase opts_type_uint64() to parse_uint_full()
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
` (3 preceding siblings ...)
2013-08-20 16:10 ` [Qemu-devel] [PULL 04/11] OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 06/11] OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS Luiz Capitulino
` (5 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Laszlo Ersek <lersek@redhat.com>
Simplify the code in preparation for the next patch.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/opts-visitor.c | 23 +++++------------------
1 file changed, 5 insertions(+), 18 deletions(-)
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 90be583..d8f9a0e 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -407,6 +407,7 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
const QemuOpt *opt;
const char *str;
+ unsigned long long val;
if (ov->list_mode == LM_UNSIGNED_INTERVAL) {
*obj = ov->range_next.u;
@@ -417,26 +418,12 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
if (!opt) {
return;
}
-
str = opt->str;
- if (str != NULL) {
- while (isspace((unsigned char)*str)) {
- ++str;
- }
-
- if (*str != '-' && *str != '\0') {
- unsigned long long val;
- char *endptr;
- /* non-empty, non-negative subject sequence */
- errno = 0;
- val = strtoull(str, &endptr, 0);
- if (*endptr == '\0' && errno == 0 && val <= UINT64_MAX) {
- *obj = val;
- processed(ov, name);
- return;
- }
- }
+ if (parse_uint_full(str, &val, 0) == 0 && val <= UINT64_MAX) {
+ *obj = val;
+ processed(ov, name);
+ return;
}
error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
"an uint64 value");
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PULL 06/11] OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
` (4 preceding siblings ...)
2013-08-20 16:10 ` [Qemu-devel] [PULL 05/11] OptsVisitor: rebase opts_type_uint64() to parse_uint_full() Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 07/11] OptsVisitor: don't try to flatten overlong integer ranges Luiz Capitulino
` (4 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Laszlo Ersek <lersek@redhat.com>
When a well-formed range value, bounded by unsigned integers, is
encountered while processing a repeated option, enter LM_UNSIGNED_INTERVAL
and return the low bound.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/opts-visitor.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index d8f9a0e..d54d373 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -408,6 +408,7 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
const QemuOpt *opt;
const char *str;
unsigned long long val;
+ char *endptr;
if (ov->list_mode == LM_UNSIGNED_INTERVAL) {
*obj = ov->range_next.u;
@@ -420,13 +421,34 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
}
str = opt->str;
- if (parse_uint_full(str, &val, 0) == 0 && val <= UINT64_MAX) {
- *obj = val;
- processed(ov, name);
- return;
+ /* we've gotten past lookup_scalar() */
+ assert(ov->list_mode == LM_NONE || ov->list_mode == LM_IN_PROGRESS);
+
+ if (parse_uint(str, &val, &endptr, 0) == 0 && val <= UINT64_MAX) {
+ if (*endptr == '\0') {
+ *obj = val;
+ processed(ov, name);
+ return;
+ }
+ if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) {
+ unsigned long long val2;
+
+ str = endptr + 1;
+ if (parse_uint_full(str, &val2, 0) == 0 &&
+ val2 <= UINT64_MAX && val <= val2) {
+ ov->range_next.u = val;
+ ov->range_limit.u = val2;
+ ov->list_mode = LM_UNSIGNED_INTERVAL;
+
+ /* as if entering on the top */
+ *obj = ov->range_next.u;
+ return;
+ }
+ }
}
error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
- "an uint64 value");
+ (ov->list_mode == LM_NONE) ? "a uint64 value" :
+ "a uint64 value or range");
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PULL 07/11] OptsVisitor: don't try to flatten overlong integer ranges
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
` (5 preceding siblings ...)
2013-08-20 16:10 ` [Qemu-devel] [PULL 06/11] OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore Luiz Capitulino
` (3 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Laszlo Ersek <lersek@redhat.com>
Prevent mistyped command line options from incurring high memory and CPU
usage at startup. 64K elements in a range should be enough for everyone
(TM).
The OPTS_VISITOR_RANGE_MAX macro is public so that unit tests can
construct corner cases with it.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
include/qapi/opts-visitor.h | 6 ++++++
qapi/opts-visitor.c | 7 +++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/include/qapi/opts-visitor.h b/include/qapi/opts-visitor.h
index 5939eee..fd48c14 100644
--- a/include/qapi/opts-visitor.h
+++ b/include/qapi/opts-visitor.h
@@ -16,6 +16,12 @@
#include "qapi/visitor.h"
#include "qemu/option.h"
+/* Inclusive upper bound on the size of any flattened range. This is a safety
+ * (= anti-annoyance) measure; wrong ranges should not cause long startup
+ * delays nor exhaust virtual memory.
+ */
+#define OPTS_VISITOR_RANGE_MAX 65536
+
typedef struct OptsVisitor OptsVisitor;
/* Contrarily to qemu-option.c::parse_option_number(), OptsVisitor's "int"
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index d54d373..96ed858 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -384,7 +384,9 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
str = endptr + 1;
val2 = strtoll(str, &endptr, 0);
if (errno == 0 && endptr > str && *endptr == '\0' &&
- INT64_MIN <= val2 && val2 <= INT64_MAX && val <= val2) {
+ INT64_MIN <= val2 && val2 <= INT64_MAX && val <= val2 &&
+ (val > INT64_MAX - OPTS_VISITOR_RANGE_MAX ||
+ val2 < val + OPTS_VISITOR_RANGE_MAX)) {
ov->range_next.s = val;
ov->range_limit.s = val2;
ov->list_mode = LM_SIGNED_INTERVAL;
@@ -435,7 +437,8 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
str = endptr + 1;
if (parse_uint_full(str, &val2, 0) == 0 &&
- val2 <= UINT64_MAX && val <= val2) {
+ val2 <= UINT64_MAX && val <= val2 &&
+ val2 - val < OPTS_VISITOR_RANGE_MAX) {
ov->range_next.u = val;
ov->range_limit.u = val2;
ov->list_mode = LM_UNSIGNED_INTERVAL;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
` (6 preceding siblings ...)
2013-08-20 16:10 ` [Qemu-devel] [PULL 07/11] OptsVisitor: don't try to flatten overlong integer ranges Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:23 ` Andreas Färber
2013-08-20 16:10 ` [Qemu-devel] [PULL 09/11] OptsVisitor: introduce unit tests, with test cases for range flattening Luiz Capitulino
` (2 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Laszlo Ersek <lersek@redhat.com>
"test-int128" was probably missed in commit 6046c620
("int128: optimize and add test cases").
"test-bitops" was probably missed in commit 3464700f
("tests: Add test-bitops.c with some sextract tests").
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
.gitignore | 2 ++
1 file changed, 2 insertions(+)
diff --git a/.gitignore b/.gitignore
index 0fe114d..a8e0f17 100644
--- a/.gitignore
+++ b/.gitignore
@@ -45,7 +45,9 @@ qemu-bridge-helper
qemu-monitor.texi
vscclient
QMP/qmp-commands.txt
+test-bitops
test-coroutine
+test-int128
test-qmp-input-visitor
test-qmp-output-visitor
test-string-input-visitor
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore
2013-08-20 16:10 ` [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore Luiz Capitulino
@ 2013-08-20 16:23 ` Andreas Färber
2013-08-20 18:39 ` Luiz Capitulino
0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2013-08-20 16:23 UTC (permalink / raw)
To: Luiz Capitulino, Laszlo Ersek; +Cc: qemu-devel, anthony
Am 20.08.2013 18:10, schrieb Luiz Capitulino:
> From: Laszlo Ersek <lersek@redhat.com>
>
> "test-int128" was probably missed in commit 6046c620
> ("int128: optimize and add test cases").
>
> "test-bitops" was probably missed in commit 3464700f
> ("tests: Add test-bitops.c with some sextract tests").
>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> .gitignore | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/.gitignore b/.gitignore
> index 0fe114d..a8e0f17 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -45,7 +45,9 @@ qemu-bridge-helper
> qemu-monitor.texi
> vscclient
> QMP/qmp-commands.txt
> +test-bitops
> test-coroutine
> +test-int128
> test-qmp-input-visitor
> test-qmp-output-visitor
> test-string-input-visitor
Markus' patch earlier today added the same two entries to
tests/.gitignore instead, which seems to have more test-* entries.
Should we consolidate these two files by moving the above test-* into
tests/.gitignore, or do we need to mirror everything into both files for
some reason?
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore
2013-08-20 16:23 ` Andreas Färber
@ 2013-08-20 18:39 ` Luiz Capitulino
2013-08-20 19:16 ` Andreas Färber
0 siblings, 1 reply; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 18:39 UTC (permalink / raw)
To: Andreas Färber; +Cc: Laszlo Ersek, qemu-devel, anthony
On Tue, 20 Aug 2013 18:23:00 +0200
Andreas Färber <afaerber@suse.de> wrote:
> Am 20.08.2013 18:10, schrieb Luiz Capitulino:
> > From: Laszlo Ersek <lersek@redhat.com>
> >
> > "test-int128" was probably missed in commit 6046c620
> > ("int128: optimize and add test cases").
> >
> > "test-bitops" was probably missed in commit 3464700f
> > ("tests: Add test-bitops.c with some sextract tests").
> >
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> > Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> > .gitignore | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/.gitignore b/.gitignore
> > index 0fe114d..a8e0f17 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -45,7 +45,9 @@ qemu-bridge-helper
> > qemu-monitor.texi
> > vscclient
> > QMP/qmp-commands.txt
> > +test-bitops
> > test-coroutine
> > +test-int128
> > test-qmp-input-visitor
> > test-qmp-output-visitor
> > test-string-input-visitor
>
> Markus' patch earlier today added the same two entries to
> tests/.gitignore instead, which seems to have more test-* entries.
>
> Should we consolidate these two files by moving the above test-* into
> tests/.gitignore, or do we need to mirror everything into both files for
> some reason?
I think we could consolidate everything in tests/.gitignore. But can we
do it as an incremental patch? It's a very small cosmetic thing to respin
the series and my pull request.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore
2013-08-20 18:39 ` Luiz Capitulino
@ 2013-08-20 19:16 ` Andreas Färber
2013-08-21 7:57 ` Laszlo Ersek
0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2013-08-20 19:16 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: Laszlo Ersek, qemu-devel, anthony
Am 20.08.2013 20:39, schrieb Luiz Capitulino:
> On Tue, 20 Aug 2013 18:23:00 +0200
> Andreas Färber <afaerber@suse.de> wrote:
>> Am 20.08.2013 18:10, schrieb Luiz Capitulino:
>>> diff --git a/.gitignore b/.gitignore
>>> index 0fe114d..a8e0f17 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -45,7 +45,9 @@ qemu-bridge-helper
>>> qemu-monitor.texi
>>> vscclient
>>> QMP/qmp-commands.txt
>>> +test-bitops
>>> test-coroutine
>>> +test-int128
>>> test-qmp-input-visitor
>>> test-qmp-output-visitor
>>> test-string-input-visitor
>>
>> Markus' patch earlier today added the same two entries to
>> tests/.gitignore instead, which seems to have more test-* entries.
>>
>> Should we consolidate these two files by moving the above test-* into
>> tests/.gitignore, or do we need to mirror everything into both files for
>> some reason?
>
> I think we could consolidate everything in tests/.gitignore. But can we
> do it as an incremental patch? It's a very small cosmetic thing to respin
> the series and my pull request.
Sure, it's not a bug.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore
2013-08-20 19:16 ` Andreas Färber
@ 2013-08-21 7:57 ` Laszlo Ersek
2013-08-21 20:48 ` Eric Blake
0 siblings, 1 reply; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-21 7:57 UTC (permalink / raw)
To: Andreas Färber; +Cc: qemu-devel, anthony, Luiz Capitulino
On 08/20/13 21:16, Andreas Färber wrote:
> Am 20.08.2013 20:39, schrieb Luiz Capitulino:
>> On Tue, 20 Aug 2013 18:23:00 +0200
>> Andreas Färber <afaerber@suse.de> wrote:
>>> Am 20.08.2013 18:10, schrieb Luiz Capitulino:
>>>> diff --git a/.gitignore b/.gitignore
>>>> index 0fe114d..a8e0f17 100644
>>>> --- a/.gitignore
>>>> +++ b/.gitignore
>>>> @@ -45,7 +45,9 @@ qemu-bridge-helper
>>>> qemu-monitor.texi
>>>> vscclient
>>>> QMP/qmp-commands.txt
>>>> +test-bitops
>>>> test-coroutine
>>>> +test-int128
>>>> test-qmp-input-visitor
>>>> test-qmp-output-visitor
>>>> test-string-input-visitor
>>>
>>> Markus' patch earlier today added the same two entries to
>>> tests/.gitignore instead, which seems to have more test-* entries.
>>>
>>> Should we consolidate these two files by moving the above test-* into
>>> tests/.gitignore, or do we need to mirror everything into both files for
>>> some reason?
>>
>> I think we could consolidate everything in tests/.gitignore. But can we
>> do it as an incremental patch? It's a very small cosmetic thing to respin
>> the series and my pull request.
>
> Sure, it's not a bug.
Is it OK if I submit a patch moving the test-* entries over to
tests/.gitignore once Anthony applies this? (I'm offering because it's
not clear to me who's planning to do it.)
Thanks
Laszlo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore
2013-08-21 7:57 ` Laszlo Ersek
@ 2013-08-21 20:48 ` Eric Blake
2013-08-22 6:32 ` Markus Armbruster
0 siblings, 1 reply; 18+ messages in thread
From: Eric Blake @ 2013-08-21 20:48 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Luiz Capitulino, Andreas Färber, anthony, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]
On 08/21/2013 01:57 AM, Laszlo Ersek wrote:
>>> I think we could consolidate everything in tests/.gitignore. But can we
>>> do it as an incremental patch? It's a very small cosmetic thing to respin
>>> the series and my pull request.
>>
>> Sure, it's not a bug.
>
> Is it OK if I submit a patch moving the test-* entries over to
> tests/.gitignore once Anthony applies this? (I'm offering because it's
> not clear to me who's planning to do it.)
Bikeshedding alert - I personally like having a single top-level
.gitignore for everything. We don't live in CVS days where you had to
have per-directory ignores, and having one file is easier to maintain
(in the future, it's easier to know WHICH .gitignore to add a new entry
to if there is only ONE .gitignore). I don't care enough to write the
patch myself, nor to care if you end up keeping the split .gitignore,
but if I did care, I'd be consolidating in the opposite direction (move
tests/.gitignore to the toplevel).
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore
2013-08-21 20:48 ` Eric Blake
@ 2013-08-22 6:32 ` Markus Armbruster
0 siblings, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2013-08-22 6:32 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, Laszlo Ersek, Andreas Färber, anthony,
Luiz Capitulino
Eric Blake <eblake@redhat.com> writes:
> On 08/21/2013 01:57 AM, Laszlo Ersek wrote:
>>>> I think we could consolidate everything in tests/.gitignore. But can we
>>>> do it as an incremental patch? It's a very small cosmetic thing to respin
>>>> the series and my pull request.
>>>
>>> Sure, it's not a bug.
>>
>> Is it OK if I submit a patch moving the test-* entries over to
>> tests/.gitignore once Anthony applies this? (I'm offering because it's
>> not clear to me who's planning to do it.)
>
> Bikeshedding alert - I personally like having a single top-level
> .gitignore for everything. We don't live in CVS days where you had to
> have per-directory ignores, and having one file is easier to maintain
> (in the future, it's easier to know WHICH .gitignore to add a new entry
> to if there is only ONE .gitignore). I don't care enough to write the
> patch myself, nor to care if you end up keeping the split .gitignore,
> but if I did care, I'd be consolidating in the opposite direction (move
> tests/.gitignore to the toplevel).
One more thing you probably care about (I do): everything that is
ignored in a directory should be ignored in the same .gitignore.
Trivial if the is only one.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PULL 09/11] OptsVisitor: introduce unit tests, with test cases for range flattening
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
` (7 preceding siblings ...)
2013-08-20 16:10 ` [Qemu-devel] [PULL 08/11] add "test-int128" and "test-bitops" to .gitignore Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 10/11] monitor: print the invalid char in error message Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 11/11] scripts/qapi.py: Avoid syntax not supported by Python 2.4 Luiz Capitulino
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Laszlo Ersek <lersek@redhat.com>
According to commit 4f193e34
("tests: Use qapi-schema-test.json as schema parser test")
the "tests/qapi-schema/qapi-schema-test.out" file must be updated as well.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
.gitignore | 1 +
tests/Makefile | 6 +-
tests/qapi-schema/qapi-schema-test.json | 15 ++
tests/qapi-schema/qapi-schema-test.out | 6 +-
tests/test-opts-visitor.c | 275 ++++++++++++++++++++++++++++++++
5 files changed, 300 insertions(+), 3 deletions(-)
create mode 100644 tests/test-opts-visitor.c
diff --git a/.gitignore b/.gitignore
index a8e0f17..d2c5c2f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -48,6 +48,7 @@ QMP/qmp-commands.txt
test-bitops
test-coroutine
test-int128
+test-opts-visitor
test-qmp-input-visitor
test-qmp-output-visitor
test-string-input-visitor
diff --git a/tests/Makefile b/tests/Makefile
index b0200fd..baba9e9 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -23,6 +23,8 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF)
gcov-files-test-string-input-visitor-y = qapi/string-input-visitor.c
check-unit-y += tests/test-string-output-visitor$(EXESUF)
gcov-files-test-string-output-visitor-y = qapi/string-output-visitor.c
+check-unit-y += tests/test-opts-visitor$(EXESUF)
+gcov-files-test-opts-visitor-y = qapi/opts-visitor.c
check-unit-y += tests/test-coroutine$(EXESUF)
gcov-files-test-coroutine-y = coroutine-$(CONFIG_COROUTINE_BACKEND).c
check-unit-y += tests/test-visitor-serialization$(EXESUF)
@@ -100,7 +102,8 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
tests/test-qmp-commands.o tests/test-visitor-serialization.o \
- tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o
+ tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
+ tests/test-opts-visitor.o
test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
@@ -148,6 +151,7 @@ tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qap
tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) qapi-types.o qapi-visit.o libqemuutil.a libqemustub.a
tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
+tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
tests/test-bitops$(EXESUF): tests/test-bitops.o libqemuutil.a
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 4434fa3..fe5af75 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -51,3 +51,18 @@
{ 'command': 'user_def_cmd', 'data': {} }
{ 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
{ 'command': 'user_def_cmd2', 'data': {'ud1a': 'UserDefOne', 'ud1b': 'UserDefOne'}, 'returns': 'UserDefTwo' }
+
+# For testing integer range flattening in opts-visitor. The following schema
+# corresponds to the option format:
+#
+# -userdef i64=3-6,i64=-5--1,u64=2,u16=1,u16=7-12
+#
+# For simplicity, this example doesn't use [type=]discriminator nor optargs
+# specific to discriminator values.
+{ 'type': 'UserDefOptions',
+ 'data': {
+ '*i64' : [ 'int' ],
+ '*u64' : [ 'uint64' ],
+ '*u16' : [ 'uint16' ],
+ '*i64x': 'int' ,
+ '*u64x': 'uint64' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index fb00344..3851880 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -9,11 +9,13 @@
OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]),
OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),
- OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')])]
+ OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
+ OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
['EnumOne', 'UserDefUnionKind', 'UserDefNativeListUnionKind']
[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefOne'), ('data', OrderedDict([('integer', 'int'), ('string', 'str'), ('*enum1', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), ('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
- OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))])]
+ OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
+ OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
new file mode 100644
index 0000000..9f902b5
--- /dev/null
+++ b/tests/test-opts-visitor.c
@@ -0,0 +1,275 @@
+/*
+ * Options Visitor unit-tests.
+ *
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * Authors:
+ * Laszlo Ersek <lersek@redhat.com> (based on test-string-output-visitor)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+
+#include "qemu/config-file.h" /* qemu_add_opts() */
+#include "qemu/option.h" /* qemu_opts_parse() */
+#include "qapi/opts-visitor.h" /* opts_visitor_new() */
+#include "test-qapi-visit.h" /* visit_type_UserDefOptions() */
+#include "qapi/dealloc-visitor.h" /* qapi_dealloc_visitor_new() */
+
+static QemuOptsList userdef_opts = {
+ .name = "userdef",
+ .head = QTAILQ_HEAD_INITIALIZER(userdef_opts.head),
+ .desc = { { 0 } } /* validated with OptsVisitor */
+};
+
+/* fixture (= glib test case context) and test case manipulation */
+
+typedef struct OptsVisitorFixture {
+ UserDefOptions *userdef;
+ Error *err;
+} OptsVisitorFixture;
+
+
+static void
+setup_fixture(OptsVisitorFixture *f, gconstpointer test_data)
+{
+ const char *opts_string = test_data;
+ QemuOpts *opts;
+ OptsVisitor *ov;
+
+ opts = qemu_opts_parse(qemu_find_opts("userdef"), opts_string, 0);
+ g_assert(opts != NULL);
+
+ ov = opts_visitor_new(opts);
+ visit_type_UserDefOptions(opts_get_visitor(ov), &f->userdef, NULL,
+ &f->err);
+ opts_visitor_cleanup(ov);
+ qemu_opts_del(opts);
+}
+
+
+static void
+teardown_fixture(OptsVisitorFixture *f, gconstpointer test_data)
+{
+ if (f->userdef != NULL) {
+ QapiDeallocVisitor *dv;
+
+ dv = qapi_dealloc_visitor_new();
+ visit_type_UserDefOptions(qapi_dealloc_get_visitor(dv), &f->userdef,
+ NULL, NULL);
+ qapi_dealloc_visitor_cleanup(dv);
+ }
+ error_free(f->err);
+}
+
+
+static void
+add_test(const char *testpath,
+ void (*test_func)(OptsVisitorFixture *f, gconstpointer test_data),
+ gconstpointer test_data)
+{
+ g_test_add(testpath, OptsVisitorFixture, test_data, setup_fixture,
+ test_func, teardown_fixture);
+}
+
+/* test output evaluation */
+
+static void
+expect_ok(OptsVisitorFixture *f, gconstpointer test_data)
+{
+ g_assert(f->err == NULL);
+ g_assert(f->userdef != NULL);
+}
+
+
+static void
+expect_fail(OptsVisitorFixture *f, gconstpointer test_data)
+{
+ g_assert(f->err != NULL);
+
+ /* The error message is printed when this test utility is invoked directly
+ * (ie. without gtester) and the --verbose flag is passed:
+ *
+ * tests/test-opts-visitor --verbose
+ */
+ g_test_message("'%s': %s", (const char *)test_data,
+ error_get_pretty(f->err));
+}
+
+
+static void
+test_value(OptsVisitorFixture *f, gconstpointer test_data)
+{
+ uint64_t magic, bitval;
+ intList *i64;
+ uint64List *u64;
+ uint16List *u16;
+
+ expect_ok(f, test_data);
+
+ magic = 0;
+ for (i64 = f->userdef->i64; i64 != NULL; i64 = i64->next) {
+ g_assert(-16 <= i64->value && i64->value < 64-16);
+ bitval = 1ull << (i64->value + 16);
+ g_assert((magic & bitval) == 0);
+ magic |= bitval;
+ }
+ g_assert(magic == 0xDEADBEEF);
+
+ magic = 0;
+ for (u64 = f->userdef->u64; u64 != NULL; u64 = u64->next) {
+ g_assert(u64->value < 64);
+ bitval = 1ull << u64->value;
+ g_assert((magic & bitval) == 0);
+ magic |= bitval;
+ }
+ g_assert(magic == 0xBADC0FFEE0DDF00D);
+
+ magic = 0;
+ for (u16 = f->userdef->u16; u16 != NULL; u16 = u16->next) {
+ g_assert(u16->value < 64);
+ bitval = 1ull << u16->value;
+ g_assert((magic & bitval) == 0);
+ magic |= bitval;
+ }
+ g_assert(magic == 0xD15EA5E);
+}
+
+
+static void
+expect_i64_min(OptsVisitorFixture *f, gconstpointer test_data)
+{
+ expect_ok(f, test_data);
+ g_assert(f->userdef->has_i64);
+ g_assert(f->userdef->i64->next == NULL);
+ g_assert(f->userdef->i64->value == INT64_MIN);
+}
+
+
+static void
+expect_i64_max(OptsVisitorFixture *f, gconstpointer test_data)
+{
+ expect_ok(f, test_data);
+ g_assert(f->userdef->has_i64);
+ g_assert(f->userdef->i64->next == NULL);
+ g_assert(f->userdef->i64->value == INT64_MAX);
+}
+
+
+static void
+expect_zero(OptsVisitorFixture *f, gconstpointer test_data)
+{
+ expect_ok(f, test_data);
+ g_assert(f->userdef->has_u64);
+ g_assert(f->userdef->u64->next == NULL);
+ g_assert(f->userdef->u64->value == 0);
+}
+
+
+static void
+expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data)
+{
+ expect_ok(f, test_data);
+ g_assert(f->userdef->has_u64);
+ g_assert(f->userdef->u64->next == NULL);
+ g_assert(f->userdef->u64->value == UINT64_MAX);
+}
+
+/* test cases */
+
+int
+main(int argc, char **argv)
+{
+ g_test_init(&argc, &argv, NULL);
+
+ qemu_add_opts(&userdef_opts);
+
+ /* Three hexadecimal magic numbers, "dead beef", "bad coffee, odd food" and
+ * "disease", from
+ * <http://en.wikipedia.org/wiki/Magic_number_%28programming%29>, were
+ * converted to binary and dissected into bit ranges. Each magic number is
+ * going to be recomposed using the lists called "i64", "u64" and "u16",
+ * respectively.
+ *
+ * (Note that these types pertain to the individual bit shift counts, not
+ * the magic numbers themselves; the intent is to exercise opts_type_int()
+ * and opts_type_uint64().)
+ *
+ * The "i64" shift counts have been decreased by 16 (decimal) in order to
+ * test negative values as well. Finally, the full list of QemuOpt elements
+ * has been permuted with "shuf".
+ *
+ * Both "i64" and "u64" have some (distinct) single-element ranges
+ * represented as both "a" and "a-a". "u16" is a special case of "i64" (see
+ * visit_type_uint16()), so it wouldn't add a separate test in this regard.
+ */
+
+ add_test("/visitor/opts/flatten/value", &test_value,
+ "i64=-1-0,u64=12-16,u64=2-3,i64=-11--9,u64=57,u16=9,i64=5-5,"
+ "u16=1-4,u16=20,u64=63-63,i64=-16--13,u64=50-52,i64=14-15,u16=11,"
+ "i64=7,u16=18,i64=2-3,u16=6,u64=54-55,u64=0,u64=18-20,u64=33-43,"
+ "i64=9-12,u16=26-27,u64=59-61,u16=13-16,u64=29-31,u64=22-23,"
+ "u16=24,i64=-7--3");
+
+ add_test("/visitor/opts/i64/val1/errno", &expect_fail,
+ "i64=0x8000000000000000");
+ add_test("/visitor/opts/i64/val1/empty", &expect_fail, "i64=");
+ add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z");
+ add_test("/visitor/opts/i64/nonlist", &expect_fail, "i64x=5-6");
+ add_test("/visitor/opts/i64/val2/errno", &expect_fail,
+ "i64=0x7fffffffffffffff-0x8000000000000000");
+ add_test("/visitor/opts/i64/val2/empty", &expect_fail, "i64=5-");
+ add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z");
+ add_test("/visitor/opts/i64/range/empty", &expect_fail, "i64=6-5");
+ add_test("/visitor/opts/i64/range/minval", &expect_i64_min,
+ "i64=-0x8000000000000000--0x8000000000000000");
+ add_test("/visitor/opts/i64/range/maxval", &expect_i64_max,
+ "i64=0x7fffffffffffffff-0x7fffffffffffffff");
+
+ add_test("/visitor/opts/u64/val1/errno", &expect_fail, "u64=-1");
+ add_test("/visitor/opts/u64/val1/empty", &expect_fail, "u64=");
+ add_test("/visitor/opts/u64/val1/trailing", &expect_fail, "u64=5z");
+ add_test("/visitor/opts/u64/nonlist", &expect_fail, "u64x=5-6");
+ add_test("/visitor/opts/u64/val2/errno", &expect_fail,
+ "u64=0xffffffffffffffff-0x10000000000000000");
+ add_test("/visitor/opts/u64/val2/empty", &expect_fail, "u64=5-");
+ add_test("/visitor/opts/u64/val2/trailing", &expect_fail, "u64=5-6z");
+ add_test("/visitor/opts/u64/range/empty", &expect_fail, "u64=6-5");
+ add_test("/visitor/opts/u64/range/minval", &expect_zero, "u64=0-0");
+ add_test("/visitor/opts/u64/range/maxval", &expect_u64_max,
+ "u64=0xffffffffffffffff-0xffffffffffffffff");
+
+ /* Test maximum range sizes. The macro value is open-coded here
+ * *intentionally*; the test case must use concrete values by design. If
+ * OPTS_VISITOR_RANGE_MAX is changed, the following values need to be
+ * recalculated as well. The assert and this comment should help with it.
+ */
+ g_assert(OPTS_VISITOR_RANGE_MAX == 65536);
+
+ /* The unsigned case is simple, a u64-u64 difference can always be
+ * represented as a u64.
+ */
+ add_test("/visitor/opts/u64/range/max", &expect_ok, "u64=0-65535");
+ add_test("/visitor/opts/u64/range/2big", &expect_fail, "u64=0-65536");
+
+ /* The same cannot be said about an i64-i64 difference. */
+ add_test("/visitor/opts/i64/range/max/pos/a", &expect_ok,
+ "i64=0x7fffffffffff0000-0x7fffffffffffffff");
+ add_test("/visitor/opts/i64/range/max/pos/b", &expect_ok,
+ "i64=0x7ffffffffffeffff-0x7ffffffffffffffe");
+ add_test("/visitor/opts/i64/range/2big/pos", &expect_fail,
+ "i64=0x7ffffffffffeffff-0x7fffffffffffffff");
+ add_test("/visitor/opts/i64/range/max/neg/a", &expect_ok,
+ "i64=-0x8000000000000000--0x7fffffffffff0001");
+ add_test("/visitor/opts/i64/range/max/neg/b", &expect_ok,
+ "i64=-0x7fffffffffffffff--0x7fffffffffff0000");
+ add_test("/visitor/opts/i64/range/2big/neg", &expect_fail,
+ "i64=-0x8000000000000000--0x7fffffffffff0000");
+ add_test("/visitor/opts/i64/range/2big/full", &expect_fail,
+ "i64=-0x8000000000000000-0x7fffffffffffffff");
+
+ g_test_run();
+ return 0;
+}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PULL 10/11] monitor: print the invalid char in error message
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
` (8 preceding siblings ...)
2013-08-20 16:10 ` [Qemu-devel] [PULL 09/11] OptsVisitor: introduce unit tests, with test cases for range flattening Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
2013-08-20 16:10 ` [Qemu-devel] [PULL 11/11] scripts/qapi.py: Avoid syntax not supported by Python 2.4 Luiz Capitulino
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Fam Zheng <famz@redhat.com>
It's more friendly to print which char is invalid to user, especially
when user tries to input a float value and expect the monitor to round
it to int. Since we don't round float number when we look for a integer,
telling which char is invalid is less confusing.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
monitor.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/monitor.c b/monitor.c
index 5dc0aa9..da9c9a2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3171,9 +3171,13 @@ static const MonitorDef monitor_defs[] = {
{ NULL },
};
-static void expr_error(Monitor *mon, const char *msg)
+static void expr_error(Monitor *mon, const char *fmt, ...)
{
- monitor_printf(mon, "%s\n", msg);
+ va_list ap;
+ va_start(ap, fmt);
+ monitor_vprintf(mon, fmt, ap);
+ monitor_printf(mon, "\n");
+ va_end(ap);
siglongjmp(expr_env, 1);
}
@@ -3291,7 +3295,7 @@ static int64_t expr_unary(Monitor *mon)
expr_error(mon, "number too large");
}
if (pch == p) {
- expr_error(mon, "invalid char in expression");
+ expr_error(mon, "invalid char '%c' in expression", *p);
}
pch = p;
while (qemu_isspace(*pch))
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PULL 11/11] scripts/qapi.py: Avoid syntax not supported by Python 2.4
2013-08-20 16:10 [Qemu-devel] [PULL 00/11] QMP queue Luiz Capitulino
` (9 preceding siblings ...)
2013-08-20 16:10 ` [Qemu-devel] [PULL 10/11] monitor: print the invalid char in error message Luiz Capitulino
@ 2013-08-20 16:10 ` Luiz Capitulino
10 siblings, 0 replies; 18+ messages in thread
From: Luiz Capitulino @ 2013-08-20 16:10 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Peter Maydell <peter.maydell@linaro.org>
The Python "except Foo as x" syntax was only introduced in
Python 2.6, but we aim to support Python 2.4 and later.
Use the old-style "except Foo, x" syntax instead, thus
fixing configure/compile on systems with older Python.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0ebea94..1069310 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -161,7 +161,7 @@ class QAPISchema:
def parse_schema(fp):
try:
schema = QAPISchema(fp)
- except QAPISchemaError as e:
+ except QAPISchemaError, e:
print >>sys.stderr, e
exit(1)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread