[Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion
Currently, when parsing a stream of tokens we make a copy of the token
list at the beginning of each level of recursion so that we do not
modify the original list in cases where we need to fall back to an
earlier state.
In the worst case, we will only read 1 or 2 tokens off the list before
recursing again, which means an upper bound of roughly N^2 token allocations.
For a "reasonably" sized QMP request (in this a QMP representation of
cirrus_vga's device state, generated via QIDL, being passed in via
qom-set), this caused my 16GB's of memory to be exhausted before any
noticeable progress was made by the parser. The command is here for
reference, and can be issued against upstream QMP to reproduce (failure
occurs before any qmp command routing/execution):
http://pastebin.com/mJrZ3Ctg
This patch works around the issue by using single copy of the token list
in the form of an indexable array so that we can save/restore state by
manipulating indices. With this patch applied the above QMP/JSON request
can be parsed in under a second.
Tested with valgrind, make check, and QMP.
Signed-off-by: Michael Roth
---
json-parser.c | 234 +++--
1 file changed, 146 insertions(+), 88 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 849e215..b4e6a60 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -27,6 +27,11 @@
typedef struct JSONParserContext
{
Error *err;
+struct {
+QObject **buf;
+size_t pos;
+size_t count;
+} tokens;
} JSONParserContext;
#define BUG_ON(cond) assert(!(cond))
@@ -40,7 +45,7 @@ typedef struct JSONParserContext
* 4) deal with premature EOI
*/
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list
*ap);
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
/**
* Token manipulators
@@ -270,27 +275,115 @@ out:
return NULL;
}
+static QObject *parser_context_pop_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+ctxt->tokens.pos++;
+return token;
+}
+
+/* Note: parser_context_{peek|pop}_token do not increment the
+ * token object's refcount. In both cases the references will continue
+ * to be * tracked and cleanup in parser_context_free()
+ */
+static QObject *parser_context_peek_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+return token;
+}
+
+static JSONParserContext parser_context_save(JSONParserContext *ctxt)
+{
+JSONParserContext saved_ctxt;
+saved_ctxt.tokens.pos = ctxt->tokens.pos;
+saved_ctxt.tokens.count = ctxt->tokens.count;
+saved_ctxt.tokens.buf = ctxt->tokens.buf;
+return saved_ctxt;
+}
+
+static void parser_context_restore(JSONParserContext *ctxt,
+ JSONParserContext saved_ctxt)
+{
+ctxt->tokens.pos = saved_ctxt.tokens.pos;
+ctxt->tokens.count = saved_ctxt.tokens.count;
+ctxt->tokens.buf = saved_ctxt.tokens.buf;
+}
+
+static void tokens_count_from_iter(QObject *obj, void *opaque)
+{
+size_t *count = opaque;
+(*count)++;
+}
+
+static void tokens_append_from_iter(QObject *obj, void *opaque)
+{
+JSONParserContext *ctxt = opaque;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ctxt->tokens.buf[ctxt->tokens.pos++] = obj;
+qobject_incref(obj);
+}
+
+static JSONParserContext *parser_context_new(QList *tokens)
+{
+JSONParserContext *ctxt;
+size_t count = 0;
+
+if (!tokens) {
+return NULL;
+}
+
+qlist_iter(tokens, tokens_count_from_iter, &count);
+if (count == 0) {
+return NULL;
+}
+
+ctxt = g_malloc0(sizeof(JSONParserContext));
+ctxt->tokens.pos = 0;
+ctxt->tokens.count = count;
+ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
+qlist_iter(tokens, tokens_append_from_iter, ctxt);
+ctxt->tokens.pos = 0;
+
+return ctxt;
+}
+
+static void parser_context_free(JSONParserContext *ctxt)
+{
+int i;
+if (ctxt) {
+for (i = 0; i < ctxt->tokens.count; i++) {
+qobject_decref(ctxt->tokens.buf[i]);
+}
+g_free(ctxt->tokens.buf);
+g_free(ctxt);
+}
+}
+
/**
* Parsing rules
*/
-static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens,
va_list *ap)
+static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
{
QObject *key = NULL, *token = NULL, *value, *peek;
-QList *working = qlist_copy(*tokens);
+JSONParserContext saved_ctxt = parser_context_save(ctxt);
-peek = qlist_peek(working);
+peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
g
Re: [Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion
On Mon, Aug 13, 2012 at 03:01:56PM -0300, Luiz Capitulino wrote:
> On Fri, 10 Aug 2012 18:24:10 -0500
> Michael Roth wrote:
>
> > Currently, when parsing a stream of tokens we make a copy of the token
> > list at the beginning of each level of recursion so that we do not
> > modify the original list in cases where we need to fall back to an
> > earlier state.
> >
> > In the worst case, we will only read 1 or 2 tokens off the list before
> > recursing again, which means an upper bound of roughly N^2 token
> > allocations.
> >
> > For a "reasonably" sized QMP request (in this a QMP representation of
> > cirrus_vga's device state, generated via QIDL, being passed in via
> > qom-set), this caused my 16GB's of memory to be exhausted before any
> > noticeable progress was made by the parser. The command is here for
> > reference, and can be issued against upstream QMP to reproduce (failure
> > occurs before any qmp command routing/execution):
> >
> > http://pastebin.com/mJrZ3Ctg
> >
> > This patch works around the issue by using single copy of the token list
> > in the form of an indexable array so that we can save/restore state by
> > manipulating indices. With this patch applied the above QMP/JSON request
> > can be parsed in under a second.
>
> Approach looks sane to me, a few review comments below.
>
> Anthony, could you provide your reviewed-by please?
>
> >
> > Tested with valgrind, make check, and QMP.
> >
> > Signed-off-by: Michael Roth
> > ---
> > json-parser.c | 234
> > +++--
> > 1 file changed, 146 insertions(+), 88 deletions(-)
> >
> > diff --git a/json-parser.c b/json-parser.c
> > index 849e215..b4e6a60 100644
> > --- a/json-parser.c
> > +++ b/json-parser.c
> > @@ -27,6 +27,11 @@
> > typedef struct JSONParserContext
> > {
> > Error *err;
> > +struct {
> > +QObject **buf;
> > +size_t pos;
> > +size_t count;
> > +} tokens;
> > } JSONParserContext;
> >
> > #define BUG_ON(cond) assert(!(cond))
> > @@ -40,7 +45,7 @@ typedef struct JSONParserContext
> > * 4) deal with premature EOI
> > */
> >
> > -static QObject *parse_value(JSONParserContext *ctxt, QList **tokens,
> > va_list *ap);
> > +static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
> >
> > /**
> > * Token manipulators
> > @@ -270,27 +275,115 @@ out:
> > return NULL;
> > }
> >
> > +static QObject *parser_context_pop_token(JSONParserContext *ctxt)
> > +{
> > +QObject *token;
> > +g_assert(ctxt->tokens.pos < ctxt->tokens.count);
> > +token = ctxt->tokens.buf[ctxt->tokens.pos];
> > +ctxt->tokens.pos++;
> > +return token;
> > +}
> > +
> > +/* Note: parser_context_{peek|pop}_token do not increment the
> > + * token object's refcount. In both cases the references will continue
> > + * to be * tracked and cleanup in parser_context_free()
> > + */
> > +static QObject *parser_context_peek_token(JSONParserContext *ctxt)
> > +{
> > +QObject *token;
> > +g_assert(ctxt->tokens.pos < ctxt->tokens.count);
> > +token = ctxt->tokens.buf[ctxt->tokens.pos];
> > +return token;
> > +}
> > +
> > +static JSONParserContext parser_context_save(JSONParserContext *ctxt)
> > +{
> > +JSONParserContext saved_ctxt;
> > +saved_ctxt.tokens.pos = ctxt->tokens.pos;
> > +saved_ctxt.tokens.count = ctxt->tokens.count;
> > +saved_ctxt.tokens.buf = ctxt->tokens.buf;
>
> Shouldn't saved_ctxt.err be initialized to NULL?
>
Yes.
> > +return saved_ctxt;
>
> I was going to say that saved_ctxt had to be static, but then I realized
> this will end up copying the struct to the caller.
>
Yah, every caller needs to manage it's own context, so I went with
passing by copy since allocating/deallocating these "half-contexts" (we
only care about token state with these) seems unecessary.
seemed
> > +}
> > +
> > +static void parser_context_restore(JSONParserContext *ctxt,
> > + JSONParserContext saved_ctxt)
> > +{
> > +ctxt->tokens.pos = saved_ctxt.tokens.pos;
> > +ctxt->tokens.count = saved_ctxt.tokens.count;
> > +ctxt->tokens.buf = saved_ctxt.tokens.buf;
> > +}
> > +
> > +static void tokens_count_from_iter(QObject *obj,
Re: [Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion
On Mon, Aug 13, 2012 at 08:49:26PM +0200, Markus Armbruster wrote: > Michael Roth writes: > > > Currently, when parsing a stream of tokens we make a copy of the token > > list at the beginning of each level of recursion so that we do not > > modify the original list in cases where we need to fall back to an > > earlier state. > > > > In the worst case, we will only read 1 or 2 tokens off the list before > > recursing again, which means an upper bound of roughly N^2 token > > allocations. > > > > For a "reasonably" sized QMP request (in this a QMP representation of > > cirrus_vga's device state, generated via QIDL, being passed in via > > qom-set), this caused my 16GB's of memory to be exhausted before any > > noticeable progress was made by the parser. The command is here for > > reference, and can be issued against upstream QMP to reproduce (failure > > occurs before any qmp command routing/execution): > > > > http://pastebin.com/mJrZ3Ctg > > Commit messages are forever, pastebins aren't. > > What about preserving your test case for eternity under tests/? We might be able to generate some json objects that cause the behavior and add them to check-qjson. > > [...] >
[Qemu-devel] Add infrastructure for supporting QIDL annotations
These patches are based are origin/master, and can also be obtained from: git://github.com/mdroth/qemu.git qidl-base This is a cleanup of the infrastructure bits from "[RFC v2] Use QEMU IDL for device serialization/introspection". I know this is pretty late into the release cycle, but the patches in this series have gotten decent review, and I have a seperate branch (qidl-conv1 in my github repo) that uses these bits to serialize i440fx, piix3/piix ide, usb, cirrus_vga, and a few others via QIDL. So there shouldn't be too much churn going forward. Changes since rfc v2: - Parser/Codegen fix-ups for cases encountered converting piix ide and usb. - Fixed license headers. - Stricter arg-checking for QIDL macros when passing to codegen. - Added asserts to detect mis-use of QIDL property/visitor interfaces. - Renamed QAPI visit_*_array interfaces to visit_*_carray to clarify that these are serialization routines for single-dimension C arrays.
[Qemu-devel] [PATCH 11/20] qapi: qapi.py, make json parser more robust
Currently the QAPI JSON parser expects a very particular style of code
indentation, the major one being that terminating curly/square brackets are
not on placed on a seperate line. This is incompatible with most
pretty-print formats, so make it a little more robust by supporting
these cases.
Signed-off-by: Michael Roth
---
scripts/qapi.py |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a347203..47cd672 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -81,7 +81,7 @@ def parse_schema(fp):
if line.startswith('#') or line == '\n':
continue
-if line.startswith(' '):
+if line[0] in ['}', ']', ' ', '\t']:
expr += line
elif expr:
expr_eval = evaluate(expr)
--
1.7.9.5
[Qemu-devel] [PATCH 13/20] qom-fuse: workaround for truncated properties > 4096
We currently hard-code property size at 4096 for the purposes of
getattr()/stat()/etc. For 'state' properties we can exceed this easily,
leading to truncated responses.
Instead, for a particular property, make it
max(4096, most_recent_property_size * 2). This allows some
head-room for properties that change size periodically (numbers,
strings, state properties containing arrays, etc)
Also, implement a simple property cache to avoid spinning on qom-get
if an application reads beyond the actual size. This also allows us
to use a snapshot of a single qom-get that persists across read()'s.
Old Cache entries are evicted as soon as we attempt to read() from
offset 0 again.
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
QMP/qom-fuse | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/QMP/qom-fuse b/QMP/qom-fuse
index 5c6754a..bd43f29 100755
--- a/QMP/qom-fuse
+++ b/QMP/qom-fuse
@@ -26,6 +26,7 @@ class QOMFS(Fuse):
self.qmp.connect()
self.ino_map = {}
self.ino_count = 1
+self.prop_cache = {}
def get_ino(self, path):
if self.ino_map.has_key(path):
@@ -67,12 +68,16 @@ class QOMFS(Fuse):
if not self.is_property(path):
return -ENOENT
-path, prop = path.rsplit('/', 1)
-try:
-data = str(self.qmp.command('qom-get', path=path, property=prop))
-data += '\n' # make values shell friendly
-except:
-return -EPERM
+# avoid extra calls to qom-get by using cached value when offset > 0
+if offset == 0 or not self.prop_cache.has_key(path):
+directory, prop = path.rsplit('/', 1)
+try:
+resp = str(self.qmp.command('qom-get', path=directory,
property=prop))
+self.prop_cache[path] = resp + '\n' # make values shell
friendly
+except:
+return -EPERM
+
+data = self.prop_cache[path]
if offset > len(data):
return ''
@@ -111,13 +116,18 @@ class QOMFS(Fuse):
0,
0))
elif self.is_property(path):
+directory, prop = path.rsplit('/', 1)
+try:
+resp = str(self.qmp.command('qom-get', path=directory,
property=prop))
+except:
+return -ENOENT
value = posix.stat_result((0644 | stat.S_IFREG,
self.get_ino(path),
0,
1,
1000,
1000,
- 4096,
+ max(len(resp) * 2, 4096),
0,
0,
0))
--
1.7.9.5
[Qemu-devel] [PATCH 05/20] qapi: qapi_visit.py, support arrays and complex qapi definitions
Add support for arrays in the code generators.
Complex field descriptions can now be used to provide additional
information to the visitor generators, such as the max size of an array,
or the field within a struct to use to determine how many elements are
present in the array to avoid serializing uninitialized elements.
Add handling for these in the code generators as well.
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
scripts/qapi.py |8 --
scripts/qapi_commands.py |8 +++---
scripts/qapi_types.py|2 +-
scripts/qapi_visit.py| 64 +-
4 files changed, 68 insertions(+), 14 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 122b4cb..a347203 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -110,12 +110,16 @@ def parse_args(typeinfo):
argentry = typeinfo[member]
optional = False
structured = False
+annotated = False
if member.startswith('*'):
argname = member[1:]
optional = True
if isinstance(argentry, OrderedDict):
-structured = True
-yield (argname, argentry, optional, structured)
+if argentry.has_key(''):
+annotated = True
+else:
+structured = True
+yield (argname, argentry, optional, structured, annotated)
def de_camel_case(name):
new_name = ''
diff --git a/scripts/qapi_commands.py b/scripts/qapi_commands.py
index 3c4678d..a2e0c23 100644
--- a/scripts/qapi_commands.py
+++ b/scripts/qapi_commands.py
@@ -32,7 +32,7 @@ void %(visitor)s(Visitor *m, %(name)s * obj, const char
*name, Error **errp);
def generate_command_decl(name, args, ret_type):
arglist=""
-for argname, argtype, optional, structured in parse_args(args):
+for argname, argtype, optional, structured, annotated in parse_args(args):
argtype = c_type(argtype)
if argtype == "char *":
argtype = "const char *"
@@ -50,7 +50,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
retval=""
if ret_type:
retval = "retval = "
-for argname, argtype, optional, structured in parse_args(args):
+for argname, argtype, optional, structured, annotated in parse_args(args):
if optional:
arglist += "has_%s, " % c_var(argname)
arglist += "%s, " % (c_var(argname))
@@ -106,7 +106,7 @@ Visitor *v;
def gen_visitor_input_vars_decl(args):
ret = ""
push_indent()
-for argname, argtype, optional, structured in parse_args(args):
+for argname, argtype, optional, structured, annotated in parse_args(args):
if optional:
ret += mcgen('''
bool has_%(argname)s = false;
@@ -145,7 +145,7 @@ v = qmp_input_get_visitor(mi);
''',
obj=obj)
-for argname, argtype, optional, structured in parse_args(args):
+for argname, argtype, optional, structured, annotated in parse_args(args):
if optional:
ret += mcgen('''
visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp);
diff --git a/scripts/qapi_types.py b/scripts/qapi_types.py
index cf601ae..8babfe1 100644
--- a/scripts/qapi_types.py
+++ b/scripts/qapi_types.py
@@ -35,7 +35,7 @@ struct %(name)s
''',
name=structname)
-for argname, argentry, optional, structured in parse_args(members):
+for argname, argentry, optional, structured, annotated in
parse_args(members):
if optional:
ret += mcgen('''
bool has_%(c_name)s;
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index 25707f5..aac2172 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -16,6 +16,52 @@ import sys
import os
import getopt
import errno
+import types
+
+def generate_visit_carray_body(name, info):
+if info['array_size'][0].isdigit():
+array_size = info['array_size']
+elif info['array_size'][0] == '(' and info['array_size'][-1] == ')':
+array_size = info['array_size']
+else:
+array_size = "(*obj)->%s" % info['array_size']
+
+if info.has_key('array_capacity'):
+array_capacity = info['array_capacity']
+else:
+array_capacity = array_size
+
+if camel_case(de_camel_case(info['type'][0])) == info['type'][0]:
+ret = mcgen('''
+visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s,
sizeof(%(type)s), errp);
+int %(name)s_i;
+for (%(name)s_i = 0; %(name)s_i < %(array_size)s; %(name)s_i++) {
+%(type)s %(name)s_ptr = &(*obj)->%(name)s[%(name)s_i];
+visit_type_%(type_short)s(m, &%(na
[Qemu-devel] [PATCH 04/20] qapi: qapi_visit.py, make code useable as module
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
scripts/qapi_visit.py | 143 +
1 file changed, 74 insertions(+), 69 deletions(-)
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index 04ef7c4..25707f5 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -224,55 +224,57 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj,
const char *name, Error **e
''',
name=name)
-try:
-opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
- ["source", "header", "prefix=",
"output-dir="])
-except getopt.GetoptError, err:
-print str(err)
-sys.exit(1)
-
-output_dir = ""
-prefix = ""
-c_file = 'qapi-visit.c'
-h_file = 'qapi-visit.h'
-
-do_c = False
-do_h = False
-
-for o, a in opts:
-if o in ("-p", "--prefix"):
-prefix = a
-elif o in ("-o", "--output-dir"):
-output_dir = a + "/"
-elif o in ("-c", "--source"):
+def main(argv=[]):
+try:
+opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
+ ["source", "header", "prefix=",
+"output-dir="])
+except getopt.GetoptError, err:
+print str(err)
+sys.exit(1)
+
+output_dir = ""
+prefix = ""
+c_file = 'qapi-visit.c'
+h_file = 'qapi-visit.h'
+
+do_c = False
+do_h = False
+
+for o, a in opts:
+if o in ("-p", "--prefix"):
+prefix = a
+elif o in ("-o", "--output-dir"):
+output_dir = a + "/"
+elif o in ("-c", "--source"):
+do_c = True
+elif o in ("-h", "--header"):
+do_h = True
+
+if not do_c and not do_h:
do_c = True
-elif o in ("-h", "--header"):
do_h = True
-if not do_c and not do_h:
-do_c = True
-do_h = True
+c_file = output_dir + prefix + c_file
+h_file = output_dir + prefix + h_file
-c_file = output_dir + prefix + c_file
-h_file = output_dir + prefix + h_file
+try:
+os.makedirs(output_dir)
+except os.error, e:
+if e.errno != errno.EEXIST:
+raise
-try:
-os.makedirs(output_dir)
-except os.error, e:
-if e.errno != errno.EEXIST:
-raise
-
-def maybe_open(really, name, opt):
-if really:
-return open(name, opt)
-else:
-import StringIO
-return StringIO.StringIO()
+def maybe_open(really, name, opt):
+if really:
+return open(name, opt)
+else:
+import StringIO
+return StringIO.StringIO()
-fdef = maybe_open(do_c, c_file, 'w')
-fdecl = maybe_open(do_h, h_file, 'w')
+fdef = maybe_open(do_c, c_file, 'w')
+fdecl = maybe_open(do_h, h_file, 'w')
-fdef.write(mcgen('''
+fdef.write(mcgen('''
/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
/*
@@ -292,7 +294,7 @@ fdef.write(mcgen('''
''',
header=basename(h_file)))
-fdecl.write(mcgen('''
+fdecl.write(mcgen('''
/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
/*
@@ -316,37 +318,40 @@ fdecl.write(mcgen('''
''',
prefix=prefix, guard=guardname(h_file)))
-exprs = parse_schema(sys.stdin)
+exprs = parse_schema(sys.stdin)
-for expr in exprs:
-if expr.has_key('type'):
-ret = generate_visit_struct(expr['type'], expr['data'])
-ret += generate_visit_list(expr['type'], expr['data'])
-fdef.write(ret)
+for expr in exprs:
+if expr.has_key('type'):
+ret = generate_visit_struct(expr['type'], expr['data'])
+ret += generate_visit_list(expr['type'], expr['data'])
+fdef.write(ret)
-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_list(expr['union'], expr['data'])
-fdef.write(ret)
+ret = generate_declaration(expr['type'], expr['data'])
+fdecl.write(ret)
+elif expr.has_key('union'):
+ret = generate_visit_union(expr['union'], expr['data'])
+ret
[Qemu-devel] [PATCH 17/20] qidl: parser, initial import from qc.git
Signed-off-by: Michael Roth
---
scripts/qidl_parser.py | 512
1 file changed, 512 insertions(+)
create mode 100644 scripts/qidl_parser.py
diff --git a/scripts/qidl_parser.py b/scripts/qidl_parser.py
new file mode 100644
index 000..831b3f5
--- /dev/null
+++ b/scripts/qidl_parser.py
@@ -0,0 +1,512 @@
+#
+# QEMU IDL Parser
+#
+# Copyright IBM, Corp. 2012
+#
+# Authors:
+# Anthony Liguori
+# Michael Roth
+#
+# This work is licensed under the terms of the GNU GPLv2.
+# See the COPYING file in the top-level directory.
+#
+# The lexer code is based off of:
+# http://www.lysator.liu.se/c/ANSI-C-grammar-l.html
+
+import sys, json
+
+class Input(object):
+def __init__(self, text):
+self.fp = text
+self.buf = text
+self.eof = False
+
+def pop(self):
+if len(self.buf) == 0:
+self.eof = True
+return ''
+ch = self.buf[0]
+self.buf = self.buf[1:]
+return ch
+
+def in_range(ch, start, end):
+if ch >= start and ch <= end:
+return True
+return False
+
+# D[0-9]
+# L[a-zA-Z_]
+# H[a-fA-F0-9]
+# E[Ee][+-]?{D}+
+# FS (f|F|l|L)
+# IS (u|U|l|L)*
+
+def is_D(ch):
+return in_range(ch, '0', '9')
+
+def is_L(ch):
+return in_range(ch, 'a', 'z') or in_range(ch, 'A', 'Z') or ch == '_'
+
+def is_H(ch):
+return in_range(ch, 'a', 'f') or in_range(ch, 'A', 'F') or is_D(ch)
+
+def is_FS(ch):
+return ch in 'fFlL'
+
+def is_IS(ch):
+return ch in 'uUlL'
+
+def lexer(fp):
+ch = fp.pop()
+
+while not fp.eof:
+token = ''
+
+if is_L(ch):
+token += ch
+
+ch = fp.pop()
+while is_L(ch) or is_D(ch):
+token += ch
+ch = fp.pop()
+if token in [ 'auto', 'break', 'case', 'const', 'continue',
+ 'default', 'do', 'else', 'enum', 'extern',
+ 'for', 'goto', 'if', 'register', 'return', 'signed',
+ 'sizeof',
+ 'static', 'struct', 'typedef', 'union', 'unsigned',
+ 'volatile', 'while' ]:
+yield (token, token)
+else:
+yield ('symbol', token)
+elif ch == "'":
+token += ch
+
+ch = fp.pop()
+if ch == '\\':
+token += ch
+token += fp.pop()
+else:
+token += ch
+token += fp.pop()
+ch = fp.pop()
+yield ('literal', token)
+elif ch == '"':
+token += ch
+
+ch = fp.pop()
+while ch not in ['', '"']:
+token += ch
+if ch == '\\':
+token += fp.pop()
+ch = fp.pop()
+token += ch
+yield ('literal', token)
+ch = fp.pop()
+elif ch in '.><+-*/%&^|!;{},:=()[]~?':
+token += ch
+ch = fp.pop()
+tmp_token = token + ch
+if tmp_token in ['<:']:
+yield ('operator', '[')
+ch = fp.pop()
+elif tmp_token in [':>']:
+yield ('operator', ']')
+ch = fp.pop()
+elif tmp_token in ['<%']:
+yield ('operator', '{')
+ch = fp.pop()
+elif tmp_token in ['%>']:
+yield ('operator', '}')
+ch = fp.pop()
+elif tmp_token == '//':
+token = tmp_token
+ch = fp.pop()
+while ch != '\n' and ch != '':
+token += ch
+ch = fp.pop()
+yield ('comment', token)
+elif tmp_token == '/*':
+token = tmp_token
+
+ch = fp.pop()
+while True:
+while ch != '*':
+token += ch
+ch = fp.pop()
+token += ch
+ch = fp.pop()
+if ch == '/':
+to
[Qemu-devel] [PATCH 19/20] qidl: qidl.h, definitions for qidl annotations
Signed-off-by: Michael Roth
---
qidl.h | 63 +++
1 file changed, 63 insertions(+)
create mode 100644 qidl.h
diff --git a/qidl.h b/qidl.h
new file mode 100644
index 000..210f4c6
--- /dev/null
+++ b/qidl.h
@@ -0,0 +1,63 @@
+/*
+ * QEMU IDL Macros/stubs
+ *
+ * See docs/qidl.txt for usage information.
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Michael Roth
+ *
+ * This work is licensed under the terms of the GNU GPLv2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QIDL_H
+#define QIDL_H
+
+#include
+#include "qapi/qapi-visit-core.h"
+#include "qemu/object.h"
+#include "hw/qdev-properties.h"
+
+#ifdef QIDL_GEN
+
+/* we pass the code through the preprocessor with QIDL_GEN defined to parse
+ * structures as they'd appear after preprocessing, and use the following
+ * definitions mostly to re-insert the initial macros/annotations so they
+ * stick around for the parser to process
+ */
+#define QIDL(...) QIDL(__VA_ARGS__)
+#define QIDL_START(name, ...) QIDL_START(name, ##__VA_ARGS__)
+#define QIDL_END(name) QIDL_END(name)
+
+#define QIDL_VISIT_TYPE(name, v, s, f, e)
+#define QIDL_SCHEMA_ADD_LINK(name, obj, path, errp)
+#define QIDL_PROPERTIES(name)
+
+#else /* !QIDL_GEN */
+
+#define QIDL(...)
+#define QIDL_START(name, ...)
+#define QIDL_END(name) \
+static struct { \
+void (*visitor)(Visitor *, struct name **, const char *, Error **); \
+const char *schema_json_text; \
+Object *schema_obj; \
+Property *properties; \
+} qidl_data_##name;
+
+#define QIDL_VISIT_TYPE(name, v, s, f, e) \
+g_assert(qidl_data_##name.visitor); \
+qidl_data_##name.visitor(v, s, f, e)
+#define QIDL_SCHEMA_ADD_LINK(name, obj, path, errp) \
+g_assert(qidl_data_##name.schema_obj); \
+object_property_add_link(obj, path, "container", \
+ &qidl_data_##name.schema_obj, errp)
+#define QIDL_PROPERTIES(name) \
+qidl_data_##name.properties
+
+#endif /* QIDL_GEN */
+
+#endif
--
1.7.9.5
[Qemu-devel] [PATCH 12/20] qapi: add open-coded visitor for struct tm types
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
qapi/Makefile.objs |1 +
qapi/misc-qapi-visit.c | 14 ++
qapi/qapi-visit-core.h |3 +++
3 files changed, 18 insertions(+)
create mode 100644 qapi/misc-qapi-visit.c
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 5f5846e..7604b52 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -1,3 +1,4 @@
qapi-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qmp-input-visitor.o
qapi-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
qapi-obj-y += string-input-visitor.o string-output-visitor.o opts-visitor.o
+qapi-obj-y += misc-qapi-visit.o
diff --git a/qapi/misc-qapi-visit.c b/qapi/misc-qapi-visit.c
new file mode 100644
index 000..a44773d
--- /dev/null
+++ b/qapi/misc-qapi-visit.c
@@ -0,0 +1,14 @@
+#include
+#include "qidl.h"
+
+void visit_type_tm(Visitor *v, struct tm *obj, const char *name, Error **errp)
+{
+visit_start_struct(v, NULL, "struct tm", name, 0, errp);
+visit_type_int32(v, &obj->tm_year, "tm_year", errp);
+visit_type_int32(v, &obj->tm_mon, "tm_mon", errp);
+visit_type_int32(v, &obj->tm_mday, "tm_mday", errp);
+visit_type_int32(v, &obj->tm_hour, "tm_hour", errp);
+visit_type_int32(v, &obj->tm_min, "tm_min", errp);
+visit_type_int32(v, &obj->tm_sec, "tm_sec", errp);
+visit_end_struct(v, errp);
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 5eb1616..10ec044 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -100,4 +100,7 @@ void visit_start_carray(Visitor *v, void **obj, const char
*name,
void visit_next_carray(Visitor *v, Error **errp);
void visit_end_carray(Visitor *v, Error **errp);
+/* misc. visitors */
+void visit_type_tm(Visitor *m, struct tm *obj, const char *name, Error **errp);
+
#endif
--
1.7.9.5
[Qemu-devel] [PATCH 18/20] qidl: codegen, initial commit
Signed-off-by: Michael Roth
---
scripts/qidl.py | 282 +++
1 file changed, 282 insertions(+)
create mode 100644 scripts/qidl.py
diff --git a/scripts/qidl.py b/scripts/qidl.py
new file mode 100644
index 000..71c89bc
--- /dev/null
+++ b/scripts/qidl.py
@@ -0,0 +1,282 @@
+#
+# QIDL Code Generator
+#
+# Copyright IBM, Corp. 2012
+#
+# Authors:
+# Anthony Liguori
+# Michael Roth
+#
+# This work is licensed under the terms of the GNU GPLv2.
+# See the COPYING file in the top-level directory.
+
+from ordereddict import OrderedDict
+from qidl_parser import parse_file
+from qapi import parse_schema, mcgen, camel_case, de_camel_case
+from qapi_visit import generate_visit_struct, push_indent, pop_indent
+import sys
+import json
+import getopt
+import os
+import errno
+
+def qapi_schema(node):
+schema = OrderedDict()
+data = OrderedDict()
+fields = None
+if node.has_key('typedef'):
+schema['type'] = node['typedef']
+fields = node['type']['fields']
+elif node.has_key('struct'):
+schema['type'] = node['struct']
+fields = node['fields']
+else:
+raise Exception("top-level neither typedef nor struct")
+
+for field in fields:
+if filter(lambda x: field.has_key(x), \
+['is_derived', 'is_immutable', 'is_broken', 'is_function', \
+ 'is_nested_decl', 'is_elsewhere']):
+continue
+
+description = {}
+
+if field['type'].endswith('_t'):
+typename = field['type'][:-2]
+elif field['type'].startswith('struct '):
+typename = field['type'].split(" ")[1]
+elif field['type'].startswith('enum '):
+typename = 'int'
+elif field['type'] == "_Bool":
+typename = 'bool'
+elif field['type'].endswith("char") and field.has_key('is_pointer'):
+typename = 'str';
+elif field['type'] == 'int':
+typename = 'int32'
+elif field['type'] == 'unsigned int':
+typename = 'uint32'
+elif field['type'] == 'char':
+typename = 'uint8'
+else:
+typename = field['type']
+
+if field.has_key('is_array') and field['is_array']:
+description['type'] = [typename]
+description[''] = 'true'
+if field.has_key('array_size'):
+description['array_size'] = field['array_size']
+if field.has_key('array_capacity'):
+description['array_capacity'] = field['array_capacity']
+elif camel_case(de_camel_case(typename)) == typename and \
+(not field.has_key('is_pointer') or not field['is_pointer']):
+description[''] = 'true'
+description[''] = 'true'
+description['type'] = typename
+else:
+description = typename
+
+if field.has_key('is_optional') and field['is_optional']:
+data["*" + field['variable']] = description
+else:
+data[field['variable']] = description
+
+schema['data'] = data
+return schema
+
+
+def parse_schema_file(filename):
+return parse_schema(open(filename, 'r'))
+
+def write_file(output, filename):
+if filename:
+output_file = open(filename, 'w')
+else:
+output_file = sys.stdout
+output_file.write(output)
+if filename:
+output_file.close()
+
+def property_list(node):
+prop_list = []
+fields = None
+if node.has_key('typedef'):
+state = node['typedef']
+fields = node['type']['fields']
+elif node.has_key('struct'):
+state = node['struct']
+fields = node['fields']
+else:
+raise Exception("top-level neither typedef nor struct")
+
+for field in fields:
+if not field.has_key('is_property'):
+continue
+
+for arglist in field['property_fields']:
+if field['variable'] == 'devfn':
+typename = 'pci_devfn'
+elif field['type'].endswith('_t'):
+typename = field['type'][:-2]
+
[Qemu-devel] [PATCH 07/20] qapi: qapi_visit.py, support generating static functions
qidl embeds visitor code into object files rather than linking against
seperate files, so allow for static declarations when we're using
qapi_visit.py as a library as we do with qidl.py
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
scripts/qapi_visit.py | 51 -
1 file changed, 33 insertions(+), 18 deletions(-)
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index aac2172..d146013 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -141,13 +141,16 @@ visit_end_optional(m, &err);
''')
return ret
-def generate_visit_struct(name, members):
+def generate_visit_struct(name, members, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char
*name, Error **errp)
{
''',
-name=name)
+name=name, ret_type=ret_type)
push_indent()
ret += generate_visit_struct_body("", name, members)
@@ -158,10 +161,13 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj,
const char *name, Error **
''')
return ret
-def generate_visit_list(name, members):
+def generate_visit_list(name, members, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
return mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp)
+%(ret_type)s visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const
char *name, Error **errp)
{
GenericList *i, **prev = (GenericList **)obj;
Error *err = NULL;
@@ -183,19 +189,22 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
}
}
''',
-name=name)
+name=name, ret_type=ret_type)
-def generate_visit_enum(name, members):
+def generate_visit_enum(name, members, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp)
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name,
Error **errp)
{
visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp);
}
''',
- name=name)
+ name=name, ret_type=ret_type)
-def generate_visit_union(name, members):
+def generate_visit_union(name, members, static=False):
ret = generate_visit_enum('%sKind' % name, members.keys())
ret += mcgen('''
@@ -252,27 +261,33 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj,
const char *name, Error **
return ret
-def generate_declaration(name, members, genlist=True):
+def generate_declaration(name, members, genlist=True, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp);
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char
*name, Error **errp);
''',
-name=name)
+name=name, ret_type=ret_type)
if genlist:
ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp);
+%(ret_type)s visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const
char *name, Error **errp);
''',
- name=name)
+ name=name, ret_type=ret_type)
return ret
-def generate_decl_enum(name, members, genlist=True):
+def generate_decl_enum(name, members, genlist=True, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp);
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name,
Error **errp);
''',
-name=name)
+name=name, ret_type=ret_type)
def main(argv=[]):
try:
--
1.7.9.5
[Qemu-devel] [PATCH 20/20] qidl: unit tests
Signed-off-by: Michael Roth
---
Makefile |2 +
rules.mak | 15 +++-
tests/Makefile |8 +-
tests/test-qidl-included.h | 31
tests/test-qidl-linked.c | 91 +++
tests/test-qidl-linked.h | 18 +
tests/test-qidl.c | 177
7 files changed, 339 insertions(+), 3 deletions(-)
create mode 100644 tests/test-qidl-included.h
create mode 100644 tests/test-qidl-linked.c
create mode 100644 tests/test-qidl-linked.h
create mode 100644 tests/test-qidl.c
diff --git a/Makefile b/Makefile
index 7733e4c..f02251f 100644
--- a/Makefile
+++ b/Makefile
@@ -233,6 +233,7 @@ clean:
if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
rm -f $$d/qemu-options.def; \
done
+ find -depth -name qidl-generated -type d -exec rm -rf {} \;
VERSION ?= $(shell cat VERSION)
@@ -403,6 +404,7 @@ qemu-doc.dvi qemu-doc.html qemu-doc.info qemu-doc.pdf: \
# rebuilt before other object files
Makefile: $(GENERATED_HEADERS)
+
# Include automatically generated dependency files
# Dependencies in Makefile.objs files come from our recursive subdir rules
-include $(wildcard *.d tests/*.d)
diff --git a/rules.mak b/rules.mak
index a284946..48e7a95 100644
--- a/rules.mak
+++ b/rules.mak
@@ -15,7 +15,20 @@ MAKEFLAGS += -rR
QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
%.o: %.c
- $(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS)
$(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<," CC$(TARGET_DIR)$@")
+
+%.qidl.c: %.c $(SRC_PATH)/qidl.h $(addprefix $(SRC_PATH)/scripts/,qidl.py
qidl_parser.py qapi.py qapi_visit.py)
+ $(call rm -f $(*D)/qidl-generated/$(*F).qidl.c)
+ $(call quiet-command, $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(CFLAGS)
-E -c -DQIDL_GEN $< | \
+ $(PYTHON) $(SRC_PATH)/scripts/qidl.py
--output-filepath=$(*D)/qidl-generated/$(*F).qidl.c || [ "$$?" -eq 2 ])
+
+%.o: %.c %.qidl.c
+ $(if $(strip $(shell test -f $(*D)/qidl-generated/$(*F).qidl.c && echo
"true")), \
+ $(call quiet-command, \
+ $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
+ -include $< -o $@ $(*D)/qidl-generated/$(*F).qidl.c,"qidl CC
$@"), \
+ $(call quiet-command, \
+ $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
+ -o $@ $<," CC$@"))
ifeq ($(LIBTOOL),)
%.lo: %.c
diff --git a/tests/Makefile b/tests/Makefile
index ec6a050..a387643 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -15,6 +15,7 @@ check-unit-y += tests/test-string-output-visitor$(EXESUF)
check-unit-y += tests/test-coroutine$(EXESUF)
check-unit-y += tests/test-visitor-serialization$(EXESUF)
check-unit-y += tests/test-iov$(EXESUF)
+check-unit-y += tests/test-qidl$(EXESUF)
check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
@@ -34,11 +35,12 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o
tests/check-qdict.o \
tests/test-coroutine.o tests/test-string-output-visitor.o \
tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
- tests/test-qmp-commands.o tests/test-visitor-serialization.o
+ tests/test-qmp-commands.o tests/test-visitor-serialization.o \
+ tests/test-qidl.o
test-qapi-obj-y = $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y)
test-qapi-obj-y += tests/test-qapi-visit.o tests/test-qapi-types.o
-test-qapi-obj-y += module.o
+test-qapi-obj-y += module.o $(qom-obj-y)
$(test-obj-y): QEMU_INCLUDES += -Itests
@@ -84,6 +86,8 @@ check-qtest-$(CONFIG_POSIX)=$(foreach TARGET,$(TARGETS),
$(check-qtest-$(TARGET)
qtest-obj-y = tests/libqtest.o $(oslib-obj-y)
$(check-qtest-y): $(qtest-obj-y)
+tests/test-qidl$(EXESUF): tests/test-qidl.o tests/test-qidl-linked.o
$(test-qapi-obj-y) qapi/misc-qapi-visit.o
+
.PHONY: check-help
check-help:
@echo "Regression testing targets:"
diff --git a/tests/test-qidl-included.h b/tests/test-qidl-included.h
new file mode 100644
index 000..2aae51e
--- /dev/null
+++ b/tests/test-qidl-included.h
@@ -0,0 +1,31 @@
+/*
+ * Unit-tests for QIDL-generated visitors/code
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Michael Roth
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef TEST_QIDL_INCLUDED_H
+#define TEST_QIDL_INCLUDED_H
+
+#include "qidl.h"
+
+QIDL_START(TestStructIncluded, state, properties)
+typedef struct TestStructIncluded {
+int32_t a QIDL(immutable);
+int32_t b;
+uint32_t c QIDL(immutable);
+uint32_t d;
+uint64_t e QIDL(immutable);
+uint64_t f QIDL(property, "f", 42);
+char *g QIDL(property, "g");
+char *h QIDL(immutable) QIDL(proper
[Qemu-devel] [PATCH 10/20] qapi: QmpInputVisitor, implement array handling
Signed-off-by: Michael Roth
---
qapi/qmp-input-visitor.c | 32 +++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 107d8d3..c4388f3 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -132,7 +132,7 @@ static void qmp_input_start_struct(Visitor *v, void **obj,
const char *kind,
return;
}
-if (obj) {
+if (obj && *obj == NULL) {
*obj = g_malloc0(size);
}
}
@@ -274,6 +274,33 @@ static void qmp_input_start_optional(Visitor *v, bool
*present,
*present = true;
}
+static void qmp_input_start_carray(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size,
+ Error **errp)
+{
+if (obj && *obj == NULL) {
+*obj = g_malloc0(elem_count * elem_size);
+}
+qmp_input_start_list(v, name, errp);
+}
+
+static void qmp_input_next_carray(Visitor *v, Error **errp)
+{
+QmpInputVisitor *qiv = to_qiv(v);
+StackObject *so = &qiv->stack[qiv->nb_stack - 1];
+
+if (so->entry == NULL) {
+so->entry = qlist_first(qobject_to_qlist(so->obj));
+} else {
+so->entry = qlist_next(so->entry);
+}
+}
+
+static void qmp_input_end_carray(Visitor *v, Error **errp)
+{
+qmp_input_end_list(v, errp);
+}
+
Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
{
return &v->visitor;
@@ -302,6 +329,9 @@ 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.start_carray = qmp_input_start_carray;
+v->visitor.next_carray = qmp_input_next_carray;
+v->visitor.end_carray = qmp_input_end_carray;
qmp_input_push(v, obj, NULL);
qobject_incref(obj);
--
1.7.9.5
[Qemu-devel] [PATCH 16/20] qidl: Add documentation
Signed-off-by: Michael Roth
---
docs/qidl.txt | 343 +
1 file changed, 343 insertions(+)
create mode 100644 docs/qidl.txt
diff --git a/docs/qidl.txt b/docs/qidl.txt
new file mode 100644
index 000..19976d9
--- /dev/null
+++ b/docs/qidl.txt
@@ -0,0 +1,343 @@
+How to Serialize Device State with QIDL
+==
+
+This document describes how to implement save/restore of a device in QEMU using
+the QIDL compiler. The QIDL compiler makes it easier to support live
+migration in devices by converging the serialization description with the
+device type declaration. It has the following features:
+
+ 1. Single description of device state and how to serialize
+
+ 2. Fully inclusive serialization description--fields that aren't serialized
+are explicitly marked as such including the reason why.
+
+ 3. Optimized for the common case. Even without any special annotations,
+many devices will Just Work out of the box.
+
+ 4. Build time schema definition. Since QIDL runs at build time, we have full
+access to the schema during the build which means we can fail the build if
+the schema breaks.
+
+For the rest, of the document, the following simple device will be used as an
+example.
+
+typedef struct SerialDevice {
+SysBusDevice parent;
+
+uint8_t thr;// transmit holding register
+uint8_t lsr;// line status register
+uint8_t ier;// interrupt enable register
+
+int int_pending;// whether we have a pending queued interrupt
+CharDriverState *chr; // backend
+} SerialDevice;
+
+Getting Started
+---
+
+The first step is to move your device struct definition to a header file. This
+header file should only contain the struct definition and any preprocessor
+declarations you need to define the structure. This header file will act as
+the source for the QIDL compiler.
+
+Do not include any function declarations in this header file as QIDL does not
+understand function declarations.
+
+Determining What State Gets Saved
+-
+
+By default, QIDL saves every field in a structure it sees. This provides
maximum
+correctness by default. However, device structures generally contain state
+that reflects state that is in someway duplicated or not guest visible. This
+more often that not reflects design implementation details.
+
+Since design implementation details change over time, saving this state makes
+compatibility hard to maintain since it would effectively lock down a device's
+implementation.
+
+QIDL allows a device author to suppress certain fields from being saved
although
+there are very strict rules about when this is allowed and what needs to be
done
+to ensure that this does not impact correctness.
+
+There are three cases where state can be suppressed: when it is **immutable**,
+**derived**, or **broken**. In addition, QIDL can decide at run time whether
to
+suppress a field by assigning it a **default** value.
+
+## Immutable Fields
+
+If a field is only set during device construction, based on parameters passed
to
+the device's constructor, then there is no need to send save and restore this
+value. We call these fields immutable and we tell QIDL about this fact by
using
+a **immutable** marker.
+
+In our *SerialDevice* example, the *CharDriverState* pointer reflects the host
+backend that we use to send serial output to the user. This is only assigned
+during device construction and never changes. This means we can add an
+**immutable** marker to it:
+
+QIDL_START(SerialDevice, state)
+typedef struct SerialDevice {
+SysBusDevice parent;
+
+uint8_t thr;// transmit holding register
+uint8_t lsr;// line status register
+uint8_t ier;// interrupt enable register
+
+int int_pending;// whether we have a pending queued interrupt
+CharDriverState *chr QIDL(immutable);
+} SerialDevice;
+QIDL_END(SerialDevice)
+
+When reviewing patches that make use of the **immutable** marker, the following
+guidelines should be followed to determine if the marker is being used
+correctly.
+
+ 1. Check to see if the field is assigned anywhere other than the device
+initialization function.
+
+ 2. Check to see if any function is being called that modifies the state of the
+field outside of the initialization function.
+
+It can be subtle whether a field is truly immutable. A good example is a
+*QEMUTimer*. Timer's will usually have their timeout modified with a call to
+*qemu_mod_timer()* even though they are only assigned in the device
+initialization function.
+
+If the timer is always modified with a fixed value that is not dependent on
+guest state, then the timer is immutable since it's unaffected by the state of
+the gu
[Qemu-devel] [PATCH 01/20] qapi: qapi-visit.py -> qapi_visit.py so we can import
Python doesn't allow "-" in module names, so we need to rename the file
so we can re-use bits of the codegen
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
Makefile |8
scripts/{qapi-visit.py => qapi_visit.py} |0
tests/Makefile |4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{qapi-visit.py => qapi_visit.py} (100%)
diff --git a/Makefile b/Makefile
index d736ea5..08c7d0e 100644
--- a/Makefile
+++ b/Makefile
@@ -187,8 +187,8 @@ qga/qapi-generated/qga-qapi-types.c
qga/qapi-generated/qga-qapi-types.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
$(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
@@ -197,8 +197,8 @@ qapi-types.c qapi-types.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o "." < $<, " GEN $@")
qapi-visit.c qapi-visit.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py
$(gen-out-type) -o "." < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o "." < $<, " GEN $@")
qmp-commands.h qmp-marshal.c :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -m -o "." < $<, " GEN $@")
diff --git a/scripts/qapi-visit.py b/scripts/qapi_visit.py
similarity index 100%
rename from scripts/qapi-visit.py
rename to scripts/qapi_visit.py
diff --git a/tests/Makefile b/tests/Makefile
index f3f4159..3c05bdf 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -55,8 +55,8 @@ tests/test-qapi-types.c tests/test-qapi-types.h :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qapi-visit.c tests/test-qapi-visit.h :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_visit.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
--
1.7.9.5
[Qemu-devel] [PATCH 08/20] qapi: qapi_visit.py, support for visiting non-pointer/embedded structs
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
scripts/qapi_visit.py |9 +
1 file changed, 9 insertions(+)
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index d146013..ab44f11 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -107,6 +107,15 @@ if (obj && (*obj)->%(prefix)shas_%(c_name)s) {
if annotated:
if isinstance(argentry['type'], types.ListType):
ret += generate_visit_carray_body(argname, argentry)
+elif argentry.has_key(''):
+tmp_ptr_name = "%s_%s_ptr" %
(c_var(field_prefix).replace(".", ""), c_var(argname))
+ret += mcgen('''
+%(type)s *%(tmp_ptr)s = &(*obj)->%(c_prefix)s%(c_name)s;
+visit_type_%(type)s(m, (obj && *obj) ? &%(tmp_ptr)s : NULL, "%(name)s", errp);
+''',
+ c_prefix=c_var(field_prefix),
prefix=field_prefix,
+ type=type_name(argentry['type']),
c_name=c_var(argname),
+ name=argname, tmp_ptr=tmp_ptr_name)
else:
ret += mcgen('''
visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL,
"%(name)s", errp);
--
1.7.9.5
[Qemu-devel] [PATCH 02/20] qapi: qapi-types.py -> qapi_types.py
Python doesn't allow "-" in module names, so we need to rename the file
so we can re-use bits of the codegen
Signed-off-by: Michael Roth
---
Makefile |8
scripts/{qapi-types.py => qapi_types.py} |0
tests/Makefile |4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{qapi-types.py => qapi_types.py} (100%)
diff --git a/Makefile b/Makefile
index 08c7d0e..e92ea6b 100644
--- a/Makefile
+++ b/Makefile
@@ -184,8 +184,8 @@ endif
qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_types.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_types.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
@@ -194,8 +194,8 @@ $(SRC_PATH)/qapi-schema-guest.json
$(SRC_PATH)/scripts/qapi-commands.py $(qapi-p
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qapi-types.c qapi-types.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o "." < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_types.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_types.py
$(gen-out-type) -o "." < $<, " GEN $@")
qapi-visit.c qapi-visit.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o "." < $<, " GEN $@")
diff --git a/scripts/qapi-types.py b/scripts/qapi_types.py
similarity index 100%
rename from scripts/qapi-types.py
rename to scripts/qapi_types.py
diff --git a/tests/Makefile b/tests/Makefile
index 3c05bdf..156105c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -52,8 +52,8 @@ tests/test-coroutine$(EXESUF): tests/test-coroutine.o
$(coroutine-obj-y) $(tools
tests/test-iov$(EXESUF): tests/test-iov.o iov.o
tests/test-qapi-types.c tests/test-qapi-types.h :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_types.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_types.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qapi-visit.c tests/test-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_visit.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
--
1.7.9.5
[Qemu-devel] [PATCH 03/20] qapi: qapi-commands.py -> qapi_commands.py
Python doesn't allow "-" in module names, so we need to rename the file
so we can re-use bits of the codegen.
Signed-off-by: Michael Roth
---
Makefile |8
scripts/{qapi-commands.py => qapi_commands.py} |0
tests/Makefile |4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{qapi-commands.py => qapi_commands.py} (100%)
diff --git a/Makefile b/Makefile
index e92ea6b..7733e4c 100644
--- a/Makefile
+++ b/Makefile
@@ -190,8 +190,8 @@ qga/qapi-generated/qga-qapi-visit.c
qga/qapi-generated/qga-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
$(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_commands.py
$(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_commands.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qapi-types.c qapi-types.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_types.py $(qapi-py)
@@ -200,8 +200,8 @@ qapi-visit.c qapi-visit.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o "." < $<, " GEN $@")
qmp-commands.h qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -m -o "." < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_commands.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_commands.py
$(gen-out-type) -m -o "." < $<, " GEN $@")
QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h
qga-qmp-commands.h)
$(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
diff --git a/scripts/qapi-commands.py b/scripts/qapi_commands.py
similarity index 100%
rename from scripts/qapi-commands.py
rename to scripts/qapi_commands.py
diff --git a/tests/Makefile b/tests/Makefile
index 156105c..ec6a050 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -58,8 +58,8 @@ tests/test-qapi-visit.c tests/test-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_visit.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_commands.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_commands.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o
$(test-qapi-obj-y)
--
1.7.9.5
[Qemu-devel] [PATCH 14/20] module additions for schema registration
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
module.h |2 ++
vl.c |1 +
2 files changed, 3 insertions(+)
diff --git a/module.h b/module.h
index c4ccd57..cb81aa2 100644
--- a/module.h
+++ b/module.h
@@ -25,6 +25,7 @@ typedef enum {
MODULE_INIT_MACHINE,
MODULE_INIT_QAPI,
MODULE_INIT_QOM,
+MODULE_INIT_QIDL,
MODULE_INIT_MAX
} module_init_type;
@@ -32,6 +33,7 @@ typedef enum {
#define machine_init(function) module_init(function, MODULE_INIT_MACHINE)
#define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
#define type_init(function) module_init(function, MODULE_INIT_QOM)
+#define qidl_init(function) module_init(function, MODULE_INIT_QIDL)
void register_module_init(void (*fn)(void), module_init_type type);
diff --git a/vl.c b/vl.c
index d01256a..7895e9d 100644
--- a/vl.c
+++ b/vl.c
@@ -2358,6 +2358,7 @@ int main(int argc, char **argv, char **envp)
}
module_call_init(MODULE_INIT_QOM);
+module_call_init(MODULE_INIT_QIDL);
runstate_init();
--
1.7.9.5
[Qemu-devel] [PATCH 15/20] qdev: move Property-related declarations to qdev-properties.h
Signed-off-by: Michael Roth
---
hw/qdev-properties.h | 151 ++
hw/qdev.h| 126 +
2 files changed, 152 insertions(+), 125 deletions(-)
create mode 100644 hw/qdev-properties.h
diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
new file mode 100644
index 000..ce79a79
--- /dev/null
+++ b/hw/qdev-properties.h
@@ -0,0 +1,151 @@
+/*
+ * Property definitions for qdev
+ *
+ * Copyright (c) 2009 CodeSourcery
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QDEV_PROPERTIES_H
+#define QDEV_PROPERTIES_H
+
+#include "qemu/object.h"
+#include "qemu-queue.h"
+
+typedef struct Property Property;
+
+typedef struct PropertyInfo PropertyInfo;
+
+typedef struct CompatProperty CompatProperty;
+
+struct Property {
+const char *name;
+PropertyInfo *info;
+int offset;
+uint8_t bitnr;
+uint8_t qtype;
+int64_t defval;
+};
+
+struct PropertyInfo {
+const char *name;
+const char *legacy_name;
+const char **enum_table;
+int (*parse)(DeviceState *dev, Property *prop, const char *str);
+int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
+ObjectPropertyAccessor *get;
+ObjectPropertyAccessor *set;
+ObjectPropertyRelease *release;
+};
+
+typedef struct GlobalProperty {
+const char *driver;
+const char *property;
+const char *value;
+QTAILQ_ENTRY(GlobalProperty) next;
+} GlobalProperty;
+
+extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_uint8;
+extern PropertyInfo qdev_prop_uint16;
+extern PropertyInfo qdev_prop_uint32;
+extern PropertyInfo qdev_prop_int32;
+extern PropertyInfo qdev_prop_uint64;
+extern PropertyInfo qdev_prop_hex8;
+extern PropertyInfo qdev_prop_hex32;
+extern PropertyInfo qdev_prop_hex64;
+extern PropertyInfo qdev_prop_string;
+extern PropertyInfo qdev_prop_chr;
+extern PropertyInfo qdev_prop_ptr;
+extern PropertyInfo qdev_prop_macaddr;
+extern PropertyInfo qdev_prop_losttickpolicy;
+extern PropertyInfo qdev_prop_bios_chs_trans;
+extern PropertyInfo qdev_prop_drive;
+extern PropertyInfo qdev_prop_netdev;
+extern PropertyInfo qdev_prop_vlan;
+extern PropertyInfo qdev_prop_pci_devfn;
+extern PropertyInfo qdev_prop_blocksize;
+extern PropertyInfo qdev_prop_pci_host_devaddr;
+
+#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
+.name = (_name),\
+.info = &(_prop), \
+.offset= offsetof(_state, _field)\
++ type_check(_type,typeof_field(_state, _field)),\
+}
+#define DEFINE_PROP_DEFAULT(_name, _state, _field, _defval, _prop, _type) { \
+.name = (_name), \
+.info = &(_prop), \
+.offset= offsetof(_state, _field) \
++ type_check(_type,typeof_field(_state, _field)), \
+.qtype = QTYPE_QINT,\
+.defval= (_type)_defval,\
+}
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \
+.name = (_name),\
+.info = &(qdev_prop_bit), \
+.bitnr= (_bit), \
+.offset= offsetof(_state, _field)\
++ type_check(uint32_t,typeof_field(_state, _field)), \
+.qtype = QTYPE_QBOOL,\
+.defval= (bool)_defval, \
+}
+
+#define DEFINE_PROP_UINT8(_n, _s, _f, _d) \
+DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
+#define DEFINE_PROP_UINT16(_n, _s, _f, _d) \
+DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint16, uint16_t)
+#define DEFINE_PROP_UINT32(_n, _s, _f, _d) \
+DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint32, uint32_t)
+#define DEFINE_PROP_INT32(_n, _s, _f, _d) \
+DEFINE_PROP_DEFAULT(_n, _s, _f,
[Qemu-devel] [PATCH 06/20] qapi: add visitor interfaces for C arrays
Generally these will be serialized into lists, but the
representation can be of any form so long as it can
be deserialized into a single-dimension C array.
Signed-off-by: Michael Roth
---
qapi/qapi-visit-core.c | 25 +
qapi/qapi-visit-core.h |8
2 files changed, 33 insertions(+)
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 7a82b63..9a74ed0 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -311,3 +311,28 @@ void input_type_enum(Visitor *v, int *obj, const char
*strings[],
g_free(enum_str);
*obj = value;
}
+
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+size_t elem_count, size_t elem_size, Error **errp)
+{
+g_assert(v->start_carray);
+if (!error_is_set(errp)) {
+v->start_carray(v, obj, name, elem_count, elem_size, errp);
+}
+}
+
+void visit_next_carray(Visitor *v, Error **errp)
+{
+g_assert(v->next_carray);
+if (!error_is_set(errp)) {
+v->next_carray(v, errp);
+}
+}
+
+void visit_end_carray(Visitor *v, Error **errp)
+{
+g_assert(v->end_carray);
+if (!error_is_set(errp)) {
+v->end_carray(v, errp);
+}
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 60aceda..5eb1616 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -43,6 +43,10 @@ struct Visitor
void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
void (*type_number)(Visitor *v, double *obj, const char *name,
Error **errp);
+void (*start_carray)(Visitor *v, void **obj, const char *name,
+size_t elem_count, size_t elem_size, Error **errp);
+void (*next_carray)(Visitor *v, Error **errp);
+void (*end_carray)(Visitor *v, Error **errp);
/* May be NULL */
void (*start_optional)(Visitor *v, bool *present, const char *name,
@@ -91,5 +95,9 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char
*name, Error **errp);
void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
void visit_type_number(Visitor *v, double *obj, const char *name, Error
**errp);
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size, Error **errp);
+void visit_next_carray(Visitor *v, Error **errp);
+void visit_end_carray(Visitor *v, Error **errp);
#endif
--
1.7.9.5
[Qemu-devel] [PATCH 09/20] qapi: QmpOutputVisitor, implement array handling
Signed-off-by: Michael Roth
---
qapi/qmp-output-visitor.c | 20
1 file changed, 20 insertions(+)
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 2bce9d5..ea08795 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -181,6 +181,23 @@ static void qmp_output_type_number(Visitor *v, double
*obj, const char *name,
qmp_output_add(qov, name, qfloat_from_double(*obj));
}
+static void qmp_output_start_carray(Visitor *v, void **obj, const char *name,
+size_t elem_count, size_t elem_size,
+Error **errp)
+{
+qmp_output_start_list(v, name, errp);
+}
+
+static void qmp_output_next_carray(Visitor *v, Error **errp)
+{
+}
+
+static void qmp_output_end_carray(Visitor *v, Error **errp)
+{
+QmpOutputVisitor *qov = to_qov(v);
+qmp_output_pop(qov);
+}
+
QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
{
QObject *obj = qmp_output_first(qov);
@@ -228,6 +245,9 @@ QmpOutputVisitor *qmp_output_visitor_new(void)
v->visitor.type_bool = qmp_output_type_bool;
v->visitor.type_str = qmp_output_type_str;
v->visitor.type_number = qmp_output_type_number;
+v->visitor.start_carray = qmp_output_start_carray;
+v->visitor.next_carray = qmp_output_next_carray;
+v->visitor.end_carray = qmp_output_end_carray;
QTAILQ_INIT(&v->stack);
--
1.7.9.5
Re: [Qemu-devel] [PATCH 16/20] qidl: Add documentation
On Tue, Aug 14, 2012 at 08:41:56PM +0100, Peter Maydell wrote:
> On 14 August 2012 17:27, Michael Roth wrote:
> >
> > Signed-off-by: Michael Roth
> > ---
> > docs/qidl.txt | 343
> > +
> > 1 file changed, 343 insertions(+)
> > create mode 100644 docs/qidl.txt
> >
> > diff --git a/docs/qidl.txt b/docs/qidl.txt
> > new file mode 100644
> > index 000..19976d9
> > --- /dev/null
> > +++ b/docs/qidl.txt
> > @@ -0,0 +1,343 @@
> > +How to Serialize Device State with QIDL
> > +==
> > +
> > +This document describes how to implement save/restore of a device in QEMU
> > using
> > +the QIDL compiler. The QIDL compiler makes it easier to support live
> > +migration in devices by converging the serialization description with the
> > +device type declaration. It has the following features:
> > +
> > + 1. Single description of device state and how to serialize
> > +
> > + 2. Fully inclusive serialization description--fields that aren't
> > serialized
> > +are explicitly marked as such including the reason why.
> > +
> > + 3. Optimized for the common case. Even without any special annotations,
> > +many devices will Just Work out of the box.
> > +
> > + 4. Build time schema definition. Since QIDL runs at build time, we have
> > full
> > +access to the schema during the build which means we can fail the
> > build if
> > +the schema breaks.
> > +
> > +For the rest, of the document, the following simple device will be used as
> > an
> > +example.
> > +
> > +typedef struct SerialDevice {
> > +SysBusDevice parent;
> > +
> > +uint8_t thr;// transmit holding register
> > +uint8_t lsr;// line status register
> > +uint8_t ier;// interrupt enable register
> > +
> > +int int_pending;// whether we have a pending queued
> > interrupt
> > +CharDriverState *chr; // backend
> > +} SerialDevice;
>
> "//" style comments are a breach of the QEMU coding style so we shouldn't
> be using them in documentation examples...
>
> > +
> > +In our *SerialDevice* example, the *CharDriverState* pointer reflects the
> > host
> > +backend that we use to send serial output to the user. This is only
> > assigned
> > +during device construction and never changes. This means we can add an
> > +**immutable** marker to it:
> > +
> > +QIDL_START(SerialDevice, state)
> > +typedef struct SerialDevice {
> > +SysBusDevice parent;
> > +
> > +uint8_t thr;// transmit holding register
> > +uint8_t lsr;// line status register
> > +uint8_t ier;// interrupt enable register
> > +
> > +int int_pending;// whether we have a pending queued
> > interrupt
> > +CharDriverState *chr QIDL(immutable);
> > +} SerialDevice;
> > +QIDL_END(SerialDevice)
>
> I think it would be nicer to have a QIDL input format from which the structure
> is generated as one of the outputs; that would avoid having to have some of
> this ugly QIDL() markup.
Some kind of inline/embedded input format, or external (like QAPI
schemas)?
In terms of the ugliness of things, Anthony suggested we re-introduce
the simpler form of the annotations by doing something like:
#define _immutable QIDL(immutable)
as syntactic sugar for the more commonly used annotations/macros. This
can all be done in qidl.h, transparently to the parser (thanks to the
fact that we pass the code through the preprocessor prior to parsing).
There's also a few things we can do to replace the QIDL_START/QIDL_END
pairs with a single indicator.
>
> We could also use this to autogenerate a lot of the QOM required boilerplate,
> qdev Property arrays, etc etc.
QIDL handles properties, and could possibly be used to generate some QOM
boilerplate. We can get as creative as need be really, it's just that
properties/serialization were the primary/initial use-cases.
>
> -- PMM
>
Re: [Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion
On Mon, Aug 13, 2012 at 09:25:44PM -0500, Michael Roth wrote:
> On Mon, Aug 13, 2012 at 03:01:56PM -0300, Luiz Capitulino wrote:
> > On Fri, 10 Aug 2012 18:24:10 -0500
> > Michael Roth wrote:
> >
> > > +qlist_iter(tokens, tokens_count_from_iter, &count);
> > > +if (count == 0) {
> > > +return NULL;
> > > +}
> >
> > Please, add qlist_size() instead.
> >
>
> Gladly :) I spent a good amount for a function that did this, not sure
> how I missed it.
>
Heh, read that as "please, use qlist_size() instead". I've added it
after further searching for it :)
> > > +
> > > +ctxt = g_malloc0(sizeof(JSONParserContext));
> > > +ctxt->tokens.pos = 0;
> > > +ctxt->tokens.count = count;
> > > +ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
> > > +qlist_iter(tokens, tokens_append_from_iter, ctxt);
> > > +ctxt->tokens.pos = 0;
> > > +
> > > +return ctxt;
> > > +}
> > > +
> > > +static void parser_context_free(JSONParserContext *ctxt)
> > > +{
> > > +int i;
> > > +if (ctxt) {
> > > +for (i = 0; i < ctxt->tokens.count; i++) {
> > > +qobject_decref(ctxt->tokens.buf[i]);
> > > +}
> > > +g_free(ctxt->tokens.buf);
> > > +g_free(ctxt);
> >
> > Isn't this leaking ctxt->err?
> >
>
> Indeed. Looks like we were leaking this previously as well. I'll get it
> in V2.
>
Apparently not, actually. It looks like error_propagate() handles
whether to free the error or pass it up the stack, so we can't mess with
it in the free function. I've added a comment to note that ctxt->err
needs to be freed seperately.
V2 is incoming with comments addressed.
[Qemu-devel] [PATCH for-1.2 v2 1/3] qlist: add qlist_size()
Signed-off-by: Michael Roth
---
qlist.c | 13 +
qlist.h |1 +
2 files changed, 14 insertions(+)
diff --git a/qlist.c b/qlist.c
index 88498b1..b48ec5b 100644
--- a/qlist.c
+++ b/qlist.c
@@ -124,6 +124,19 @@ int qlist_empty(const QList *qlist)
return QTAILQ_EMPTY(&qlist->head);
}
+static void qlist_size_iter(QObject *obj, void *opaque)
+{
+size_t *count = opaque;
+(*count)++;
+}
+
+size_t qlist_size(const QList *qlist)
+{
+size_t count = 0;
+qlist_iter(qlist, qlist_size_iter, &count);
+return count;
+}
+
/**
* qobject_to_qlist(): Convert a QObject into a QList
*/
diff --git a/qlist.h b/qlist.h
index d426bd4..ae776f9 100644
--- a/qlist.h
+++ b/qlist.h
@@ -49,6 +49,7 @@ void qlist_iter(const QList *qlist,
QObject *qlist_pop(QList *qlist);
QObject *qlist_peek(QList *qlist);
int qlist_empty(const QList *qlist);
+size_t qlist_size(const QList *qlist);
QList *qobject_to_qlist(const QObject *obj);
static inline const QListEntry *qlist_first(const QList *qlist)
--
1.7.9.5
[Qemu-devel] [PATCH for-1.2 v2 2/3] json-parser: don't replicate tokens at each level of recursion
Currently, when parsing a stream of tokens we make a copy of the token
list at the beginning of each level of recursion so that we do not
modify the original list in cases where we need to fall back to an
earlier state.
In the worst case, we will only read 1 or 2 tokens off the list before
recursing again, which means an upper bound of roughly N^2 token allocations.
For a "reasonably" sized QMP request (in this a QMP representation of
cirrus_vga's device state, generated via QIDL, being passed in via
qom-set), this caused my 16GB's of memory to be exhausted before any
noticeable progress was made by the parser.
This patch works around the issue by using single copy of the token list
in the form of an indexable array so that we can save/restore state by
manipulating indices.
A subsequent commit adds a "large_dict" test case which exhibits the
same behavior as above. With this patch applied the test case successfully
completes in under a second.
Tested with valgrind, make check, and QMP.
Signed-off-by: Michael Roth
---
json-parser.c | 229 +++--
1 file changed, 141 insertions(+), 88 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 849e215..65a87c7 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -27,6 +27,11 @@
typedef struct JSONParserContext
{
Error *err;
+struct {
+QObject **buf;
+size_t pos;
+size_t count;
+} tokens;
} JSONParserContext;
#define BUG_ON(cond) assert(!(cond))
@@ -40,7 +45,7 @@ typedef struct JSONParserContext
* 4) deal with premature EOI
*/
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list
*ap);
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
/**
* Token manipulators
@@ -270,27 +275,110 @@ out:
return NULL;
}
+static QObject *parser_context_pop_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+ctxt->tokens.pos++;
+return token;
+}
+
+/* Note: parser_context_{peek|pop}_token do not increment the
+ * token object's refcount. In both cases the references will continue
+ * to be * tracked and cleanup in parser_context_free()
+ */
+static QObject *parser_context_peek_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+return token;
+}
+
+static JSONParserContext parser_context_save(JSONParserContext *ctxt)
+{
+JSONParserContext saved_ctxt = {0};
+saved_ctxt.tokens.pos = ctxt->tokens.pos;
+saved_ctxt.tokens.count = ctxt->tokens.count;
+saved_ctxt.tokens.buf = ctxt->tokens.buf;
+return saved_ctxt;
+}
+
+static void parser_context_restore(JSONParserContext *ctxt,
+ JSONParserContext saved_ctxt)
+{
+ctxt->tokens.pos = saved_ctxt.tokens.pos;
+ctxt->tokens.count = saved_ctxt.tokens.count;
+ctxt->tokens.buf = saved_ctxt.tokens.buf;
+}
+
+static void tokens_append_from_iter(QObject *obj, void *opaque)
+{
+JSONParserContext *ctxt = opaque;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ctxt->tokens.buf[ctxt->tokens.pos++] = obj;
+qobject_incref(obj);
+}
+
+static JSONParserContext *parser_context_new(QList *tokens)
+{
+JSONParserContext *ctxt;
+size_t count;
+
+if (!tokens) {
+return NULL;
+}
+
+count = qlist_size(tokens);
+if (count == 0) {
+return NULL;
+}
+
+ctxt = g_malloc0(sizeof(JSONParserContext));
+ctxt->tokens.pos = 0;
+ctxt->tokens.count = count;
+ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
+qlist_iter(tokens, tokens_append_from_iter, ctxt);
+ctxt->tokens.pos = 0;
+
+return ctxt;
+}
+
+/* to support error propagation, ctxt->err must be freed seperately */
+static void parser_context_free(JSONParserContext *ctxt)
+{
+int i;
+if (ctxt) {
+for (i = 0; i < ctxt->tokens.count; i++) {
+qobject_decref(ctxt->tokens.buf[i]);
+}
+g_free(ctxt->tokens.buf);
+g_free(ctxt);
+}
+}
+
/**
* Parsing rules
*/
-static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens,
va_list *ap)
+static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
{
QObject *key = NULL, *token = NULL, *value, *peek;
-QList *working = qlist_copy(*tokens);
+JSONParserContext saved_ctxt = parser_context_save(ctxt);
-peek = qlist_peek(working);
+peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
-key = parse_value(ctxt, &working, ap);
+key = parse_value(ctxt, ap);
if (!key || qobject_type(key) !=
[Qemu-devel] [PATCH for-1.2 v2 3/3] check-qjson: add test for large JSON objects
Signed-off-by: Michael Roth
---
tests/check-qjson.c | 53 +++
1 file changed, 53 insertions(+)
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 526e25e..ef9b529 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -690,6 +690,58 @@ static void unterminated_literal(void)
g_assert(obj == NULL);
}
+/*
+ * this generates json of the form:
+ * a(0,m) = [0, 1, ..., m-1]
+ * a(n,m) = {
+ *'key0': a(0,m),
+ *'key1': a(1,m),
+ *...
+ *'key(n-1)': a(n-1,m)
+ * }
+ */
+static void gen_test_json(GString *gstr, int nest_level_max,
+ int elem_count)
+{
+int i;
+
+g_assert(gstr);
+if (nest_level_max == 0) {
+g_string_append(gstr, "[");
+for (i = 0; i < elem_count; i++) {
+g_string_append_printf(gstr, "%d", i);
+if (i < elem_count - 1) {
+g_string_append_printf(gstr, ", ");
+}
+}
+g_string_append(gstr, "]");
+return;
+}
+
+g_string_append(gstr, "{");
+for (i = 0; i < nest_level_max; i++) {
+g_string_append_printf(gstr, "'key%d': ", i);
+gen_test_json(gstr, i, elem_count);
+if (i < nest_level_max - 1) {
+g_string_append(gstr, ",");
+}
+}
+g_string_append(gstr, "}");
+}
+
+static void large_dict(void)
+{
+GString *gstr = g_string_new("");
+QObject *obj;
+
+gen_test_json(gstr, 10, 100);
+obj = qobject_from_json(gstr->str);
+g_assert(obj != NULL);
+
+qobject_decref(obj);
+g_string_free(gstr, true);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -706,6 +758,7 @@ int main(int argc, char **argv)
g_test_add_func("/literals/keyword", keyword_literal);
g_test_add_func("/dicts/simple_dict", simple_dict);
+g_test_add_func("/dicts/large_dict", large_dict);
g_test_add_func("/lists/simple_list", simple_list);
g_test_add_func("/whitespace/simple_whitespace", simple_whitespace);
--
1.7.9.5
Re: [Qemu-devel] [PATCH for-1.2 v2 2/3] json-parser: don't replicate tokens at each level of recursion
On Wed, Aug 15, 2012 at 12:09:09PM -0600, Eric Blake wrote:
> On 08/15/2012 11:56 AM, Michael Roth wrote:
> > Currently, when parsing a stream of tokens we make a copy of the token
> > list at the beginning of each level of recursion so that we do not
> > modify the original list in cases where we need to fall back to an
> > earlier state.
> >
> > +
> > +/* Note: parser_context_{peek|pop}_token do not increment the
> > + * token object's refcount. In both cases the references will continue
> > + * to be * tracked and cleanup in parser_context_free()
>
> Bad comment reflow? s/be * tracked/be tracked/
>
> > +
> > +/* to support error propagation, ctxt->err must be freed seperately */
>
> s/seperately/separately/
>
Thanks, fixed in incoming v3
> --
> Eric Blake ebl...@redhat.com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
[Qemu-devel] [PATCH for-1.2 v3 1/3] qlist: add qlist_size()
Signed-off-by: Michael Roth
---
qlist.c | 13 +
qlist.h |1 +
2 files changed, 14 insertions(+)
diff --git a/qlist.c b/qlist.c
index 88498b1..b48ec5b 100644
--- a/qlist.c
+++ b/qlist.c
@@ -124,6 +124,19 @@ int qlist_empty(const QList *qlist)
return QTAILQ_EMPTY(&qlist->head);
}
+static void qlist_size_iter(QObject *obj, void *opaque)
+{
+size_t *count = opaque;
+(*count)++;
+}
+
+size_t qlist_size(const QList *qlist)
+{
+size_t count = 0;
+qlist_iter(qlist, qlist_size_iter, &count);
+return count;
+}
+
/**
* qobject_to_qlist(): Convert a QObject into a QList
*/
diff --git a/qlist.h b/qlist.h
index d426bd4..ae776f9 100644
--- a/qlist.h
+++ b/qlist.h
@@ -49,6 +49,7 @@ void qlist_iter(const QList *qlist,
QObject *qlist_pop(QList *qlist);
QObject *qlist_peek(QList *qlist);
int qlist_empty(const QList *qlist);
+size_t qlist_size(const QList *qlist);
QList *qobject_to_qlist(const QObject *obj);
static inline const QListEntry *qlist_first(const QList *qlist)
--
1.7.9.5
[Qemu-devel] [PATCH for-1.2 v3 3/3] check-qjson: add test for large JSON objects
Signed-off-by: Michael Roth
---
tests/check-qjson.c | 53 +++
1 file changed, 53 insertions(+)
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 526e25e..3b896f5 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -466,6 +466,58 @@ static void simple_dict(void)
}
}
+/*
+ * this generates json of the form:
+ * a(0,m) = [0, 1, ..., m-1]
+ * a(n,m) = {
+ *'key0': a(0,m),
+ *'key1': a(1,m),
+ *...
+ *'key(n-1)': a(n-1,m)
+ * }
+ */
+static void gen_test_json(GString *gstr, int nest_level_max,
+ int elem_count)
+{
+int i;
+
+g_assert(gstr);
+if (nest_level_max == 0) {
+g_string_append(gstr, "[");
+for (i = 0; i < elem_count; i++) {
+g_string_append_printf(gstr, "%d", i);
+if (i < elem_count - 1) {
+g_string_append_printf(gstr, ", ");
+}
+}
+g_string_append(gstr, "]");
+return;
+}
+
+g_string_append(gstr, "{");
+for (i = 0; i < nest_level_max; i++) {
+g_string_append_printf(gstr, "'key%d': ", i);
+gen_test_json(gstr, i, elem_count);
+if (i < nest_level_max - 1) {
+g_string_append(gstr, ",");
+}
+}
+g_string_append(gstr, "}");
+}
+
+static void large_dict(void)
+{
+GString *gstr = g_string_new("");
+QObject *obj;
+
+gen_test_json(gstr, 10, 100);
+obj = qobject_from_json(gstr->str);
+g_assert(obj != NULL);
+
+qobject_decref(obj);
+g_string_free(gstr, true);
+}
+
static void simple_list(void)
{
int i;
@@ -706,6 +758,7 @@ int main(int argc, char **argv)
g_test_add_func("/literals/keyword", keyword_literal);
g_test_add_func("/dicts/simple_dict", simple_dict);
+g_test_add_func("/dicts/large_dict", large_dict);
g_test_add_func("/lists/simple_list", simple_list);
g_test_add_func("/whitespace/simple_whitespace", simple_whitespace);
--
1.7.9.5
[Qemu-devel] [PATCH for-1.2 v3 2/3] json-parser: don't replicate tokens at each level of recursion
Currently, when parsing a stream of tokens we make a copy of the token
list at the beginning of each level of recursion so that we do not
modify the original list in cases where we need to fall back to an
earlier state.
In the worst case, we will only read 1 or 2 tokens off the list before
recursing again, which means an upper bound of roughly N^2 token allocations.
For a "reasonably" sized QMP request (in this a QMP representation of
cirrus_vga's device state, generated via QIDL, being passed in via
qom-set), this caused my 16GB's of memory to be exhausted before any
noticeable progress was made by the parser.
This patch works around the issue by using single copy of the token list
in the form of an indexable array so that we can save/restore state by
manipulating indices.
A subsequent commit adds a "large_dict" test case which exhibits the
same behavior as above. With this patch applied the test case successfully
completes in under a second.
Tested with valgrind, make check, and QMP.
Signed-off-by: Michael Roth
---
json-parser.c | 230 +++--
1 file changed, 142 insertions(+), 88 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 849e215..457291b 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -27,6 +27,11 @@
typedef struct JSONParserContext
{
Error *err;
+struct {
+QObject **buf;
+size_t pos;
+size_t count;
+} tokens;
} JSONParserContext;
#define BUG_ON(cond) assert(!(cond))
@@ -40,7 +45,7 @@ typedef struct JSONParserContext
* 4) deal with premature EOI
*/
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list
*ap);
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
/**
* Token manipulators
@@ -270,27 +275,111 @@ out:
return NULL;
}
+static QObject *parser_context_pop_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+ctxt->tokens.pos++;
+return token;
+}
+
+/* Note: parser_context_{peek|pop}_token do not increment the
+ * token object's refcount. In both cases the references will continue
+ * to be tracked and cleaned up in parser_context_free(), so do not
+ * attempt to free the token object.
+ */
+static QObject *parser_context_peek_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+return token;
+}
+
+static JSONParserContext parser_context_save(JSONParserContext *ctxt)
+{
+JSONParserContext saved_ctxt = {0};
+saved_ctxt.tokens.pos = ctxt->tokens.pos;
+saved_ctxt.tokens.count = ctxt->tokens.count;
+saved_ctxt.tokens.buf = ctxt->tokens.buf;
+return saved_ctxt;
+}
+
+static void parser_context_restore(JSONParserContext *ctxt,
+ JSONParserContext saved_ctxt)
+{
+ctxt->tokens.pos = saved_ctxt.tokens.pos;
+ctxt->tokens.count = saved_ctxt.tokens.count;
+ctxt->tokens.buf = saved_ctxt.tokens.buf;
+}
+
+static void tokens_append_from_iter(QObject *obj, void *opaque)
+{
+JSONParserContext *ctxt = opaque;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ctxt->tokens.buf[ctxt->tokens.pos++] = obj;
+qobject_incref(obj);
+}
+
+static JSONParserContext *parser_context_new(QList *tokens)
+{
+JSONParserContext *ctxt;
+size_t count;
+
+if (!tokens) {
+return NULL;
+}
+
+count = qlist_size(tokens);
+if (count == 0) {
+return NULL;
+}
+
+ctxt = g_malloc0(sizeof(JSONParserContext));
+ctxt->tokens.pos = 0;
+ctxt->tokens.count = count;
+ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
+qlist_iter(tokens, tokens_append_from_iter, ctxt);
+ctxt->tokens.pos = 0;
+
+return ctxt;
+}
+
+/* to support error propagation, ctxt->err must be freed separately */
+static void parser_context_free(JSONParserContext *ctxt)
+{
+int i;
+if (ctxt) {
+for (i = 0; i < ctxt->tokens.count; i++) {
+qobject_decref(ctxt->tokens.buf[i]);
+}
+g_free(ctxt->tokens.buf);
+g_free(ctxt);
+}
+}
+
/**
* Parsing rules
*/
-static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens,
va_list *ap)
+static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
{
QObject *key = NULL, *token = NULL, *value, *peek;
-QList *working = qlist_copy(*tokens);
+JSONParserContext saved_ctxt = parser_context_save(ctxt);
-peek = qlist_peek(working);
+peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
-key = parse_value(ctxt, &working, ap);
+key = parse_value(ctxt, ap
Re: [Qemu-devel] [PATCH for-1.2 v3 1/3] qlist: add qlist_size()
On Wed, Aug 15, 2012 at 01:52:33PM -0600, Eric Blake wrote: > On 08/15/2012 12:45 PM, Michael Roth wrote: > > Signed-off-by: Michael Roth > > --- > > qlist.c | 13 + > > qlist.h |1 + > > 2 files changed, 14 insertions(+) > > No cover-letter? Started off as 1 patch with the explanation in the commit > > Reviewed-by: Eric Blake > > -- > Eric Blake ebl...@redhat.com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
Re: [Qemu-devel] [PATCH for-1.2 v3 2/3] json-parser: don't replicate tokens at each level of recursion
On Wed, Aug 15, 2012 at 02:04:52PM -0600, Eric Blake wrote:
> On 08/15/2012 12:45 PM, Michael Roth wrote:
> > Currently, when parsing a stream of tokens we make a copy of the token
> > list at the beginning of each level of recursion so that we do not
> > modify the original list in cases where we need to fall back to an
> > earlier state.
> >
> > In the worst case, we will only read 1 or 2 tokens off the list before
> > recursing again, which means an upper bound of roughly N^2 token
> > allocations.
> >
> > For a "reasonably" sized QMP request (in this a QMP representation of
> > cirrus_vga's device state, generated via QIDL, being passed in via
> > qom-set), this caused my 16GB's of memory to be exhausted before any
> > noticeable progress was made by the parser.
> >
> > This patch works around the issue by using single copy of the token list
> > in the form of an indexable array so that we can save/restore state by
> > manipulating indices.
> >
> > A subsequent commit adds a "large_dict" test case which exhibits the
> > same behavior as above. With this patch applied the test case successfully
> > completes in under a second.
> >
> > Tested with valgrind, make check, and QMP.
> >
> > Signed-off-by: Michael Roth
> > ---
> > json-parser.c | 230
> > +++--
> > 1 file changed, 142 insertions(+), 88 deletions(-)
>
> I'm not the most familiar with this code, so take my review with a grain
> of salt, but I read through it and the transformation looks sane (and my
> non-code findings from v2 were fixed).
>
> Reviewed-by: Eric Blake
>
> > +static JSONParserContext parser_context_save(JSONParserContext *ctxt)
> > +{
> > +JSONParserContext saved_ctxt = {0};
> > +saved_ctxt.tokens.pos = ctxt->tokens.pos;
> > +saved_ctxt.tokens.count = ctxt->tokens.count;
> > +saved_ctxt.tokens.buf = ctxt->tokens.buf;
>
> Is it any simpler to condense 3 lines to 1:
>
> saved_cts.tokens = ctxt->tokens;
>
> > +return saved_ctxt;
> > +}
> > +
> > +static void parser_context_restore(JSONParserContext *ctxt,
> > + JSONParserContext saved_ctxt)
> > +{
> > +ctxt->tokens.pos = saved_ctxt.tokens.pos;
> > +ctxt->tokens.count = saved_ctxt.tokens.count;
> > +ctxt->tokens.buf = saved_ctxt.tokens.buf;
>
> and again, ctxt->tokens = saved_ctxt.tokens;
Poor function naming: save/restore apply to the token state, the other
fields in ctxt are unused, so I opted to set the fields explicitly.
Can probably make this read a little better by breaking token state off
into it's own struct, but I think we can clean that up later.
Thanks for the review!
>
> --
> Eric Blake ebl...@redhat.com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Re: [Qemu-devel] [PATCH for-1.2 v3 2/3] json-parser: don't replicate tokens at each level of recursion
On Thu, Aug 16, 2012 at 11:11:26AM -0300, Luiz Capitulino wrote:
> On Wed, 15 Aug 2012 13:45:43 -0500
> Michael Roth wrote:
>
> > Currently, when parsing a stream of tokens we make a copy of the token
> > list at the beginning of each level of recursion so that we do not
> > modify the original list in cases where we need to fall back to an
> > earlier state.
> >
> > In the worst case, we will only read 1 or 2 tokens off the list before
> > recursing again, which means an upper bound of roughly N^2 token
> > allocations.
> >
> > For a "reasonably" sized QMP request (in this a QMP representation of
> > cirrus_vga's device state, generated via QIDL, being passed in via
> > qom-set), this caused my 16GB's of memory to be exhausted before any
> > noticeable progress was made by the parser.
> >
> > This patch works around the issue by using single copy of the token list
> > in the form of an indexable array so that we can save/restore state by
> > manipulating indices.
> >
> > A subsequent commit adds a "large_dict" test case which exhibits the
> > same behavior as above. With this patch applied the test case successfully
> > completes in under a second.
> >
> > Tested with valgrind, make check, and QMP.
> >
> > Signed-off-by: Michael Roth
> > ---
> > json-parser.c | 230
> > +++--
> > 1 file changed, 142 insertions(+), 88 deletions(-)
> >
> > diff --git a/json-parser.c b/json-parser.c
> > index 849e215..457291b 100644
> > --- a/json-parser.c
> > +++ b/json-parser.c
> > @@ -27,6 +27,11 @@
> > typedef struct JSONParserContext
> > {
> > Error *err;
> > +struct {
> > +QObject **buf;
> > +size_t pos;
> > +size_t count;
> > +} tokens;
> > } JSONParserContext;
> >
> > #define BUG_ON(cond) assert(!(cond))
> > @@ -40,7 +45,7 @@ typedef struct JSONParserContext
> > * 4) deal with premature EOI
> > */
> >
> > -static QObject *parse_value(JSONParserContext *ctxt, QList **tokens,
> > va_list *ap);
> > +static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
> >
> > /**
> > * Token manipulators
> > @@ -270,27 +275,111 @@ out:
> > return NULL;
> > }
> >
> > +static QObject *parser_context_pop_token(JSONParserContext *ctxt)
> > +{
> > +QObject *token;
> > +g_assert(ctxt->tokens.pos < ctxt->tokens.count);
> > +token = ctxt->tokens.buf[ctxt->tokens.pos];
> > +ctxt->tokens.pos++;
>
> Shouldn't pos be decremented instead? Haven't tried it yet to confirm, but
> if I'm right I can fix it myself (unless this requires fixes in other areas).
>
No that's intentional, since qlist_pop() (which this replaces) actually "pops"
from the beginning of the list. So in this case we make the next element the
new "head" by simply incrementing the starting position (so it's easy to roll
back later).
> > +return token;
> > +}
> > +
> > +/* Note: parser_context_{peek|pop}_token do not increment the
> > + * token object's refcount. In both cases the references will continue
> > + * to be tracked and cleaned up in parser_context_free(), so do not
> > + * attempt to free the token object.
> > + */
> > +static QObject *parser_context_peek_token(JSONParserContext *ctxt)
> > +{
> > +QObject *token;
> > +g_assert(ctxt->tokens.pos < ctxt->tokens.count);
> > +token = ctxt->tokens.buf[ctxt->tokens.pos];
> > +return token;
> > +}
> > +
> > +static JSONParserContext parser_context_save(JSONParserContext *ctxt)
> > +{
> > +JSONParserContext saved_ctxt = {0};
> > +saved_ctxt.tokens.pos = ctxt->tokens.pos;
> > +saved_ctxt.tokens.count = ctxt->tokens.count;
> > +saved_ctxt.tokens.buf = ctxt->tokens.buf;
> > +return saved_ctxt;
> > +}
> > +
> > +static void parser_context_restore(JSONParserContext *ctxt,
> > + JSONParserContext saved_ctxt)
> > +{
> > +ctxt->tokens.pos = saved_ctxt.tokens.pos;
> > +ctxt->tokens.count = saved_ctxt.tokens.count;
> > +ctxt->tokens.buf = saved_ctxt.tokens.buf;
> > +}
> > +
> > +static void tokens_append_from_iter(QObject *obj, void *opaque)
> > +{
> > +JSONParserContext *ctxt = opaque;
> > +g_assert(ctxt->tokens.pos < ctxt-
Re: [Qemu-devel] [PATCH for-1.2 v3 1/3] qlist: add qlist_size()
On Thu, Aug 16, 2012 at 10:40:05AM -0300, Luiz Capitulino wrote:
> On Wed, 15 Aug 2012 13:45:42 -0500
> Michael Roth wrote:
>
> >
> > Signed-off-by: Michael Roth
>
> I've applied this series to the qmp branch for 1.2. I'll run some tests and if
> all goes alright will send a pull request shortly.
>
I just noticed via our fancy #qemu github bot that Anthony
applied these already, but I asked about and if you want to
test/submit through your queue it should all work out. So either
way.
Thanks!
> > ---
> > qlist.c | 13 +
> > qlist.h |1 +
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/qlist.c b/qlist.c
> > index 88498b1..b48ec5b 100644
> > --- a/qlist.c
> > +++ b/qlist.c
> > @@ -124,6 +124,19 @@ int qlist_empty(const QList *qlist)
> > return QTAILQ_EMPTY(&qlist->head);
> > }
> >
> > +static void qlist_size_iter(QObject *obj, void *opaque)
> > +{
> > +size_t *count = opaque;
> > +(*count)++;
> > +}
> > +
> > +size_t qlist_size(const QList *qlist)
> > +{
> > +size_t count = 0;
> > +qlist_iter(qlist, qlist_size_iter, &count);
> > +return count;
> > +}
> > +
> > /**
> > * qobject_to_qlist(): Convert a QObject into a QList
> > */
> > diff --git a/qlist.h b/qlist.h
> > index d426bd4..ae776f9 100644
> > --- a/qlist.h
> > +++ b/qlist.h
> > @@ -49,6 +49,7 @@ void qlist_iter(const QList *qlist,
> > QObject *qlist_pop(QList *qlist);
> > QObject *qlist_peek(QList *qlist);
> > int qlist_empty(const QList *qlist);
> > +size_t qlist_size(const QList *qlist);
> > QList *qobject_to_qlist(const QObject *obj);
> >
> > static inline const QListEntry *qlist_first(const QList *qlist)
>
Re: [Qemu-devel] qemu-ga : Guest Agent : Windows 2008 : Unknown command guest-fsfreeze-freeze
On Mon, Aug 20, 2012 at 10:54:29AM -0300, Luiz Capitulino wrote: > On Fri, 17 Aug 2012 10:11:29 -0700 (PDT) > desi babu wrote: > > > Guest-Agent : Windows 2008 Error : Relase 1.1.90 > > > > error : internal error unable to execute QEMU command > > 'guest-fsfreeze-freeze': this feature or command is not currently supported. > > That's correct, fsfreze is not supported on Windows. > > > Guest-info shows the command is available.  Is there any information > > available on the list of commands supported inside Windows ? Appreciate if > > you have any pointers. > > That's a qemu-ga bug. CC'ing Michael to check if he has a fix in mind for > this. > I've been thinking about this one for a while. It's considered expected behavior, but I realize it sucks for discoverability. I doubt we want to do platform-specific QAPI schema definitions, so the only option I can think of is some kind of [de-]registration mechanism where we can mark commands as being not available for a particular build/platform in the cases where we stub out command implementations. I think we can expose this to existing clients by no longer listing commands marked as unsupported in the list provided by the guest-info command. It should "just work". Can probably do it for 1.3. For now, clients will have to catch it in the error-handling path.
Re: [Qemu-devel] [Bug 1038070] [NEW] > qemu-kvm-1.1.1-r1 - USB activkey doesn't work anymore
On Fri, Aug 17, 2012 at 12:50:14PM -, linuxale wrote: > Public bug reported: > > Linux Distro: Gentoo > > Smartcard Activkey doesn't work anymore. I use it without problem till version > qemu-kvm-1.0.1. > > Follow a log extraction: > 2012-08-14 16:27:34.751+: 5487: error : qemuProcessReadLogOutput:1298 : > internal error Process exited while reading console log output: char device > redirected to /dev/pts/40 > ccid-card-emulated: failed to initialize vcard > qemu-system-x86_64: -device > ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0: Device > 'ccid-card-emulated' could not be initialized > > 2012-08-14 16:28:01.018+: 5486: error : qemuProcessReadLogOutput:1298 : > internal error Process exited while reading console log output: char device > redirected to /dev/pts/40 > ccid-card-emulated: failed to initialize vcard > qemu-system-x86_64: -device > ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0: Device > 'ccid-card-emulated' could not be initialized > > If you need any other info please tell me. I've tried 1.1.1 and current upstream with a Windows 7 guest and the device seems to show up and function properly in both cases. One way that I *can* reproduce the error is by running your command-line with an NSS database at some place other than the default search path (/etc/pki/nssdb in 1.1 and upstream). I don't think this has changed since 1.0, but maybe something changed on the distro side? Can you try reproducing by compiling from upstream source? > > I've tried with git version with same problem. > > ** Affects: qemu > Importance: Undecided > Status: New > > -- > You received this bug notification because you are a member of qemu- > devel-ml, which is subscribed to QEMU. > https://bugs.launchpad.net/bugs/1038070 > > Title: > > qemu-kvm-1.1.1-r1 - USB activkey doesn't work anymore > > Status in QEMU: > New > > Bug description: > Linux Distro: Gentoo > > Smartcard Activkey doesn't work anymore. I use it without problem till > version > qemu-kvm-1.0.1. > > Follow a log extraction: > 2012-08-14 16:27:34.751+: 5487: error : qemuProcessReadLogOutput:1298 : > internal error Process exited while reading console log output: char device > redirected to /dev/pts/40 > ccid-card-emulated: failed to initialize vcard > qemu-system-x86_64: -device > ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0: Device > 'ccid-card-emulated' could not be initialized > > 2012-08-14 16:28:01.018+: 5486: error : qemuProcessReadLogOutput:1298 : > internal error Process exited while reading console log output: char device > redirected to /dev/pts/40 > ccid-card-emulated: failed to initialize vcard > qemu-system-x86_64: -device > ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0: Device > 'ccid-card-emulated' could not be initialized > > If you need any other info please tell me. > > I've tried with git version with same problem. > > To manage notifications about this bug go to: > https://bugs.launchpad.net/qemu/+bug/1038070/+subscriptions >
Re: [Qemu-devel] [PATCH for-1.2 v3] qemu-ga: report success-response field in guest-info
On Tue, Aug 21, 2012 at 02:39:35PM +0200, Michal Privoznik wrote:
> This is a useful hint for users (or libvirt) whether a command returns
> anything success and hence wait for reply or not.
I'm not necessarilly against this patch, but I think we should clarify it's
purpose.
Commands that don't return success should still be "waited" on, since
they may still return an error. You can only stop waiting once you
recieve an error, a client-side timeout, or some "external" indicator
in the case of things like shutdown and suspend.
For guest-suspend*, this has always been the case, so there's no reason
to probe for their behavior.
For guest-shutdown, the semantics did change, but were considered
backward-compatible in that the new behavior would trigger a
client-side timeout for older clients, which was always a required
aspect of the protocol (since it's a connectionless protocol, possibly
malicious agent, etc), and the success response was always documented as
async and do-nothing.
So we only need to worry about how new clients deal with guest-shutdown,
Specifically, whether or not we will get a success response from the
agent. But since the success response was always meaningless, why
couldn't we just throw it away and continue waiting for a shutdown event
from QMP, or querying query-status for runstate change?
Would this solve the ambiguity? If so, we can get just update the
guest-shutdown documentation to detail how to handle "success" responses from
older agents (ignore them, basically). I think this is a nicer way to
handle newer clients than adding more things to probe for.
> ---
> diff to v2:
> -fixed typo and field description
>
> diff to v1:
> -changed condition in qmp_command_reports_success per Paolo's advice
>
> qapi-schema-guest.json |6 +-
> qapi/qmp-core.h|1 +
> qapi/qmp-registry.c| 12
> qga/commands.c |1 +
> 4 files changed, 19 insertions(+), 1 deletions(-)
>
> diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
> index d955cf1..60872e5 100644
> --- a/qapi-schema-guest.json
> +++ b/qapi-schema-guest.json
> @@ -91,10 +91,14 @@
> #
> # @enabled: whether command is currently enabled by guest admin
> #
> +# @success-response: true unless the command does not respond on success
> +#See 'guest-shutdown' command for more detailed info.
> +#
> # Since 1.1.0
> ##
> { 'type': 'GuestAgentCommandInfo',
> - 'data': { 'name': 'str', 'enabled': 'bool' } }
> + 'data': { 'name': 'str', 'enabled': 'bool',
> +'success-response': 'bool'} }
>
> ##
> # @GuestAgentInfo
> diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h
> index 00446cf..271c3f2 100644
> --- a/qapi/qmp-core.h
> +++ b/qapi/qmp-core.h
> @@ -48,6 +48,7 @@ QObject *qmp_dispatch(QObject *request);
> void qmp_disable_command(const char *name);
> void qmp_enable_command(const char *name);
> bool qmp_command_is_enabled(const char *name);
> +bool qmp_command_reports_success(const char *name);
> char **qmp_get_command_list(void);
> QObject *qmp_build_error_object(Error *errp);
>
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index 5414613..3f7303c 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -77,6 +77,18 @@ bool qmp_command_is_enabled(const char *name)
> return false;
> }
>
> +bool qmp_command_reports_success(const char *name) {
> +QmpCommand *cmd;
> +
> +QTAILQ_FOREACH(cmd, &qmp_commands, node) {
> +if (strcmp(cmd->name, name) == 0) {
> +return (cmd->options & QCO_NO_SUCCESS_RESP) == 0;
> +}
> +}
> +
> +return true;
> +}
> +
> char **qmp_get_command_list(void)
> {
> QmpCommand *cmd;
> diff --git a/qga/commands.c b/qga/commands.c
> index 46b0b08..9811221 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -63,6 +63,7 @@ struct GuestAgentInfo *qmp_guest_info(Error **err)
> cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
> cmd_info->name = strdup(*cmd_list);
> cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
> +cmd_info->success_response =
> qmp_command_reports_success(cmd_info->name);
>
> cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
> cmd_info_list->value = cmd_info;
> --
> 1.7.8.6
>
>
[Qemu-devel] [PATCH for-1.2] libcacard: Fix segfault when no backend specified
When a user does not specify a backend, a segfault can be triggered due
to a strcmp() being called on a NULL char*:
$ gdb --args qemu -usb -device usb-ccid \
-device ccid-card-emulated,id=smartcard0 \
...
(gdb) s
emulated_initfn (base=0x56cb2e70) at
/home/mdroth/w/qemu.git/hw/ccid-card-emulated.c:503
503 card->backend = parse_enumeration(card->backend_str,
backend_enum_table, 0);
(gdb) s
parse_enumeration (not_found_value=0, table=0x55b9b3c0, str=0x0) at
/home/mdroth/w/qemu.git/hw/ccid-card-emulated.c:476
476 while (table->name != NULL) {
(gdb) s
477 if (strcmp(table->name, str) == 0) {
(gdb) s
Program received signal SIGSEGV, Segmentation fault.
0x760df09a in ?? () from /lib/x86_64-linux-gnu/libc.so.6
After the fix:
$ qemu -usb -device usb-ccid \
-device ccid-card-emulated,id=smartcard0 \
...
unknown backend, must be one of:
nss-emulated
certificates
qemu-system-x86_64: -device
ccid-card-emulated,id=smartcard0,backend=nss-certificates: Device
'ccid-card-emulated' could not be initialized
Signed-off-by: Michael Roth
---
hw/ccid-card-emulated.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/ccid-card-emulated.c b/hw/ccid-card-emulated.c
index f4a6da4..740e8df 100644
--- a/hw/ccid-card-emulated.c
+++ b/hw/ccid-card-emulated.c
@@ -473,6 +473,9 @@ static uint32_t parse_enumeration(char *str,
{
uint32_t ret = not_found_value;
+if (!str) {
+return ret;
+}
while (table->name != NULL) {
if (strcmp(table->name, str) == 0) {
ret = table->value;
--
1.7.9.5
[Qemu-devel] [PATCH 16/23] check-qjson: add test for large JSON objects
Signed-off-by: Michael Roth
Signed-off-by: Anthony Liguori
(cherry picked from commit 7109edfeb69c1d3c2164175837784dfcd210fed0)
Signed-off-by: Michael Roth
---
tests/check-qjson.c | 53 +++
1 file changed, 53 insertions(+)
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 526e25e..3b896f5 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -466,6 +466,58 @@ static void simple_dict(void)
}
}
+/*
+ * this generates json of the form:
+ * a(0,m) = [0, 1, ..., m-1]
+ * a(n,m) = {
+ *'key0': a(0,m),
+ *'key1': a(1,m),
+ *...
+ *'key(n-1)': a(n-1,m)
+ * }
+ */
+static void gen_test_json(GString *gstr, int nest_level_max,
+ int elem_count)
+{
+int i;
+
+g_assert(gstr);
+if (nest_level_max == 0) {
+g_string_append(gstr, "[");
+for (i = 0; i < elem_count; i++) {
+g_string_append_printf(gstr, "%d", i);
+if (i < elem_count - 1) {
+g_string_append_printf(gstr, ", ");
+}
+}
+g_string_append(gstr, "]");
+return;
+}
+
+g_string_append(gstr, "{");
+for (i = 0; i < nest_level_max; i++) {
+g_string_append_printf(gstr, "'key%d': ", i);
+gen_test_json(gstr, i, elem_count);
+if (i < nest_level_max - 1) {
+g_string_append(gstr, ",");
+}
+}
+g_string_append(gstr, "}");
+}
+
+static void large_dict(void)
+{
+GString *gstr = g_string_new("");
+QObject *obj;
+
+gen_test_json(gstr, 10, 100);
+obj = qobject_from_json(gstr->str);
+g_assert(obj != NULL);
+
+qobject_decref(obj);
+g_string_free(gstr, true);
+}
+
static void simple_list(void)
{
int i;
@@ -706,6 +758,7 @@ int main(int argc, char **argv)
g_test_add_func("/literals/keyword", keyword_literal);
g_test_add_func("/dicts/simple_dict", simple_dict);
+g_test_add_func("/dicts/large_dict", large_dict);
g_test_add_func("/lists/simple_list", simple_list);
g_test_add_func("/whitespace/simple_whitespace", simple_whitespace);
--
1.7.9.5
[Qemu-devel] [PATCH 02/23] configure: Don't override user's --cpu on MacOS and Solaris
From: Peter Maydell
Both MacOS and Solaris have special case handling for the CPU
type, because the check_define probes will return i386 even if
the hardware is 64 bit and x86_64 would be preferable. Move
these checks earlier in the configure probing so that we can
do them only if the user didn't specify a CPU with --cpu. This
fixes a bug where the user's command line argument was being
ignored.
Reviewed-by: Andreas F=E4rber
Signed-off-by: Peter Maydell
Signed-off-by: Anthony Liguori
(cherry picked from commit bbea4050802a2e7e0296a21823c0925782c02b93)
Signed-off-by: Michael Roth
---
configure | 60 +++-
1 file changed, 35 insertions(+), 25 deletions(-)
diff --git a/configure b/configure
index 72d16a4..df29c2f 100755
--- a/configure
+++ b/configure
@@ -275,6 +275,41 @@ EOF
compile_object
}
+if check_define __linux__ ; then
+ targetos="Linux"
+elif check_define _WIN32 ; then
+ targetos='MINGW32'
+elif check_define __OpenBSD__ ; then
+ targetos='OpenBSD'
+elif check_define __sun__ ; then
+ targetos='SunOS'
+elif check_define __HAIKU__ ; then
+ targetos='Haiku'
+else
+ targetos=`uname -s`
+fi
+
+# Some host OSes need non-standard checks for which CPU to use.
+# Note that these checks are broken for cross-compilation: if you're
+# cross-compiling to one of these OSes then you'll need to specify
+# the correct CPU with the --cpu option.
+case $targetos in
+Darwin)
+ # on Leopard most of the system is 32-bit, so we have to ask the kernel if
we can
+ # run 64-bit userspace code.
+ # If the user didn't specify a CPU explicitly and the kernel says this is
+ # 64 bit hw, then assume x86_64. Otherwise fall through to the usual
detection code.
+ if test -z "$cpu" && test "$(sysctl -n hw.optional.x86_64)" = "1"; then
+cpu="x86_64"
+ fi
+ ;;
+SunOS)
+ # `uname -m` returns i86pc even on an x86_64 box, so default based on isainfo
+ if test -z "$cpu" && test "$(isainfo -k)" = "amd64"; then
+cpu="x86_64"
+ fi
+esac
+
if test ! -z "$cpu" ; then
# command line argument
:
@@ -349,19 +384,6 @@ if test -z "$ARCH"; then
fi
# OS specific
-if check_define __linux__ ; then
- targetos="Linux"
-elif check_define _WIN32 ; then
- targetos='MINGW32'
-elif check_define __OpenBSD__ ; then
- targetos='OpenBSD'
-elif check_define __sun__ ; then
- targetos='SunOS'
-elif check_define __HAIKU__ ; then
- targetos='Haiku'
-else
- targetos=`uname -s`
-fi
case $targetos in
CYGWIN*)
@@ -411,12 +433,6 @@ OpenBSD)
Darwin)
bsd="yes"
darwin="yes"
- # on Leopard most of the system is 32-bit, so we have to ask the kernel it
if we can
- # run 64-bit userspace code
- if [ "$cpu" = "i386" ] ; then
-is_x86_64=`sysctl -n hw.optional.x86_64`
-[ "$is_x86_64" = "1" ] && cpu=x86_64
- fi
if [ "$cpu" = "x86_64" ] ; then
QEMU_CFLAGS="-arch x86_64 $QEMU_CFLAGS"
LDFLAGS="-arch x86_64 $LDFLAGS"
@@ -437,12 +453,6 @@ SunOS)
smbd="${SMBD-/usr/sfw/sbin/smbd}"
needs_libsunmath="no"
solarisrev=`uname -r | cut -f2 -d.`
- # have to select again, because `uname -m` returns i86pc
- # even on an x86_64 box.
- solariscpu=`isainfo -k`
- if test "${solariscpu}" = "amd64" ; then
-cpu="x86_64"
- fi
if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
if test "$solarisrev" -le 9 ; then
if test -f /opt/SUNWspro/prod/lib/libsunmath.so.1; then
--
1.7.9.5
[Qemu-devel] [PATCH 23/23] update VERSION for 1.1.2
Signed-off-by: Michael Roth --- VERSION |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 524cb55..45a1b3f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.1.1 +1.1.2 -- 1.7.9.5
[Qemu-devel] [PATCH 15/23] json-parser: don't replicate tokens at each level of recursion
Currently, when parsing a stream of tokens we make a copy of the token
list at the beginning of each level of recursion so that we do not
modify the original list in cases where we need to fall back to an
earlier state.
In the worst case, we will only read 1 or 2 tokens off the list before
recursing again, which means an upper bound of roughly N^2 token allocations.
For a "reasonably" sized QMP request (in this a QMP representation of
cirrus_vga's device state, generated via QIDL, being passed in via
qom-set), this caused my 16GB's of memory to be exhausted before any
noticeable progress was made by the parser.
This patch works around the issue by using single copy of the token list
in the form of an indexable array so that we can save/restore state by
manipulating indices.
A subsequent commit adds a "large_dict" test case which exhibits the
same behavior as above. With this patch applied the test case successfully
completes in under a second.
Tested with valgrind, make check, and QMP.
Reviewed-by: Eric Blake
Signed-off-by: Michael Roth
Signed-off-by: Anthony Liguori
(cherry picked from commit 65c0f1e9558c7c762cdb333406243fff1d687117)
Signed-off-by: Michael Roth
---
json-parser.c | 230 +++--
1 file changed, 142 insertions(+), 88 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 849e215..457291b 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -27,6 +27,11 @@
typedef struct JSONParserContext
{
Error *err;
+struct {
+QObject **buf;
+size_t pos;
+size_t count;
+} tokens;
} JSONParserContext;
#define BUG_ON(cond) assert(!(cond))
@@ -40,7 +45,7 @@ typedef struct JSONParserContext
* 4) deal with premature EOI
*/
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list
*ap);
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
/**
* Token manipulators
@@ -270,27 +275,111 @@ out:
return NULL;
}
+static QObject *parser_context_pop_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+ctxt->tokens.pos++;
+return token;
+}
+
+/* Note: parser_context_{peek|pop}_token do not increment the
+ * token object's refcount. In both cases the references will continue
+ * to be tracked and cleaned up in parser_context_free(), so do not
+ * attempt to free the token object.
+ */
+static QObject *parser_context_peek_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+return token;
+}
+
+static JSONParserContext parser_context_save(JSONParserContext *ctxt)
+{
+JSONParserContext saved_ctxt = {0};
+saved_ctxt.tokens.pos = ctxt->tokens.pos;
+saved_ctxt.tokens.count = ctxt->tokens.count;
+saved_ctxt.tokens.buf = ctxt->tokens.buf;
+return saved_ctxt;
+}
+
+static void parser_context_restore(JSONParserContext *ctxt,
+ JSONParserContext saved_ctxt)
+{
+ctxt->tokens.pos = saved_ctxt.tokens.pos;
+ctxt->tokens.count = saved_ctxt.tokens.count;
+ctxt->tokens.buf = saved_ctxt.tokens.buf;
+}
+
+static void tokens_append_from_iter(QObject *obj, void *opaque)
+{
+JSONParserContext *ctxt = opaque;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ctxt->tokens.buf[ctxt->tokens.pos++] = obj;
+qobject_incref(obj);
+}
+
+static JSONParserContext *parser_context_new(QList *tokens)
+{
+JSONParserContext *ctxt;
+size_t count;
+
+if (!tokens) {
+return NULL;
+}
+
+count = qlist_size(tokens);
+if (count == 0) {
+return NULL;
+}
+
+ctxt = g_malloc0(sizeof(JSONParserContext));
+ctxt->tokens.pos = 0;
+ctxt->tokens.count = count;
+ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
+qlist_iter(tokens, tokens_append_from_iter, ctxt);
+ctxt->tokens.pos = 0;
+
+return ctxt;
+}
+
+/* to support error propagation, ctxt->err must be freed separately */
+static void parser_context_free(JSONParserContext *ctxt)
+{
+int i;
+if (ctxt) {
+for (i = 0; i < ctxt->tokens.count; i++) {
+qobject_decref(ctxt->tokens.buf[i]);
+}
+g_free(ctxt->tokens.buf);
+g_free(ctxt);
+}
+}
+
/**
* Parsing rules
*/
-static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens,
va_list *ap)
+static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
{
QObject *key = NULL, *token = NULL, *value, *peek;
-QList *working = qlist_copy(*tokens);
+JSONParserContext saved_ctxt = parser_context_save(ctxt);
-peek = qlist_peek(working);
+peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
[Qemu-devel] [PATCH 06/23] qdev: fix use-after-free in the error path of qdev_init_nofail
From: Anthony Liguori
>From Markus:
Before:
$ qemu-system-x86_64 -display none -drive if=ide
qemu-system-x86_64: Device needs media, but drive is empty
qemu-system-x86_64: Initialization of device ide-hd failed
[Exit 1 ]
After:
$ qemu-system-x86_64 -display none -drive if=ide
qemu-system-x86_64: Device needs media, but drive is empty
Segmentation fault (core dumped)
[Exit 139 (SIGSEGV)]
This error always existed as qdev_init() frees the object. But QOM
goes a bit further and purposefully sets the class pointer to NULL to
help find use-after-free. It worked :-)
Cc: Andreas Faerber
Reported-by: Markus Armbruster
Signed-off-by: Anthony Liguori
(cherry picked from commit 7de3abe505e34398cef5bddf6c4d0bd9ee47007f)
Signed-off-by: Michael Roth
---
hw/qdev.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index af419b9..8e8ca3f 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -256,9 +256,10 @@ int qdev_simple_unplug_cb(DeviceState *dev)
way is somewhat unclean, and best avoided. */
void qdev_init_nofail(DeviceState *dev)
{
+const char *typename = object_get_typename(OBJECT(dev));
+
if (qdev_init(dev) < 0) {
-error_report("Initialization of device %s failed",
- object_get_typename(OBJECT(dev)));
+error_report("Initialization of device %s failed", typename);
exit(1);
}
}
--
1.7.9.5
[Qemu-devel] [PATCH 05/23] kvmvapic: Disable if there is insufficient memory
From: Jan Kiszka
We need at least 1M of RAM to map the option ROM. Otherwise, we will
corrupt host memory or even crash:
$ qemu-system-x86_64 -nodefaults --enable-kvm -vnc :0 -m 640k
Segmentation fault (core dumped)
Reported-and-tested-by: Markus Armbruster
Signed-off-by: Jan Kiszka
Signed-off-by: Marcelo Tosatti
(cherry picked from commit a9605e0317c7a6d5e68f3a3b6708c8ef1096f4bc)
Signed-off-by: Michael Roth
---
hw/apic_common.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/apic_common.c b/hw/apic_common.c
index 60b8259..e4612bb 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -289,7 +289,9 @@ static int apic_init_common(SysBusDevice *dev)
sysbus_init_mmio(dev, &s->io_memory);
-if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
+/* Note: We need at least 1M to map the VAPIC option ROM */
+if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
+ram_size >= 1024 * 1024) {
vapic = sysbus_create_simple("kvmvapic", -1, NULL);
}
s->vapic = vapic;
--
1.7.9.5
[Qemu-devel] [PATCH 01/23] qtest: fix infinite loop when QEMU aborts abruptly
From: Anthony Liguori
>From Markus:
Makes "make check" hang:
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 gtester -k --verbose
-m=quick tests/crash-test tests/rtc-test
TEST: tests/crash-test... (pid=972)
qemu-system-x86_64: Device needs media, but drive is empty
[Nothing happens, wait a while, then hit ^C]
make: *** [check-qtest-x86_64] Interrupt
This was due to the fact that we weren't checked for errors when
reading from the QMP socket. This patch adds appropriate error
checking.
Reported-by: Markus Armbruster
Signed-off-by: Anthony Liguori
(cherry picked from commit 039380a8e18f618cdacf72486449c04dc1b70eef)
Signed-off-by: Michael Roth
---
tests/libqtest.c |5 +
1 file changed, 5 insertions(+)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 6d333ef..0664323 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -290,6 +290,11 @@ void qtest_qmp(QTestState *s, const char *fmt, ...)
continue;
}
+if (len == -1 || len == 0) {
+fprintf(stderr, "Broken pipe\n");
+exit(1);
+}
+
switch (c) {
case '{':
nesting++;
--
1.7.9.5
[Qemu-devel] [PATCH 22/23] apic: Defer interrupt updates to VCPU thread
From: Jan Kiszka
KVM performs TPR raising asynchronously to QEMU, specifically outside
QEMU's global lock. When an interrupt is injected into the APIC and TPR
is checked to decide if this can be delivered, a stale TPR value may be
used, causing spurious interrupts in the end.
Fix this by deferring apic_update_irq to the context of the target VCPU.
We introduce a new interrupt flag for this, CPU_INTERRUPT_POLL. When it
is set, the VCPU calls apic_poll_irq before checking for further pending
interrupts. To avoid special-casing KVM, we also implement this logic
for TCG mode.
Signed-off-by: Jan Kiszka
Signed-off-by: Avi Kivity
(cherry picked from commit 5d62c43a17edaa7f6a88821c9086e6c8e0e5327d)
Signed-off-by: Michael Roth
---
cpu-exec.c |6 ++
hw/apic.c |5 -
hw/apic.h |1 +
hw/apic_internal.h |1 -
target-i386/cpu.h |4 +++-
target-i386/kvm.c |4
6 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index 0344cd5..6db32cd 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -285,6 +285,12 @@ int cpu_exec(CPUArchState *env)
}
#endif
#if defined(TARGET_I386)
+#if !defined(CONFIG_USER_ONLY)
+if (interrupt_request & CPU_INTERRUPT_POLL) {
+env->interrupt_request &= ~CPU_INTERRUPT_POLL;
+apic_poll_irq(env->apic_state);
+}
+#endif
if (interrupt_request & CPU_INTERRUPT_INIT) {
svm_check_intercept(env, SVM_EXIT_INIT);
do_cpu_init(env);
diff --git a/hw/apic.c b/hw/apic.c
index e96402d..b2c6373 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -16,6 +16,7 @@
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, see <http://www.gnu.org/licenses/>
*/
+#include "qemu-thread.h"
#include "apic_internal.h"
#include "apic.h"
#include "ioapic.h"
@@ -369,7 +370,9 @@ static void apic_update_irq(APICCommonState *s)
if (!(s->spurious_vec & APIC_SV_ENABLE)) {
return;
}
-if (apic_irq_pending(s) > 0) {
+if (!qemu_cpu_is_self(s->cpu_env)) {
+cpu_interrupt(s->cpu_env, CPU_INTERRUPT_POLL);
+} else if (apic_irq_pending(s) > 0) {
cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
}
}
diff --git a/hw/apic.h b/hw/apic.h
index 62179ce..a89542b 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -20,6 +20,7 @@ void apic_init_reset(DeviceState *s);
void apic_sipi(DeviceState *s);
void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
TPRAccess access);
+void apic_poll_irq(DeviceState *d);
/* pc.c */
int cpu_is_bsp(CPUX86State *env);
diff --git a/hw/apic_internal.h b/hw/apic_internal.h
index 60a6a8b..4d8ff49 100644
--- a/hw/apic_internal.h
+++ b/hw/apic_internal.h
@@ -141,7 +141,6 @@ void apic_report_irq_delivered(int delivered);
bool apic_next_timer(APICCommonState *s, int64_t current_time);
void apic_enable_tpr_access_reporting(DeviceState *d, bool enable);
void apic_enable_vapic(DeviceState *d, target_phys_addr_t paddr);
-void apic_poll_irq(DeviceState *d);
void vapic_report_tpr_access(DeviceState *dev, void *cpu, target_ulong ip,
TPRAccess access);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 2460f63..8ff3f30 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -477,6 +477,7 @@
for syscall instruction */
/* i386-specific interrupt pending bits. */
+#define CPU_INTERRUPT_POLL CPU_INTERRUPT_TGT_EXT_1
#define CPU_INTERRUPT_SMI CPU_INTERRUPT_TGT_EXT_2
#define CPU_INTERRUPT_NMI CPU_INTERRUPT_TGT_EXT_3
#define CPU_INTERRUPT_MCE CPU_INTERRUPT_TGT_EXT_4
@@ -1029,7 +1030,8 @@ static inline void cpu_clone_regs(CPUX86State *env,
target_ulong newsp)
static inline bool cpu_has_work(CPUX86State *env)
{
-return ((env->interrupt_request & CPU_INTERRUPT_HARD) &&
+return ((env->interrupt_request & (CPU_INTERRUPT_HARD |
+ CPU_INTERRUPT_POLL)) &&
(env->eflags & IF_MASK)) ||
(env->interrupt_request & (CPU_INTERRUPT_NMI |
CPU_INTERRUPT_INIT |
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index e74a9e4..d8bbe4f 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1725,6 +1725,10 @@ int kvm_arch_process_async_events(CPUX86State *env)
return 0;
}
+if (env->interrupt_request & CPU_INTERRUPT_POLL) {
+env->interrupt_request &= ~CPU_INTERRUPT_POLL;
+apic_poll_irq(env->apic_state);
+}
if (((env->interrupt_request & CPU_INTERRUPT_HARD) &&
(env->eflags & IF_MASK)) ||
(env->interrupt_request & CPU_INTERRUPT_NMI)) {
--
1.7.9.5
[Qemu-devel] [PATCH 14/23] qlist: add qlist_size()
Signed-off-by: Michael Roth
Reviewed-by: Eric Blake
Signed-off-by: Michael Roth
Signed-off-by: Anthony Liguori
(cherry picked from commit a86a4c2f7b7f0b72816ea1c219d8140699b6665b)
Signed-off-by: Michael Roth
---
qlist.c | 13 +
qlist.h |1 +
2 files changed, 14 insertions(+)
diff --git a/qlist.c b/qlist.c
index 88498b1..b48ec5b 100644
--- a/qlist.c
+++ b/qlist.c
@@ -124,6 +124,19 @@ int qlist_empty(const QList *qlist)
return QTAILQ_EMPTY(&qlist->head);
}
+static void qlist_size_iter(QObject *obj, void *opaque)
+{
+size_t *count = opaque;
+(*count)++;
+}
+
+size_t qlist_size(const QList *qlist)
+{
+size_t count = 0;
+qlist_iter(qlist, qlist_size_iter, &count);
+return count;
+}
+
/**
* qobject_to_qlist(): Convert a QObject into a QList
*/
diff --git a/qlist.h b/qlist.h
index d426bd4..ae776f9 100644
--- a/qlist.h
+++ b/qlist.h
@@ -49,6 +49,7 @@ void qlist_iter(const QList *qlist,
QObject *qlist_pop(QList *qlist);
QObject *qlist_peek(QList *qlist);
int qlist_empty(const QList *qlist);
+size_t qlist_size(const QList *qlist);
QList *qobject_to_qlist(const QObject *obj);
static inline const QListEntry *qlist_first(const QList *qlist)
--
1.7.9.5
[Qemu-devel] [PATCH 11/23] usb: restore USBDevice->attached on vmload
From: Gerd Hoffmann
Signed-off-by: Gerd Hoffmann
(cherry picked from commit 495d544798151206bafca65ec588c0388637eb40)
Signed-off-by: Michael Roth
---
hw/usb/bus.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 2068640..77b2b99 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -27,10 +27,23 @@ static struct BusInfo usb_bus_info = {
static int next_usb_bus = 0;
static QTAILQ_HEAD(, USBBus) busses = QTAILQ_HEAD_INITIALIZER(busses);
+static int usb_device_post_load(void *opaque, int version_id)
+{
+USBDevice *dev = opaque;
+
+if (dev->state == USB_STATE_NOTATTACHED) {
+dev->attached = 0;
+} else {
+dev->attached = 1;
+}
+return 0;
+}
+
const VMStateDescription vmstate_usb_device = {
.name = "USBDevice",
.version_id = 1,
.minimum_version_id = 1,
+.post_load = usb_device_post_load,
.fields = (VMStateField []) {
VMSTATE_UINT8(addr, USBDevice),
VMSTATE_INT32(state, USBDevice),
--
1.7.9.5
[Qemu-devel] [PATCH 09/23] ehci: don't flush cache on doorbell rings.
From: Gerd Hoffmann
Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
zap any unlinked queue heads when the guest rings the doorbell.
While hacking up uas support this turned out to be a problem. The linux
kernel can unlink and instantly relink the very same queue head, thereby
killing any async packets in flight. That alone isn't an issue yet, the
packet will canceled and resubmitted and everything is fine. We'll run
into trouble though in case the async packet is completed already, so we
can't cancel it any more. The transaction is simply lost then.
usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f122 qtds
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f122 qtds
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f0c2 qtds
29dbce40,29dbc4e0,0009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: alloc
usb_packet_state_change bus 0, port 2, ep 1, packet 0x7f95fdec32e0, state undef
-> setup
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: process
usb_uas_command dev 2, tag 0x2, lun 0, lun64 -
scsi_req_parsed target 0 lun 0 tag 2 command 42 dir 2 length 16384
scsi_req_parsed_lba target 0 lun 0 tag 2 command 42 lba 5933312
scsi_req_alloc target 0 lun 0 tag 2
scsi_req_continue target 0 lun 0 tag 2
scsi_req_data target 0 lun 0 tag 2 len 16384
usb_uas_scsi_data dev 2, tag 0x2, bytes 16384
usb_uas_write_ready dev 2, tag 0x2
usb_packet_state_change bus 0, port 2, ep 1, packet 0x7f95fdec32e0, state setup
-> complete
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: free
usb_ehci_qh_ptrs q 0x7f95fdec3210 - QH @ 39c4f0c0: next 39c4f002 qtds
29dbce40,0001,0009
usb_ehci_qh_fields QH @ 39c4f0c0 - rl 4, mplen 512, eps 2, ep 2, dev 2
usb_ehci_queue_action q 0x7f95fe5152a0: free
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state async
-> complete
^^^ async packets completes.
usb_ehci_packet_action q 0x7f95fdec3210 p 0x7f95feba9130: wakeup
usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f122 qtds
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f122 qtds
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f002 qtds
29dbc4e0,29dbc8a0,0009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2
usb_ehci_queue_action q 0x7f95fdec3210: free
usb_ehci_packet_action q 0x7f95fdec3210 p 0x7f95feba9130: free
^^^ endpoint #2 queue head removed from schedule, doorbell makes ehci zap the
queue,
the (completed) usb packet is freed too and gets lost.
usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f0c2 qtds
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f0c2 qtds
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_queue_action q 0x7f9600dff570: alloc
usb_ehci_qh_ptrs q 0x7f9600dff570 - QH @ 39c4f0c0: next 39c4f122 qtds
29dbce40,0001,0009
usb_ehci_qh_fields QH @ 39c4f0c0 - rl 4, mplen 512, eps 2, ep 2, dev 2
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: alloc
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state undef
-> setup
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: process
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state setup
-> async
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: async
^^^ linux kernel relinked the queue head, ehci creates a new usb packet,
but we should have delivered the completed one instead.
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f002 qtds
29dbc4e0,29dbc8a0,0009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2
So instead of instantly zapping the queue we'll set a flag that the
queue needs revalidation in case we'll see it again in the schedule.
ehci then checks that the queue head fields addressing / describing the
endpoint and the qtd pointer match the cached content before reusing it.
Cc: Hans de Goede
Signed-off-by: Gerd Hoffmann
(cherry picked from commit 9bc3a3a216e2689bfcdd36c3e079333bbdbf3ba0)
Conflicts:
hw/usb/hcd-ehci.c
Signed-off-by: Michael Roth
---
hw/usb/hcd-ehci.c | 34 --
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 5855c5d..ce8b405 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -348,6 +348,7 @@ struct
[Qemu-devel] [PATCH 18/23] slirp: Ensure smbd and shared directory exist when enable smb
From: Dunrong Huang
Users may pass the following parameters to qemu:
$ qemu-kvm -net nic -net user,smb= ...
$ qemu-kvm -net nic -net user,smb ...
$ qemu-kvm -net nic -net user,smb=bad_directory ...
In these cases, qemu started successfully while samba server
failed to start. Users will confuse since samba server
failed silently without any indication of what it did wrong.
To avoid it, we check whether the shared directory exist and
if users have permission to access this directory when QEMU's
"built-in" SMB server is enabled.
Signed-off-by: Dunrong Huang
Signed-off-by: Jan Kiszka
(cherry picked from commit 927d811b282ffdf5386bd63f435c1507634ba49a)
Signed-off-by: Michael Roth
---
net/slirp.c | 12
1 file changed, 12 insertions(+)
diff --git a/net/slirp.c b/net/slirp.c
index c73610e..d3fcbf4 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -497,6 +497,18 @@ static int slirp_smb(SlirpState* s, const char
*exported_dir,
return -1;
}
+if (access(CONFIG_SMBD_COMMAND, F_OK)) {
+error_report("could not find '%s', please install it",
+ CONFIG_SMBD_COMMAND);
+return -1;
+}
+
+if (access(exported_dir, R_OK | X_OK)) {
+error_report("no such directory '%s', or you do not have permission "
+ "to access it, please check it", exported_dir);
+return -1;
+}
+
snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.%ld-%d",
(long)getpid(), instance++);
if (mkdir(s->smb_dir, 0700) < 0) {
--
1.7.9.5
[Qemu-devel] [PATCH 04/23] s390: Fix error handling and condition code of service call
From: Christian Borntraeger
Invalid sccb addresses will cause specification or addressing exception.
Lets add those checks. Furthermore, the good case (cc=0) was incorrect
for KVM, we did not set the CC at all. We now use return codes < 0
as program checks and return codes > 0 as condition code values.
Signed-off-by: Christian Borntraeger
Signed-off-by: Alexander Graf
(cherry picked from commit 9abf567d95a4e840df868ca993219175fbef8c22)
Signed-off-by: Michael Roth
---
target-s390x/kvm.c |5 +++--
target-s390x/op_helper.c | 27 ++-
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 90aad61..e4e6f15 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -238,9 +238,10 @@ static int kvm_sclp_service_call(CPUS390XState *env,
struct kvm_run *run,
code = env->regs[(ipbh0 & 0xf0) >> 4];
r = sclp_service_call(env, sccb, code);
-if (r) {
-setcc(env, 3);
+if (r < 0) {
+enter_pgmcheck(env, -r);
}
+setcc(env, r);
return 0;
}
diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c
index 7b72473..91dd8dc 100644
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -19,6 +19,8 @@
*/
#include "cpu.h"
+#include "memory.h"
+#include "cputlb.h"
#include "dyngen-exec.h"
#include "host-utils.h"
#include "helper.h"
@@ -2366,6 +2368,9 @@ static void ext_interrupt(CPUS390XState *env, int type,
uint32_t param,
cpu_inject_ext(env, type, param, param64);
}
+/*
+ * ret < 0 indicates program check, ret = 0,1,2,3 -> cc
+ */
int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code)
{
int r = 0;
@@ -2375,10 +2380,12 @@ int sclp_service_call(CPUS390XState *env, uint32_t
sccb, uint64_t code)
printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code);
#endif
+/* basic checks */
+if (!memory_region_is_ram(phys_page_find(sccb >> TARGET_PAGE_BITS)->mr)) {
+return -PGM_ADDRESSING;
+}
if (sccb & ~0x7ff8ul) {
-fprintf(stderr, "KVM: invalid sccb address 0x%x\n", sccb);
-r = -1;
-goto out;
+return -PGM_SPECIFICATION;
}
switch(code) {
@@ -2405,22 +2412,24 @@ int sclp_service_call(CPUS390XState *env, uint32_t
sccb, uint64_t code)
#ifdef DEBUG_HELPER
printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb,
code);
#endif
-r = -1;
+r = 3;
break;
}
-out:
return r;
}
/* SCLP service call */
uint32_t HELPER(servc)(uint32_t r1, uint64_t r2)
{
-if (sclp_service_call(env, r1, r2)) {
-return 3;
-}
+int r;
-return 0;
+r = sclp_service_call(env, r1, r2);
+if (r < 0) {
+program_interrupt(env, -r, 4);
+return 0;
+}
+return r;
}
/* DIAG */
--
1.7.9.5
[Qemu-devel] [PATCH 03/23] ppc: Fix bug in handling of PAPR hypercall exits
From: David Gibson
Currently for powerpc, kvm_arch_handle_exit() always returns 1, meaning
that its caller - kvm_cpu_exec() - will always exit immediately afterwards
to the loop in qemu_kvm_cpu_thread_fn().
There's no need to do this. Once we've handled the hypercall there's no
reason we can't go straight around and KVM_RUN again, which is what ret = 0
will signal. The only exception might be for hypercalls which affect the
state of cpu_can_run(), however the only one that might do this is H_CEDE
and for kvm that is always handled in the kernel, not qemu.
Furtherm setting ret = 0 means that when exit_requested is set from a
hypercall, we will enter KVM_RUN once more with a signal which lets the
the kernel do its internal logic to complete the hypercall with out
actually executing any more guest code. This is important if our hypercall
also triggered a reset, which previously would re-initialize everything
without completing the hypercall. This caused the kernel to get confused
because it thought the guest was still in the middle of a hypercall when
it has actually been reset.
This patch therefore changes to ret = 0, which is both a bugfix and a small
optimization.
Signed-off-by: David Gibson
Signed-off-by: Alexander Graf
(cherry picked from commit 78e8fde26c032931ca2ae13bfc7c59e38afd17ee)
Signed-off-by: Michael Roth
---
target-ppc/kvm.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index c09cc39..29997af 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -558,7 +558,7 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run
*run)
dprintf("handle PAPR hypercall\n");
run->papr_hcall.ret = spapr_hypercall(env, run->papr_hcall.nr,
run->papr_hcall.args);
-ret = 1;
+ret = 0;
break;
#endif
default:
--
1.7.9.5
[Qemu-devel] [PATCH 07/23] virtio-blk: fix use-after-free while handling scsi commands
From: Avi Kivity The scsi passthrough handler falls through after completing a request into the failure path, resulting in a use after free. Reproducible by running a guest with aio=native on a block device. Reported-by: Stefan Priebe Signed-off-by: Avi Kivity Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf (cherry picked from commit 730a9c53b4e52681fcfe31cf38854cbf91e132c7) Signed-off-by: Michael Roth --- hw/virtio-blk.c |1 + 1 file changed, 1 insertion(+) diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index fe07746..f44d244 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -253,6 +253,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) virtio_blk_req_complete(req, status); g_free(req); +return; #else abort(); #endif -- 1.7.9.5
[Qemu-devel] [PATCH 21/23] apic: Reevaluate pending interrupts on LVT_LINT0 changes
From: Jan Kiszka
When the guest modifies the LVT_LINT0 register, we need to check if some
pending PIC interrupt can now be delivered.
Signed-off-by: Jan Kiszka
Signed-off-by: Avi Kivity
(cherry picked from commit a94820ddc36f8c452b37f9dcb323f55ffdbc75f9)
Signed-off-by: Michael Roth
---
hw/apic.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/apic.c b/hw/apic.c
index ec8f9ba..e96402d 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -540,6 +540,15 @@ static void apic_deliver(DeviceState *d, uint8_t dest,
uint8_t dest_mode,
apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
}
+static bool apic_check_pic(APICCommonState *s)
+{
+if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) {
+return false;
+}
+apic_deliver_pic_intr(&s->busdev.qdev, 1);
+return true;
+}
+
int apic_get_interrupt(DeviceState *d)
{
APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
@@ -567,9 +576,7 @@ int apic_get_interrupt(DeviceState *d)
apic_sync_vapic(s, SYNC_TO_VAPIC);
/* re-inject if there is still a pending PIC interrupt */
-if (apic_accept_pic_intr(&s->busdev.qdev) && pic_get_output(isa_pic)) {
-apic_deliver_pic_intr(&s->busdev.qdev, 1);
-}
+apic_check_pic(s);
apic_update_irq(s);
@@ -812,8 +819,11 @@ static void apic_mem_writel(void *opaque,
target_phys_addr_t addr, uint32_t val)
{
int n = index - 0x32;
s->lvt[n] = val;
-if (n == APIC_LVT_TIMER)
+if (n == APIC_LVT_TIMER) {
apic_timer_update(s, qemu_get_clock_ns(vm_clock));
+} else if (n == APIC_LVT_LINT0 && apic_check_pic(s)) {
+apic_update_irq(s);
+}
}
break;
case 0x38:
--
1.7.9.5
[Qemu-devel] [PATCH 20/23] apic: Resolve potential endless loop around apic_update_irq
From: Jan Kiszka
Commit d96e173769 refactored the reinjection of pending PIC interrupts.
However, it missed the potential loop of apic_update_irq ->
apic_deliver_pic_intr -> apic_local_deliver -> apic_set_irq ->
apic_update_irq that /could/ occur if LINT0 is injected as APIC_DM_FIXED
and that vector is currently blocked via TPR.
Resolve this by reinjecting only where it matters: inside
apic_get_interrupt. This function may clear a vector while a
PIC-originated reason still exists.
Signed-off-by: Jan Kiszka
Signed-off-by: Avi Kivity
(cherry picked from commit 3db3659bf60094657e1465cc809acb09551816ee)
Signed-off-by: Michael Roth
---
hw/apic.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/apic.c b/hw/apic.c
index 4eeaf88..ec8f9ba 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -371,9 +371,6 @@ static void apic_update_irq(APICCommonState *s)
}
if (apic_irq_pending(s) > 0) {
cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
-} else if (apic_accept_pic_intr(&s->busdev.qdev) &&
- pic_get_output(isa_pic)) {
-apic_deliver_pic_intr(&s->busdev.qdev, 1);
}
}
@@ -568,7 +565,14 @@ int apic_get_interrupt(DeviceState *d)
reset_bit(s->irr, intno);
set_bit(s->isr, intno);
apic_sync_vapic(s, SYNC_TO_VAPIC);
+
+/* re-inject if there is still a pending PIC interrupt */
+if (apic_accept_pic_intr(&s->busdev.qdev) && pic_get_output(isa_pic)) {
+apic_deliver_pic_intr(&s->busdev.qdev, 1);
+}
+
apic_update_irq(s);
+
return intno;
}
--
1.7.9.5
[Qemu-devel] [PATCH 13/23] usb-ehci: Fix an assert whenever isoc transfers are used
From: Hans de Goede hcd-ehci.c is missing an usb_packet_init() call for the ipacket UsbPacket it uses for isoc transfers, triggering an assert (taking the entire vm down) in usb_packet_setup as soon as any isoc transfers are done by a high speed USB device. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann (cherry picked from commit 7341ea075c09258b98a1d0efc60efd402cbfc9b4) Signed-off-by: Michael Roth --- hw/usb/hcd-ehci.c |1 + 1 file changed, 1 insertion(+) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index ce8b405..41c9d84 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2324,6 +2324,7 @@ static int usb_ehci_initfn(PCIDevice *dev) s->frame_timer = qemu_new_timer_ns(vm_clock, ehci_frame_timer, s); QTAILQ_INIT(&s->aqueues); QTAILQ_INIT(&s->pqueues); +usb_packet_init(&s->ipacket); qemu_register_reset(ehci_reset, s); -- 1.7.9.5
[Qemu-devel] [PATCH 19/23] slirp: Improve error reporting of inaccessible smb directories
From: Jan Kiszka
Instead of guessing, print the error code returned by access.
Signed-off-by: Jan Kiszka
(cherry picked from commit 22a61f365df83d5d7884cceb1c462295977cb2db)
Signed-off-by: Michael Roth
---
net/slirp.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index d3fcbf4..49f55bc 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -504,8 +504,8 @@ static int slirp_smb(SlirpState* s, const char
*exported_dir,
}
if (access(exported_dir, R_OK | X_OK)) {
-error_report("no such directory '%s', or you do not have permission "
- "to access it, please check it", exported_dir);
+error_report("error accessing shared directory '%s': %s",
+ exported_dir, strerror(errno));
return -1;
}
--
1.7.9.5
[Qemu-devel] [PATCH 12/23] usb-redir: Correctly handle the usb_redir_babble usbredir status
From: Hans de Goede
Signed-off-by: Hans de Goede
Signed-off-by: Gerd Hoffmann
(cherry picked from commit adae502c0ae4572ef08f71cb5b5ed5a8e90299fe)
Signed-off-by: Michael Roth
---
hw/usb/redirect.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 51c27b4..1fd903a 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1031,6 +1031,8 @@ static int usbredir_handle_status(USBRedirDevice *dev,
case usb_redir_inval:
WARNING("got invalid param error from usb-host?\n");
return USB_RET_NAK;
+case usb_redir_babble:
+return USB_RET_BABBLE;
case usb_redir_ioerror:
case usb_redir_timeout:
default:
--
1.7.9.5
[Qemu-devel] [PATCH 17/23] slirp: Enforce host-side user of smb share
From: Jan Kiszka
Windows 7 (and possibly other versions) cannot connect to the samba
share if the exported host directory is not world-readable. This can be
resolved by forcing the username used for access checks to the one
under which QEMU and smbd are running.
Signed-off-by: Jan Kiszka
(cherry picked from commit 1cb1c5d10bb9e180bd3f7be2c10b212ed86a97b4)
Signed-off-by: Michael Roth
---
net/slirp.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index 96f5032..c73610e 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -26,6 +26,7 @@
#include "config-host.h"
#ifndef _WIN32
+#include
#include
#endif
#include "net.h"
@@ -487,8 +488,15 @@ static int slirp_smb(SlirpState* s, const char
*exported_dir,
static int instance;
char smb_conf[128];
char smb_cmdline[128];
+struct passwd *passwd;
FILE *f;
+passwd = getpwuid(geteuid());
+if (!passwd) {
+error_report("failed to retrieve user name");
+return -1;
+}
+
snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.%ld-%d",
(long)getpid(), instance++);
if (mkdir(s->smb_dir, 0700) < 0) {
@@ -517,14 +525,16 @@ static int slirp_smb(SlirpState* s, const char
*exported_dir,
"[qemu]\n"
"path=%s\n"
"read only=no\n"
-"guest ok=yes\n",
+"guest ok=yes\n"
+"force user=%s\n",
s->smb_dir,
s->smb_dir,
s->smb_dir,
s->smb_dir,
s->smb_dir,
s->smb_dir,
-exported_dir
+exported_dir,
+passwd->pw_name
);
fclose(f);
--
1.7.9.5
[Qemu-devel] [PATCH 10/23] uhci: fix uhci_async_cancel_all
From: Gerd Hoffmann
We update the QTAILQ in the loop, thus we must use the SAFE version
to make sure we don't touch the queue struct after freeing it.
https://bugzilla.novell.com/show_bug.cgi?id=766310
Signed-off-by: Gerd Hoffmann
(cherry picked from commit 77fa9aee38758a078870e25f0dcf642066b4d5cc)
Signed-off-by: Michael Roth
---
hw/usb/hcd-uhci.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 9e211a0..3803f52 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -288,10 +288,10 @@ static void uhci_async_cancel_device(UHCIState *s,
USBDevice *dev)
static void uhci_async_cancel_all(UHCIState *s)
{
-UHCIQueue *queue;
+UHCIQueue *queue, *nq;
UHCIAsync *curr, *n;
-QTAILQ_FOREACH(queue, &s->queues, next) {
+QTAILQ_FOREACH_SAFE(queue, &s->queues, next, nq) {
QTAILQ_FOREACH_SAFE(curr, &queue->asyncs, next, n) {
uhci_async_unlink(curr);
uhci_async_cancel(curr);
--
1.7.9.5
[Qemu-devel] [stable-1.1] Patch Round-up for stable 1.1.2
This is the current list of candidates for QEMU 1.1.2, which we're planning to release on Friday. Please review and respond with any additional patches you think should be included. These are also available at: git://github.com/mdroth/qemu.git stable-1.1-staging There's a few that Michael Tokarov had mentioned to me that are not upstream yet that I'd also like to pull in: "ahci: fix cdrom read corruption" http://thread.gmane.org/gmane.comp.emulators.qemu/163543 "commit da57febfed "qdev: give all devices a canonical path" broke usb_ [...]" http://thread.gmane.org/gmane.comp.emulators.qemu/164180/focus=164185 And one I just sent out: "libcacard: Fix segfault when no backend specified" Queued patches follow: Anthony Liguori (2): qtest: fix infinite loop when QEMU aborts abruptly qdev: fix use-after-free in the error path of qdev_init_nofail Avi Kivity (1): virtio-blk: fix use-after-free while handling scsi commands Christian Borntraeger (1): s390: Fix error handling and condition code of service call David Gibson (1): ppc: Fix bug in handling of PAPR hypercall exits Dunrong Huang (1): slirp: Ensure smbd and shared directory exist when enable smb Gerd Hoffmann (4): ehci: fix reset ehci: don't flush cache on doorbell rings. uhci: fix uhci_async_cancel_all usb: restore USBDevice->attached on vmload Hans de Goede (2): usb-redir: Correctly handle the usb_redir_babble usbredir status usb-ehci: Fix an assert whenever isoc transfers are used Jan Kiszka (6): kvmvapic: Disable if there is insufficient memory slirp: Enforce host-side user of smb share slirp: Improve error reporting of inaccessible smb directories apic: Resolve potential endless loop around apic_update_irq apic: Reevaluate pending interrupts on LVT_LINT0 changes apic: Defer interrupt updates to VCPU thread Michael Roth (4): qlist: add qlist_size() json-parser: don't replicate tokens at each level of recursion check-qjson: add test for large JSON objects update VERSION for 1.1.2 Peter Maydell (1): configure: Don't override user's --cpu on MacOS and Solaris VERSION |2 +- configure| 60 +++- cpu-exec.c |6 ++ hw/apic.c| 27 +- hw/apic.h|1 + hw/apic_common.c |4 +- hw/apic_internal.h |1 - hw/qdev.c|5 +- hw/usb/bus.c | 13 +++ hw/usb/hcd-ehci.c| 45 +++-- hw/usb/hcd-uhci.c|4 +- hw/usb/redirect.c|2 + hw/virtio-blk.c |1 + json-parser.c| 230 -- net/slirp.c | 26 +- qlist.c | 13 +++ qlist.h |1 + target-i386/cpu.h|4 +- target-i386/kvm.c|4 + target-ppc/kvm.c |2 +- target-s390x/kvm.c |5 +- target-s390x/op_helper.c | 27 -- tests/check-qjson.c | 53 +++ tests/libqtest.c |5 + 24 files changed, 391 insertions(+), 150 deletions(-)
[Qemu-devel] [PATCH 08/23] ehci: fix reset
From: Gerd Hoffmann
Check for the reset bit first when processing USBCMD register writes.
Also break out of the switch, there is no need to check the other bits.
Signed-off-by: Gerd Hoffmann
(cherry picked from commit 7046530c36fa3a3f87692bdb54556f5d891a9c03)
Signed-off-by: Michael Roth
---
hw/usb/hcd-ehci.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index e759c99..5855c5d 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1064,6 +1064,12 @@ static void ehci_mem_writel(void *ptr,
target_phys_addr_t addr, uint32_t val)
/* Do any register specific pre-write processing here. */
switch(addr) {
case USBCMD:
+if (val & USBCMD_HCRESET) {
+ehci_reset(s);
+val = s->usbcmd;
+break;
+}
+
if ((val & USBCMD_RUNSTOP) && !(s->usbcmd & USBCMD_RUNSTOP)) {
qemu_mod_timer(s->frame_timer, qemu_get_clock_ns(vm_clock));
SET_LAST_RUN_CLOCK(s);
@@ -1077,10 +1083,6 @@ static void ehci_mem_writel(void *ptr,
target_phys_addr_t addr, uint32_t val)
ehci_set_usbsts(s, USBSTS_HALT);
}
-if (val & USBCMD_HCRESET) {
-ehci_reset(s);
-val = s->usbcmd;
-}
/* not supporting dynamic frame list size at the moment */
if ((val & USBCMD_FLS) && !(s->usbcmd & USBCMD_FLS)) {
--
1.7.9.5
Re: [Qemu-devel] [PATCH] Add guest-get-hostname to retrieve the guests current hostname
On Thu, Aug 23, 2012 at 09:48:54AM -0300, Luiz Capitulino wrote:
> On Tue, 21 Aug 2012 13:57:54 +0200
> Guido Günther wrote:
>
> > This allows to retrieve the guest's hostname via gethostname(2).
> >
> > This can be useful to identify a VM e.g. one without network.
> >
> > Signed-off-by: Guido Günther
> > ---
> > We have an API in libvirt for that (virDomainGetHostname).
> > Cheers
> > -- Guido
> >
> > qapi-schema-guest.json | 12
> > qga/commands-posix.c | 12
> > qga/commands-win32.c |6 ++
> > 3 files changed, 30 insertions(+)
> >
> > diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
> > index d955cf1..8c7a4a5 100644
> > --- a/qapi-schema-guest.json
> > +++ b/qapi-schema-guest.json
> > @@ -515,3 +515,15 @@
> > ##
> > { 'command': 'guest-network-get-interfaces',
> >'returns': ['GuestNetworkInterface'] }
> > +
> > +##
> > +# @guest-get-hostname:
> > +#
> > +# Get the guest's hostname
> > +#
> > +# Returns: The guest's hostname
> > +#
> > +# Since: 1.2
>
> Won't make it for 1.2 because we're in hard-freeze.
>
> > +##
> > +{ 'command': 'guest-get-hostname',
> > + 'returns': 'str' }
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index ce90421..9223f18 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -15,6 +15,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include "qga/guest-agent-core.h"
> > #include "qga-qmp-commands.h"
> > #include "qerror.h"
> > @@ -993,6 +994,17 @@ void qmp_guest_fstrim(bool has_minimum, int64_t
> > minimum, Error **err)
> > }
> > #endif
> >
> > +char *qmp_guest_get_hostname(Error **err)
> > +{
> > +char hostname[HOST_NAME_MAX];
> > +
> > +if (gethostname(hostname, HOST_NAME_MAX)) {
> > +error_set(err, QERR_QGA_COMMAND_FAILED, strerror(errno));
>
> You shouldn't use that macro, you can do something like this instead:
>
> error_set(err, "can't get hostname: %s", strerror(errno));
>
> Michael, do you think that's fine for new error messages or do you think it's
> worth it to add a "guest-agent: " prefix to all guest agent error messages?
>
I think that's fine. libvirt or QMP (if we integrate the qemu-ga
commands with QMP) can tack on a prefix for cases where we need to distinguish
where in the stack the error occurred, but for direct communication with
qemu-ga it's unambiguous, so might as well save the keystrokes.
> > +return NULL;
> > +}
> > +return g_strdup(hostname);
> > +}
> > +
> > /* register init/cleanup routines for stateful command groups */
> > void ga_command_state_init(GAState *s, GACommandState *cs)
> > {
> > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > index 54bc546..55e8162 100644
> > --- a/qga/commands-win32.c
> > +++ b/qga/commands-win32.c
> > @@ -280,6 +280,12 @@ GuestNetworkInterfaceList
> > *qmp_guest_network_get_interfaces(Error **err)
> > return NULL;
> > }
> >
> > +char *qmp_guest_get_hostname(Error **err)
> > +{
> > +error_set(err, QERR_UNSUPPORTED);
> > +return NULL;
> > +}
> > +
> > /* register init/cleanup routines for stateful command groups */
> > void ga_command_state_init(GAState *s, GACommandState *cs)
> > {
>
>
Re: [Qemu-devel] [PATCH 1/7] qemu-ga: move channel/transport functionality into wrapper class
On 01/23/2012 02:24 PM, Anthony Liguori wrote:
On 01/23/2012 08:21 AM, Michael Roth wrote:
This is monstly in preparation for the win32 port, which won't use
GIO channels for reasons that will be made clearer later. Here the
GAChannel class is just a loose wrapper around GIOChannel
calls/callbacks, but we also roll the logic/configuration for handling
FDs and also for managing unix socket connections into it, which makes
the abstraction much more complete and further aids in the win32 port
since isa-serial/unix-listen will not be supported initially.
There's also a bit of refactoring in the main logic to consolidate the
exit paths so we can do common cleanup for things like pid files, which
weren't always cleaned up previously.
Signed-off-by: Michael Roth
This doesn't apply very well. Can you rebase?
I think the issue is that it depends on the 4 patches I mentioned that
are currently floating around on the list (the first 2 for patchability,
the latter 2 for functional/performance issues):
[PATCH] qemu-ga: Add schema documentation for types
[PATCH] qemu-ga: add guest-set-support-level command
[PATCH] main-loop: Fix SetEvent() on uninitialized handle on win32
[PATCH] main-loop: For tools, initialize timers as part of
qemu_init_main_loop()
I think the first 2 are more likely to get merged along with Luiz's
guest-suspend series, so I was hesitant to re-include them here as 1
unit (since that would break Luiz's patches).
The git repo has the whole series though. Can also resubmit in a
different form if preferred.
Regards,
Anthony Liguori
---
Makefile.objs | 1 +
qemu-ga.c | 306
qga/channel-posix.c | 246 ++
qga/channel.h | 33 +
qga/guest-agent-core.h | 2 +-
5 files changed, 355 insertions(+), 233 deletions(-)
create mode 100644 qga/channel-posix.c
create mode 100644 qga/channel.h
diff --git a/Makefile.objs b/Makefile.objs
index f94c881..72141cc 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -417,6 +417,7 @@ common-obj-y += qmp.o hmp.o
# guest agent
qga-nested-y = guest-agent-commands.o guest-agent-command-state.o
+qga-nested-y += channel-posix.o
qga-obj-y = $(addprefix qga/, $(qga-nested-y))
qga-obj-y += qemu-ga.o qemu-sockets.o module.o qemu-option.o
qga-obj-$(CONFIG_WIN32) += oslib-win32.o
diff --git a/qemu-ga.c b/qemu-ga.c
index 8e5b075..5f89ab9 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -15,9 +15,7 @@
#include
#include
#include
-#include
#include
-#include "qemu_socket.h"
#include "json-streamer.h"
#include "json-parser.h"
#include "qint.h"
@@ -29,19 +27,15 @@
#include "error_int.h"
#include "qapi/qmp-core.h"
#include "qga-qapi-types.h"
+#include "qga/channel.h"
#define QGA_VIRTIO_PATH_DEFAULT
"/dev/virtio-ports/org.qemu.guest_agent.0"
#define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid"
-#define QGA_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */
-#define QGA_TIMEOUT_DEFAULT 30*1000 /* ms */
struct GAState {
JSONMessageParser parser;
GMainLoop *main_loop;
- GIOChannel *conn_channel;
- GIOChannel *listen_channel;
- const char *path;
- const char *method;
+ GAChannel *channel;
bool virtio; /* fastpath to check for virtio to deal with poll()
quirks */
GACommandState *command_state;
GLogLevelFlags log_level;
@@ -60,7 +54,7 @@ static void quit_handler(int sig)
}
}
-static void register_signal_handlers(void)
+static gboolean register_signal_handlers(void)
{
struct sigaction sigact;
int ret;
@@ -71,12 +65,14 @@ static void register_signal_handlers(void)
ret = sigaction(SIGINT,&sigact, NULL);
if (ret == -1) {
g_error("error configuring signal handler: %s", strerror(errno));
- exit(EXIT_FAILURE);
+ return false;
}
ret = sigaction(SIGTERM,&sigact, NULL);
if (ret == -1) {
g_error("error configuring signal handler: %s", strerror(errno));
+ return false;
}
+ return true;
}
static void usage(const char *cmd)
@@ -101,8 +97,6 @@ static void usage(const char *cmd)
, cmd, QGA_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT);
}
-static void conn_channel_close(GAState *s);
-
static GuestAgentSupportLevel ga_support_level;
static int ga_cmp_support_levels(GuestAgentSupportLevel a,
@@ -250,40 +244,13 @@ fail:
exit(EXIT_FAILURE);
}
-static int conn_channel_send_buf(GIOChannel *channel, const char *buf,
- gsize count)
+static int send_response(GAState *s, QObject *payload)
{
- GError *err = NULL;
- gsize written = 0;
- GIOStatus status;
-
- while (count) {
- status = g_io_channel_write_chars(channel, buf, count,&written,&err);
- g_debug("sending data, count: %d", (int)count);
- if (err != NULL) {
- g_warning("error sending newline: %s", err->message);
- return err->code;
- }
- if (status == G_IO_STATUS_ERROR || status == G_IO_STATUS_EOF) {
- return -EPIPE;
- }
-
- if (status == G_IO_STATUS_NORMAL) {
- c
[Qemu-devel] [PULL 0/6] qemu-ga queue and qemu-tool fixes
This pull adds 2 RPCs (guest-suspend and guest-set-support-level) to qemu-ga and some includes fixes/workarounds for building tools on Windows. The following changes since commit 5b4448d27d7c6ff6e18a1edc8245cb1db783e37c: Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2012-01-23 11:00:26 -0600) are available in the git repository at: git://github.com/mdroth/qemu.git qga-pull-2012-01-23 Luiz Capitulino (2): qemu-ga: set O_NONBLOCK for serial channels qemu-ga: Add the guest-suspend command Michael Roth (4): main-loop: Fix SetEvent() on uninitialized handle on win32 main-loop: For tools, initialize timers as part of qemu_init_main_loop() qemu-ga: Add schema documentation for types qemu-ga: add guest-set-support-level command main-loop.c|7 +- main-loop.h| 12 ++ qapi-schema-guest.json | 217 qemu-ga.c | 66 - qemu-tool.c|3 +- qga/guest-agent-commands.c | 239 qga/guest-agent-core.h | 11 ++ vl.c |5 + 8 files changed, 533 insertions(+), 27 deletions(-)
[Qemu-devel] [PATCH 1/6] main-loop: Fix SetEvent() on uninitialized handle on win32
The __attribute__((constructor)) init_main_loop() automatically get
called if qemu-tool.o is linked in. On win32, this leads to
a qemu_notify_event() call which attempts to SetEvent() on a HANDLE that
won't be initialized until qemu_init_main_loop() is manually called,
breaking qemu-tools.o programs on Windows at runtime.
This patch checks for an initialized event handle before attempting to
set it, which is analoguous to how we deal with an unitialized
io_thread_fd in the posix implementation.
Signed-off-by: Michael Roth
---
main-loop.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index 692381c..62d95b9 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -164,7 +164,7 @@ static int qemu_signal_init(void)
#else /* _WIN32 */
-HANDLE qemu_event_handle;
+HANDLE qemu_event_handle = NULL;
static void dummy_event_handler(void *opaque)
{
@@ -183,6 +183,9 @@ static int qemu_event_init(void)
void qemu_notify_event(void)
{
+if (!qemu_event_handle) {
+return;
+}
if (!SetEvent(qemu_event_handle)) {
fprintf(stderr, "qemu_notify_event: SetEvent failed: %ld\n",
GetLastError());
--
1.7.4.1
[Qemu-devel] [PATCH 5/6] qemu-ga: set O_NONBLOCK for serial channels
From: Luiz Capitulino
This fixes a bug when using -m isa-serial where qemu-ga will
hang on a read()'s when communicating to the host via isa-serial.
Original fix by Michael Roth.
Signed-off-by: Luiz Capitulino
Signed-off-by: Michael Roth
---
qemu-ga.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-ga.c b/qemu-ga.c
index 8e5b075..d63b17c 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -544,7 +544,7 @@ static void init_guest_agent(GAState *s)
exit(EXIT_FAILURE);
}
} else if (strcmp(s->method, "isa-serial") == 0) {
-fd = qemu_open(s->path, O_RDWR | O_NOCTTY);
+fd = qemu_open(s->path, O_RDWR | O_NOCTTY | O_NONBLOCK);
if (fd == -1) {
g_critical("error opening channel: %s", strerror(errno));
exit(EXIT_FAILURE);
--
1.7.4.1
[Qemu-devel] [PATCH 6/6] qemu-ga: Add the guest-suspend command
From: Luiz Capitulino
The guest-suspend command supports three modes:
o hibernate (suspend to disk)
o sleep (suspend to ram)
o hybrid(save RAM contents to disk, but suspend instead of
powering off)
Before trying to suspend, the command queries the guest in order
to know whether the given mode is supported. The sleep and hybrid
modes are only supported in QEMU 1.1 and later though, because
QEMU's S3 support is broken in previous versions.
The guest-suspend command will use the scripts provided by the
pm-utils package if they are available. If they aren't, a manual
process which directly writes to the "/sys/power/state" file is
used.
To reap terminated children, a new signal handler is installed in
the parent to catch SIGCHLD signals and a non-blocking call to
waitpid() is done to collect their exit statuses. The statuses,
however, are discarded.
The approach used to query the guest for suspend support deserves
some explanation. It's implemented by bios_supports_mode() and shown
below:
qemu-ga
|
create pipe
|
fork()
-
| |
| |
| fork()
| --
| ||
| ||
| | exec('pm-is-supported')
| |
| wait()
| write exit status to pipe
| exit
|
read pipe
This might look complex, but the resulting code is quite simple.
The purpose of that approach is to allow qemu-ga to reap its
children (semi-)automatically from its SIGCHLD handler.
Implementing this the obvious way, that's, doing the exec() call
from the first child process, would force us to introduce a more
complex way to reap qemu-ga's children. Like registering PIDs to
be reaped and having a way to wait for them when returning their
exit status to qemu-ga is necessary. The approach explained
above avoids that complexity.
Signed-off-by: Luiz Capitulino
Signed-off-by: Michael Roth
---
qapi-schema-guest.json | 32 ++
qemu-ga.c | 18 -
qga/guest-agent-commands.c | 226
3 files changed, 275 insertions(+), 1 deletions(-)
diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index 81cd773..9b1ab1c 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -360,3 +360,35 @@
##
{ 'command': 'guest-fsfreeze-thaw',
'returns': 'int' }
+
+##
+# @guest-suspend
+#
+# Suspend guest execution by entering ACPI power state S3 or S4.
+#
+# This command tries to execute the scripts provided by the pm-utils
+# package. If they are not available, it will perform the suspend
+# operation by manually writing to a sysfs file.
+#
+# For the best results it's strongly recommended to have the pm-utils
+# package installed in the guest.
+#
+# @mode: 'hibernate' RAM content is saved to the disk and the guest is
+#powered off (this corresponds to ACPI S4)
+#'sleep' execution is suspended but the RAM retains its contents
+#(this corresponds to ACPI S3)
+#'hybrid'RAM content is saved to the disk but the guest is
+#suspended instead of powering off
+#
+# Returns: nothing on success
+# If @mode is not supported by the guest, Unsupported
+#
+# Notes: o This is an asynchronous request. There's no guarantee a response
+# will be sent.
+#o Errors will be logged to guest's syslog.
+#o It's strongly recommended to issue the guest-sync command before
+# sending commands when the guest resumes.
+#
+# Since: 1.1
+##
+{ 'command': 'guest-suspend', 'data': { 'mode': 'str' } }
diff --git a/qemu-ga.c b/qemu-ga.c
index d63b17c..f8b12c7 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include "qemu_socket.h"
#include "json-streamer.h"
#include "json-parser.h"
@@ -60,9 +61,16 @@ static void quit_handler(int sig)
}
}
+/* reap _all_ terminated children */
+static void child_handler(int sig)
+{
+int status;
+while (waitpid(-1, &status, WNOHANG) > 0) /* NOTHING */;
+}
+
static void register_signal_handlers(void)
{
-struct sigaction sigact;
+struct sigaction sigact, sigact_chld;
int ret;
memset(&sigact, 0, sizeof(struct sigaction));
@@ -77,6 +85,14 @@ static void register_signal_handlers(void)
if (ret == -1) {
g_error("error configuring signal handler: %s", strerror(errno));
}
+
+memset(&sigact_chld, 0, sizeof(struct sigaction));
+sigact_chld.sa_handler = child_handler;
+sigact_ch
[Qemu-devel] [PATCH 3/6] qemu-ga: Add schema documentation for types
Document guest agent schema types in similar fashion as qmp schema
types.
Signed-off-by: Michael Roth
---
qapi-schema-guest.json | 118 +++-
1 files changed, 97 insertions(+), 21 deletions(-)
diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index 5f8a18d..706925d 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -37,17 +37,42 @@
{ 'command': 'guest-ping' }
##
-# @guest-info:
+# @GuestAgentCommandInfo:
#
-# Get some information about the guest agent.
+# Information about guest agent commands.
#
-# Since: 0.15.0
+# @name: name of the command
+#
+# @enabled: whether command is currently enabled by guest admin
+#
+# Since 1.1.0
##
{ 'type': 'GuestAgentCommandInfo',
'data': { 'name': 'str', 'enabled': 'bool' } }
+
+##
+# @GuestAgentInfo
+#
+# Information about guest agent.
+#
+# @version: guest agent version
+#
+# @supported_commands: Information about guest agent commands
+#
+# Since 0.15.0
+##
{ 'type': 'GuestAgentInfo',
'data': { 'version': 'str',
'supported_commands': ['GuestAgentCommandInfo'] } }
+##
+# @guest-info:
+#
+# Get some information about the guest agent.
+#
+# Returns: @GuestAgentInfo
+#
+# Since: 0.15.0
+##
{ 'command': 'guest-info',
'returns': 'GuestAgentInfo' }
@@ -98,6 +123,23 @@
'data': { 'handle': 'int' } }
##
+# @GuestFileRead
+#
+# Result of guest agent file-read operation
+#
+# @count: number of bytes read (note: count is *before*
+# base64-encoding is applied)
+#
+# @buf-b64: base64-encoded bytes read
+#
+# @eof: whether EOF was encountered during read operation.
+#
+# Since: 0.15.0
+##
+{ 'type': 'GuestFileRead',
+ 'data': { 'count': 'int', 'buf-b64': 'str', 'eof': 'bool' } }
+
+##
# @guest-file-read:
#
# Read from an open file in the guest. Data will be base64-encoded
@@ -106,19 +148,30 @@
#
# @count: #optional maximum number of bytes to read (default is 4KB)
#
-# Returns: GuestFileRead on success. Note: count is number of bytes read
-# *before* base64 encoding bytes read.
+# Returns: @GuestFileRead on success.
#
# Since: 0.15.0
##
-{ 'type': 'GuestFileRead',
- 'data': { 'count': 'int', 'buf-b64': 'str', 'eof': 'bool' } }
-
{ 'command': 'guest-file-read',
'data':{ 'handle': 'int', '*count': 'int' },
'returns': 'GuestFileRead' }
##
+# @GuestFileWrite
+#
+# Result of guest agent file-write operation
+#
+# @count: number of bytes written (note: count is actual bytes
+# written, after base64-decoding of provided buffer)
+#
+# @eof: whether EOF was encountered during write operation.
+#
+# Since: 0.15.0
+##
+{ 'type': 'GuestFileWrite',
+ 'data': { 'count': 'int', 'eof': 'bool' } }
+
+##
# @guest-file-write:
#
# Write to an open file in the guest.
@@ -130,17 +183,29 @@
# @count: #optional bytes to write (actual bytes, after base64-decode),
# default is all content in buf-b64 buffer after base64 decoding
#
-# Returns: GuestFileWrite on success. Note: count is the number of bytes
-# base64-decoded bytes written
+# Returns: @GuestFileWrite on success.
#
# Since: 0.15.0
##
-{ 'type': 'GuestFileWrite',
- 'data': { 'count': 'int', 'eof': 'bool' } }
{ 'command': 'guest-file-write',
'data':{ 'handle': 'int', 'buf-b64': 'str', '*count': 'int' },
'returns': 'GuestFileWrite' }
+
+##
+# @GuestFileSeek
+#
+# Result of guest agent file-seek operation
+#
+# @position: current file position
+#
+# @eof: whether EOF was encountered during file seek
+#
+# Since: 0.15.0
+##
+{ 'type': 'GuestFileSeek',
+ 'data': { 'position': 'int', 'eof': 'bool' } }
+
##
# @guest-file-seek:
#
@@ -154,13 +219,10 @@
#
# @whence: SEEK_SET, SEEK_CUR, or SEEK_END, as with fseek()
#
-# Returns: GuestFileSeek on success.
+# Returns: @GuestFileSeek on success.
#
# Since: 0.15.0
##
-{ 'type': 'GuestFileSeek',
- 'data': { 'position': 'int', 'eof': 'bool' } }
-
{ 'command': 'guest-file-seek',
'data':{ 'handle': 'int', 'offset': 'int', 'whence': 'in
[Qemu-devel] [PATCH 4/6] qemu-ga: add guest-set-support-level command
Recently commands where introduced on the mailing that involved adding
commands to the guest agent that could potentially break older versions
of QEMU. While it's okay to expect that qemu-ga can be updated to support
newer host features, it's unrealistic to require a host to be updated to
support qemu-ga features, or to expect that qemu-ga should be downgraded
in these circumstances, especially considering that a large mix of
guests/agents may be running on a particular host.
To address this, we introduce here a mechanism to set qemu-ga's
feature-set to one that is known to be compatible with
the host/QEMU running the guest. As new commands/options are added, we
can maintain backward-compatibility by adding conditional checks to filter
out host-incompatible functionality based on the host-specified support
level (generally analogous to the host QEMU version) set by the
client.
The current default/minimum support level supports all versions of QEMU
that have had qemu-ga in-tree (0.15.0, 1.0.0) and so should be
backward-compatible with existing hosts/clients.
Signed-off-by: Michael Roth
---
qapi-schema-guest.json | 67 +++-
qemu-ga.c | 46 ++
qga/guest-agent-commands.c | 13
qga/guest-agent-core.h | 11 +++
4 files changed, 136 insertions(+), 1 deletions(-)
diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index 706925d..81cd773 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -51,6 +51,27 @@
'data': { 'name': 'str', 'enabled': 'bool' } }
##
+# @GuestAgentSupportLevel
+#
+# Guest agent support/compatability level (as configured by
+# guest agent defaults of guest-set-support-level command)
+#
+# Three numerical fields describe the support level:
+#
+# ..
+#
+# @major: As above
+#
+# @minor: As above
+#
+# @micro: As above
+#
+# Since 1.1.0
+##
+{ 'type': 'GuestAgentSupportLevel',
+ 'data': { 'major': 'int', 'minor': 'int', 'micro': 'int' } }
+
+##
# @GuestAgentInfo
#
# Information about guest agent.
@@ -59,11 +80,17 @@
#
# @supported_commands: Information about guest agent commands
#
+# @support_level: Current support/compatability level, as set by
+# guest agent defaults or guest-set-support-level
+# command.
+#
# Since 0.15.0
##
{ 'type': 'GuestAgentInfo',
'data': { 'version': 'str',
-'supported_commands': ['GuestAgentCommandInfo'] } }
+'supported_commands': ['GuestAgentCommandInfo']
+'support_level': 'GuestAgentSupportLevel' } }
+
##
# @guest-info:
#
@@ -77,6 +104,44 @@
'returns': 'GuestAgentInfo' }
##
+# @guest-set-support-level:
+#
+# Set guest agent feature-set to one that is compatible with/supported by
+# the host.
+#
+# Certain commands/options may have dependencies on host
+# version/support-level, such as specific QEMU features (such
+# dependencies will be noted in this schema). By default we assume 1.0.0,
+# which is backward-compatible with QEMU 0.15.0/1.0, and should be compatible
+# with later versions of QEMU as well. To enable newer guest agent features,
+# this command must be issued to raise the support-level to one corresponding
+# to supported host QEMU/KVM/etc capabilities.
+#
+# The host support-level is generally <= host QEMU version
+# level. Note that undefined behavior may occur if a support-level is
+# provided that exceeds the capabilities of the version of QEMU currently
+# running the guest. Any level specified below 1.0.0 will default to 1.0.0.
+#
+# The currently set support level is obtainable via the guest-info command.
+#
+# Three numerical fields describe the host support/compatability level:
+#
+# ..
+#
+# @major: As above
+#
+# @minor: As above
+#
+# @micro: As above
+#
+# Returns: Nothing on success
+#
+# Since: 1.1.0
+##
+{ 'command': 'guest-set-support-level',
+ 'data':{ 'major': 'int', 'minor': 'int', '*micro': 'int' } }
+
+##
# @guest-shutdown:
#
# Initiate guest-activated shutdown. Note: this is an asynchronous
diff --git a/qemu-ga.c b/qemu-ga.c
index 29e4f64..8e5b075 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -28,6 +28,7 @@
#include "qerror.h"
#include "error_int.h"
#include "qapi/qmp-core.h"
+#include "qga-qapi-types.h"
#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
#define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid"
@@ -102,6 +103,45 @@ static void usage(const char *cmd)
static void conn_channel_close(GAState *s);
+static GuestAgentSupportL
[Qemu-devel] [PATCH 2/6] main-loop: For tools, initialize timers as part of qemu_init_main_loop()
In some cases initializing the alarm timers can lead to non-negligable
overhead from programs that link against qemu-tool.o. At least,
setting a max-resolution WinMM alarm timer via mm_start_timer() (the
current default for Windows) can increase the "tick rate" on Windows
OSs and affect frequency scaling, and in the case of tools that run
in guest OSs such has qemu-ga, the impact can be fairly dramatic
(+20%/20% user/sys time on a core 2 processor was observed from an idle
Windows XP guest).
This patch doesn't address the issue directly (not sure what a good
solution would be for Windows, or what other situations it might be
noticeable), but it at least limits the scope of the issue to programs
that "opt-in" to using the main-loop.c functions by only enabling alarm
timers when qemu_init_main_loop() is called, which is already required
to make use of those facilities, so existing users shouldn't be
affected.
Signed-off-by: Michael Roth
---
main-loop.c |2 +-
main-loop.h | 12
qemu-tool.c |3 ++-
vl.c|5 +
4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index 62d95b9..db23de0 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -199,7 +199,7 @@ static int qemu_signal_init(void)
}
#endif
-int qemu_init_main_loop(void)
+int main_loop_init(void)
{
int ret;
diff --git a/main-loop.h b/main-loop.h
index f971013..4987041 100644
--- a/main-loop.h
+++ b/main-loop.h
@@ -41,10 +41,22 @@
* SIGUSR2, thread signals (SIGFPE, SIGILL, SIGSEGV, SIGBUS) and real-time
* signals if available. Remember that Windows in practice does not have
* signals, though.
+ *
+ * In the case of QEMU tools, this will also start/initialize timers.
*/
int qemu_init_main_loop(void);
/**
+ * main_loop_init: Initializes main loop
+ *
+ * Internal (but shared for compatibility reasons) initialization routine
+ * for the main loop. This should not be used by applications directly,
+ * use qemu_init_main_loop() instead.
+ *
+ */
+int main_loop_init(void);
+
+/**
* main_loop_wait: Run one iteration of the main loop.
*
* If @nonblocking is true, poll for events, otherwise suspend until
diff --git a/qemu-tool.c b/qemu-tool.c
index 6b69668..183a583 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -83,11 +83,12 @@ void qemu_clock_warp(QEMUClock *clock)
{
}
-static void __attribute__((constructor)) init_main_loop(void)
+int qemu_init_main_loop(void)
{
init_clocks();
init_timer_alarm();
qemu_clock_enable(vm_clock, false);
+return main_loop_init();
}
void slirp_select_fill(int *pnfds, fd_set *readfds,
diff --git a/vl.c b/vl.c
index 57378b6..9d14cc8 100644
--- a/vl.c
+++ b/vl.c
@@ -2159,6 +2159,11 @@ static void free_and_trace(gpointer mem)
free(mem);
}
+int qemu_init_main_loop(void)
+{
+return main_loop_init();
+}
+
int main(int argc, char **argv, char **envp)
{
const char *gdbstub_dev = NULL;
--
1.7.4.1
Re: [Qemu-devel] [libvirt] [PATCH RFC 0/4] Allow hibernation on guests
On 01/26/2012 02:13 PM, Luiz Capitulino wrote:
On Thu, 26 Jan 2012 20:41:13 +0100
Michal Privoznik wrote:
On 26.01.2012 20:35, Luiz Capitulino wrote:
On Thu, 26 Jan 2012 08:18:03 -0700
Eric Blake wrote:
[adding qemu-devel]
On 01/26/2012 07:46 AM, Daniel P. Berrange wrote:
One thing, that you'll probably notice is this
'set-support-level' command. Basically, it tells GA what qemu version
is it running on. Ideally, this should be done as soon as
GA starts up. However, that cannot be determined from outside
world as GA doesn't emit any events yet.
Ideally^2 this command should be left out as it should be qemu
who tells its own agent this kind of information.
Anyway, I was going to call this command in qemuProcess{Startup,
Reconnect,Attach}, but it won't work. We need to un-pause guest CPUs
so guest can boot and start GA, but that implies returning from qemuProcess*.
So I am setting this just before 'guest-suspend' command, as
there is one more thing about GA. It is unable to remember anything
upon its restart (GA process). Which has BTW show flaw
in our current code with FS freeze& thaw. If we freeze guest
FS, and somebody restart GA, the simple FS Thaw will not succeed as
GA thinks FS are not frozen. But that's a different cup of tea.
Because of what written above, we need to call set-level
on every suspend.
IMHO all this says that the 'set-level' command is a conceptually
unfixably broken design& should be killed in QEMU before it turns
into an even bigger mess.
Can you elaborate on this? Michal and I talked on irc about making the
compatibility level persistent, would that help?
Once we're in a situation where we need to call 'set-level' prior
to every single invocation, you might as well just allow the QEMU
version number to be passed in directly as an arg to the command
you are running directly thus avoiding this horrificness.
Qemu folks, would you care to chime in on this?
Exactly how is the set-level command supposed to work? As I understand
it, the goal is that if the guest has qemu-ga 1.1 installed, but is
being run by qemu 1.0, then we want to ensure that any guest agent
command supported by qemu-ga 1.1 but requiring features of qemu not
present in qemu 1.0 will be properly rejected.
Not exactly, the default support of qemu-ga is qemu 1.0. This means that by
default qemu-ga will only support qemu 1.0 even when running on qemu 2.0. This
way the set-support-level command allows you to specify that qemu 2.0 features
are supported.
Note that this is only about specific features that depend on host support,
like S3 suspend which is known to be buggy in current and old qemu.
But whose job is it to tell the guest agent what version of qemu is
running? Based on the above conversation, it looks like the current
qemu implementation does not do any handshaking on its own when the
guest agent first comes alive, which means that you are forcing the work
on the management app (libvirt). And this is inherently racy - if the
guest is allowed to restart its qemu-ga process at will, and each
restart of that guest process triggers a need to redo the handshake,
then libvirt can never reliably know what version the agent is running at.
Making the set-support-level persistent would solve it, wouldn't it?
Yes and no. We still need an event when GA come to live. Because if
anybody tries to write something for GA which is not running (and for
purpose of this scenario assume it never will), like 'set-support-level'
and wait for answer (which will never come) he will be blocked
indefinitely. However, if he writes it after 1st event come, everything
is OK.
What if the event never reach libvirt?
This problem is a lot more general and is not related to the
set-support-level command. Maybe adding shutdown& start events can serve as
good hints, but they won't fix the problem.
Yah, start up events are a good indicator to issue the guest-sync
sequence (we had them at one point, and planned to re-add them for QMP
integration, and since libvirt is taking on this role for now it might
make sense to re-add it now), but once that sequence is issued the agent
can still be manually stopped, or the guest-sync sequence itself can
timeout.
And there's no way to reliably send a stop indicator, maybe to capture
shutdown events, but not consistently enough that we can base the
protocol on it (agent might get pkill -9'd for instance, and
virtio-serial doesn't currently plumb a guest-side disconnect to the
chardev front-end, so you'd never know).
So, the only indication you'll ever get that your "session" ended is
either a timeout, or, if we add it, a start up event. In either case the
response is to issue the reset sequence.
The way it would need to work with resets is everytime a command times
out you:
1) report the timeout error to libvirt client/management app. set
guest-agent_available = 0, such that further libvirt calls that depend
on it would return "not currently available",
Re: [Qemu-devel] [libvirt] [PATCH RFC 0/4] Allow hibernation on guests
On 01/26/2012 09:18 AM, Eric Blake wrote:
[adding qemu-devel]
On 01/26/2012 07:46 AM, Daniel P. Berrange wrote:
One thing, that you'll probably notice is this
'set-support-level' command. Basically, it tells GA what qemu version
is it running on. Ideally, this should be done as soon as
GA starts up. However, that cannot be determined from outside
world as GA doesn't emit any events yet.
Ideally^2 this command should be left out as it should be qemu
who tells its own agent this kind of information.
Anyway, I was going to call this command in qemuProcess{Startup,
Reconnect,Attach}, but it won't work. We need to un-pause guest CPUs
so guest can boot and start GA, but that implies returning from qemuProcess*.
So I am setting this just before 'guest-suspend' command, as
there is one more thing about GA. It is unable to remember anything
upon its restart (GA process). Which has BTW show flaw
in our current code with FS freeze& thaw. If we freeze guest
FS, and somebody restart GA, the simple FS Thaw will not succeed as
GA thinks FS are not frozen. But that's a different cup of tea.
We could drop the state tracking (guest-fsfreeze-status) and issue the
freeze/thaw unconditionally. Talked with Anthony and going stateless in
general seems to solve a lot of nastiness that might pop up with the
current implementation. I'll send some patches out soon for fsfreeze,
guest-file* may end up getting similar treatment in the
not-too-distant-future.
[Qemu-devel] [PATCH v2] main-loop: Fix SetEvent() on uninitialized handle on win32
The __attribute__((constructor)) init_main_loop() automatically get
called if qemu-tool.o is linked in. On win32, this leads to
a qemu_notify_event() call which attempts to SetEvent() on a HANDLE that
won't be initialized until qemu_init_main_loop() is manually called,
breaking qemu-tools.o programs on Windows at runtime.
This patch checks for an initialized event handle before attempting to
set it, which is analoguous to how we deal with an unitialized
io_thread_fd in the posix implementation.
Signed-off-by: Michael Roth
---
main-loop.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index 692381c..db90ace 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -183,6 +183,9 @@ static int qemu_event_init(void)
void qemu_notify_event(void)
{
+if (!qemu_event_handle) {
+return;
+}
if (!SetEvent(qemu_event_handle)) {
fprintf(stderr, "qemu_notify_event: SetEvent failed: %ld\n",
GetLastError());
--
1.7.4.1
Re: [Qemu-devel] [libvirt] [PATCH RFC 0/4] Allow hibernation on guests
On 01/30/2012 06:57 AM, Luiz Capitulino wrote:
On Thu, 26 Jan 2012 16:57:01 -0600
Anthony Liguori wrote:
On 01/26/2012 01:35 PM, Luiz Capitulino wrote:
On Thu, 26 Jan 2012 08:18:03 -0700
Eric Blake wrote:
[adding qemu-devel]
On 01/26/2012 07:46 AM, Daniel P. Berrange wrote:
One thing, that you'll probably notice is this
'set-support-level' command. Basically, it tells GA what qemu version
is it running on. Ideally, this should be done as soon as
GA starts up. However, that cannot be determined from outside
world as GA doesn't emit any events yet.
Ideally^2 this command should be left out as it should be qemu
who tells its own agent this kind of information.
Anyway, I was going to call this command in qemuProcess{Startup,
Reconnect,Attach}, but it won't work. We need to un-pause guest CPUs
so guest can boot and start GA, but that implies returning from qemuProcess*.
So I am setting this just before 'guest-suspend' command, as
there is one more thing about GA. It is unable to remember anything
upon its restart (GA process). Which has BTW show flaw
in our current code with FS freeze& thaw. If we freeze guest
FS, and somebody restart GA, the simple FS Thaw will not succeed as
GA thinks FS are not frozen. But that's a different cup of tea.
Because of what written above, we need to call set-level
on every suspend.
IMHO all this says that the 'set-level' command is a conceptually
unfixably broken design& should be killed in QEMU before it turns
into an even bigger mess.
Can you elaborate on this? Michal and I talked on irc about making the
compatibility level persistent, would that help?
Once we're in a situation where we need to call 'set-level' prior
to every single invocation, you might as well just allow the QEMU
version number to be passed in directly as an arg to the command
you are running directly thus avoiding this horrificness.
Qemu folks, would you care to chime in on this?
Exactly how is the set-level command supposed to work? As I understand
it, the goal is that if the guest has qemu-ga 1.1 installed, but is
being run by qemu 1.0, then we want to ensure that any guest agent
command supported by qemu-ga 1.1 but requiring features of qemu not
present in qemu 1.0 will be properly rejected.
Not exactly, the default support of qemu-ga is qemu 1.0. This means that by
default qemu-ga will only support qemu 1.0 even when running on qemu 2.0. This
way the set-support-level command allows you to specify that qemu 2.0 features
are supported.
Version numbers are meaningless. What happens when a bunch of features get
backported by RHEL such that qemu-ga 1.0 ends up being a frankenstein version of
2.0?
The feature negotiation mechanism we have in QMP is the existence of a command.
If we're in a position where we're trying to disable part of a command, it
simply means that we should have multiple commands such that we can just remove
the disabled part entirely.
You may have a point that we shouldn't be using the version number for that,
but just switching to multiple commands doesn't solve the fundamental problem.
Agreed, but the multiple commands isn't really the fix here, it's
libvirt querying for the "wakeup" command that Gerd's patches add.
We implemented set-version-level as a way to let management tools
obliviously report something simple, like the version information it
parses from QEMU already, to let the guest agent Do The Right Thing
without any deep insight into what it's host requirements are (which is
also why we opted for versions over specific capabilities flags).
But we can't rely on version levels applying in all cases, a distro
might backport s3 support for 1.0, modify the guest agent version level
dependencies accordingly, and thus render the that agent incompatible
with, say, RHEL.
So it was a naive approach on my part, and the issues the libvirt folks
noted with not knowing if a reset occurred since the last
set-support-level make it even less desirable (we could persist it as
you suggested, and I am working on patches to persist fsfreeze state,
but it's not worth trying to fix set-support-level).
And at least in this case we have an easy out: libvirt knows it can't
resume a guest without the presence of a QMP command to do so, and that
doesn't require any intimate knowledge of how the agent works, it's just
common sense.
Breaking the suspend/hibernate stuff into multiple commands avoids the
need for libvirt to special case based on specific parameters to the
command. We can also imagine distro-specific implementations of the
agent disabling s4 due to things like not having s4 patches for virtio
in place, so it makes it easier (possible) to discover those situations
as well. It may be that we opt for a single command for libvirt, but at
least on the agent side it should be multiple discoverable ones.
Hopefully this doesn't set anyone back too much :( IMHO it's the right
way to go though.
The fundamental problem is t
Re: [Qemu-devel] [libvirt] [PATCH RFC 0/4] Allow hibernation on guests
On 01/30/2012 08:44 AM, Luiz Capitulino wrote:
On Mon, 30 Jan 2012 07:54:56 -0600
Anthony Liguori wrote:
On 01/30/2012 06:57 AM, Luiz Capitulino wrote:
On Thu, 26 Jan 2012 16:57:01 -0600
Anthony Liguori wrote:
On 01/26/2012 01:35 PM, Luiz Capitulino wrote:
On Thu, 26 Jan 2012 08:18:03 -0700
Eric Blakewrote:
[adding qemu-devel]
On 01/26/2012 07:46 AM, Daniel P. Berrange wrote:
One thing, that you'll probably notice is this
'set-support-level' command. Basically, it tells GA what qemu version
is it running on. Ideally, this should be done as soon as
GA starts up. However, that cannot be determined from outside
world as GA doesn't emit any events yet.
Ideally^2 this command should be left out as it should be qemu
who tells its own agent this kind of information.
Anyway, I was going to call this command in qemuProcess{Startup,
Reconnect,Attach}, but it won't work. We need to un-pause guest CPUs
so guest can boot and start GA, but that implies returning from qemuProcess*.
So I am setting this just before 'guest-suspend' command, as
there is one more thing about GA. It is unable to remember anything
upon its restart (GA process). Which has BTW show flaw
in our current code with FS freeze&thaw. If we freeze guest
FS, and somebody restart GA, the simple FS Thaw will not succeed as
GA thinks FS are not frozen. But that's a different cup of tea.
Because of what written above, we need to call set-level
on every suspend.
IMHO all this says that the 'set-level' command is a conceptually
unfixably broken design&should be killed in QEMU before it turns
into an even bigger mess.
Can you elaborate on this? Michal and I talked on irc about making the
compatibility level persistent, would that help?
Once we're in a situation where we need to call 'set-level' prior
to every single invocation, you might as well just allow the QEMU
version number to be passed in directly as an arg to the command
you are running directly thus avoiding this horrificness.
Qemu folks, would you care to chime in on this?
Exactly how is the set-level command supposed to work? As I understand
it, the goal is that if the guest has qemu-ga 1.1 installed, but is
being run by qemu 1.0, then we want to ensure that any guest agent
command supported by qemu-ga 1.1 but requiring features of qemu not
present in qemu 1.0 will be properly rejected.
Not exactly, the default support of qemu-ga is qemu 1.0. This means that by
default qemu-ga will only support qemu 1.0 even when running on qemu 2.0. This
way the set-support-level command allows you to specify that qemu 2.0 features
are supported.
Version numbers are meaningless. What happens when a bunch of features get
backported by RHEL such that qemu-ga 1.0 ends up being a frankenstein version of
2.0?
The feature negotiation mechanism we have in QMP is the existence of a command.
If we're in a position where we're trying to disable part of a command, it
simply means that we should have multiple commands such that we can just remove
the disabled part entirely.
You may have a point that we shouldn't be using the version number for that,
but just switching to multiple commands doesn't solve the fundamental problem.
The fundamental problem is that, S3 in current (and old) qemu has two known
bugs:
1. The screen is left black after S3 (it's a bug in seabios)
2. QEMU resumes the guest immediately (Gerd posted patches to address this)
We're going to address both issues in 1.1. However, if qemu-ga is installed in
an old qemu and S3 is used, the bugs will be triggered.
It's a management tool problem.
Before a management tool issues a command, it should query the existence of the
command to determine whether this version of QEMU has that capability. If the
tool needs to use two commands, it should query the existence of both of them.
In this case, the management tool needs a qemu-ga command *and* a QEMU command
(to resume from suspend) so it should query both of them.
Obviously, we wouldn't have a resume-from-suspend command in QEMU unless it S3
worked in QEMU as expected.
That's right, it's a coincidence, but I don't see why this wouldn't work.
I think we should do the following then:
1. Drop the set-support-level command
2. Split the guest-suspend command into guest-suspend-ram,
guest-suspend-hybrid,
guest-suspend-disk
3. Libvirt should query for _QEMU_'s 'wakeup' command before issuing
the guest-suspend-ram command
Michal, Michael, do you agree?
Yup yup, looks sound to me.
Alternatively, if there really was no reason to have a resume-from-suspend
command, this would be the point where we would add a capabilities command
adding the "working-s3" capability.
But with capabilities, this is a direct QEMU->management tool interaction, not a
proxy through the guest agent.
We shouldn't trust the guest agent and we certainly don't want to rely on the
guest agent to avoid sending an improper command to QEMU! That would be a
security
Re: [Qemu-devel] [libvirt] [PATCH RFC 0/4] Allow hibernation on guests
On 01/30/2012 09:58 AM, Eric Blake wrote: On 01/30/2012 07:44 AM, Luiz Capitulino wrote: I think we should do the following then: 1. Drop the set-support-level command 2. Split the guest-suspend command into guest-suspend-ram, guest-suspend-hybrid, guest-suspend-disk 3. Libvirt should query for _QEMU_'s 'wakeup' command before issuing the guest-suspend-ram command Michal, Michael, do you agree? I'm not Michal, but speaking for libvirt, this definitely sounds like the way to go. Questions: Should libvirt also check for 'wakeup' before attempting guest-suspend-hybrid? With guest-suspend-disk, what happens when the suspend completes? Does this look like a normal case of the guest powering down, which qemu then passes as an event to libvirt and libvirt then ends the qemu process? That would mean that the only difference from a normal guest shutdown is that on the next guest boot the guest's disk image allows to recover state from disk rather than booting from scratch; either way, a new qemu process is created to resume the guest, and qemu is doing nothing different in how it starts the guest (it's just that the guest itself does something different based on what it stored into the disk images before shutting down). Also, I know there has been talk about a qemu-ga command to resync clocks after a resume from S3 and/or 'loadvm'; is this command fully in place yet, and is it another command that libvirt should be checking for prior to allowing any S3 attempts? Hi Eric, I'm not aware of a clock re-sync command being worked on.. are we maybe talking about the guest-sync command qemu-ga currently has in place to re-sync the protocol stream after a client-side timeout?
Re: [Qemu-devel] [RFC Patch 3/7]Qemu: Cmd "block_set_hostcache" for dynamic cache change
On 01/31/2012 09:06 PM, Supriya Kannery wrote:
New command "block_set_hostcache" added for dynamically changing
host pagecache setting of a block device.
Usage:
block_set_hostcache
= block device
= on/off
Example:
(qemu) block_set_hostcache ide0-hd0 off
Signed-off-by: Supriya Kannery
---
block.c | 54 ++
block.h |2 ++
blockdev.c | 26 ++
blockdev.h |2 ++
hmp-commands.hx | 14 ++
qmp-commands.hx | 27 +++
6 files changed, 125 insertions(+)
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -808,6 +808,35 @@ unlink_and_fail:
return ret;
}
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
+{
+BlockDriver *drv = bs->drv;
+int ret = 0, open_flags;
+
+/* Quiesce IO for the given block device */
+qemu_aio_flush();
+ret = bdrv_flush(bs);
+if (ret != 0) {
+qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
+return ret;
+}
+open_flags = bs->open_flags;
+bdrv_close(bs);
+
+ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
+if (ret< 0) {
+/* Reopen failed. Try to open with original flags */
+qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+ret = bdrv_open(bs, bs->filename, open_flags, drv);
+if (ret< 0) {
+/* Reopen failed with orig and modified flags */
+abort();
+}
+}
+
+return ret;
+}
+
void bdrv_close(BlockDriverState *bs)
{
if (bs->drv) {
@@ -870,6 +899,33 @@ void bdrv_drain_all(void)
}
}
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache)
+{
+int bdrv_flags = bs->open_flags;
+
+/* set hostcache flags (without changing WCE/flush bits) */
+if (enable_host_cache) {
+bdrv_flags&= ~BDRV_O_NOCACHE;
+} else {
+bdrv_flags |= BDRV_O_NOCACHE;
+}
+
+/* If no change in flags, no need to reopen */
+if (bdrv_flags == bs->open_flags) {
+return 0;
+}
+
+if (bdrv_is_inserted(bs)) {
+/* Reopen file with changed set of flags */
+bdrv_flags&= ~BDRV_O_CACHE_WB;
+return bdrv_reopen(bs, bdrv_flags);
It seems like the real interface we're wanting here is bdrv_set_flags(),
or something along that line, with the re-opening being more of an
implementation detail.
For instance, with raw-posix.c:raw_reopen_prepare() we'll end up
skipping the re-opening completely if fcntl() is sufficient.
+} else {
+/* Save hostcache change for future use */
+bs->open_flags = bdrv_flags;
+return 0;
+}
+}
+
/* make a BlockDriverState anonymous by removing from bdrv_state list.
Also, NULL terminate the device_name to prevent double remove */
void bdrv_make_anon(BlockDriverState *bs)
Index: qemu/block.h
===
--- qemu.orig/block.h
+++ qemu/block.h
@@ -119,6 +119,7 @@ int bdrv_parse_cache_flags(const char *m
int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
BlockDriver *drv);
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
void bdrv_close(BlockDriverState *bs);
int bdrv_attach_dev(BlockDriverState *bs, void *dev);
void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
@@ -160,6 +161,7 @@ void bdrv_commit_all(void);
int bdrv_change_backing_file(BlockDriverState *bs,
const char *backing_file, const char *backing_fmt);
void bdrv_register(BlockDriver *bdrv);
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache);
typedef struct BdrvCheckResult {
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -1080,3 +1080,29 @@ BlockJobInfoList *qmp_query_block_jobs(E
bdrv_iterate(do_qmp_query_block_jobs_one,&prev);
return dummy.next;
}
+
+
+/*
+ * Change host page cache setting while guest is running.
+*/
+int do_block_set_hostcache(Monitor *mon, const QDict *qdict,
+ QObject **ret_data)
+{
+BlockDriverState *bs = NULL;
+int enable;
+const char *device;
+
+/* Validate device */
+device = qdict_get_str(qdict, "device");
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+/* Read hostcache setting */
+enable = qdict_get_bool(qdict, "option");
+return bdrv_change_hostcache(bs, enable);
+
+}
+
Index: qemu/blockdev.h
===
--- qemu.orig/blockdev.h
+++ qemu/blockdev.h
@@ -62,4 +62,6 @@ void qmp_change_blockdev(const char *dev
bool has_format, c
Re: [Qemu-devel] [RFC Patch 5/7]Qemu: raw-posix image file reopen
On 01/31/2012 09:07 PM, Supriya Kannery wrote:
raw-posix driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache dynamically is handled here.
Signed-off-by: Supriya Kannery
Index: qemu/block/raw.c
===
--- qemu.orig/block/raw.c
+++ qemu/block/raw.c
@@ -9,6 +9,22 @@ static int raw_open(BlockDriverState *bs
return 0;
}
+static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+ int flags)
+{
+return bdrv_reopen_prepare(bs->file, prs, flags);
+}
+
+static void raw_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+bdrv_reopen_commit(bs->file, rs);
+}
+
+static void raw_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+bdrv_reopen_abort(bs->file, rs);
+}
+
static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
@@ -109,6 +125,10 @@ static BlockDriver bdrv_raw = {
.instance_size = 1,
.bdrv_open = raw_open,
+.bdrv_reopen_prepare
+= raw_reopen_prepare,
+.bdrv_reopen_commit = raw_reopen_commit,
+.bdrv_reopen_abort = raw_reopen_abort,
.bdrv_close = raw_close,
.bdrv_co_readv = raw_co_readv,
Index: qemu/block/raw-posix.c
===
--- qemu.orig/block/raw-posix.c
+++ qemu/block/raw-posix.c
@@ -136,6 +136,11 @@ typedef struct BDRVRawState {
#endif
} BDRVRawState;
+typedef struct BDRVRawReopenState {
+BDRVReopenState reopen_state;
+BDRVRawState *stash_s;
+} BDRVRawReopenState;
+
static int fd_open(BlockDriverState *bs);
static int64_t raw_getlength(BlockDriverState *bs);
@@ -279,6 +284,71 @@ static int raw_open(BlockDriverState *bs
return raw_open_common(bs, filename, flags, 0);
}
+static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+ int flags)
+{
+BDRVRawReopenState *raw_rs = g_malloc0(sizeof(BDRVRawReopenState));
+BDRVRawState *s = bs->opaque;
+int ret = 0;
+
+raw_rs->reopen_state.bs = bs;
+
+/* stash state before reopen */
+raw_rs->stash_s = g_malloc0(sizeof(BDRVRawState));
+memcpy(raw_rs->stash_s, s, sizeof(BDRVRawState));
+s->fd = dup(raw_rs->stash_s->fd);
+
+*prs =&(raw_rs->reopen_state);
+
+/* Flags that can be set using fcntl */
+int fcntl_flags = BDRV_O_NOCACHE;
+
+if ((bs->open_flags& ~fcntl_flags) == (flags& ~fcntl_flags)) {
+if ((flags& BDRV_O_NOCACHE)) {
+s->open_flags |= O_DIRECT;
+} else {
+s->open_flags&= ~O_DIRECT;
+}
+printf("O_DIRECT flag\n");
+ret = fcntl_setfl(s->fd, s->open_flags);
raw-posix.c:raw_aio_submit() does some extra alignment work if
s->aligned_buf was set due to the image being opened O_DIRECT initially,
not sure what the impact is but probably want to clean that up here.
+} else {
+
+printf("close and open with new flags\n");
+/* close and reopen using new flags */
+bs->drv->bdrv_close(bs);
+ret = bs->drv->bdrv_file_open(bs, bs->filename, flags);
+}
+return ret;
+}
+
+static void raw_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+BDRVRawReopenState *raw_rs;
+
+raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+/* clean up stashed state */
+close(raw_rs->stash_s->fd);
+g_free(raw_rs->stash_s);
+g_free(raw_rs);
+}
+
+static void raw_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+BDRVRawReopenState *raw_rs;
+BDRVRawState *s = bs->opaque;
+
+raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+/* revert to stashed state */
+if (s->fd != -1) {
+close(s->fd);
+}
+memcpy(s, raw_rs->stash_s, sizeof(BDRVRawState));
+g_free(raw_rs->stash_s);
+g_free(raw_rs);
+}
+
/* XXX: use host sector size if necessary with:
#ifdef DIOCGSECTORSIZE
{
@@ -631,6 +701,9 @@ static BlockDriver bdrv_file = {
.instance_size = sizeof(BDRVRawState),
.bdrv_probe = NULL, /* no probe for protocols */
.bdrv_file_open = raw_open,
+.bdrv_reopen_prepare = raw_reopen_prepare,
+.bdrv_reopen_commit = raw_reopen_commit,
+.bdrv_reopen_abort = raw_reopen_abort,
.bdrv_close = raw_close,
.bdrv_create = raw_create,
.bdrv_co_discard = raw_co_discard,
[Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
These patches apply on top of qemu.git master, and can also be obtained from: git://github.com/mdroth/qemu.git qga-win32-v2 Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series since the s3 situation isn't fully sorted out yet. The file structure is a little different now, posix/linux-specific stuff goes in qga/commands-posix.c, win32-specific stuff in qga/commands-win32.c, but other than that it should be a straightforward rebase if this gets merged first. CHANGES SINCE V1: - Dropped guest-set-support-level patch dependency - Rebased on master and re-tested - Spelling/grammar fixes in commits/comments OVERVIEW: These patches add support for Windows to the QEMU guest agent. With these patches the following guest agent commands are supported on Windows: guest-ping guest-info guest-sync guest-shutdown The guest-file* commands can essentially be enabled for Windows as-is, but since mingw does not honor the O_NONBLOCK flag, they'll need to be reworked if we're to retain the current non-blocking behavior. The rest of the commands are currently stubbed out for Windows (qemu-ga will return an "unsupported" error), but it should be easy to implement these going forward with basic Windows support/infrastructure in place. The build was tested using Fedora15 with a MinGW cross-build target via: configure --enable-guest-agent --cross-prefix=i686-pc-mingw32- make qemu-ga.exe The executable was tested using Windows XP SP3, and partially tested using Windows Server 2008 and Windows 7 (no I/O for the latter 2, having issues with virtio-win drivers). GLib 2.28+ for Windows is required. You can install qemu-ga as a load-on-boot service by running: ./qemu-ga --service install And start/stop manually via: net start qemu-ga net stop qemu-ga Many thanks to Gal Hammer for contributing the service integration and shutdown code. Makefile |2 +- Makefile.objs |8 +- configure |2 +- qapi-schema-guest.json | 118 -- qemu-ga.c | 413 ++- qga/channel-posix.c| 246 +++ qga/channel-win32.c| 337 + qga/channel.h | 33 +++ qga/commands-posix.c | 528 +++ qga/commands-win32.c | 130 ++ qga/commands.c | 73 ++ qga/guest-agent-commands.c | 585 qga/guest-agent-core.h |3 +- qga/service-win32.c| 114 + qga/service-win32.h| 30 +++ 15 files changed, 1782 insertions(+), 840 deletions(-)
[Qemu-devel] [PATCH v2 2/8] qemu-ga: move channel/transport functionality into wrapper class
This is mostly in preparation for the win32 port, which won't use
GIO channels for reasons that will be made clearer later. Here the
GAChannel class is just a loose wrapper around GIOChannel
calls/callbacks, but we also roll in the logic/configuration for
various channel types and managing unix socket connections, which makes
the abstraction much more complete and further aids in the win32 port
since isa-serial/unix-listen will not be supported initially.
There's also a bit of refactoring in the main logic to consolidate the
exit paths so we can do common cleanup for things like pid files, which
weren't always cleaned up previously.
Signed-off-by: Michael Roth
---
Makefile.objs |1 +
qemu-ga.c | 306
qga/channel-posix.c| 246 ++
qga/channel.h | 33 +
qga/guest-agent-core.h |2 +-
5 files changed, 355 insertions(+), 233 deletions(-)
create mode 100644 qga/channel-posix.c
create mode 100644 qga/channel.h
diff --git a/Makefile.objs b/Makefile.objs
index b942625..27ff919 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -425,6 +425,7 @@ common-obj-y += qmp.o hmp.o
# guest agent
qga-nested-y = guest-agent-commands.o guest-agent-command-state.o
+qga-nested-y += channel-posix.o
qga-obj-y = $(addprefix qga/, $(qga-nested-y))
qga-obj-y += qemu-ga.o qemu-sockets.o module.o qemu-option.o
qga-obj-$(CONFIG_WIN32) += oslib-win32.o
diff --git a/qemu-ga.c b/qemu-ga.c
index 29e4f64..2e8af02 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -15,9 +15,7 @@
#include
#include
#include
-#include
#include
-#include "qemu_socket.h"
#include "json-streamer.h"
#include "json-parser.h"
#include "qint.h"
@@ -28,19 +26,15 @@
#include "qerror.h"
#include "error_int.h"
#include "qapi/qmp-core.h"
+#include "qga/channel.h"
#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
#define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid"
-#define QGA_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */
-#define QGA_TIMEOUT_DEFAULT 30*1000 /* ms */
struct GAState {
JSONMessageParser parser;
GMainLoop *main_loop;
-GIOChannel *conn_channel;
-GIOChannel *listen_channel;
-const char *path;
-const char *method;
+GAChannel *channel;
bool virtio; /* fastpath to check for virtio to deal with poll() quirks */
GACommandState *command_state;
GLogLevelFlags log_level;
@@ -59,7 +53,7 @@ static void quit_handler(int sig)
}
}
-static void register_signal_handlers(void)
+static gboolean register_signal_handlers(void)
{
struct sigaction sigact;
int ret;
@@ -70,12 +64,14 @@ static void register_signal_handlers(void)
ret = sigaction(SIGINT, &sigact, NULL);
if (ret == -1) {
g_error("error configuring signal handler: %s", strerror(errno));
-exit(EXIT_FAILURE);
+return false;
}
ret = sigaction(SIGTERM, &sigact, NULL);
if (ret == -1) {
g_error("error configuring signal handler: %s", strerror(errno));
+return false;
}
+return true;
}
static void usage(const char *cmd)
@@ -100,8 +96,6 @@ static void usage(const char *cmd)
, cmd, QGA_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT);
}
-static void conn_channel_close(GAState *s);
-
static const char *ga_log_level_str(GLogLevelFlags level)
{
switch (level & G_LOG_LEVEL_MASK) {
@@ -210,40 +204,13 @@ fail:
exit(EXIT_FAILURE);
}
-static int conn_channel_send_buf(GIOChannel *channel, const char *buf,
- gsize count)
-{
-GError *err = NULL;
-gsize written = 0;
-GIOStatus status;
-
-while (count) {
-status = g_io_channel_write_chars(channel, buf, count, &written, &err);
-g_debug("sending data, count: %d", (int)count);
-if (err != NULL) {
-g_warning("error sending newline: %s", err->message);
-return err->code;
-}
-if (status == G_IO_STATUS_ERROR || status == G_IO_STATUS_EOF) {
-return -EPIPE;
-}
-
-if (status == G_IO_STATUS_NORMAL) {
-count -= written;
-}
-}
-
-return 0;
-}
-
-static int conn_channel_send_payload(GIOChannel *channel, QObject *payload)
+static int send_response(GAState *s, QObject *payload)
{
-int ret = 0;
const char *buf;
QString *payload_qstr;
-GError *err = NULL;
+GIOStatus status;
-g_assert(payload && channel);
+g_assert(payload && s->channel);
payload_qstr = qobject_to_json(payload);
if (!payload_qstr) {
@@ -252,24 +219,13 @@ static int conn_channel_send_payload(GIOChannel *channel,
QObject *payload)
qstring_append_chr(payload_qstr, '\n&#x
[Qemu-devel] [PATCH v2 1/8] qemu-ga: Add schema documentation for types
Document guest agent schema types in similar fashion as qmp schema
types.
Signed-off-by: Michael Roth
---
qapi-schema-guest.json | 118 +++-
1 files changed, 97 insertions(+), 21 deletions(-)
diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index 5f8a18d..706925d 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -37,17 +37,42 @@
{ 'command': 'guest-ping' }
##
-# @guest-info:
+# @GuestAgentCommandInfo:
#
-# Get some information about the guest agent.
+# Information about guest agent commands.
#
-# Since: 0.15.0
+# @name: name of the command
+#
+# @enabled: whether command is currently enabled by guest admin
+#
+# Since 1.1.0
##
{ 'type': 'GuestAgentCommandInfo',
'data': { 'name': 'str', 'enabled': 'bool' } }
+
+##
+# @GuestAgentInfo
+#
+# Information about guest agent.
+#
+# @version: guest agent version
+#
+# @supported_commands: Information about guest agent commands
+#
+# Since 0.15.0
+##
{ 'type': 'GuestAgentInfo',
'data': { 'version': 'str',
'supported_commands': ['GuestAgentCommandInfo'] } }
+##
+# @guest-info:
+#
+# Get some information about the guest agent.
+#
+# Returns: @GuestAgentInfo
+#
+# Since: 0.15.0
+##
{ 'command': 'guest-info',
'returns': 'GuestAgentInfo' }
@@ -98,6 +123,23 @@
'data': { 'handle': 'int' } }
##
+# @GuestFileRead
+#
+# Result of guest agent file-read operation
+#
+# @count: number of bytes read (note: count is *before*
+# base64-encoding is applied)
+#
+# @buf-b64: base64-encoded bytes read
+#
+# @eof: whether EOF was encountered during read operation.
+#
+# Since: 0.15.0
+##
+{ 'type': 'GuestFileRead',
+ 'data': { 'count': 'int', 'buf-b64': 'str', 'eof': 'bool' } }
+
+##
# @guest-file-read:
#
# Read from an open file in the guest. Data will be base64-encoded
@@ -106,19 +148,30 @@
#
# @count: #optional maximum number of bytes to read (default is 4KB)
#
-# Returns: GuestFileRead on success. Note: count is number of bytes read
-# *before* base64 encoding bytes read.
+# Returns: @GuestFileRead on success.
#
# Since: 0.15.0
##
-{ 'type': 'GuestFileRead',
- 'data': { 'count': 'int', 'buf-b64': 'str', 'eof': 'bool' } }
-
{ 'command': 'guest-file-read',
'data':{ 'handle': 'int', '*count': 'int' },
'returns': 'GuestFileRead' }
##
+# @GuestFileWrite
+#
+# Result of guest agent file-write operation
+#
+# @count: number of bytes written (note: count is actual bytes
+# written, after base64-decoding of provided buffer)
+#
+# @eof: whether EOF was encountered during write operation.
+#
+# Since: 0.15.0
+##
+{ 'type': 'GuestFileWrite',
+ 'data': { 'count': 'int', 'eof': 'bool' } }
+
+##
# @guest-file-write:
#
# Write to an open file in the guest.
@@ -130,17 +183,29 @@
# @count: #optional bytes to write (actual bytes, after base64-decode),
# default is all content in buf-b64 buffer after base64 decoding
#
-# Returns: GuestFileWrite on success. Note: count is the number of bytes
-# base64-decoded bytes written
+# Returns: @GuestFileWrite on success.
#
# Since: 0.15.0
##
-{ 'type': 'GuestFileWrite',
- 'data': { 'count': 'int', 'eof': 'bool' } }
{ 'command': 'guest-file-write',
'data':{ 'handle': 'int', 'buf-b64': 'str', '*count': 'int' },
'returns': 'GuestFileWrite' }
+
+##
+# @GuestFileSeek
+#
+# Result of guest agent file-seek operation
+#
+# @position: current file position
+#
+# @eof: whether EOF was encountered during file seek
+#
+# Since: 0.15.0
+##
+{ 'type': 'GuestFileSeek',
+ 'data': { 'position': 'int', 'eof': 'bool' } }
+
##
# @guest-file-seek:
#
@@ -154,13 +219,10 @@
#
# @whence: SEEK_SET, SEEK_CUR, or SEEK_END, as with fseek()
#
-# Returns: GuestFileSeek on success.
+# Returns: @GuestFileSeek on success.
#
# Since: 0.15.0
##
-{ 'type': 'GuestFileSeek',
- 'data': { 'position': 'int', 'eof': 'bool' } }
-
{ 'command': 'guest-file-seek',
'data':{ 'handle': 'int', 'offset': 'int', 'whence': 'in
[Qemu-devel] [PATCH v2 8/8] qemu-ga: add win32 guest-shutdown command
Implement guest-shutdown RPC for Windows. Functionally this should be
equivalent to the posix implementation.
Original patch by Gal Hammer
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 41 -
1 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index d96f1ad..4aa0f0d 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -15,9 +15,48 @@
#include "qga-qmp-commands.h"
#include "qerror.h"
+#ifndef SHTDN_REASON_FLAG_PLANNED
+#define SHTDN_REASON_FLAG_PLANNED 0x8000
+#endif
+
void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
{
-error_set(err, QERR_UNSUPPORTED);
+HANDLE token;
+TOKEN_PRIVILEGES priv;
+UINT shutdown_flag = EWX_FORCE;
+
+slog("guest-shutdown called, mode: %s", mode);
+
+if (!has_mode || strcmp(mode, "powerdown") == 0) {
+shutdown_flag |= EWX_POWEROFF;
+} else if (strcmp(mode, "halt") == 0) {
+shutdown_flag |= EWX_SHUTDOWN;
+} else if (strcmp(mode, "reboot") == 0) {
+shutdown_flag |= EWX_REBOOT;
+} else {
+error_set(err, QERR_INVALID_PARAMETER_VALUE, "mode",
+ "halt|powerdown|reboot");
+return;
+}
+
+/* Request a shutdown privilege, but try to shut down the system
+ anyway. */
+if (OpenProcessToken(GetCurrentProcess(),
+TOKEN_ADJUST_PRIVILEGES|TOKEN_QUERY, &token))
+{
+LookupPrivilegeValue(NULL, SE_SHUTDOWN_NAME,
+&priv.Privileges[0].Luid);
+
+priv.PrivilegeCount = 1;
+priv.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
+
+AdjustTokenPrivileges(token, FALSE, &priv, 0, NULL, 0);
+}
+
+if (!ExitWindowsEx(shutdown_flag, SHTDN_REASON_FLAG_PLANNED)) {
+slog("guest-shutdown failed: %d", GetLastError());
+error_set(err, QERR_UNDEFINED_ERROR);
+}
}
int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
Error **err)
--
1.7.4.1
[Qemu-devel] [PATCH v2 3/8] qemu-ga: separate out common commands from posix-specific ones
Many of the current RPC implementations are very much POSIX-specific
and require complete re-writes for Windows. There are however a small
set of core guest agent commands that are common to both, and other
commands such as guest-file-* which *may* be portable. So we introduce
commands.c for the latter, and will rename guest-agent-commands.c to
commands-posix.c in a future commit. Windows implementations will go in
commands-win32.c, eventually.
Signed-off-by: Michael Roth
---
Makefile.objs |2 +-
qga/commands.c | 73
qga/guest-agent-commands.c | 59 +---
qga/guest-agent-core.h |1 +
4 files changed, 76 insertions(+), 59 deletions(-)
create mode 100644 qga/commands.c
diff --git a/Makefile.objs b/Makefile.objs
index 27ff919..d70cebe 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -424,7 +424,7 @@ common-obj-y += qmp.o hmp.o
##
# guest agent
-qga-nested-y = guest-agent-commands.o guest-agent-command-state.o
+qga-nested-y = commands.o guest-agent-commands.o guest-agent-command-state.o
qga-nested-y += channel-posix.o
qga-obj-y = $(addprefix qga/, $(qga-nested-y))
qga-obj-y += qemu-ga.o qemu-sockets.o module.o qemu-option.o
diff --git a/qga/commands.c b/qga/commands.c
new file mode 100644
index 000..b27407d
--- /dev/null
+++ b/qga/commands.c
@@ -0,0 +1,73 @@
+/*
+ * QEMU Guest Agent common/cross-platform command implementations
+ *
+ * Copyright IBM Corp. 2012
+ *
+ * Authors:
+ * Michael Roth
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include
+#include "qga/guest-agent-core.h"
+#include "qga-qmp-commands.h"
+#include "qerror.h"
+
+/* Note: in some situations, like with the fsfreeze, logging may be
+ * temporarilly disabled. if it is necessary that a command be able
+ * to log for accounting purposes, check ga_logging_enabled() beforehand,
+ * and use the QERR_QGA_LOGGING_DISABLED to generate an error
+ */
+void slog(const gchar *fmt, ...)
+{
+va_list ap;
+
+va_start(ap, fmt);
+g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap);
+va_end(ap);
+}
+
+int64_t qmp_guest_sync(int64_t id, Error **errp)
+{
+return id;
+}
+
+void qmp_guest_ping(Error **err)
+{
+slog("guest-ping called");
+}
+
+struct GuestAgentInfo *qmp_guest_info(Error **err)
+{
+GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
+GuestAgentCommandInfo *cmd_info;
+GuestAgentCommandInfoList *cmd_info_list;
+char **cmd_list_head, **cmd_list;
+
+info->version = g_strdup(QGA_VERSION);
+
+cmd_list_head = cmd_list = qmp_get_command_list();
+if (*cmd_list_head == NULL) {
+goto out;
+}
+
+while (*cmd_list) {
+cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
+cmd_info->name = strdup(*cmd_list);
+cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
+
+cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
+cmd_info_list->value = cmd_info;
+cmd_info_list->next = info->supported_commands;
+info->supported_commands = cmd_info_list;
+
+g_free(*cmd_list);
+cmd_list++;
+}
+
+out:
+g_free(cmd_list_head);
+return info;
+}
diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
index a09c8ca..126127a 100644
--- a/qga/guest-agent-commands.c
+++ b/qga/guest-agent-commands.c
@@ -1,5 +1,5 @@
/*
- * QEMU Guest Agent commands
+ * QEMU Guest Agent POSIX-specific command implementations
*
* Copyright IBM Corp. 2011
*
@@ -30,63 +30,6 @@
static GAState *ga_state;
-/* Note: in some situations, like with the fsfreeze, logging may be
- * temporarilly disabled. if it is necessary that a command be able
- * to log for accounting purposes, check ga_logging_enabled() beforehand,
- * and use the QERR_QGA_LOGGING_DISABLED to generate an error
- */
-static void slog(const char *fmt, ...)
-{
-va_list ap;
-
-va_start(ap, fmt);
-g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap);
-va_end(ap);
-}
-
-int64_t qmp_guest_sync(int64_t id, Error **errp)
-{
-return id;
-}
-
-void qmp_guest_ping(Error **err)
-{
-slog("guest-ping called");
-}
-
-struct GuestAgentInfo *qmp_guest_info(Error **err)
-{
-GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
-GuestAgentCommandInfo *cmd_info;
-GuestAgentCommandInfoList *cmd_info_list;
-char **cmd_list_head, **cmd_list;
-
-info->version = g_strdup(QGA_VERSION);
-
-cmd_list_head = cmd_list = qmp_get_command_list();
-if (*cmd_list_head == NULL) {
-goto out;
-}
-
-while (*cmd_list) {
-cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
-cmd_info->name = strdup(*cmd_list);
-cmd
[Qemu-devel] [PATCH v2 6/8] qemu-ga: add initial win32 support
This adds a win32 channel implementation that makes qemu-ga functional
on Windows using virtio-serial (unix-listen/isa-serial not currently
implemented). Unlike with the posix implementation, we do not use
GIOChannel for the following reasons:
- glib calls stat() on an fd to check whether S_IFCHR is set, which is
the case for virtio-serial on win32. Because of that, a one-time
check to determine whether the channel is readable is done by making
a call to PeekConsoleInput(), which reports the underlying handle is
not a valid console handle, and thus we can never read from the
channel.
- if one goes as far as to "trick" glib into thinking it is a normal
file descripter, the buffering is done in such a way that data
written to the output stream will subsequently result in that same
data being read back as if it were input, causing an error loop.
furthermore, a forced flush of the channel only moves the data into a
secondary buffer managed by glib, so there's no way to prevent output
from getting read back as input.
The implementation here ties into the glib main loop by implementing a
custom GSource that continually submits asynchronous/overlapped I/O to
fill an GAChannel-managed read buffer, and tells glib to poll the
corresponding event handle for a completion whenever there is no
data/RPC in the read buffer to notify the main application about.
Signed-off-by: Michael Roth
---
Makefile.objs |2 +-
qemu-ga.c |4 +
qga/channel-win32.c | 337 +++
3 files changed, 342 insertions(+), 1 deletions(-)
create mode 100644 qga/channel-win32.c
diff --git a/Makefile.objs b/Makefile.objs
index 18e79ce..e1cb54a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -426,7 +426,7 @@ common-obj-y += qmp.o hmp.o
qga-nested-y = commands.o guest-agent-command-state.o
qga-nested-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
-qga-nested-$(CONFIG_WIN32) += commands-win32.o
+qga-nested-$(CONFIG_WIN32) += commands-win32.o channel-win32.o
qga-obj-y = $(addprefix qga/, $(qga-nested-y))
qga-obj-y += qemu-ga.o module.o
qga-obj-$(CONFIG_WIN32) += oslib-win32.o
diff --git a/qemu-ga.c b/qemu-ga.c
index 93ebc3e..8e517b5 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -30,7 +30,11 @@
#include "qapi/qmp-core.h"
#include "qga/channel.h"
+#ifndef _WIN32
#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
+#else
+#define QGA_VIRTIO_PATH_DEFAULT ".\\Global\\org.qemu.guest_agent.0"
+#endif
#define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid"
struct GAState {
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
new file mode 100644
index 000..9d8601a
--- /dev/null
+++ b/qga/channel-win32.c
@@ -0,0 +1,337 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "qga/guest-agent-core.h"
+#include "qga/channel.h"
+
+typedef struct GAChannelReadState {
+guint thread_id;
+uint8_t *buf;
+size_t buf_size;
+size_t cur; /* current buffer start */
+size_t pending; /* pending buffered bytes to read */
+OVERLAPPED ov;
+bool ov_pending; /* whether on async read is outstanding */
+} GAChannelReadState;
+
+struct GAChannel {
+HANDLE handle;
+GAChannelCallback cb;
+gpointer user_data;
+GAChannelReadState rstate;
+GIOCondition pending_events; /* TODO: use GAWatch.pollfd.revents */
+GSource *source;
+};
+
+typedef struct GAWatch {
+GSource source;
+GPollFD pollfd;
+GAChannel *channel;
+GIOCondition events_mask;
+} GAWatch;
+
+/*
+ * Called by glib prior to polling to set up poll events if polling is needed.
+ *
+ */
+static gboolean ga_channel_prepare(GSource *source, gint *timeout_ms)
+{
+GAWatch *watch = (GAWatch *)source;
+GAChannel *c = (GAChannel *)watch->channel;
+GAChannelReadState *rs = &c->rstate;
+DWORD count_read, count_to_read = 0;
+bool success;
+GIOCondition new_events = 0;
+
+g_debug("prepare");
+/* go ahead and submit another read if there's room in the buffer
+ * and no previous reads are outstanding
+ */
+if (!rs->ov_pending) {
+if (rs->cur + rs->pending >= rs->buf_size) {
+if (rs->cur) {
+memmove(rs->buf, rs->buf + rs->cur, rs->pending);
+rs->cur = 0;
+}
+}
+count_to_read = rs->buf_size - rs->cur - rs->pending;
+}
+
+if (rs->ov_pending || count_to_read <= 0) {
+goto out;
+}
+
+/* submit the read */
+success = ReadFile(c->handle, rs->buf + rs->cur + rs->pending,
+ count_to_read, &count_read, &rs->ov);
+if (success) {
+rs->pending += count_read;
+rs->ov_pending = false;
+} else {
+if (GetLastError() ==
[Qemu-devel] [PATCH v2 4/8] qemu-ga: rename guest-agent-commands.c -> commands-posix.c
Signed-off-by: Michael Roth
---
Makefile.objs |2 +-
qga/commands-posix.c | 528
qga/guest-agent-commands.c | 528
3 files changed, 529 insertions(+), 529 deletions(-)
create mode 100644 qga/commands-posix.c
delete mode 100644 qga/guest-agent-commands.c
diff --git a/Makefile.objs b/Makefile.objs
index d70cebe..2e2efb4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -424,7 +424,7 @@ common-obj-y += qmp.o hmp.o
##
# guest agent
-qga-nested-y = commands.o guest-agent-commands.o guest-agent-command-state.o
+qga-nested-y = commands.o commands-posix.o guest-agent-command-state.o
qga-nested-y += channel-posix.o
qga-obj-y = $(addprefix qga/, $(qga-nested-y))
qga-obj-y += qemu-ga.o qemu-sockets.o module.o qemu-option.o
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
new file mode 100644
index 000..126127a
--- /dev/null
+++ b/qga/commands-posix.c
@@ -0,0 +1,528 @@
+/*
+ * QEMU Guest Agent POSIX-specific command implementations
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ * Michael Roth
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include
+
+#if defined(__linux__)
+#include
+#include
+
+#if defined(__linux__) && defined(FIFREEZE)
+#define CONFIG_FSFREEZE
+#endif
+#endif
+
+#include
+#include
+#include "qga/guest-agent-core.h"
+#include "qga-qmp-commands.h"
+#include "qerror.h"
+#include "qemu-queue.h"
+
+static GAState *ga_state;
+
+void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
+{
+int ret;
+const char *shutdown_flag;
+
+slog("guest-shutdown called, mode: %s", mode);
+if (!has_mode || strcmp(mode, "powerdown") == 0) {
+shutdown_flag = "-P";
+} else if (strcmp(mode, "halt") == 0) {
+shutdown_flag = "-H";
+} else if (strcmp(mode, "reboot") == 0) {
+shutdown_flag = "-r";
+} else {
+error_set(err, QERR_INVALID_PARAMETER_VALUE, "mode",
+ "halt|powerdown|reboot");
+return;
+}
+
+ret = fork();
+if (ret == 0) {
+/* child, start the shutdown */
+setsid();
+fclose(stdin);
+fclose(stdout);
+fclose(stderr);
+
+ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
+"hypervisor initiated shutdown", (char*)NULL);
+if (ret) {
+slog("guest-shutdown failed: %s", strerror(errno));
+}
+exit(!!ret);
+} else if (ret < 0) {
+error_set(err, QERR_UNDEFINED_ERROR);
+}
+}
+
+typedef struct GuestFileHandle {
+uint64_t id;
+FILE *fh;
+QTAILQ_ENTRY(GuestFileHandle) next;
+} GuestFileHandle;
+
+static struct {
+QTAILQ_HEAD(, GuestFileHandle) filehandles;
+} guest_file_state;
+
+static void guest_file_handle_add(FILE *fh)
+{
+GuestFileHandle *gfh;
+
+gfh = g_malloc0(sizeof(GuestFileHandle));
+gfh->id = fileno(fh);
+gfh->fh = fh;
+QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
+}
+
+static GuestFileHandle *guest_file_handle_find(int64_t id)
+{
+GuestFileHandle *gfh;
+
+QTAILQ_FOREACH(gfh, &guest_file_state.filehandles, next)
+{
+if (gfh->id == id) {
+return gfh;
+}
+}
+
+return NULL;
+}
+
+int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
Error **err)
+{
+FILE *fh;
+int fd;
+int64_t ret = -1;
+
+if (!has_mode) {
+mode = "r";
+}
+slog("guest-file-open called, filepath: %s, mode: %s", path, mode);
+fh = fopen(path, mode);
+if (!fh) {
+error_set(err, QERR_OPEN_FILE_FAILED, path);
+return -1;
+}
+
+/* set fd non-blocking to avoid common use cases (like reading from a
+ * named pipe) from hanging the agent
+ */
+fd = fileno(fh);
+ret = fcntl(fd, F_GETFL);
+ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
+if (ret == -1) {
+error_set(err, QERR_QGA_COMMAND_FAILED, "fcntl() failed");
+fclose(fh);
+return -1;
+}
+
+guest_file_handle_add(fh);
+slog("guest-file-open, handle: %d", fd);
+return fd;
+}
+
+void qmp_guest_file_close(int64_t handle, Error **err)
+{
+GuestFileHandle *gfh = guest_file_handle_find(handle);
+int ret;
+
+slog("guest-file-close called, handle: %ld", handle);
+if (!gfh) {
+error_set(err, QERR_FD_NOT_FOUND, "handle");
+return;
+}
+
+ret = fclose(gfh->fh);
+if (ret == -1) {
+error_set(err, QERR_QGA_COMMAND
[Qemu-devel] [PATCH v2 7/8] qemu-ga: add Windows service integration
This allows qemu-ga to function as a Windows service:
- to install the service (will auto-start on boot):
qemu-ga --service install
- to start the service:
net start qemu-ga
- to stop the service:
net stop qemu-ga
- to uninstall service:
qemu-ga --service uninstall
Original patch by Gal Hammer
Signed-off-by: Michael Roth
---
Makefile.objs |2 +-
qemu-ga.c | 103 --
qga/service-win32.c | 114 +++
qga/service-win32.h | 30 +
4 files changed, 244 insertions(+), 5 deletions(-)
create mode 100644 qga/service-win32.c
create mode 100644 qga/service-win32.h
diff --git a/Makefile.objs b/Makefile.objs
index e1cb54a..3b08e70 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -426,7 +426,7 @@ common-obj-y += qmp.o hmp.o
qga-nested-y = commands.o guest-agent-command-state.o
qga-nested-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
-qga-nested-$(CONFIG_WIN32) += commands-win32.o channel-win32.o
+qga-nested-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o
qga-obj-y = $(addprefix qga/, $(qga-nested-y))
qga-obj-y += qemu-ga.o module.o
qga-obj-$(CONFIG_WIN32) += oslib-win32.o
diff --git a/qemu-ga.c b/qemu-ga.c
index 8e517b5..92f81ed 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -29,6 +29,10 @@
#include "error_int.h"
#include "qapi/qmp-core.h"
#include "qga/channel.h"
+#ifdef _WIN32
+#include "qga/service-win32.h"
+#include
+#endif
#ifndef _WIN32
#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
@@ -46,11 +50,19 @@ struct GAState {
GLogLevelFlags log_level;
FILE *log_file;
bool logging_enabled;
+#ifdef _WIN32
+GAService service;
+#endif
};
static struct GAState *ga_state;
-#ifndef _WIN32
+#ifdef _WIN32
+DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
+ LPVOID ctx);
+VOID WINAPI service_main(DWORD argc, TCHAR *argv[]);
+#endif
+
static void quit_handler(int sig)
{
g_debug("received signal num %d, quitting", sig);
@@ -60,6 +72,7 @@ static void quit_handler(int sig)
}
}
+#ifndef _WIN32
static gboolean register_signal_handlers(void)
{
struct sigaction sigact;
@@ -95,8 +108,9 @@ static void usage(const char *cmd)
" -f, --pidfile specify pidfile (default is %s)\n"
" -v, --verbose log extra debugging information\n"
" -V, --version print version information and exit\n"
-#ifndef _WIN32
" -d, --daemonize become a daemon\n"
+#ifdef _WIN32
+" -s, --service service commands: install, uninstall\n"
#endif
" -b, --blacklist comma-separated list of RPCs to disable (no spaces, \"?\""
"to list available RPCs)\n"
@@ -394,10 +408,64 @@ static gboolean channel_init(GAState *s, const gchar
*method, const gchar *path)
return true;
}
+#ifdef _WIN32
+DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
+ LPVOID ctx)
+{
+DWORD ret = NO_ERROR;
+GAService *service = &ga_state->service;
+
+switch (ctrl)
+{
+case SERVICE_CONTROL_STOP:
+case SERVICE_CONTROL_SHUTDOWN:
+quit_handler(SIGTERM);
+service->status.dwCurrentState = SERVICE_STOP_PENDING;
+SetServiceStatus(service->status_handle, &service->status);
+break;
+
+default:
+ret = ERROR_CALL_NOT_IMPLEMENTED;
+}
+return ret;
+}
+
+VOID WINAPI service_main(DWORD argc, TCHAR *argv[])
+{
+GAService *service = &ga_state->service;
+
+service->status_handle = RegisterServiceCtrlHandlerEx(QGA_SERVICE_NAME,
+service_ctrl_handler, NULL);
+
+if (service->status_handle == 0) {
+g_critical("Failed to register extended requests function!\n");
+return;
+}
+
+service->status.dwServiceType = SERVICE_WIN32;
+service->status.dwCurrentState = SERVICE_RUNNING;
+service->status.dwControlsAccepted = SERVICE_ACCEPT_STOP |
SERVICE_ACCEPT_SHUTDOWN;
+service->status.dwWin32ExitCode = NO_ERROR;
+service->status.dwServiceSpecificExitCode = NO_ERROR;
+service->status.dwCheckPoint = 0;
+service->status.dwWaitHint = 0;
+SetServiceStatus(service->status_handle, &service->status);
+
+g_main_loop_run(ga_state->main_loop);
+
+service->status.dwCurrentState = SERVICE_STOPPED;
+SetServiceStatus(service->status_handle, &service->status);
+}
+#endif
+
int main(int argc, char **argv)
{
-const char *sopt = "hVvdm:p:l:f:b:";
+const char *sopt = "hVvdm:p:l:f:b:s:";
const char *method = NULL, *path = NULL, *pidfile = QGA_PIDFILE_DEFAULT;
+const char *log_file_name =
[Qemu-devel] [PATCH v2 5/8] qemu-ga: fixes for win32 build of qemu-ga
Various stubs and #ifdefs to compile for Windows using mingw
cross-build. Still has 1 linker error due to a dependency on the
forthcoming win32 versions of the GAChannel/transport class.
Signed-off-by: Michael Roth
---
Makefile |2 +-
Makefile.objs|9 +++--
configure|2 +-
qemu-ga.c| 16 +
qga/commands-win32.c | 91 ++
5 files changed, 114 insertions(+), 6 deletions(-)
create mode 100644 qga/commands-win32.c
diff --git a/Makefile b/Makefile
index 2560b59..9baa532 100644
--- a/Makefile
+++ b/Makefile
@@ -199,7 +199,7 @@ QGALIB_GEN=$(addprefix $(qapi-dir)/, qga-qapi-types.h
qga-qapi-visit.h qga-qmp-c
$(QGALIB_OBJ): $(QGALIB_GEN) $(GENERATED_HEADERS)
$(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) $(GENERATED_HEADERS)
-qemu-ga$(EXESUF): qemu-ga.o $(qga-obj-y) $(qapi-obj-y) $(tools-obj-y)
$(qobject-obj-y) $(version-obj-y) $(QGALIB_OBJ)
+qemu-ga$(EXESUF): qemu-ga.o $(qga-obj-y) $(tools-obj-y) $(qapi-obj-y)
$(qobject-obj-y) $(version-obj-y) $(QGALIB_OBJ)
QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
diff --git a/Makefile.objs b/Makefile.objs
index 2e2efb4..18e79ce 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -424,12 +424,13 @@ common-obj-y += qmp.o hmp.o
##
# guest agent
-qga-nested-y = commands.o commands-posix.o guest-agent-command-state.o
-qga-nested-y += channel-posix.o
+qga-nested-y = commands.o guest-agent-command-state.o
+qga-nested-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
+qga-nested-$(CONFIG_WIN32) += commands-win32.o
qga-obj-y = $(addprefix qga/, $(qga-nested-y))
-qga-obj-y += qemu-ga.o qemu-sockets.o module.o qemu-option.o
+qga-obj-y += qemu-ga.o module.o
qga-obj-$(CONFIG_WIN32) += oslib-win32.o
-qga-obj-$(CONFIG_POSIX) += oslib-posix.o
+qga-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-sockets.o qemu-option.o
vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
diff --git a/configure b/configure
index 3b0b300..aaf8171 100755
--- a/configure
+++ b/configure
@@ -508,7 +508,7 @@ if test "$mingw32" = "yes" ; then
bindir="\${prefix}"
sysconfdir="\${prefix}"
confsuffix=""
- guest_agent="no"
+ libs_qga="-lws2_32 -lwinmm $lib_qga"
fi
werror=""
diff --git a/qemu-ga.c b/qemu-ga.c
index 2e8af02..93ebc3e 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -15,7 +15,9 @@
#include
#include
#include
+#ifndef _WIN32
#include
+#endif
#include "json-streamer.h"
#include "json-parser.h"
#include "qint.h"
@@ -44,6 +46,7 @@ struct GAState {
static struct GAState *ga_state;
+#ifndef _WIN32
static void quit_handler(int sig)
{
g_debug("received signal num %d, quitting", sig);
@@ -73,6 +76,7 @@ static gboolean register_signal_handlers(void)
}
return true;
}
+#endif
static void usage(const char *cmd)
{
@@ -87,7 +91,9 @@ static void usage(const char *cmd)
" -f, --pidfile specify pidfile (default is %s)\n"
" -v, --verbose log extra debugging information\n"
" -V, --version print version information and exit\n"
+#ifndef _WIN32
" -d, --daemonize become a daemon\n"
+#endif
" -b, --blacklist comma-separated list of RPCs to disable (no spaces, \"?\""
"to list available RPCs)\n"
" -h, --helpdisplay this help and exit\n"
@@ -143,9 +149,13 @@ static void ga_log(const gchar *domain, GLogLevelFlags
level,
}
level &= G_LOG_LEVEL_MASK;
+#ifndef _WIN32
if (domain && strcmp(domain, "syslog") == 0) {
syslog(LOG_INFO, "%s: %s", level_str, msg);
} else if (level & s->log_level) {
+#else
+if (level & s->log_level) {
+#endif
g_get_current_time(&time);
fprintf(s->log_file,
"%lu.%lu: %s: %s\n", time.tv_sec, time.tv_usec, level_str,
msg);
@@ -153,6 +163,7 @@ static void ga_log(const gchar *domain, GLogLevelFlags
level,
}
}
+#ifndef _WIN32
static void become_daemon(const char *pidfile)
{
pid_t pid, sid;
@@ -203,6 +214,7 @@ fail:
g_critical("failed to daemonize");
exit(EXIT_FAILURE);
}
+#endif
static int send_response(GAState *s, QObject *payload)
{
@@ -466,10 +478,12 @@ int main(int argc, char **argv)
}
}
+#ifndef _WIN32
if (daemonize) {
g_debug("starting daemon");
become_daemon(pidfile);
}
+#endif
s = g_malloc0(sizeof(GAState));
s->log_file = log_file;
@@ -482,10 +496,12 @@ int main(int argc, char **argv)
ga_command_state_init_all(s->command_state);
json_message_parser_init(&s->parser, process_event);
ga_state = s;
+#ifndef _WIN32
if (!register_signal_handlers()) {
g_critic
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On 02/03/2012 08:18 AM, Luiz Capitulino wrote: On Thu, 2 Feb 2012 13:58:52 -0600 Michael Roth wrote: These patches apply on top of qemu.git master, and can also be obtained from: git://github.com/mdroth/qemu.git qga-win32-v2 Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series since the s3 situation isn't fully sorted out yet. The file structure is a little different now, posix/linux-specific stuff goes in qga/commands-posix.c, win32-specific stuff in qga/commands-win32.c, but other than that it should be a straightforward rebase if this gets merged first. I think I'll have to rebase my series on top of this one, when do you plan to merge this? Hopefully soon, was planning on waiting for the suspend/hibernate bits but we seem to be blocked on the s3 issues and I have other patches accumulating on top of win32 (hesitant to base those on master since this patchset does a lot of refactoring that might affect them), so I figured I'd push this for merge since it doesn't have any dependencies outside master.
Re: [Qemu-devel] [PATCH v2 2/8] qemu-ga: move channel/transport functionalit
On 02/02/2012 10:25 PM, MATSUDA, Daiki wrote: Hi, Michael! Thank you for your working. And I have a question the process id written in pid file. If qemu-ga is ran as daemon, the parent process id not child is written in pid file. So, id gotten by 'ps' command is different. Is it correct work? Many other daemon writes child process id. Regards MATSUDA Daiki Hi Matsuda, Thank you for testing! In the become_daemon() function, the parent exits immediately after the fork(), so only the child has the opportunity to write to the pid file. It calls getpid() to get the pid to write, which should be it's own lwpid. So I'm not seeing where there's an opportunity for the parent pid to be written. Can you confirm? It seems to behave as expected for me: [root@vm ~]# /home/mdroth/w/qemu-build/qemu-ga -d ** (process:7441): DEBUG: starting daemon [root@vm ~]# ps aux | grep qemu-ga root 7442 0.0 0.0 13792 348 ?Ss 10:56 0:00 /home/mdroth/w/qemu-build/qemu-ga -d root 7471 0.0 0.1 109108 816 pts/2R+ 11:00 0:00 grep --color=auto qemu-ga [root@vm ~]# cat /var/run/qemu-ga.pid 7442
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On 02/03/2012 10:45 AM, Luiz Capitulino wrote: On Fri, 03 Feb 2012 10:37:25 -0600 Michael Roth wrote: On 02/03/2012 08:18 AM, Luiz Capitulino wrote: On Thu, 2 Feb 2012 13:58:52 -0600 Michael Roth wrote: These patches apply on top of qemu.git master, and can also be obtained from: git://github.com/mdroth/qemu.git qga-win32-v2 Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series since the s3 situation isn't fully sorted out yet. The file structure is a little different now, posix/linux-specific stuff goes in qga/commands-posix.c, win32-specific stuff in qga/commands-win32.c, but other than that it should be a straightforward rebase if this gets merged first. I think I'll have to rebase my series on top of this one, when do you plan to merge this? Hopefully soon, was planning on waiting for the suspend/hibernate bits but we seem to be blocked on the s3 issues and I have other patches accumulating on top of win32 (hesitant to base those on master since this patchset does a lot of refactoring that might affect them), so I figured I'd push this for merge since it doesn't have any dependencies outside master. The S3 issues seem sorted to me, but I don't oppose having this series in first. Thanks, in retrospect I probably should've just gotten these out of the way weeks ago since they'd immediately clobber git blame. I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need for guest-suspend, is that still the case? I guess those are coming through your QMP queue?
[Qemu-devel] [PATCH 0/1] qemu-ga: add guest-sync-delimited
Sorry for the somewhat redundant cover letter, but needed to note that: This applies on top of "[PATCH v2 0/8] qemu-ga: add support for Windows", and can also be obtained from: git://github.com/mdroth/qemu.git qga-guest-sync-delimited As noted in the commit there's a wiki write-up with more details on what exactly this is for: http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol It's not absolutely required, but it does make dealing with some communication corner cases a heck of a lot easier.
[Qemu-devel] [PATCH] qemu-ga: add guest-sync-delimited
guest-sync leaves it as an exercise to the user as to how to reliably
obtain the response to guest-sync if the client had previously read in a
partial response (due qemu-ga previously being restarted mid-"sentence"
due to reboot, forced restart, etc).
qemu-ga handles this situation on its end by having a client precede
their guest-sync request with a 0xFF byte (invalid UTF-8), which
qemu-ga/QEMU JSON parsers will treat as a flush event. Thus we can
reliably flush the qemu-ga parser state in preparation for receiving
the guest-sync request.
guest-sync-delimited provides the same functionality for a client: when
a guest-sync-delimited is issued, qemu-ga will precede it's response
with a 0xFF byte that the client can use as an indicator to flush its
buffer/parser state in preparation for reliably receiving the
guest-sync-delimited response.
More information available on the wiki:
http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol
Signed-off-by: Michael Roth
---
qapi-schema-guest.json | 42 +-
qemu-ga.c | 29 +++--
qga/commands-posix.c |3 ---
qga/commands.c |6 ++
qga/guest-agent-core.h |2 ++
5 files changed, 72 insertions(+), 10 deletions(-)
diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index 706925d..1a44357 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -1,6 +1,40 @@
# *-*- Mode: Python -*-*
##
+#
+# Echo back a unique integer value, and prepend to response a
+# leading sentinel byte (0xFF) the client can check scan for.
+#
+# This is used by clients talking to the guest agent over the
+# wire to ensure the stream is in sync and doesn't contain stale
+# data from previous client. It must be issued upon initial
+# connection, and after any client-side timeouts (including
+# timeouts on receiving a response to this command).
+#
+# After issuing this request, all guest agent responses should be
+# ignored until the response containing the unique integer value
+# the client passed in is returned. Receival of the 0xFF sentinel
+# byte must be handled as an indication that the client's
+# lexer/tokenizer/parser state should be flushed/reset in
+# preparation for reliably receiving the subsequent response.
+#
+# Similarly, clients should also precede this *request*
+# with a 0xFF byte to make sure the guest agent flushes any
+# partially read JSON data from a previous client connection.
+#
+# This command deprecates the guest-sync command.
+#
+# @id: randomly generated 64-bit integer
+#
+# Returns: The unique integer id passed in by the client
+#
+# Since: 1.1
+# ##
+{ 'command': 'guest-sync-delimited'
+ 'data':{ 'id': 'int' },
+ 'returns': 'int' }
+
+##
# @guest-sync:
#
# Echo back a unique integer value
@@ -13,8 +47,12 @@
# partially-delivered JSON text in such a way that this response
# can be obtained.
#
+# In cases where a partial stale response was previously
+# received by the client, this cannot always be done reliably.
+# Use guest-sync-delimited instead.
+#
# Such clients should also precede this command
-# with a 0xFF byte to make such the guest agent flushes any
+# with a 0xFF byte to make sure the guest agent flushes any
# partially read JSON data from a previous session.
#
# @id: randomly generated 64-bit integer
@@ -22,6 +60,8 @@
# Returns: The unique integer id passed in by the client
#
# Since: 0.15.0
+# Deprecated since: 1.1
+# Deprecated by: guest-sync-delimited
##
{ 'command': 'guest-sync'
'data':{ 'id': 'int' },
diff --git a/qemu-ga.c b/qemu-ga.c
index 92f81ed..d567241 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -40,6 +40,7 @@
#define QGA_VIRTIO_PATH_DEFAULT ".\\Global\\org.qemu.guest_agent.0"
#endif
#define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid"
+#define QGA_SENTINEL_BYTE 0xFF
struct GAState {
JSONMessageParser parser;
@@ -53,9 +54,10 @@ struct GAState {
#ifdef _WIN32
GAService service;
#endif
+bool delimit_response;
};
-static struct GAState *ga_state;
+struct GAState *ga_state;
#ifdef _WIN32
DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
@@ -140,6 +142,11 @@ static const char *ga_log_level_str(GLogLevelFlags level)
}
}
+void ga_set_response_delimited(GAState *s)
+{
+s->delimit_response = true;
+}
+
bool ga_logging_enabled(GAState *s)
{
return s->logging_enabled;
@@ -236,8 +243,8 @@ fail:
static int send_response(GAState *s, QObject *payload)
{
-const char *buf;
-QString *payload_qstr;
+const char *buf, *buf2;
+QString *payload_qstr, *response_qstr;
GIOStatus status;
g_assert(payload && s->channel);
@@ -247,10 +254,20 @@ static int send_response(GAState *s, QObject *payload)
Re: [Qemu-devel] [PATCH 0/1] qemu-ga: add guest-sync-delimited
On Mon, Feb 06, 2012 at 06:07:34PM -0600, Michael Roth wrote: > Sorry for the somewhat redundant cover letter, but needed to note that: > > This applies on top of "[PATCH v2 0/8] qemu-ga: add support for Windows", and > can also be obtained from: > git://github.com/mdroth/qemu.git qga-guest-sync-delimited > > As noted in the commit there's a wiki write-up with more details on what > exactly this is for: > > http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol > > It's not absolutely required, but it does make dealing with some communication > corner cases a heck of a lot easier. > Actually, let me re-spin this without the deprecated flag for guest-sync, since I suspect a more common client implementation would do something like: qga.write(obj_to_json_string(req_obj)) resp_obj = obj_from_json_string(qga.readline()) as opposed to the JSON streamer that qemu-ga/QMP uses. You might miss a response doing the above approach if there was garbage in the channel, but you'll always been able to recover on the second attempt. So guest-sync is still useful there, assuming we codify 1-line-per-response in the QGA protocol.
[Qemu-devel] [PATCH v2 0/1] qemu-ga: add guest-sync-delimited
Sorry for the somewhat redundant cover letter, but needed to note that: This applies on top of "[PATCH v2 0/8] qemu-ga: add support for Windows", and can also be obtained from: git://github.com/mdroth/qemu.git qga-guest-sync-delimited CHANGES SINCE v1: - removed deprecation flag for guest-sync, still useful for some implementations As noted in the commit there's a wiki write-up with more details on what exactly this is for: http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol It's not absolutely required, but it does make dealing with some communication corner cases a heck of a lot easier for stream-based (as opposed to readline-based) client implementations.
[Qemu-devel] [PATCH v2] qemu-ga: add guest-sync-delimited
guest-sync leaves it as an exercise to the user as to how to reliably
obtain the response to guest-sync if the client had previously read in a
partial response (due qemu-ga previously being restarted mid-"sentence"
due to reboot, forced restart, etc).
qemu-ga handles this situation on its end by having a client precede
their guest-sync request with a 0xFF byte (invalid UTF-8), which
qemu-ga/QEMU JSON parsers will treat as a flush event. Thus we can
reliably flush the qemu-ga parser state in preparation for receiving
the guest-sync request.
guest-sync-delimited provides the same functionality for a client: when
a guest-sync-delimited is issued, qemu-ga will precede it's response
with a 0xFF byte that the client can use as an indicator to flush its
buffer/parser state in preparation for reliably receiving the
guest-sync-delimited response.
It is also useful as an optimization for clients, since, after issuing a
guest-sync-delimited, clients can safely discard all stale data read
from the channel until the 0xFF is found.
More information available on the wiki:
http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol
Signed-off-by: Michael Roth
---
qapi-schema-guest.json | 48 +++-
qemu-ga.c | 29 +++--
qga/commands-posix.c |3 ---
qga/commands.c |6 ++
qga/guest-agent-core.h |2 ++
5 files changed, 78 insertions(+), 10 deletions(-)
diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index 706925d..e2cb9ef 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -1,6 +1,41 @@
# *-*- Mode: Python -*-*
##
+#
+# Echo back a unique integer value, and prepend to response a
+# leading sentinel byte (0xFF) the client can check scan for.
+#
+# This is used by clients talking to the guest agent over the
+# wire to ensure the stream is in sync and doesn't contain stale
+# data from previous client. It must be issued upon initial
+# connection, and after any client-side timeouts (including
+# timeouts on receiving a response to this command).
+#
+# After issuing this request, all guest agent responses should be
+# ignored until the response containing the unique integer value
+# the client passed in is returned. Receival of the 0xFF sentinel
+# byte must be handled as an indication that the client's
+# lexer/tokenizer/parser state should be flushed/reset in
+# preparation for reliably receiving the subsequent response. As
+# an optimization, clients may opt to ignore all data until a
+# sentinel value is receiving to avoid unecessary processing of
+# stale data.
+#
+# Similarly, clients should also precede this *request*
+# with a 0xFF byte to make sure the guest agent flushes any
+# partially read JSON data from a previous client connection.
+#
+# @id: randomly generated 64-bit integer
+#
+# Returns: The unique integer id passed in by the client
+#
+# Since: 1.1
+# ##
+{ 'command': 'guest-sync-delimited'
+ 'data':{ 'id': 'int' },
+ 'returns': 'int' }
+
+##
# @guest-sync:
#
# Echo back a unique integer value
@@ -13,8 +48,19 @@
# partially-delivered JSON text in such a way that this response
# can be obtained.
#
+# In cases where a partial stale response was previously
+# received by the client, this cannot always be done reliably.
+# One particular scenario being if qemu-ga responses are fed
+# character-by-character into a JSON parser. In these situations,
+# using guest-sync-delimited may be optimal.
+#
+# For clients that fetch responses line by line and convert them
+# to JSON objects, guest-sync should be sufficient, but note that
+# in cases where the channel is dirty some attempts at parsing the
+# response may result in a parser error.
+#
# Such clients should also precede this command
-# with a 0xFF byte to make such the guest agent flushes any
+# with a 0xFF byte to make sure the guest agent flushes any
# partially read JSON data from a previous session.
#
# @id: randomly generated 64-bit integer
diff --git a/qemu-ga.c b/qemu-ga.c
index 92f81ed..d567241 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -40,6 +40,7 @@
#define QGA_VIRTIO_PATH_DEFAULT ".\\Global\\org.qemu.guest_agent.0"
#endif
#define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid"
+#define QGA_SENTINEL_BYTE 0xFF
struct GAState {
JSONMessageParser parser;
@@ -53,9 +54,10 @@ struct GAState {
#ifdef _WIN32
GAService service;
#endif
+bool delimit_response;
};
-static struct GAState *ga_state;
+struct GAState *ga_state;
#ifdef _WIN32
DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
@@ -140,6 +142,11 @@ static const char *ga_log_level_str(GLogLevelFlags level)
}
}
+void ga_set_response_delimited(GAState *s)
+{
+s->delimit_response = true;
+}
+
bool ga_logging_enabled(GAState *s)
{
return s->log
Re: [Qemu-devel] [PATCH v2] qemu-ga: add guest-sync-delimited
On Tue, Feb 07, 2012 at 04:17:26PM +0100, Michal Privoznik wrote: > On 07.02.2012 02:09, Michael Roth wrote: > > guest-sync leaves it as an exercise to the user as to how to reliably > > obtain the response to guest-sync if the client had previously read in a > > partial response (due qemu-ga previously being restarted mid-"sentence" > > due to reboot, forced restart, etc). > > > > qemu-ga handles this situation on its end by having a client precede > > their guest-sync request with a 0xFF byte (invalid UTF-8), which > > qemu-ga/QEMU JSON parsers will treat as a flush event. Thus we can > > reliably flush the qemu-ga parser state in preparation for receiving > > the guest-sync request. > > > > guest-sync-delimited provides the same functionality for a client: when > > a guest-sync-delimited is issued, qemu-ga will precede it's response > > with a 0xFF byte that the client can use as an indicator to flush its > > buffer/parser state in preparation for reliably receiving the > > guest-sync-delimited response. > > > > It is also useful as an optimization for clients, since, after issuing a > > guest-sync-delimited, clients can safely discard all stale data read > > from the channel until the 0xFF is found. > > > > More information available on the wiki: > > > > http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol > > > > Signed-off-by: Michael Roth > > This makes sense. And it's workable for libvirt. > IIUC, client can send 0xFF to the guest agent and vice versa, right? Yup, and it should be considered a requirement to send the 0xFF before guest-sync* to avoid some of the potential corner cases mentioned, since on the qemu-ga side the 0xFF reliance is baked in. Clients have a choice as to how they want to deal with this scenario so we offer both guest-sync (no 0xFF) and guest-sync-delimited (precede response with 0xFF). > > Michal >
