* [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options @ 2013-08-19 22:35 Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 1/8] OptsVisitor: introduce basic list modes Laszlo Ersek ` (9 more replies) 0 siblings, 10 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-19 22:35 UTC (permalink / raw) To: lcapitulino, qemu-devel v1->v2: - rebased to current master (patch #8 only applied with "git am -3"), - now patch #7 adds "test-bitops" too to .gitignore, - patch #8 updates "qapi-schema-test.out" to keep it in synch with the test schema changes warranted by the opts-visitor unit tests. rfc->v1: - addressed Paolo's comments for patches 1 and 2, - patches 7 and 8 are new (unit tests), - updated the cover letter to take native lists into account, plus cleaned it up. Consider the following QAPI schema fragment, for the purpose of command line parsing with OptsVisitor: { 'union': 'NumaOptions', 'data': { 'node': 'NumaNodeOptions', 'mem' : 'NumaMemOptions' }} { 'type': 'NumaNodeOptions', 'data': { '*nodeid': 'int', '*cpus' : ['uint16'] }} { 'type': 'NumaMemOptions', 'data': { '*nodeid': 'int', '*size' : 'size' }} (Commit eb7ee2cb ("qapi: introduce OptsVisitor") had originally documented OptsVisitor's general schema requirements for parsing repeated options such that the list element type had to be a struct with one mandatory scalar field. Accordingly, the RFC version of this series required for interval flattening that this underlying scalar type be an integer type. However, since commit a678e26c ("qapi: pad GenericList value fields to 64 bits") we've had reliable native lists; OptsVisitor turns out to support them automatically.) OptsVisitor already accepts the following command line with the above schema: -numa node,nodeid=3,cpus=0,cpus=1,cpus=2,cpus=6,cpus=7,cpus=8 Paolo suggested in <http://thread.gmane.org/gmane.comp.emulators.qemu/222589/focus=222732> that OptsVisitor should allow the following shortcut: -numa node,nodeid=3,cpus=0-2,cpus=6-8 and that the code processing the "cpus" list should encounter all six elements (0, 1, 2, 6, 7, 8) individually. The series implements this feature. Both signed and unsigned values and intervals are supported in general: * 0 (zero) * 1-5 (one to five) * 4-4 (four to four, range with one element) * -2 (minus two) * -5-8 (minus five to plus eight) * -9--6 (minus nine to minus six) The restrictions imposed by the native list element's signedness and size (in the above schema example, 'uint16') are enforced element-wise as usual. That is, for 'uint16', the command line option -numa node,nodeid=3,cpus=65534-65537 is equivalent to -numa node,nodeid=3,cpus=65534,cpus=65535,cpus=65536,cpus=65537 and visit_type_uint16() [qapi/qapi-visit-core.c] will catch the first element (= 65536) that has been parsed by opts_type_int() but cannot be represented as 'uint16'. Laszlo Ersek (8): OptsVisitor: introduce basic list modes OptsVisitor: introduce list modes for interval flattening OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS OptsVisitor: rebase opts_type_uint64() to parse_uint_full() OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS OptsVisitor: don't try to flatten overlong integer ranges add "test-int128" and "test-bitops" to .gitignore OptsVisitor: introduce unit tests, with test cases for range flattening tests/Makefile | 6 +- tests/qapi-schema/qapi-schema-test.json | 15 ++ include/qapi/opts-visitor.h | 6 + qapi/opts-visitor.c | 184 +++++++++++++++++---- tests/test-opts-visitor.c | 275 +++++++++++++++++++++++++++++++ .gitignore | 3 + tests/qapi-schema/qapi-schema-test.out | 6 +- 7 files changed, 461 insertions(+), 34 deletions(-) create mode 100644 tests/test-opts-visitor.c ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 1/8] OptsVisitor: introduce basic list modes 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek @ 2013-08-19 22:35 ` Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 2/8] OptsVisitor: introduce list modes for interval flattening Laszlo Ersek ` (8 subsequent siblings) 9 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-19 22:35 UTC (permalink / raw) To: lcapitulino, qemu-devel 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> --- qapi/opts-visitor.c | 45 +++++++++++++++++++++++++++++++++++---------- 1 files 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.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 2/8] OptsVisitor: introduce list modes for interval flattening 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 1/8] OptsVisitor: introduce basic list modes Laszlo Ersek @ 2013-08-19 22:35 ` Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 3/8] OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS Laszlo Ersek ` (7 subsequent siblings) 9 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-19 22:35 UTC (permalink / raw) To: lcapitulino, qemu-devel 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> --- qapi/opts-visitor.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 files 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.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 3/8] OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 1/8] OptsVisitor: introduce basic list modes Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 2/8] OptsVisitor: introduce list modes for interval flattening Laszlo Ersek @ 2013-08-19 22:35 ` Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 4/8] OptsVisitor: rebase opts_type_uint64() to parse_uint_full() Laszlo Ersek ` (6 subsequent siblings) 9 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-19 22:35 UTC (permalink / raw) To: lcapitulino, qemu-devel 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> --- qapi/opts-visitor.c | 34 ++++++++++++++++++++++++++++------ 1 files 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.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 4/8] OptsVisitor: rebase opts_type_uint64() to parse_uint_full() 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek ` (2 preceding siblings ...) 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 3/8] OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS Laszlo Ersek @ 2013-08-19 22:35 ` Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 5/8] OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS Laszlo Ersek ` (5 subsequent siblings) 9 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-19 22:35 UTC (permalink / raw) To: lcapitulino, qemu-devel Simplify the code in preparation for the next patch. Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- qapi/opts-visitor.c | 23 +++++------------------ 1 files 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.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 5/8] OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek ` (3 preceding siblings ...) 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 4/8] OptsVisitor: rebase opts_type_uint64() to parse_uint_full() Laszlo Ersek @ 2013-08-19 22:35 ` Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 6/8] OptsVisitor: don't try to flatten overlong integer ranges Laszlo Ersek ` (4 subsequent siblings) 9 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-19 22:35 UTC (permalink / raw) To: lcapitulino, qemu-devel 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> --- qapi/opts-visitor.c | 32 +++++++++++++++++++++++++++----- 1 files 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.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 6/8] OptsVisitor: don't try to flatten overlong integer ranges 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek ` (4 preceding siblings ...) 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 5/8] OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS Laszlo Ersek @ 2013-08-19 22:35 ` Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 7/8] add "test-int128" and "test-bitops" to .gitignore Laszlo Ersek ` (3 subsequent siblings) 9 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-19 22:35 UTC (permalink / raw) To: lcapitulino, qemu-devel 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> --- 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.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 7/8] add "test-int128" and "test-bitops" to .gitignore 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek ` (5 preceding siblings ...) 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 6/8] OptsVisitor: don't try to flatten overlong integer ranges Laszlo Ersek @ 2013-08-19 22:35 ` Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening Laszlo Ersek ` (2 subsequent siblings) 9 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-19 22:35 UTC (permalink / raw) To: lcapitulino, qemu-devel "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> --- v1->v2: - add test-bitops too .gitignore | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) 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.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek ` (6 preceding siblings ...) 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 7/8] add "test-int128" and "test-bitops" to .gitignore Laszlo Ersek @ 2013-08-19 22:35 ` Laszlo Ersek 2013-08-20 1:09 ` [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Wanlong Gao 2013-08-20 14:12 ` Luiz Capitulino 9 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-19 22:35 UTC (permalink / raw) To: lcapitulino, qemu-devel 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> --- v1->v2: - rebase to master, - synch "qapi-schema-test.out". tests/Makefile | 6 +- tests/qapi-schema/qapi-schema-test.json | 15 ++ tests/test-opts-visitor.c | 275 +++++++++++++++++++++++++++++++ .gitignore | 1 + tests/qapi-schema/qapi-schema-test.out | 6 +- 5 files changed, 300 insertions(+), 3 deletions(-) create mode 100644 tests/test-opts-visitor.c diff --git a/tests/Makefile b/tests/Makefile index d044908..27730cd 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) @@ -99,7 +101,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 @@ -141,6 +144,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/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; +} 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/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')]))])] -- 1.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek ` (7 preceding siblings ...) 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening Laszlo Ersek @ 2013-08-20 1:09 ` Wanlong Gao 2013-08-21 7:31 ` Laszlo Ersek 2013-08-20 14:12 ` Luiz Capitulino 9 siblings, 1 reply; 12+ messages in thread From: Wanlong Gao @ 2013-08-20 1:09 UTC (permalink / raw) To: Laszlo Ersek; +Cc: qemu-devel, lcapitulino On 08/20/2013 06:35 AM, Laszlo Ersek wrote: > v1->v2: Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> > - rebased to current master (patch #8 only applied with "git am -3"), > - now patch #7 adds "test-bitops" too to .gitignore, > - patch #8 updates "qapi-schema-test.out" to keep it in synch with the > test schema changes warranted by the opts-visitor unit tests. > > rfc->v1: > - addressed Paolo's comments for patches 1 and 2, > - patches 7 and 8 are new (unit tests), > - updated the cover letter to take native lists into account, plus > cleaned it up. > > Consider the following QAPI schema fragment, for the purpose of command > line parsing with OptsVisitor: > > { 'union': 'NumaOptions', > 'data': { > 'node': 'NumaNodeOptions', > 'mem' : 'NumaMemOptions' }} > > { 'type': 'NumaNodeOptions', > 'data': { > '*nodeid': 'int', > '*cpus' : ['uint16'] }} > > { 'type': 'NumaMemOptions', > 'data': { > '*nodeid': 'int', > '*size' : 'size' }} > > (Commit eb7ee2cb ("qapi: introduce OptsVisitor") had originally > documented OptsVisitor's general schema requirements for parsing > repeated options such that the list element type had to be a struct with > one mandatory scalar field. Accordingly, the RFC version of this series > required for interval flattening that this underlying scalar type be an > integer type. However, since commit a678e26c ("qapi: pad GenericList > value fields to 64 bits") we've had reliable native lists; OptsVisitor > turns out to support them automatically.) > > OptsVisitor already accepts the following command line with the above > schema: > > -numa node,nodeid=3,cpus=0,cpus=1,cpus=2,cpus=6,cpus=7,cpus=8 > > Paolo suggested in > <http://thread.gmane.org/gmane.comp.emulators.qemu/222589/focus=222732> > that OptsVisitor should allow the following shortcut: > > -numa node,nodeid=3,cpus=0-2,cpus=6-8 > > and that the code processing the "cpus" list should encounter all six > elements (0, 1, 2, 6, 7, 8) individually. > > The series implements this feature. Both signed and unsigned values and > intervals are supported in general: > > * 0 (zero) > * 1-5 (one to five) > * 4-4 (four to four, range with one element) > * -2 (minus two) > * -5-8 (minus five to plus eight) > * -9--6 (minus nine to minus six) > > The restrictions imposed by the native list element's signedness and > size (in the above schema example, 'uint16') are enforced element-wise > as usual. That is, for 'uint16', the command line option > > -numa node,nodeid=3,cpus=65534-65537 > > is equivalent to > > -numa node,nodeid=3,cpus=65534,cpus=65535,cpus=65536,cpus=65537 > > and visit_type_uint16() [qapi/qapi-visit-core.c] will catch the first > element (= 65536) that has been parsed by opts_type_int() but cannot be > represented as 'uint16'. > > Laszlo Ersek (8): > OptsVisitor: introduce basic list modes > OptsVisitor: introduce list modes for interval flattening > OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS > OptsVisitor: rebase opts_type_uint64() to parse_uint_full() > OptsVisitor: opts_type_uint64(): recognize intervals when > LM_IN_PROGRESS > OptsVisitor: don't try to flatten overlong integer ranges > add "test-int128" and "test-bitops" to .gitignore > OptsVisitor: introduce unit tests, with test cases for range > flattening > > tests/Makefile | 6 +- > tests/qapi-schema/qapi-schema-test.json | 15 ++ > include/qapi/opts-visitor.h | 6 + > qapi/opts-visitor.c | 184 +++++++++++++++++---- > tests/test-opts-visitor.c | 275 +++++++++++++++++++++++++++++++ > .gitignore | 3 + > tests/qapi-schema/qapi-schema-test.out | 6 +- > 7 files changed, 461 insertions(+), 34 deletions(-) > create mode 100644 tests/test-opts-visitor.c > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options 2013-08-20 1:09 ` [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Wanlong Gao @ 2013-08-21 7:31 ` Laszlo Ersek 0 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2013-08-21 7:31 UTC (permalink / raw) To: gaowanlong; +Cc: qemu-devel, lcapitulino On 08/20/13 03:09, Wanlong Gao wrote: > On 08/20/2013 06:35 AM, Laszlo Ersek wrote: >> v1->v2: > > > Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> Thank you. Laszlo ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek ` (8 preceding siblings ...) 2013-08-20 1:09 ` [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Wanlong Gao @ 2013-08-20 14:12 ` Luiz Capitulino 9 siblings, 0 replies; 12+ messages in thread From: Luiz Capitulino @ 2013-08-20 14:12 UTC (permalink / raw) To: Laszlo Ersek; +Cc: qemu-devel On Tue, 20 Aug 2013 00:35:32 +0200 Laszlo Ersek <lersek@redhat.com> wrote: > v1->v2: > - rebased to current master (patch #8 only applied with "git am -3"), > - now patch #7 adds "test-bitops" too to .gitignore, > - patch #8 updates "qapi-schema-test.out" to keep it in synch with the > test schema changes warranted by the opts-visitor unit tests. Applied to the qmp branch, thanks. I'm not very familiar with OptsVisitor, but looks good to me. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-08-21 7:56 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-19 22:35 [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 1/8] OptsVisitor: introduce basic list modes Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 2/8] OptsVisitor: introduce list modes for interval flattening Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 3/8] OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 4/8] OptsVisitor: rebase opts_type_uint64() to parse_uint_full() Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 5/8] OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 6/8] OptsVisitor: don't try to flatten overlong integer ranges Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 7/8] add "test-int128" and "test-bitops" to .gitignore Laszlo Ersek 2013-08-19 22:35 ` [Qemu-devel] [PATCH v2 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening Laszlo Ersek 2013-08-20 1:09 ` [Qemu-devel] [PATCH v2 0/8] OptsVisitor: support / flatten integer ranges for repeating options Wanlong Gao 2013-08-21 7:31 ` Laszlo Ersek 2013-08-20 14:12 ` Luiz Capitulino
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).