* [Qemu-devel] [PULL 01/18] qapi-types.py: Implement 'base' for unions
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 02/18] qapi-visit.py: Split off generate_visit_struct_fields() Kevin Wolf
` (16 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 ddcfed9..07bd311 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'])
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 02/18] qapi-visit.py: Split off generate_visit_struct_fields()
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 01/18] qapi-types.py: Implement 'base' for unions Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 03/18] qapi-visit.py: Implement 'base' for unions Kevin Wolf
` (15 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 6cac05a..a337d80 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()
-
- 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)
+def generate_visit_struct_fields(field_prefix, members):
+ ret = ''
- 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);
''')
- pop_indent()
+ 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;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 03/18] qapi-visit.py: Implement 'base' for unions
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 01/18] qapi-types.py: Implement 'base' for unions Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 02/18] qapi-visit.py: Split off generate_visit_struct_fields() Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 04/18] docs: Document QAPI union types Kevin Wolf
` (14 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 a337d80..db6fa44 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)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 04/18] docs: Document QAPI union types
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (2 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 03/18] qapi-visit.py: Implement 'base' for unions Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 05/18] qapi: Add visitor for implicit structs Kevin Wolf
` (13 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 cccb11e..f6f8d33 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 ==
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 05/18] qapi: Add visitor for implicit structs
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (3 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 04/18] docs: Document QAPI union types Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 06/18] qapi: Flat unions with arbitrary discriminator Kevin Wolf
` (12 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 5159964..5c1297f 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 28c21d8..bd24f85 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 401ee6e..9b4d51b 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 67fb127..59c5cac 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;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 06/18] qapi: Flat unions with arbitrary discriminator
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (4 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 05/18] qapi: Add visitor for implicit structs Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 07/18] qapi: Add consume argument to qmp_input_get_object() Kevin Wolf
` (11 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 f6f8d33..11f19cf 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 07bd311..84d46fb 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 db6fa44..87d5f15 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,20 +103,14 @@ visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
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;
-}
-''')
+ visit_type_%(name)s_fields(m, obj, &err);
+ error_propagate(errp, err);
+ err = NULL;
+ }
+''',
+ name=full_name)
pop_indent()
- pop_indent()
ret += mcgen('''
/* Always call end_struct if start_struct succeeded. */
visit_end_struct(m, &err);
@@ -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(),
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 07/18] qapi: Add consume argument to qmp_input_get_object()
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (5 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 06/18] qapi: Flat unions with arbitrary discriminator Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 08/18] qapi.py: Maintain a list of union types Kevin Wolf
` (10 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
This allows to just look at the next element without actually consuming
it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 59c5cac..70864a1 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;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 08/18] qapi.py: Maintain a list of union types
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (6 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 07/18] qapi: Add consume argument to qmp_input_get_object() Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 09/18] qapi: Anonymous unions Kevin Wolf
` (9 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi.py | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index baf1321..3a54c7f 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)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 09/18] qapi: Anonymous unions
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (7 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 08/18] qapi.py: Maintain a list of union types Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 10/18] block: Allow "driver" option on the top level Kevin Wolf
` (8 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 11f19cf..0ce045c 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 9124649..d0bbc7c 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 5c1297f..f3fa420 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 bd24f85..48a2a2e 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 <stdlib.h>
@@ -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 9b4d51b..d6a4012 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 70864a1..bf42c04 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 19085a1..6cf2511 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 84d46fb..5ee46ea 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 87d5f15..597cca4 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 3a54c7f..38c808e 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]
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 10/18] block: Allow "driver" option on the top level
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (8 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 09/18] qapi: Anonymous unions Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 11/18] QemuOpts: Add qemu_opt_unset() Kevin Wolf
` (7 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 7 +++++++
blockdev.c | 20 ++++++++++----------
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index 6cd39fa..c77cfd1 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 c5abd65..8ff8ed3 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;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 11/18] QemuOpts: Add qemu_opt_unset()
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (9 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 10/18] block: Allow "driver" option on the top level Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 12/18] blockdev: Rename I/O throttling options for QMP Kevin Wolf
` (6 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 a83c700..13f5e72 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 e0ef426..5d686c8 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)
{
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 12/18] blockdev: Rename I/O throttling options for QMP
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (10 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 11/18] QemuOpts: Add qemu_opt_unset() Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 13/18] qcow2: Use dashes instead of underscores in options Kevin Wolf
` (5 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
blockdev.c | 52 +++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 13 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 8ff8ed3..5403188 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",
},{
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 13/18] qcow2: Use dashes instead of underscores in options
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (11 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 12/18] blockdev: Rename I/O throttling options for QMP Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 14/18] blockdev: Rename 'readonly' option to 'read-only' Kevin Wolf
` (4 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 0eceefe..3376901 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 3b2d5cd..dba9771 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 1cf8bf7..1f39c6a 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 95ff245..c676538 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) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
-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) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
-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) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 14/18] blockdev: Rename 'readonly' option to 'read-only'
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (12 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 13/18] qcow2: Use dashes instead of underscores in options Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 15/18] blockdev: Split up 'cache' option Kevin Wolf
` (3 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
Option name cleanup before it becomes a QMP API.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 5403188..3b05e29 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 c676538..9588d0c 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) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
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
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 15/18] blockdev: Split up 'cache' option
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (13 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 14/18] blockdev: Rename 'readonly' option to 'read-only' Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 16/18] Implement qdict_flatten() Kevin Wolf
` (2 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
blockdev.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 47 insertions(+), 10 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 3b05e29..ef55b1a 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,
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 16/18] Implement qdict_flatten()
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (14 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 15/18] blockdev: Split up 'cache' option Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 17/18] Implement sync modes for drive-backup Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 18/18] Add tests for sync modes 'TOP' and 'NONE' Kevin Wolf
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
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 <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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 685b2e3..d6855d1 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 ed381f9..472f106 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);
+}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 17/18] Implement sync modes for drive-backup.
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (15 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 16/18] Implement qdict_flatten() Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
2013-07-26 20:20 ` [Qemu-devel] [PULL 18/18] Add tests for sync modes 'TOP' and 'NONE' Kevin Wolf
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Ian Main <imain@redhat.com>
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 <imain@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/backup.c | 105 +++++++++++++++++++++++++++++++++-------------
blockdev.c | 29 +++++++++----
include/block/block_int.h | 4 +-
qmp-commands.hx | 1 +
4 files changed, 101 insertions(+), 38 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 16105d4..6ae8a05 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;
+ }
- 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 {
+ block_job_sleep_ns(&job->common, rt_clock, 0);
+ }
- 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;
- } else {
- start--;
- continue;
+ }
+
+ 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 ef55b1a..4534864 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 c6ac871..e45f2a0 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 65a9e26..2e59b0d 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
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 18/18] Add tests for sync modes 'TOP' and 'NONE'
2013-07-26 20:20 [Qemu-devel] [PULL 00/18] Block patches Kevin Wolf
` (16 preceding siblings ...)
2013-07-26 20:20 ` [Qemu-devel] [PULL 17/18] Implement sync modes for drive-backup Kevin Wolf
@ 2013-07-26 20:20 ` Kevin Wolf
17 siblings, 0 replies; 31+ messages in thread
From: Kevin Wolf @ 2013-07-26 20:20 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Ian Main <imain@redhat.com>
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 <imain@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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 c66f8db..44bb025 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 fa16b5c..6323079 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 0000000..6389342
--- /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 <http://www.gnu.org/licenses/>.
+#
+
+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 0000000..fbc63e6
--- /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 fdc6ed1..b1d03c7 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 b028a89..33ad0ec 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,
--
1.8.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread