From e2503f5e213e30e3e9a397d454a35c10b5bdc899 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 2 Jul 2013 12:18:47 +0200 Subject: [PATCH 01/18] qapi-types.py: Implement 'base' for unions The new 'base' key in a union definition refers to a struct type, which is inlined into the union definition and can represent fields common to all kinds. For example the following schema definition... { 'type': 'BlockOptionsBase', 'data': { 'read-only': 'bool' } } { 'union': 'BlockOptions', 'base': 'BlockOptionsBase', 'data': { 'raw': 'BlockOptionsRaw' 'qcow2': 'BlockOptionsQcow2' } } ...would result in this generated C struct: struct BlockOptions { BlockOptionsKind kind; union { void *data; BlockOptionsRaw * raw; BlockOptionsQcow2 * qcow2; }; bool read_only; }; Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- scripts/qapi-types.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index ddcfed9f4b..07bd311657 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -150,7 +150,12 @@ typedef enum %(name)s return lookup_decl + enum_decl -def generate_union(name, typeinfo): +def generate_union(expr): + + name = expr['union'] + typeinfo = expr['data'] + base = expr.get('base') + ret = mcgen(''' struct %(name)s { @@ -169,6 +174,13 @@ struct %(name)s ret += mcgen(''' }; +''') + + if base: + struct = find_struct(base) + ret += generate_struct_fields(struct['data']) + + ret += mcgen(''' }; ''') @@ -352,7 +364,7 @@ for expr in exprs: ret += generate_type_cleanup_decl(expr['type']) fdef.write(generate_type_cleanup(expr['type']) + "\n") elif expr.has_key('union'): - ret += generate_union(expr['union'], expr['data']) + ret += generate_union(expr) ret += generate_type_cleanup_decl(expr['union'] + "List") fdef.write(generate_type_cleanup(expr['union'] + "List") + "\n") ret += generate_type_cleanup_decl(expr['union']) From d131c897f3dea8b76d7a487af0f7f5f11d0500a3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 2 Jul 2013 16:18:35 +0200 Subject: [PATCH 02/18] qapi-visit.py: Split off generate_visit_struct_fields() Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- scripts/qapi-visit.py | 62 ++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 6cac05acd5..a337d80f5b 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -17,34 +17,9 @@ import os import getopt import errno -def generate_visit_struct_body(field_prefix, name, members): - ret = mcgen(''' -if (!error_is_set(errp)) { -''') - push_indent() +def generate_visit_struct_fields(field_prefix, members): + ret = '' - if len(field_prefix): - field_prefix = field_prefix + "." - ret += mcgen(''' -Error **errp = &err; /* from outer scope */ -Error *err = NULL; -visit_start_struct(m, NULL, "", "%(name)s", 0, &err); -''', - name=name) - else: - ret += mcgen(''' -Error *err = NULL; -visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err); -''', - name=name) - - ret += mcgen(''' -if (!err) { - if (!obj || *obj) { -''') - - push_indent() - push_indent() for argname, argentry, optional, structured in parse_args(members): if optional: ret += mcgen(''' @@ -72,9 +47,40 @@ visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", visit_end_optional(m, &err); ''') + return ret + + +def generate_visit_struct_body(field_prefix, name, members): + ret = mcgen(''' +if (!error_is_set(errp)) { +''') + push_indent() + + if len(field_prefix): + field_prefix = field_prefix + "." + ret += mcgen(''' +Error **errp = &err; /* from outer scope */ +Error *err = NULL; +visit_start_struct(m, NULL, "", "%(name)s", 0, &err); +''', + name=name) + else: + ret += mcgen(''' +Error *err = NULL; +visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err); +''', + name=name) + + ret += mcgen(''' +if (!err) { + if (!obj || *obj) { +''') + push_indent() + push_indent() + + ret += generate_visit_struct_fields(field_prefix, members) pop_indent() ret += mcgen(''' - error_propagate(errp, err); err = NULL; } From 0aef92b90d24858eea1ebd52a51bc31563f1fb52 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 2 Jul 2013 16:20:04 +0200 Subject: [PATCH 03/18] qapi-visit.py: Implement 'base' for unions This implements the visitor part of base types for unions. Parsed into QMP, this example schema definition... { 'type': 'BlockOptionsBase', 'data': { 'read-only': 'bool' } } { 'type': 'BlockOptionsQcow2, 'data': { 'lazy-refcounts': 'bool' } } { 'union': 'BlockOptions', 'base': 'BlockOptionsBase', 'data': { 'raw': 'BlockOptionsRaw' 'qcow2': 'BlockOptionsQcow2' } } ...would describe the following JSON object: { "type": "qcow2", "read-only": true, "data": { "lazy-refcounts": false } } Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- scripts/qapi-visit.py | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index a337d80f5b..db6fa44b07 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -151,7 +151,13 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e ''', name=name) -def generate_visit_union(name, members): +def generate_visit_union(expr): + + name = expr['union'] + members = expr['data'] + + base = expr.get('base') + ret = generate_visit_enum('%sKind' % name, members.keys()) ret += mcgen(''' @@ -164,14 +170,28 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err); if (!err) { if (obj && *obj) { - visit_type_%(name)sKind(m, &(*obj)->kind, "type", &err); - if (!err) { - switch ((*obj)->kind) { ''', name=name) + push_indent() push_indent() + push_indent() + + if base: + struct = find_struct(base) + push_indent() + ret += generate_visit_struct_fields("", struct['data']) + pop_indent() + + pop_indent() + ret += mcgen(''' + visit_type_%(name)sKind(m, &(*obj)->kind, "type", &err); + if (!err) { + switch ((*obj)->kind) { +''', + name=name) + for key in members: ret += mcgen(''' case %(abbrev)s_KIND_%(enum)s: @@ -368,7 +388,7 @@ for expr in exprs: ret = generate_declaration(expr['type'], expr['data']) fdecl.write(ret) elif expr.has_key('union'): - ret = generate_visit_union(expr['union'], expr['data']) + ret = generate_visit_union(expr) ret += generate_visit_list(expr['union'], expr['data']) fdef.write(ret) From 51631493e4876081ae27078b50bd95bd4418bf37 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 16 Jul 2013 13:17:27 +0200 Subject: [PATCH 04/18] docs: Document QAPI union types Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- docs/qapi-code-gen.txt | 62 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index cccb11e562..f6f8d33863 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -34,9 +34,15 @@ OrderedDicts so that ordering is preserved. There are two basic syntaxes used, type definitions and command definitions. The first syntax defines a type and is represented by a dictionary. There are -two kinds of types that are supported: complex user-defined types, and enums. +three kinds of user-defined types that are supported: complex types, +enumeration types and union types. -A complex type is a dictionary containing a single key who's value is a +Generally speaking, types definitions should always use CamelCase for the type +names. Command names should be all lower case with words separated by a hyphen. + +=== Complex types === + +A complex type is a dictionary containing a single key whose value is a dictionary. This corresponds to a struct in C or an Object in JSON. An example of a complex type is: @@ -47,13 +53,57 @@ The use of '*' as a prefix to the name means the member is optional. Optional members should always be added to the end of the dictionary to preserve backwards compatibility. -An enumeration type is a dictionary containing a single key who's value is a +=== Enumeration types === + +An enumeration type is a dictionary containing a single key whose value is a list of strings. An example enumeration is: { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] } -Generally speaking, complex types and enums should always use CamelCase for -the type names. +=== Union types === + +Union types are used to let the user choose between several different data +types. A union type is defined using a dictionary as explained in the +following paragraphs. + + +A simple union type defines a mapping from discriminator values to data types +like in this example: + + { 'type': 'FileOptions', 'data': { 'filename': 'str' } } + { 'type': 'Qcow2Options', + 'data': { 'backing-file': 'str', 'lazy-refcounts': 'bool' } } + + { 'union': 'BlockdevOptions', + 'data': { 'file': 'FileOptions', + 'qcow2': 'Qcow2Options' } } + +In the QMP wire format, a simple union is represented by a dictionary that +contains the 'type' field as a discriminator, and a 'data' field that is of the +specified data type corresponding to the discriminator value: + + { "type": "qcow2", "data" : { "backing-file": "/some/place/my-image", + "lazy-refcounts": true } } + + +A union definition can specify a complex type as its base. In this case, the +fields of the complex type are included as top-level fields of the union +dictionary in the QMP wire format. An example definition is: + + { 'type': 'BlockdevCommonOptions', 'data': { 'readonly': 'bool' } } + { 'union': 'BlockdevOptions', + 'base': 'BlockdevCommonOptions', + 'data': { 'raw': 'RawOptions', + 'qcow2': 'Qcow2Options' } } + +And it looks like this on the wire: + + { "type": "qcow2", + "readonly": false, + "data" : { "backing-file": "/some/place/my-image", + "lazy-refcounts": true } } + +=== Commands === Commands are defined by using a list containing three members. The first member is the command name, the second member is a dictionary containing @@ -65,8 +115,6 @@ An example command is: 'data': { 'arg1': 'str', '*arg2': 'str' }, 'returns': 'str' } -Command names should be all lower case with words separated by a hyphen. - == Code generation == From 761d524dbcc5bb41213dd0f238f43c273bc2b077 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 3 Jul 2013 15:52:42 +0200 Subject: [PATCH 05/18] qapi: Add visitor for implicit structs These can be used when an embedded struct is parsed and members not belonging to the struct may be present in the input (e.g. parsing a flat namespace QMP union, where fields from both the base and one of the alternative types are mixed in the JSON object) Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- include/qapi/visitor-impl.h | 4 ++++ include/qapi/visitor.h | 3 +++ qapi/qapi-visit-core.c | 16 ++++++++++++++++ qapi/qmp-input-visitor.c | 14 ++++++++++++++ 4 files changed, 37 insertions(+) diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 5159964863..5c1297f430 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -22,6 +22,10 @@ struct Visitor const char *name, size_t size, Error **errp); void (*end_struct)(Visitor *v, Error **errp); + void (*start_implicit_struct)(Visitor *v, void **obj, size_t size, + Error **errp); + void (*end_implicit_struct)(Visitor *v, Error **errp); + void (*start_list)(Visitor *v, const char *name, Error **errp); GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp); void (*end_list)(Visitor *v, Error **errp); diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 28c21d8338..bd24f85e97 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -33,6 +33,9 @@ void visit_end_handle(Visitor *v, Error **errp); void visit_start_struct(Visitor *v, void **obj, const char *kind, const char *name, size_t size, Error **errp); void visit_end_struct(Visitor *v, Error **errp); +void visit_start_implicit_struct(Visitor *v, void **obj, size_t size, + Error **errp); +void visit_end_implicit_struct(Visitor *v, Error **errp); void visit_start_list(Visitor *v, const char *name, Error **errp); GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp); void visit_end_list(Visitor *v, Error **errp); diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 401ee6e597..9b4d51bc0a 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -45,6 +45,22 @@ void visit_end_struct(Visitor *v, Error **errp) v->end_struct(v, errp); } +void visit_start_implicit_struct(Visitor *v, void **obj, size_t size, + Error **errp) +{ + if (!error_is_set(errp) && v->start_implicit_struct) { + v->start_implicit_struct(v, obj, size, errp); + } +} + +void visit_end_implicit_struct(Visitor *v, Error **errp) +{ + assert(!error_is_set(errp)); + if (v->end_implicit_struct) { + v->end_implicit_struct(v, errp); + } +} + void visit_start_list(Visitor *v, const char *name, Error **errp) { if (!error_is_set(errp)) { diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 67fb127050..59c5cace96 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -144,6 +144,18 @@ static void qmp_input_end_struct(Visitor *v, Error **errp) qmp_input_pop(qiv, errp); } +static void qmp_input_start_implicit_struct(Visitor *v, void **obj, + size_t size, Error **errp) +{ + if (obj) { + *obj = g_malloc0(size); + } +} + +static void qmp_input_end_implicit_struct(Visitor *v, Error **errp) +{ +} + static void qmp_input_start_list(Visitor *v, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); @@ -293,6 +305,8 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj) v->visitor.start_struct = qmp_input_start_struct; v->visitor.end_struct = qmp_input_end_struct; + v->visitor.start_implicit_struct = qmp_input_start_implicit_struct; + v->visitor.end_implicit_struct = qmp_input_end_implicit_struct; v->visitor.start_list = qmp_input_start_list; v->visitor.next_list = qmp_input_next_list; v->visitor.end_list = qmp_input_end_list; From 50f2bdc75c5ee00617ad874c9ceac2cea660aa1e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 3 Jul 2013 15:58:57 +0200 Subject: [PATCH 06/18] qapi: Flat unions with arbitrary discriminator Instead of the rather verbose syntax that distinguishes base and subclass fields... { "type": "file", "read-only": true, "data": { "filename": "test" } } ...we can now have both in the same namespace, allowing a more direct mapping of the command line, and moving fields between the common base and subclasses without breaking the API: { "driver": "file", "read-only": true, "filename": "test" } Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- docs/qapi-code-gen.txt | 22 +++++++++++ scripts/qapi-types.py | 11 +++++- scripts/qapi-visit.py | 90 +++++++++++++++++++++++++++++++----------- 3 files changed, 98 insertions(+), 25 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index f6f8d33863..11f19cfa5f 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -103,6 +103,28 @@ And it looks like this on the wire: "data" : { "backing-file": "/some/place/my-image", "lazy-refcounts": true } } + +Flat union types avoid the nesting on the wire. They are used whenever a +specific field of the base type is declared as the discriminator ('type' is +then no longer generated). The discriminator must always be a string field. +The above example can then be modified as follows: + + { 'type': 'BlockdevCommonOptions', + 'data': { 'driver': 'str', 'readonly': 'bool' } } + { 'union': 'BlockdevOptions', + 'base': 'BlockdevCommonOptions', + 'discriminator': 'driver', + 'data': { 'raw': 'RawOptions', + 'qcow2': 'Qcow2Options' } } + +Resulting in this JSON object: + + { "driver": "qcow2", + "readonly": false, + "backing-file": "/some/place/my-image", + "lazy-refcounts": true } + + === Commands === Commands are defined by using a list containing three members. The first diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 07bd311657..84d46fb863 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -154,7 +154,9 @@ def generate_union(expr): name = expr['union'] typeinfo = expr['data'] + base = expr.get('base') + discriminator = expr.get('discriminator') ret = mcgen(''' struct %(name)s @@ -177,8 +179,13 @@ struct %(name)s ''') if base: - struct = find_struct(base) - ret += generate_struct_fields(struct['data']) + base_fields = find_struct(base)['data'] + if discriminator: + base_fields = base_fields.copy() + del base_fields[discriminator] + ret += generate_struct_fields(base_fields) + else: + assert not discriminator ret += mcgen(''' }; diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index db6fa44b07..87d5f15164 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -17,8 +17,30 @@ import os import getopt import errno -def generate_visit_struct_fields(field_prefix, members): +def generate_visit_struct_fields(name, field_prefix, fn_prefix, members): + substructs = [] ret = '' + full_name = name if not fn_prefix else "%s_%s" % (name, fn_prefix) + + for argname, argentry, optional, structured in parse_args(members): + if structured: + if not fn_prefix: + nested_fn_prefix = argname + else: + nested_fn_prefix = "%s_%s" % (fn_prefix, argname) + + nested_field_prefix = "%s%s." % (field_prefix, argname) + ret += generate_visit_struct_fields(name, nested_field_prefix, + nested_fn_prefix, argentry) + + ret += mcgen(''' + +static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error **errp) +{ + Error *err = NULL; +''', + name=name, full_name=full_name) + push_indent() for argname, argentry, optional, structured in parse_args(members): if optional: @@ -31,7 +53,7 @@ if (obj && (*obj)->%(prefix)shas_%(c_name)s) { push_indent() if structured: - ret += generate_visit_struct_body(field_prefix + argname, argname, argentry) + ret += generate_visit_struct_body(full_name, argname, argentry) else: ret += mcgen(''' visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", &err); @@ -47,6 +69,12 @@ visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", visit_end_optional(m, &err); ''') + pop_indent() + ret += mcgen(''' + + error_propagate(errp, err); +} +''') return ret @@ -56,8 +84,9 @@ if (!error_is_set(errp)) { ''') push_indent() + full_name = name if not field_prefix else "%s_%s" % (field_prefix, name) + if len(field_prefix): - field_prefix = field_prefix + "." ret += mcgen(''' Error **errp = &err; /* from outer scope */ Error *err = NULL; @@ -74,19 +103,13 @@ visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err); ret += mcgen(''' if (!err) { if (!obj || *obj) { -''') - push_indent() - push_indent() + visit_type_%(name)s_fields(m, obj, &err); + error_propagate(errp, err); + err = NULL; + } +''', + name=full_name) - ret += generate_visit_struct_fields(field_prefix, members) - pop_indent() - ret += mcgen(''' - error_propagate(errp, err); - err = NULL; -} -''') - - pop_indent() pop_indent() ret += mcgen(''' /* Always call end_struct if start_struct succeeded. */ @@ -98,7 +121,9 @@ if (!err) { return ret def generate_visit_struct(name, members): - ret = mcgen(''' + ret = generate_visit_struct_fields(name, "", "", members) + + ret += mcgen(''' void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp) { @@ -157,9 +182,17 @@ def generate_visit_union(expr): members = expr['data'] base = expr.get('base') + discriminator = expr.get('discriminator') ret = generate_visit_enum('%sKind' % name, members.keys()) + if base: + base_fields = find_struct(base)['data'] + if discriminator: + base_fields = base_fields.copy() + del base_fields[discriminator] + ret += generate_visit_struct_fields(name, "", "", base_fields) + ret += mcgen(''' void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp) @@ -179,23 +212,34 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** push_indent() if base: - struct = find_struct(base) - push_indent() - ret += generate_visit_struct_fields("", struct['data']) - pop_indent() + ret += mcgen(''' + visit_type_%(name)s_fields(m, obj, &err); +''', + name=name) pop_indent() ret += mcgen(''' - visit_type_%(name)sKind(m, &(*obj)->kind, "type", &err); + visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err); if (!err) { switch ((*obj)->kind) { ''', - name=name) + name=name, type="type" if not discriminator else discriminator) for key in members: + if not discriminator: + fmt = 'visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);' + else: + fmt = '''visit_start_implicit_struct(m, (void**) &(*obj)->%(c_name)s, sizeof(%(c_type)s), &err); + if (!err) { + visit_type_%(c_type)s_fields(m, &(*obj)->%(c_name)s, &err); + error_propagate(errp, err); + err = NULL; + visit_end_implicit_struct(m, &err); + }''' + ret += mcgen(''' case %(abbrev)s_KIND_%(enum)s: - visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err); + ''' + fmt + ''' break; ''', abbrev = de_camel_case(name).upper(), From e8316d7e8e8339a9ea593ba821a0aad26908c0d5 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 8 Jul 2013 11:33:07 +0200 Subject: [PATCH 07/18] qapi: Add consume argument to qmp_input_get_object() This allows to just look at the next element without actually consuming it. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- qapi/qmp-input-visitor.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 59c5cace96..70864a1af2 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -41,13 +41,14 @@ static QmpInputVisitor *to_qiv(Visitor *v) } static QObject *qmp_input_get_object(QmpInputVisitor *qiv, - const char *name) + const char *name, + bool consume) { QObject *qobj = qiv->stack[qiv->nb_stack - 1].obj; if (qobj) { if (name && qobject_type(qobj) == QTYPE_QDICT) { - if (qiv->stack[qiv->nb_stack - 1].h) { + if (qiv->stack[qiv->nb_stack - 1].h && consume) { g_hash_table_remove(qiv->stack[qiv->nb_stack - 1].h, name); } return qdict_get(qobject_to_qdict(qobj), name); @@ -117,7 +118,7 @@ static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind, const char *name, size_t size, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name); + QObject *qobj = qmp_input_get_object(qiv, name, true); Error *err = NULL; if (!qobj || qobject_type(qobj) != QTYPE_QDICT) { @@ -159,7 +160,7 @@ static void qmp_input_end_implicit_struct(Visitor *v, Error **errp) static void qmp_input_start_list(Visitor *v, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name); + QObject *qobj = qmp_input_get_object(qiv, name, true); if (!qobj || qobject_type(qobj) != QTYPE_QLIST) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -211,7 +212,7 @@ static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name); + QObject *qobj = qmp_input_get_object(qiv, name, true); if (!qobj || qobject_type(qobj) != QTYPE_QINT) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -226,7 +227,7 @@ static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name); + QObject *qobj = qmp_input_get_object(qiv, name, true); if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -241,7 +242,7 @@ static void qmp_input_type_str(Visitor *v, char **obj, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name); + QObject *qobj = qmp_input_get_object(qiv, name, true); if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -256,7 +257,7 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name); + QObject *qobj = qmp_input_get_object(qiv, name, true); if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT && qobject_type(qobj) != QTYPE_QINT)) { @@ -276,7 +277,7 @@ static void qmp_input_start_optional(Visitor *v, bool *present, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QObject *qobj = qmp_input_get_object(qiv, name); + QObject *qobj = qmp_input_get_object(qiv, name, true); if (!qobj) { *present = false; From ea66c6d8819c8fc5f73a28554992be64e5399fed Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 16 Jul 2013 10:49:41 +0200 Subject: [PATCH 08/18] qapi.py: Maintain a list of union types Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- scripts/qapi.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/scripts/qapi.py b/scripts/qapi.py index baf13213a9..3a54c7fc7c 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -105,6 +105,7 @@ def parse_schema(fp): if expr_eval.has_key('enum'): add_enum(expr_eval['enum']) elif expr_eval.has_key('union'): + add_union(expr_eval) add_enum('%sKind' % expr_eval['union']) elif expr_eval.has_key('type'): add_struct(expr_eval) @@ -188,6 +189,7 @@ def type_name(name): enum_types = [] struct_types = [] +union_types = [] def add_struct(definition): global struct_types @@ -200,6 +202,17 @@ def find_struct(name): return struct return None +def add_union(definition): + global union_types + union_types.append(definition) + +def find_union(name): + global union_types + for union in union_types: + if union['union'] == name: + return union + return None + def add_enum(name): global enum_types enum_types.append(name) From 69dd62dfd60631ba69201d8a197fde1ece4b4df3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 8 Jul 2013 16:14:21 +0200 Subject: [PATCH 09/18] qapi: Anonymous unions The discriminator for anonymous unions is the data type. This allows to have a union type that allows both of these: { 'file': 'my_existing_block_device_id' } { 'file': { 'filename': '/tmp/mydisk.qcow2', 'read-only': true } } Unions like this are specified in the schema with an empty dict as discriminator. For this example you could take: { 'union': 'BlockRef', 'discriminator': {}, 'data': { 'definition': 'BlockOptions', 'reference': 'str' } } { 'type': 'ExampleObject', 'data: { 'file': 'BlockRef' } } Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- docs/qapi-code-gen.txt | 25 ++++++++++++++++++++ include/qapi/qmp/qobject.h | 1 + include/qapi/visitor-impl.h | 2 ++ include/qapi/visitor.h | 3 +++ qapi/qapi-visit-core.c | 9 +++++++ qapi/qmp-input-visitor.c | 14 +++++++++++ qobject/qjson.c | 2 ++ scripts/qapi-types.py | 42 +++++++++++++++++++++++++++++++++ scripts/qapi-visit.py | 47 +++++++++++++++++++++++++++++++++++++ scripts/qapi.py | 15 ++++++++++++ 10 files changed, 160 insertions(+) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 11f19cfa5f..0ce045c0b3 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -125,6 +125,31 @@ Resulting in this JSON object: "lazy-refcounts": true } +A special type of unions are anonymous unions. They don't form a dictionary in +the wire format but allow the direct use of different types in their place. As +they aren't structured, they don't have any explicit discriminator but use +the (QObject) data type of their value as an implicit discriminator. This means +that they are restricted to using only one discriminator value per QObject +type. For example, you cannot have two different complex types in an anonymous +union, or two different integer types. + +Anonymous unions are declared using an empty dictionary as their discriminator. +The discriminator values never appear on the wire, they are only used in the +generated C code. Anonymous unions cannot have a base type. + + { 'union': 'BlockRef', + 'discriminator': {}, + 'data': { 'definition': 'BlockdevOptions', + 'reference': 'str' } } + +This example allows using both of the following example objects: + + { "file": "my_existing_block_device_id" } + { "file": { "driver": "file", + "readonly": false, + 'filename': "/tmp/mydisk.qcow2" } } + + === Commands === Commands are defined by using a list containing three members. The first diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h index 9124649ed2..d0bbc7c4a6 100644 --- a/include/qapi/qmp/qobject.h +++ b/include/qapi/qmp/qobject.h @@ -44,6 +44,7 @@ typedef enum { QTYPE_QFLOAT, QTYPE_QBOOL, QTYPE_QERROR, + QTYPE_MAX, } qtype_code; struct QObject; diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 5c1297f430..f3fa420245 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -32,6 +32,8 @@ struct Visitor void (*type_enum)(Visitor *v, int *obj, const char *strings[], const char *kind, const char *name, Error **errp); + void (*get_next_type)(Visitor *v, int *kind, const int *qobjects, + const char *name, Error **errp); void (*type_int)(Visitor *v, int64_t *obj, const char *name, Error **errp); void (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp); diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index bd24f85e97..48a2a2edfd 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -13,6 +13,7 @@ #ifndef QAPI_VISITOR_CORE_H #define QAPI_VISITOR_CORE_H +#include "qapi/qmp/qobject.h" #include "qapi/error.h" #include @@ -42,6 +43,8 @@ void visit_end_list(Visitor *v, Error **errp); void visit_start_optional(Visitor *v, bool *present, const char *name, Error **errp); void visit_end_optional(Visitor *v, Error **errp); +void visit_get_next_type(Visitor *v, int *obj, const int *qtypes, + const char *name, Error **errp); void visit_type_enum(Visitor *v, int *obj, const char *strings[], const char *kind, const char *name, Error **errp); void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp); diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 9b4d51bc0a..d6a4012f78 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -12,6 +12,7 @@ */ #include "qemu-common.h" +#include "qapi/qmp/qobject.h" #include "qapi/qmp/qerror.h" #include "qapi/visitor.h" #include "qapi/visitor-impl.h" @@ -98,6 +99,14 @@ void visit_end_optional(Visitor *v, Error **errp) } } +void visit_get_next_type(Visitor *v, int *obj, const int *qtypes, + const char *name, Error **errp) +{ + if (!error_is_set(errp) && v->get_next_type) { + v->get_next_type(v, obj, qtypes, name, errp); + } +} + void visit_type_enum(Visitor *v, int *obj, const char *strings[], const char *kind, const char *name, Error **errp) { diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 70864a1af2..bf42c04ea6 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -208,6 +208,19 @@ static void qmp_input_end_list(Visitor *v, Error **errp) qmp_input_pop(qiv, errp); } +static void qmp_input_get_next_type(Visitor *v, int *kind, const int *qobjects, + const char *name, Error **errp) +{ + QmpInputVisitor *qiv = to_qiv(v); + QObject *qobj = qmp_input_get_object(qiv, name, false); + + if (!qobj) { + error_set(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } + *kind = qobjects[qobject_type(qobj)]; +} + static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) { @@ -317,6 +330,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj) v->visitor.type_str = qmp_input_type_str; v->visitor.type_number = qmp_input_type_number; v->visitor.start_optional = qmp_input_start_optional; + v->visitor.get_next_type = qmp_input_get_next_type; qmp_input_push(v, obj, NULL); qobject_incref(obj); diff --git a/qobject/qjson.c b/qobject/qjson.c index 19085a1bb7..6cf2511580 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -260,6 +260,8 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent) /* XXX: should QError be emitted? */ case QTYPE_NONE: break; + case QTYPE_MAX: + abort(); } } diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 84d46fb863..5ee46ea1b3 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -150,6 +150,40 @@ typedef enum %(name)s return lookup_decl + enum_decl +def generate_anon_union_qtypes(expr): + + name = expr['union'] + members = expr['data'] + + ret = mcgen(''' +const int %(name)s_qtypes[QTYPE_MAX] = { +''', + name=name) + + for key in members: + qapi_type = members[key] + if builtin_type_qtypes.has_key(qapi_type): + qtype = builtin_type_qtypes[qapi_type] + elif find_struct(qapi_type): + qtype = "QTYPE_QDICT" + elif find_union(qapi_type): + qtype = "QTYPE_QDICT" + else: + assert False, "Invalid anonymous union member" + + ret += mcgen(''' + [ %(qtype)s ] = %(abbrev)s_KIND_%(enum)s, +''', + qtype = qtype, + abbrev = de_camel_case(name).upper(), + enum = c_fun(de_camel_case(key),False).upper()) + + ret += mcgen(''' +}; +''') + return ret + + def generate_union(expr): name = expr['union'] @@ -190,6 +224,12 @@ struct %(name)s ret += mcgen(''' }; ''') + if discriminator == {}: + ret += mcgen(''' +extern const int %(name)s_qtypes[]; +''', + name=name) + return ret @@ -342,6 +382,8 @@ for expr in exprs: ret += generate_fwd_struct(expr['union'], expr['data']) + "\n" ret += generate_enum('%sKind' % expr['union'], expr['data'].keys()) fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys())) + if expr.get('discriminator') == {}: + fdef.write(generate_anon_union_qtypes(expr)) else: continue fdecl.write(ret) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 87d5f15164..597cca4b66 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -176,6 +176,49 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e ''', name=name) +def generate_visit_anon_union(name, members): + ret = mcgen(''' + +void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp) +{ + Error *err = NULL; + + if (!error_is_set(errp)) { + visit_start_implicit_struct(m, (void**) obj, sizeof(%(name)s), &err); + visit_get_next_type(m, (int*) &(*obj)->kind, %(name)s_qtypes, name, &err); + switch ((*obj)->kind) { +''', + name=name) + + for key in members: + assert (members[key] in builtin_types + or find_struct(members[key]) + or find_union(members[key])), "Invalid anonymous union member" + + ret += mcgen(''' + case %(abbrev)s_KIND_%(enum)s: + visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err); + break; +''', + abbrev = de_camel_case(name).upper(), + enum = c_fun(de_camel_case(key),False).upper(), + c_type = type_name(members[key]), + c_name = c_fun(key)) + + ret += mcgen(''' + default: + abort(); + } + error_propagate(errp, err); + err = NULL; + visit_end_implicit_struct(m, &err); + } +} +''') + + return ret + + def generate_visit_union(expr): name = expr['union'] @@ -184,6 +227,10 @@ def generate_visit_union(expr): base = expr.get('base') discriminator = expr.get('discriminator') + if discriminator == {}: + assert not base + return generate_visit_anon_union(name, members) + ret = generate_visit_enum('%sKind' % name, members.keys()) if base: diff --git a/scripts/qapi.py b/scripts/qapi.py index 3a54c7fc7c..38c808e256 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -17,6 +17,21 @@ builtin_types = [ 'uint8', 'uint16', 'uint32', 'uint64' ] +builtin_type_qtypes = { + 'str': 'QTYPE_QSTRING', + 'int': 'QTYPE_QINT', + 'number': 'QTYPE_QFLOAT', + 'bool': 'QTYPE_QBOOL', + 'int8': 'QTYPE_QINT', + 'int16': 'QTYPE_QINT', + 'int32': 'QTYPE_QINT', + 'int64': 'QTYPE_QINT', + 'uint8': 'QTYPE_QINT', + 'uint16': 'QTYPE_QINT', + 'uint32': 'QTYPE_QINT', + 'uint64': 'QTYPE_QINT', +} + def tokenize(data): while len(data): ch = data[0] From 74fe54f2a1b5c4f4498a8fe521e1dfc936656cd4 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 9 Jul 2013 11:09:02 +0200 Subject: [PATCH 10/18] block: Allow "driver" option on the top level This is traditionally -drive format=..., which is now translated into the new driver option. This gives us a more consistent way to select the driver of BlockDriverStates that can be used in QMP context, too. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 7 +++++++ blockdev.c | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/block.c b/block.c index 6cd39fa146..c77cfd16c7 100644 --- a/block.c +++ b/block.c @@ -970,6 +970,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, char tmp_filename[PATH_MAX + 1]; BlockDriverState *file = NULL; QDict *file_options = NULL; + const char *drvname; /* NULL means an empty set of options */ if (options == NULL) { @@ -1059,6 +1060,12 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, } /* Find the right image format driver */ + drvname = qdict_get_try_str(options, "driver"); + if (drvname) { + drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR)); + qdict_del(options, "driver"); + } + if (!drv) { ret = find_image_format(file, filename, &drv); } diff --git a/blockdev.c b/blockdev.c index c5abd65182..8ff8ed325a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -322,7 +322,6 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) enum { MEDIA_DISK, MEDIA_CDROM } media; int bus_id, unit_id; int cyls, heads, secs, translation; - BlockDriver *drv = NULL; int max_devs; int index; int ro = 0; @@ -338,6 +337,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) QemuOpts *opts; QDict *bs_opts; const char *id; + bool has_driver_specific_opts; translation = BIOS_ATA_TRANSLATION_AUTO; media = MEDIA_DISK; @@ -365,6 +365,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) qdict_del(bs_opts, "id"); } + has_driver_specific_opts = !!qdict_size(bs_opts); + /* extract parameters */ bus_id = qemu_opt_get_number(opts, "bus", 0); unit_id = qemu_opt_get_number(opts, "unit", -1); @@ -477,11 +479,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) error_printf("\n"); return NULL; } - drv = bdrv_find_whitelisted_format(buf, ro); - if (!drv) { - error_report("'%s' invalid format", buf); - return NULL; - } + + qdict_put(bs_opts, "driver", qstring_from_str(buf)); } /* disk I/O throttling */ @@ -658,7 +657,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) abort(); } if (!file || !*file) { - if (qdict_size(bs_opts)) { + if (has_driver_specific_opts) { file = NULL; } else { return dinfo; @@ -695,13 +694,13 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) error_report("warning: disabling copy_on_read on readonly drive"); } - ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv); - bs_opts = NULL; + QINCREF(bs_opts); + ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, NULL); if (ret < 0) { if (ret == -EMEDIUMTYPE) { error_report("could not open disk image %s: not in %s format", - file ?: dinfo->id, drv->format_name); + file ?: dinfo->id, qdict_get_str(bs_opts, "driver")); } else { error_report("could not open disk image %s: %s", file ?: dinfo->id, strerror(-ret)); @@ -712,6 +711,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) if (bdrv_key_required(dinfo->bdrv)) autostart = 0; + QDECREF(bs_opts); qemu_opts_del(opts); return dinfo; From 0dd6c5266313c861cf36476da86599d368ec41fc Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 17 Jul 2013 14:40:37 +0200 Subject: [PATCH 11/18] QemuOpts: Add qemu_opt_unset() Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- include/qemu/option.h | 1 + util/qemu-option.c | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/include/qemu/option.h b/include/qemu/option.h index a83c700323..13f5e72a8e 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -120,6 +120,7 @@ bool qemu_opt_has_help_opt(QemuOpts *opts); bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval); uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval); uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval); +int qemu_opt_unset(QemuOpts *opts, const char *name); int qemu_opt_set(QemuOpts *opts, const char *name, const char *value); void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, Error **errp); diff --git a/util/qemu-option.c b/util/qemu-option.c index e0ef426daa..5d686c805f 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -593,6 +593,20 @@ static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc, return NULL; } +int qemu_opt_unset(QemuOpts *opts, const char *name) +{ + QemuOpt *opt = qemu_opt_find(opts, name); + + assert(opts_accepts_any(opts)); + + if (opt == NULL) { + return -1; + } else { + qemu_opt_del(opt); + return 0; + } +} + static void opt_set(QemuOpts *opts, const char *name, const char *value, bool prepend, Error **errp) { From 57975222b6a928dd4a4a8a7b37093cc8ecba5045 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 17 Jul 2013 14:41:54 +0200 Subject: [PATCH 12/18] blockdev: Rename I/O throttling options for QMP In QMP, we want to use dashes instead of underscores in QMP argument names, and use nested options for throttling. The new option names affect the command line as well, but for compatibility drive_init() will convert the old option names before calling into the code that will be shared between -drive and blockdev-add. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- blockdev.c | 52 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/blockdev.c b/blockdev.c index 8ff8ed325a..5403188005 100644 --- a/blockdev.c +++ b/blockdev.c @@ -312,7 +312,8 @@ static bool do_check_io_limits(BlockIOLimit *io_limits, Error **errp) return true; } -DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) +static DriveInfo *blockdev_init(QemuOpts *all_opts, + BlockInterfaceType block_default_type) { const char *buf; const char *file = NULL; @@ -485,17 +486,17 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) /* disk I/O throttling */ io_limits.bps[BLOCK_IO_LIMIT_TOTAL] = - qemu_opt_get_number(opts, "bps", 0); + qemu_opt_get_number(opts, "throttling.bps-total", 0); io_limits.bps[BLOCK_IO_LIMIT_READ] = - qemu_opt_get_number(opts, "bps_rd", 0); + qemu_opt_get_number(opts, "throttling.bps-read", 0); io_limits.bps[BLOCK_IO_LIMIT_WRITE] = - qemu_opt_get_number(opts, "bps_wr", 0); + qemu_opt_get_number(opts, "throttling.bps-write", 0); io_limits.iops[BLOCK_IO_LIMIT_TOTAL] = - qemu_opt_get_number(opts, "iops", 0); + qemu_opt_get_number(opts, "throttling.iops-total", 0); io_limits.iops[BLOCK_IO_LIMIT_READ] = - qemu_opt_get_number(opts, "iops_rd", 0); + qemu_opt_get_number(opts, "throttling.iops-read", 0); io_limits.iops[BLOCK_IO_LIMIT_WRITE] = - qemu_opt_get_number(opts, "iops_wr", 0); + qemu_opt_get_number(opts, "throttling.iops-write", 0); if (!do_check_io_limits(&io_limits, &error)) { error_report("%s", error_get_pretty(error)); @@ -726,6 +727,31 @@ err: return NULL; } +static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to) +{ + const char *value; + + value = qemu_opt_get(opts, from); + if (value) { + qemu_opt_set(opts, to, value); + qemu_opt_unset(opts, from); + } +} + +DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) +{ + /* Change legacy command line options into QMP ones */ + qemu_opt_rename(all_opts, "iops", "throttling.iops-total"); + qemu_opt_rename(all_opts, "iops_rd", "throttling.iops-read"); + qemu_opt_rename(all_opts, "iops_wr", "throttling.iops-write"); + + qemu_opt_rename(all_opts, "bps", "throttling.bps-total"); + qemu_opt_rename(all_opts, "bps_rd", "throttling.bps-read"); + qemu_opt_rename(all_opts, "bps_wr", "throttling.bps-write"); + + return blockdev_init(all_opts, block_default_type); +} + void do_commit(Monitor *mon, const QDict *qdict) { const char *device = qdict_get_str(qdict, "device"); @@ -1855,27 +1881,27 @@ QemuOptsList qemu_common_drive_opts = { .type = QEMU_OPT_BOOL, .help = "open drive file as read-only", },{ - .name = "iops", + .name = "throttling.iops-total", .type = QEMU_OPT_NUMBER, .help = "limit total I/O operations per second", },{ - .name = "iops_rd", + .name = "throttling.iops-read", .type = QEMU_OPT_NUMBER, .help = "limit read operations per second", },{ - .name = "iops_wr", + .name = "throttling.iops-write", .type = QEMU_OPT_NUMBER, .help = "limit write operations per second", },{ - .name = "bps", + .name = "throttling.bps-total", .type = QEMU_OPT_NUMBER, .help = "limit total bytes per second", },{ - .name = "bps_rd", + .name = "throttling.bps-read", .type = QEMU_OPT_NUMBER, .help = "limit read bytes per second", },{ - .name = "bps_wr", + .name = "throttling.bps-write", .type = QEMU_OPT_NUMBER, .help = "limit write bytes per second", },{ From 64aa99d3e0333dea73d7505190659a02ca909292 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 17 Jul 2013 14:45:34 +0200 Subject: [PATCH 13/18] qcow2: Use dashes instead of underscores in options This is what QMP wants to use. The options haven't been enabled in any release yet, so we're still free to change them. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/qcow2.c | 2 +- block/qcow2.h | 8 ++++---- tests/qemu-iotests/051 | 14 +++++++------- tests/qemu-iotests/051.out | 30 +++++++++++++++--------------- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 0eceefe2cd..3376901bd7 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -291,7 +291,7 @@ static QemuOptsList qcow2_runtime_opts = { .head = QTAILQ_HEAD_INITIALIZER(qcow2_runtime_opts.head), .desc = { { - .name = "lazy_refcounts", + .name = QCOW2_OPT_LAZY_REFCOUNTS, .type = QEMU_OPT_BOOL, .help = "Postpone refcount updates", }, diff --git a/block/qcow2.h b/block/qcow2.h index 3b2d5cda71..dba9771419 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -59,10 +59,10 @@ #define DEFAULT_CLUSTER_SIZE 65536 -#define QCOW2_OPT_LAZY_REFCOUNTS "lazy_refcounts" -#define QCOW2_OPT_DISCARD_REQUEST "pass_discard_request" -#define QCOW2_OPT_DISCARD_SNAPSHOT "pass_discard_snapshot" -#define QCOW2_OPT_DISCARD_OTHER "pass_discard_other" +#define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts" +#define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request" +#define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot" +#define QCOW2_OPT_DISCARD_OTHER "pass-discard-other" typedef struct QCowHeader { uint32_t magic; diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index 1cf8bf79b6..1f39c6ad21 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -72,11 +72,11 @@ echo echo === Enable and disable lazy refcounting on the command line, plus some invalid values === echo -run_qemu -drive file=$TEST_IMG,format=qcow2,lazy_refcounts=on -run_qemu -drive file=$TEST_IMG,format=qcow2,lazy_refcounts=off -run_qemu -drive file=$TEST_IMG,format=qcow2,lazy_refcounts= -run_qemu -drive file=$TEST_IMG,format=qcow2,lazy_refcounts=42 -run_qemu -drive file=$TEST_IMG,format=qcow2,lazy_refcounts=foo +run_qemu -drive file=$TEST_IMG,format=qcow2,lazy-refcounts=on +run_qemu -drive file=$TEST_IMG,format=qcow2,lazy-refcounts=off +run_qemu -drive file=$TEST_IMG,format=qcow2,lazy-refcounts= +run_qemu -drive file=$TEST_IMG,format=qcow2,lazy-refcounts=42 +run_qemu -drive file=$TEST_IMG,format=qcow2,lazy-refcounts=foo echo @@ -85,8 +85,8 @@ echo _make_test_img -ocompat=0.10 $size -run_qemu -drive file=$TEST_IMG,format=qcow2,lazy_refcounts=on -run_qemu -drive file=$TEST_IMG,format=qcow2,lazy_refcounts=off +run_qemu -drive file=$TEST_IMG,format=qcow2,lazy-refcounts=on +run_qemu -drive file=$TEST_IMG,format=qcow2,lazy-refcounts=off echo echo === No medium === diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 95ff245f40..c676538022 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -22,35 +22,35 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: could not === Enable and disable lazy refcounting on the command line, plus some invalid values === -Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=on +Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on QEMU 1.5.50 monitor - type 'help' for more information (qemu) qququiquit -Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=off +Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=off QEMU 1.5.50 monitor - type 'help' for more information (qemu) qququiquit -Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts= -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=: Parameter 'lazy_refcounts' expects 'on' or 'off' -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=: could not open disk image TEST_DIR/t.qcow2: Invalid argument +Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts= +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off' +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: could not open disk image TEST_DIR/t.qcow2: Invalid argument -Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=42 -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=42: Parameter 'lazy_refcounts' expects 'on' or 'off' -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=42: could not open disk image TEST_DIR/t.qcow2: Invalid argument +Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off' +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: could not open disk image TEST_DIR/t.qcow2: Invalid argument -Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=foo -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=foo: Parameter 'lazy_refcounts' expects 'on' or 'off' -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=foo: could not open disk image TEST_DIR/t.qcow2: Invalid argument +Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off' +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: could not open disk image TEST_DIR/t.qcow2: Invalid argument === With version 2 images enabling lazy refcounts must fail === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 -Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=on -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=on: Lazy refcounts require a qcow2 image with at least qemu 1.1 compatibility level -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=on: could not open disk image TEST_DIR/t.qcow2: Invalid argument +Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: Lazy refcounts require a qcow2 image with at least qemu 1.1 compatibility level +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: could not open disk image TEST_DIR/t.qcow2: Invalid argument -Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=off +Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=off QEMU 1.5.50 monitor - type 'help' for more information (qemu) qququiquit From 0f227a947004aa9043d4386f4a47d6739499b88f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 19 Jul 2013 20:07:29 +0200 Subject: [PATCH 14/18] blockdev: Rename 'readonly' option to 'read-only' Option name cleanup before it becomes a QMP API. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- blockdev.c | 10 ++++++---- tests/qemu-iotests/051.out | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/blockdev.c b/blockdev.c index 5403188005..3b05e29082 100644 --- a/blockdev.c +++ b/blockdev.c @@ -378,7 +378,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, secs = qemu_opt_get_number(opts, "secs", 0); snapshot = qemu_opt_get_bool(opts, "snapshot", 0); - ro = qemu_opt_get_bool(opts, "readonly", 0); + ro = qemu_opt_get_bool(opts, "read-only", 0); copy_on_read = qemu_opt_get_bool(opts, "copy-on-read", false); file = qemu_opt_get(opts, "file"); @@ -684,7 +684,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, } else if (ro == 1) { if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && type != IF_NONE && type != IF_PFLASH) { - error_report("readonly not supported by this bus type"); + error_report("read-only not supported by this bus type"); goto err; } } @@ -692,7 +692,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, bdrv_flags |= ro ? 0 : BDRV_O_RDWR; if (ro && copy_on_read) { - error_report("warning: disabling copy_on_read on readonly drive"); + error_report("warning: disabling copy_on_read on read-only drive"); } QINCREF(bs_opts); @@ -749,6 +749,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) qemu_opt_rename(all_opts, "bps_rd", "throttling.bps-read"); qemu_opt_rename(all_opts, "bps_wr", "throttling.bps-write"); + qemu_opt_rename(all_opts, "readonly", "read-only"); + return blockdev_init(all_opts, block_default_type); } @@ -1877,7 +1879,7 @@ QemuOptsList qemu_common_drive_opts = { .type = QEMU_OPT_STRING, .help = "pci address (virtio only)", },{ - .name = "readonly", + .name = "read-only", .type = QEMU_OPT_BOOL, .help = "open drive file as read-only", },{ diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index c676538022..9588d0c977 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -137,7 +137,7 @@ QEMU 1.5.50 monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on: readonly not supported by this bus type +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on: read-only not supported by this bus type Testing: -drive file=TEST_DIR/t.qcow2,if=virtio,readonly=on QEMU 1.5.50 monitor - type 'help' for more information From 29c4e2b50d95f4a15c3dd62b39f3402f05a34907 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 18 Jul 2013 16:31:25 +0200 Subject: [PATCH 15/18] blockdev: Split up 'cache' option The old 'cache' option really encodes three different boolean flags into a cache mode name, without providing all combinations. Make them three separate options instead and translate the old option to the new ones for drive_init(). The specific boolean options take precedence if the old cache option is specified as well, so the following options are equivalent: -drive file=x,cache=none,cache.no-flush=true -drive file=x,cache.writeback=true,cache.direct=true,cache.no-flush=true Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- blockdev.c | 57 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/blockdev.c b/blockdev.c index 3b05e29082..ef55b1a15c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -452,12 +452,15 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, } } - bdrv_flags |= BDRV_O_CACHE_WB; - if ((buf = qemu_opt_get(opts, "cache")) != NULL) { - if (bdrv_parse_cache_flags(buf, &bdrv_flags) != 0) { - error_report("invalid cache option"); - return NULL; - } + bdrv_flags = 0; + if (qemu_opt_get_bool(opts, "cache.writeback", true)) { + bdrv_flags |= BDRV_O_CACHE_WB; + } + if (qemu_opt_get_bool(opts, "cache.direct", false)) { + bdrv_flags |= BDRV_O_NOCACHE; + } + if (qemu_opt_get_bool(opts, "cache.no-flush", true)) { + bdrv_flags |= BDRV_O_NO_FLUSH; } #ifdef CONFIG_LINUX_AIO @@ -740,6 +743,8 @@ static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to) DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) { + const char *value; + /* Change legacy command line options into QMP ones */ qemu_opt_rename(all_opts, "iops", "throttling.iops-total"); qemu_opt_rename(all_opts, "iops_rd", "throttling.iops-read"); @@ -751,6 +756,31 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) qemu_opt_rename(all_opts, "readonly", "read-only"); + value = qemu_opt_get(all_opts, "cache"); + if (value) { + int flags = 0; + + if (bdrv_parse_cache_flags(value, &flags) != 0) { + error_report("invalid cache option"); + return NULL; + } + + /* Specific options take precedence */ + if (!qemu_opt_get(all_opts, "cache.writeback")) { + qemu_opt_set_bool(all_opts, "cache.writeback", + !!(flags & BDRV_O_CACHE_WB)); + } + if (!qemu_opt_get(all_opts, "cache.direct")) { + qemu_opt_set_bool(all_opts, "cache.direct", + !!(flags & BDRV_O_NOCACHE)); + } + if (!qemu_opt_get(all_opts, "cache.no-flush")) { + qemu_opt_set_bool(all_opts, "cache.no-flush", + !!(flags & BDRV_O_NO_FLUSH)); + } + qemu_opt_unset(all_opts, "cache"); + } + return blockdev_init(all_opts, block_default_type); } @@ -1850,10 +1880,17 @@ QemuOptsList qemu_common_drive_opts = { .type = QEMU_OPT_STRING, .help = "discard operation (ignore/off, unmap/on)", },{ - .name = "cache", - .type = QEMU_OPT_STRING, - .help = "host cache usage (none, writeback, writethrough, " - "directsync, unsafe)", + .name = "cache.writeback", + .type = QEMU_OPT_BOOL, + .help = "enables writeback mode for any caches", + },{ + .name = "cache.direct", + .type = QEMU_OPT_BOOL, + .help = "enables use of O_DIRECT (bypass the host page cache)", + },{ + .name = "cache.no-flush", + .type = QEMU_OPT_BOOL, + .help = "ignore any flush requests for the device", },{ .name = "aio", .type = QEMU_OPT_STRING, From f660dc6a2e97756596b2e79ce6127a3034f2308b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 8 Jul 2013 17:11:58 +0200 Subject: [PATCH 16/18] Implement qdict_flatten() qdict_flatten(): For each nested QDict with key x, all fields with key y are moved to this QDict and their key is renamed to "x.y". This operation is applied recursively for nested QDicts. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- include/qapi/qmp/qdict.h | 1 + qobject/qdict.c | 51 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h index 685b2e3fcb..d6855d112e 100644 --- a/include/qapi/qmp/qdict.h +++ b/include/qapi/qmp/qdict.h @@ -65,5 +65,6 @@ int qdict_get_try_bool(const QDict *qdict, const char *key, int def_value); const char *qdict_get_try_str(const QDict *qdict, const char *key); QDict *qdict_clone_shallow(const QDict *src); +void qdict_flatten(QDict *qdict); #endif /* QDICT_H */ diff --git a/qobject/qdict.c b/qobject/qdict.c index ed381f9a50..472f106e27 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -476,3 +476,54 @@ static void qdict_destroy_obj(QObject *obj) g_free(qdict); } + +static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix) +{ + QObject *value; + const QDictEntry *entry, *next; + const char *new_key; + bool delete; + + entry = qdict_first(qdict); + + while (entry != NULL) { + + next = qdict_next(qdict, entry); + value = qdict_entry_value(entry); + new_key = NULL; + delete = false; + + if (prefix) { + qobject_incref(value); + new_key = g_strdup_printf("%s.%s", prefix, entry->key); + qdict_put_obj(target, new_key, value); + delete = true; + } + + if (qobject_type(value) == QTYPE_QDICT) { + qdict_do_flatten(qobject_to_qdict(value), target, + new_key ? new_key : entry->key); + delete = true; + } + + if (delete) { + qdict_del(qdict, entry->key); + + /* Restart loop after modifying the iterated QDict */ + entry = qdict_first(qdict); + continue; + } + + entry = next; + } +} + +/** + * qdict_flatten(): For each nested QDict with key x, all fields with key y + * are moved to this QDict and their key is renamed to "x.y". This operation + * is applied recursively for nested QDicts. + */ +void qdict_flatten(QDict *qdict) +{ + qdict_do_flatten(qdict, qdict, NULL); +} From fc5d3f843250c9d3bfa2bcfdb7369f4753a49f0e Mon Sep 17 00:00:00 2001 From: Ian Main Date: Fri, 26 Jul 2013 11:39:04 -0700 Subject: [PATCH 17/18] Implement sync modes for drive-backup. This patch adds sync-modes to the drive-backup interface and implements the FULL, NONE and TOP modes of synchronization. FULL performs as before copying the entire contents of the drive while preserving the point-in-time using CoW. NONE only copies new writes to the target drive. TOP copies changes to the topmost drive image and preserves the point-in-time using CoW. For sync mode TOP are creating a new target image using the same backing file as the original disk image. Then any new data that has been laid on top of it since creation is copied in the main backup_run() loop. There is an extra check in the 'TOP' case so that we don't bother to copy all the data of the backing file as it already exists in the target. This is where the bdrv_co_is_allocated() is used to determine if the data exists in the topmost layer or below. Also any new data being written is intercepted via the write_notifier hook which ends up calling backup_do_cow() to copy old data out before it gets overwritten. For mode 'NONE' we create the new target image and only copy in the original data from the disk image starting from the time the call was made. This preserves the point in time data by only copying the parts that are *going to change* to the target image. This way we can reconstruct the final image by checking to see if the given block exists in the new target image first, and if it does not, you can get it from the original image. This is basically an optimization allowing you to do point-in-time snapshots with low overhead vs the 'FULL' version. Since there is no old data to copy out the loop in backup_run() for the NONE case just calls qemu_coroutine_yield() which only wakes up after an event (usually cancel in this case). The rest is handled by the before_write notifier which again calls backup_do_cow() to write out the old data so it can be preserved. Signed-off-by: Ian Main Signed-off-by: Kevin Wolf --- block/backup.c | 107 +++++++++++++++++++++++++++----------- blockdev.c | 29 ++++++++--- include/block/block_int.h | 4 +- qmp-commands.hx | 1 + 4 files changed, 102 insertions(+), 39 deletions(-) diff --git a/block/backup.c b/block/backup.c index 16105d40b1..6ae8a05a3e 100644 --- a/block/backup.c +++ b/block/backup.c @@ -37,6 +37,7 @@ typedef struct CowRequest { typedef struct BackupBlockJob { BlockJob common; BlockDriverState *target; + MirrorSyncMode sync_mode; RateLimit limit; BlockdevOnError on_source_error; BlockdevOnError on_target_error; @@ -247,40 +248,83 @@ static void coroutine_fn backup_run(void *opaque) bdrv_add_before_write_notifier(bs, &before_write); - for (; start < end; start++) { - bool error_is_read; - - if (block_job_is_cancelled(&job->common)) { - break; + if (job->sync_mode == MIRROR_SYNC_MODE_NONE) { + while (!block_job_is_cancelled(&job->common)) { + /* Yield until the job is cancelled. We just let our before_write + * notify callback service CoW requests. */ + job->common.busy = false; + qemu_coroutine_yield(); + job->common.busy = true; } + } else { + /* Both FULL and TOP SYNC_MODE's require copying.. */ + for (; start < end; start++) { + bool error_is_read; - /* we need to yield so that qemu_aio_flush() returns. - * (without, VM does not reboot) - */ - if (job->common.speed) { - uint64_t delay_ns = ratelimit_calculate_delay( - &job->limit, job->sectors_read); - job->sectors_read = 0; - block_job_sleep_ns(&job->common, rt_clock, delay_ns); - } else { - block_job_sleep_ns(&job->common, rt_clock, 0); - } - - if (block_job_is_cancelled(&job->common)) { - break; - } - - ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER, - BACKUP_SECTORS_PER_CLUSTER, &error_is_read); - if (ret < 0) { - /* Depending on error action, fail now or retry cluster */ - BlockErrorAction action = - backup_error_action(job, error_is_read, -ret); - if (action == BDRV_ACTION_REPORT) { + if (block_job_is_cancelled(&job->common)) { break; + } + + /* we need to yield so that qemu_aio_flush() returns. + * (without, VM does not reboot) + */ + if (job->common.speed) { + uint64_t delay_ns = ratelimit_calculate_delay( + &job->limit, job->sectors_read); + job->sectors_read = 0; + block_job_sleep_ns(&job->common, rt_clock, delay_ns); } else { - start--; - continue; + block_job_sleep_ns(&job->common, rt_clock, 0); + } + + if (block_job_is_cancelled(&job->common)) { + break; + } + + if (job->sync_mode == MIRROR_SYNC_MODE_TOP) { + int i, n; + int alloced = 0; + + /* Check to see if these blocks are already in the + * backing file. */ + + for (i = 0; i < BACKUP_SECTORS_PER_CLUSTER;) { + /* bdrv_co_is_allocated() only returns true/false based + * on the first set of sectors it comes accross that + * are are all in the same state. + * For that reason we must verify each sector in the + * backup cluster length. We end up copying more than + * needed but at some point that is always the case. */ + alloced = + bdrv_co_is_allocated(bs, + start * BACKUP_SECTORS_PER_CLUSTER + i, + BACKUP_SECTORS_PER_CLUSTER - i, &n); + i += n; + + if (alloced == 1) { + break; + } + } + + /* If the above loop never found any sectors that are in + * the topmost image, skip this backup. */ + if (alloced == 0) { + continue; + } + } + /* FULL sync mode we copy the whole drive. */ + ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER, + BACKUP_SECTORS_PER_CLUSTER, &error_is_read); + if (ret < 0) { + /* Depending on error action, fail now or retry cluster */ + BlockErrorAction action = + backup_error_action(job, error_is_read, -ret); + if (action == BDRV_ACTION_REPORT) { + break; + } else { + start--; + continue; + } } } } @@ -300,7 +344,7 @@ static void coroutine_fn backup_run(void *opaque) } void backup_start(BlockDriverState *bs, BlockDriverState *target, - int64_t speed, + int64_t speed, MirrorSyncMode sync_mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, BlockDriverCompletionFunc *cb, void *opaque, @@ -335,6 +379,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, job->on_source_error = on_source_error; job->on_target_error = on_target_error; job->target = target; + job->sync_mode = sync_mode; job->common.len = len; job->common.co = qemu_coroutine_create(backup_run); qemu_coroutine_enter(job->common.co, job); diff --git a/blockdev.c b/blockdev.c index ef55b1a15c..4534864802 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1489,16 +1489,13 @@ void qmp_drive_backup(const char *device, const char *target, { BlockDriverState *bs; BlockDriverState *target_bs; + BlockDriverState *source = NULL; BlockDriver *drv = NULL; Error *local_err = NULL; int flags; int64_t size; int ret; - if (sync != MIRROR_SYNC_MODE_FULL) { - error_setg(errp, "only sync mode 'full' is currently supported"); - return; - } if (!has_speed) { speed = 0; } @@ -1541,6 +1538,18 @@ void qmp_drive_backup(const char *device, const char *target, flags = bs->open_flags | BDRV_O_RDWR; + /* See if we have a backing HD we can use to create our new image + * on top of. */ + if (sync == MIRROR_SYNC_MODE_TOP) { + source = bs->backing_hd; + if (!source) { + sync = MIRROR_SYNC_MODE_FULL; + } + } + if (sync == MIRROR_SYNC_MODE_NONE) { + source = bs; + } + size = bdrv_getlength(bs); if (size < 0) { error_setg_errno(errp, -size, "bdrv_getlength failed"); @@ -1549,8 +1558,14 @@ void qmp_drive_backup(const char *device, const char *target, if (mode != NEW_IMAGE_MODE_EXISTING) { assert(format && drv); - bdrv_img_create(target, format, - NULL, NULL, NULL, size, flags, &local_err, false); + if (source) { + bdrv_img_create(target, format, source->filename, + source->drv->format_name, NULL, + size, flags, &local_err, false); + } else { + bdrv_img_create(target, format, NULL, NULL, NULL, + size, flags, &local_err, false); + } } if (error_is_set(&local_err)) { @@ -1566,7 +1581,7 @@ void qmp_drive_backup(const char *device, const char *target, return; } - backup_start(bs, target_bs, speed, on_source_error, on_target_error, + backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error, block_job_cb, bs, &local_err); if (local_err != NULL) { bdrv_delete(target_bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index c6ac871e21..e45f2a0d56 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -404,6 +404,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, * @bs: Block device to operate on. * @target: Block device to write to. * @speed: The maximum speed, in bytes per second, or 0 for unlimited. + * @sync_mode: What parts of the disk image should be copied to the destination. * @on_source_error: The action to take upon error reading from the source. * @on_target_error: The action to take upon error writing to the target. * @cb: Completion function for the job. @@ -413,7 +414,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, * until the job is cancelled or manually completed. */ void backup_start(BlockDriverState *bs, BlockDriverState *target, - int64_t speed, BlockdevOnError on_source_error, + int64_t speed, MirrorSyncMode sync_mode, + BlockdevOnError on_source_error, BlockdevOnError on_target_error, BlockDriverCompletionFunc *cb, void *opaque, Error **errp); diff --git a/qmp-commands.hx b/qmp-commands.hx index 65a9e26423..2e59b0d218 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -960,6 +960,7 @@ Arguments: Example: -> { "execute": "drive-backup", "arguments": { "device": "drive0", + "sync": "full", "target": "backup.img" } } <- { "return": {} } EQMP From e3409362bd64731e042c9d001e43cc1d13d2df5d Mon Sep 17 00:00:00 2001 From: Ian Main Date: Fri, 26 Jul 2013 11:39:05 -0700 Subject: [PATCH 18/18] Add tests for sync modes 'TOP' and 'NONE' This patch adds tests for sync modes top and none. Test for 'TOP' is separated out as it requires a backing file. Also added a test for invalid format. Signed-off-by: Ian Main Signed-off-by: Kevin Wolf --- tests/qemu-iotests/055 | 6 +++ tests/qemu-iotests/055.out | 4 +- tests/qemu-iotests/056 | 94 +++++++++++++++++++++++++++++++++++ tests/qemu-iotests/056.out | 5 ++ tests/qemu-iotests/group | 1 + tests/qemu-iotests/iotests.py | 5 ++ 6 files changed, 113 insertions(+), 2 deletions(-) create mode 100755 tests/qemu-iotests/056 create mode 100644 tests/qemu-iotests/056.out diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 index c66f8dbd7d..44bb025687 100755 --- a/tests/qemu-iotests/055 +++ b/tests/qemu-iotests/055 @@ -97,6 +97,12 @@ class TestSingleDrive(iotests.QMPTestCase): target=target_img, sync='full', mode='existing') self.assert_qmp(result, 'error/class', 'GenericError') + def test_invalid_format(self): + result = self.vm.qmp('drive-backup', device='drive0', + target=target_img, sync='full', + format='spaghetti-noodles') + self.assert_qmp(result, 'error/class', 'GenericError') + def test_device_not_found(self): result = self.vm.qmp('drive-backup', device='nonexistent', target=target_img, sync='full') diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/055.out index fa16b5ccef..6323079e08 100644 --- a/tests/qemu-iotests/055.out +++ b/tests/qemu-iotests/055.out @@ -1,5 +1,5 @@ -............. +.............. ---------------------------------------------------------------------- -Ran 13 tests +Ran 14 tests OK diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056 new file mode 100755 index 0000000000..63893423cf --- /dev/null +++ b/tests/qemu-iotests/056 @@ -0,0 +1,94 @@ +#!/usr/bin/env python +# +# Tests for drive-backup +# +# Copyright (C) 2013 Red Hat, Inc. +# +# Based on 041. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import time +import os +import iotests +from iotests import qemu_img, qemu_io, create_image + +backing_img = os.path.join(iotests.test_dir, 'backing.img') +test_img = os.path.join(iotests.test_dir, 'test.img') +target_img = os.path.join(iotests.test_dir, 'target.img') + +class TestSyncModesNoneAndTop(iotests.QMPTestCase): + image_len = 64 * 1024 * 1024 # MB + + def setUp(self): + create_image(backing_img, TestSyncModesNoneAndTop.image_len) + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img) + qemu_io('-c', 'write -P0x41 0 512', test_img) + qemu_io('-c', 'write -P0xd5 1M 32k', test_img) + qemu_io('-c', 'write -P0xdc 32M 124k', test_img) + qemu_io('-c', 'write -P0xdc 67043328 64k', test_img) + self.vm = iotests.VM().add_drive(test_img) + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + os.remove(test_img) + os.remove(backing_img) + try: + os.remove(target_img) + except OSError: + pass + + def test_complete_top(self): + self.assert_no_active_block_jobs() + result = self.vm.qmp('drive-backup', device='drive0', sync='top', + format=iotests.imgfmt, target=target_img) + self.assert_qmp(result, 'return', {}) + + # Custom completed check as we are not copying all data. + completed = False + while not completed: + for event in self.vm.get_qmp_events(wait=True): + if event['event'] == 'BLOCK_JOB_COMPLETED': + self.assert_qmp(event, 'data/device', 'drive0') + self.assert_qmp_absent(event, 'data/error') + completed = True + + self.assert_no_active_block_jobs() + self.vm.shutdown() + self.assertTrue(iotests.compare_images(test_img, target_img), + 'target image does not match source after backup') + + def test_cancel_sync_none(self): + self.assert_no_active_block_jobs() + + result = self.vm.qmp('drive-backup', device='drive0', + sync='none', target=target_img) + self.assert_qmp(result, 'return', {}) + time.sleep(1) + self.vm.hmp_qemu_io('drive0', 'write -P0x5e 0 512') + self.vm.hmp_qemu_io('drive0', 'aio_flush') + # Verify that the original contents exist in the target image. + + event = self.cancel_and_wait() + self.assert_qmp(event, 'data/type', 'backup') + + self.vm.shutdown() + time.sleep(1) + self.assertEqual(-1, qemu_io('-c', 'read -P0x41 0 512', target_img).find("verification failed")) + + +if __name__ == '__main__': + iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/056.out b/tests/qemu-iotests/056.out new file mode 100644 index 0000000000..fbc63e62f8 --- /dev/null +++ b/tests/qemu-iotests/056.out @@ -0,0 +1,5 @@ +.. +---------------------------------------------------------------------- +Ran 2 tests + +OK diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index fdc6ed14ca..b1d03c76a4 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -62,3 +62,4 @@ 053 rw auto 054 rw auto 055 rw auto +056 rw auto backing diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index b028a890e6..33ad0ecb92 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -95,6 +95,11 @@ class VM(object): self._num_drives += 1 return self + def hmp_qemu_io(self, drive, cmd): + '''Write to a given drive using an HMP command''' + return self.qmp('human-monitor-command', + command_line='qemu-io %s "%s"' % (drive, cmd)) + def add_fd(self, fd, fdset, opaque, opts=''): '''Pass a file descriptor to the VM''' options = ['fd=%d' % fd,