* [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2)
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:12 ` [Qemu-devel] " Anthony Liguori
` (2 more replies)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 02/15] qapi: add code generator for type marshallers Anthony Liguori
` (14 subsequent siblings)
15 siblings, 3 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
Only generate qmp-types.[ch]. These files contain the type definitions for
QMP along with the alloc/free functions for these types. Functions to convert
enum values to integers and vice versa are also included.
qmp-types is used both within QEMU and within libqmp
Special alloc/free functions are provided to ensure that all structures are
padded when allocated. This makes sure that libqmp can provide a forward
compatible interface since all additions to a structure will have a boolean
enable flag.
The free function is convenient since individual structures may have pointers
that also require freeing.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
- modify code generator to use multiline strings instead of prints
- support proxy commands
- support async commands
diff --git a/Makefile b/Makefile
index 6b1d716..6b9fd69 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
ifeq ($(TRACE_BACKEND),dtrace)
GENERATED_HEADERS += trace-dtrace.h
endif
+GENERATED_HEADERS += qmp-types.h
ifneq ($(wildcard config-host.mak),)
# Put the all: rule here so that config-host.mak can contain dependencies.
@@ -146,6 +147,14 @@ trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
+qmp-types.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
+ $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-body < $< > $@, " GEN $@")
+
+qmp-types.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
+ $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-header < $< > $@, " GEN $@")
+
+qmp-types.o: qmp-types.c qmp-types.h
+
version.o: $(SRC_PATH)/version.rc config-host.mak
$(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
diff --git a/Makefile.objs b/Makefile.objs
index 69f0383..710d99f 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -15,7 +15,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
-block-obj-y += error.o
+block-obj-y += error.o qmp-types.o
block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/ordereddict.py b/ordereddict.py
new file mode 100644
index 0000000..e17269f
--- /dev/null
+++ b/ordereddict.py
@@ -0,0 +1,128 @@
+# Copyright (c) 2009 Raymond Hettinger
+#
+# Permission is hereby granted, free of charge, to any person
+# obtaining a copy of this software and associated documentation files
+# (the "Software"), to deal in the Software without restriction,
+# including without limitation the rights to use, copy, modify, merge,
+# publish, distribute, sublicense, and/or sell copies of the Software,
+# and to permit persons to whom the Software is furnished to do so,
+# subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+# OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+
+from UserDict import DictMixin
+
+class OrderedDict(dict, DictMixin):
+
+ def __init__(self, *args, **kwds):
+ if len(args) > 1:
+ raise TypeError('expected at most 1 arguments, got %d' % len(args))
+ try:
+ self.__end
+ except AttributeError:
+ self.clear()
+ self.update(*args, **kwds)
+
+ def clear(self):
+ self.__end = end = []
+ end += [None, end, end] # sentinel node for doubly linked list
+ self.__map = {} # key --> [key, prev, next]
+ dict.clear(self)
+
+ def __setitem__(self, key, value):
+ if key not in self:
+ end = self.__end
+ curr = end[1]
+ curr[2] = end[1] = self.__map[key] = [key, curr, end]
+ dict.__setitem__(self, key, value)
+
+ def __delitem__(self, key):
+ dict.__delitem__(self, key)
+ key, prev, next = self.__map.pop(key)
+ prev[2] = next
+ next[1] = prev
+
+ def __iter__(self):
+ end = self.__end
+ curr = end[2]
+ while curr is not end:
+ yield curr[0]
+ curr = curr[2]
+
+ def __reversed__(self):
+ end = self.__end
+ curr = end[1]
+ while curr is not end:
+ yield curr[0]
+ curr = curr[1]
+
+ def popitem(self, last=True):
+ if not self:
+ raise KeyError('dictionary is empty')
+ if last:
+ key = reversed(self).next()
+ else:
+ key = iter(self).next()
+ value = self.pop(key)
+ return key, value
+
+ def __reduce__(self):
+ items = [[k, self[k]] for k in self]
+ tmp = self.__map, self.__end
+ del self.__map, self.__end
+ inst_dict = vars(self).copy()
+ self.__map, self.__end = tmp
+ if inst_dict:
+ return (self.__class__, (items,), inst_dict)
+ return self.__class__, (items,)
+
+ def keys(self):
+ return list(self)
+
+ setdefault = DictMixin.setdefault
+ update = DictMixin.update
+ pop = DictMixin.pop
+ values = DictMixin.values
+ items = DictMixin.items
+ iterkeys = DictMixin.iterkeys
+ itervalues = DictMixin.itervalues
+ iteritems = DictMixin.iteritems
+
+ def __repr__(self):
+ if not self:
+ return '%s()' % (self.__class__.__name__,)
+ return '%s(%r)' % (self.__class__.__name__, self.items())
+
+ def copy(self):
+ return self.__class__(self)
+
+ @classmethod
+ def fromkeys(cls, iterable, value=None):
+ d = cls()
+ for key in iterable:
+ d[key] = value
+ return d
+
+ def __eq__(self, other):
+ if isinstance(other, OrderedDict):
+ if len(self) != len(other):
+ return False
+ for p, q in zip(self.items(), other.items()):
+ if p != q:
+ return False
+ return True
+ return dict.__eq__(self, other)
+
+ def __ne__(self, other):
+ return not self == other
+
diff --git a/qmp-gen.py b/qmp-gen.py
new file mode 100644
index 0000000..cded2f6
--- /dev/null
+++ b/qmp-gen.py
@@ -0,0 +1,516 @@
+##
+# QAPI Code Generator
+#
+# Copyright IBM, Corp. 2011
+#
+# Authors:
+# Anthony Liguori <aliguori@us.ibm.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2. See
+# the COPYING file in the top-level directory.
+##
+import sys
+from ordereddict import OrderedDict
+
+enum_types = []
+event_types = {}
+
+def qmp_is_proxy_cmd(name):
+ return name.startswith('guest-')
+
+def qmp_is_async_cmd(name):
+ return name.startswith('guest-')
+
+def qmp_is_stateful_cmd(name):
+ return name in ['qmp_capabilities', 'put-event', 'getfd', 'closefd']
+
+def c_var(name):
+ return '_'.join(name.split('-'))
+
+def genindent(count):
+ ret = ""
+ for i in range(count):
+ ret += " "
+ return ret
+
+indent_level = 0
+
+def push_indent():
+ global indent_level
+ indent_level += 4
+
+def pop_indent():
+ global indent_level
+ indent_level -= 4
+
+def cgen(code, **kwds):
+ indent = genindent(indent_level)
+ lines = code.split('\n')
+ lines = map(lambda x: indent + x, lines)
+ return '\n'.join(lines) % kwds + '\n'
+
+def mcgen(code, **kwds):
+ return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
+
+def is_dict(obj):
+ if type(obj) in [dict, OrderedDict]:
+ return True
+ return False
+
+def qmp_array_type_to_c(typename):
+ if type(typename) == list or is_dict(typename):
+ return qmp_type_to_c(typename)
+ elif typename == 'int':
+ return 'IntArray *'
+ elif typename == 'str':
+ return 'StringArray *'
+ elif typename == 'bool':
+ return 'BoolArray *'
+ elif typename == 'number':
+ return 'DoubleArray *'
+ else:
+ return qmp_type_to_c(typename)
+
+def qmp_type_should_free(typename):
+ if (type(typename) == list or
+ typename == 'str' or
+ (typename not in ['int', 'bool', 'number'] and
+ typename not in enum_types and not typename.isupper())):
+ return True
+ return False
+
+def qmp_free_func(typename):
+ if type(typename) == list:
+ return qmp_free_func(typename[0])
+ elif typename == 'str':
+ return 'qemu_free'
+ else:
+ return 'qmp_free_%s' % (de_camel_case(typename))
+
+def qmp_type_is_event(typename):
+ if type(typename) == str and typename.isupper():
+ return True
+ return False
+
+def qmp_type_to_c(typename, retval=False, indent=0):
+ if type(typename) == list:
+ return qmp_array_type_to_c(typename[0])
+ elif is_dict(typename):
+ string = 'struct {\n'
+ for argname, argtype, optional in parse_args(typename):
+ if optional:
+ string += "%sbool has_%s;\n" % (genindent(indent + 4), c_var(argname))
+ string += "%s%s %s;\n" % (genindent(indent + 4),
+ qmp_type_to_c(argtype, True,
+ indent=(indent + 4)),
+ c_var(argname))
+ string += "%s}" % genindent(indent)
+ return string
+ elif typename == 'int':
+ return 'int64_t'
+ elif not retval and typename == 'str':
+ return 'const char *'
+ elif retval and typename == 'str':
+ return 'char *'
+ elif typename == 'bool':
+ return 'bool'
+ elif typename == 'number':
+ return 'double'
+ elif typename == 'none':
+ return 'void'
+ elif typename in enum_types:
+ return typename
+ elif qmp_type_is_event(typename):
+ return 'struct %s *' % qmp_event_to_c(typename)
+ else:
+ return 'struct %s *' % typename
+
+def qmp_type_to_qobj(typename):
+ return 'qmp_marshal_type_%s' % typename
+
+def qmp_type_from_qobj(typename):
+ return 'qmp_unmarshal_type_%s' % typename
+
+def parse_args(typeinfo):
+ for member in typeinfo:
+ argname = member
+ argtype = typeinfo[member]
+ optional = False
+ if member.startswith('*'):
+ argname = member[1:]
+ optional = True
+ yield (argname, argtype, optional)
+
+def de_camel_case(name):
+ new_name = ''
+ for ch in name:
+ if ch.isupper() and new_name:
+ new_name += '_'
+ new_name += ch.lower()
+ return new_name
+
+def camel_case(name):
+ new_name = ''
+ first = True
+ for ch in name:
+ if ch in ['_', '-']:
+ first = True
+ elif first:
+ new_name += ch.upper()
+ first = False
+ else:
+ new_name += ch.lower()
+ return new_name
+
+def qmp_event_to_c(name):
+ return '%sEvent' % camel_case(name)
+
+def qmp_event_func_to_c(name):
+ return '%sFunc' % camel_case(name)
+
+def enum_abbreviation(name):
+ ab = ''
+ for ch in name:
+ if ch.isupper():
+ ab += ch
+ return ab
+
+def gen_type_declaration(name, typeinfo):
+ ret = ''
+ if type(typeinfo) == str:
+ ret += mcgen('''
+
+typedef %(type)s %(name)s;
+''',
+ type=qmp_type_to_c(typeinfo),
+ name=name)
+ elif is_dict(typeinfo) and not name.isupper():
+ ret += mcgen('''
+
+typedef struct %(name)s %(name)s;
+struct %(name)s {
+''', name=name)
+ for argname, argtype, optional in parse_args(typeinfo):
+ if optional:
+ ret += cgen(' bool has_%(c_name)s;',
+ c_name=c_var(argname))
+ ret += cgen(' %(type)s %(c_name)s;',
+ type=qmp_type_to_c(argtype, True, indent=4),
+ c_name=c_var(argname))
+ ret += mcgen('''
+ %(c_name)s *next;
+};
+
+%(name)s *qmp_alloc_%(dcc_name)s(void);
+void qmp_free_%(dcc_name)s(%(name)s *obj);
+''',
+ c_name=c_var(name), name=name,
+ dcc_name=de_camel_case(name))
+ elif is_dict(typeinfo) and name.isupper():
+ arglist = ['void *opaque']
+ for argname, argtype, optional in parse_args(typeinfo):
+ arglist.append('%s %s' % (qmp_type_to_c(argtype), argname))
+ ret += mcgen('''
+
+typedef void (%(event_func)s)(%(args)s);
+
+typedef struct %(c_event)s {
+ QmpSignal *signal;
+ %(event_func)s *func;
+} %(c_event)s;
+''',
+ event_func=qmp_event_func_to_c(name),
+ args=', '.join(arglist),
+ c_event=qmp_event_to_c(name))
+ return ret
+
+def gen_metatype_free(typeinfo, prefix):
+ ret = ''
+
+ for argname, argtype, optional in parse_args(typeinfo):
+ if type(argtype) == list:
+ argtype = argtype[0]
+
+ if is_dict(argtype):
+ if optional:
+ ret += cgen(' if (%(prefix)shas_%(c_name)s) {',
+ prefix=prefix, c_name=c_var(argname))
+ push_indent()
+ ret += gen_metatype_free(argtype, '%s%s.' % (prefix, argname))
+ if optional:
+ pop_indent()
+ ret += cgen(' }')
+ elif qmp_type_should_free(argtype):
+ if optional:
+ ret += mcgen('''
+ if (%(prefix)shas_%(c_name)s) {
+ %(free)s(%(prefix)s%(c_name)s);
+ }
+''',
+ prefix=prefix, c_name=c_var(argname),
+ free=qmp_free_func(argtype))
+ else:
+ ret += mcgen('''
+ %(free)s(%(prefix)s%(c_name)s);
+''',
+ prefix=prefix, c_name=c_var(argname),
+ free=qmp_free_func(argtype))
+
+ return ret
+
+def gen_type_definition(name, typeinfo):
+ return mcgen('''
+
+void qmp_free_%(dcc_name)s(%(name)s *obj)
+{
+ if (!obj) {
+ return;
+ }
+%(type_free)s
+
+ %(free)s(obj->next);
+ qemu_free(obj);
+}
+
+%(name)s *qmp_alloc_%(dcc_name)s(void)
+{
+ BUILD_ASSERT(sizeof(%(name)s) < 512);
+ return qemu_mallocz(512);
+}
+''',
+ dcc_name=de_camel_case(name), name=name,
+ free=qmp_free_func(name),
+ type_free=gen_metatype_free(typeinfo, 'obj->'))
+
+def gen_enum_declaration(name, entries):
+ ret = mcgen('''
+
+typedef enum %(name)s {
+''', name=name)
+ i = 0
+ for entry in entries:
+ ret += cgen(' %(abrev)s_%(name)s = %(value)d,',
+ abrev=enum_abbreviation(name),
+ name=entry.upper(), value=i)
+ i += 1
+ ret += mcgen('''
+} %(name)s;
+
+%(name)s qmp_type_%(dcc_name)s_from_str(const char *str, Error **errp);
+const char *qmp_type_%(dcc_name)s_to_str(%(name)s value, Error **errp);
+''',
+ name=name, dcc_name=de_camel_case(name))
+ return ret
+
+def gen_enum_definition(name, entries):
+ ret = mcgen('''
+
+%(name)s qmp_type_%(dcc_name)s_from_str(const char *str, Error **errp)
+{
+''',
+ name=name,
+ dcc_name=de_camel_case(name))
+ first = True
+ for entry in entries:
+ prefix = '} else '
+ if first:
+ prefix = ''
+ first = False
+ ret += mcgen('''
+ %(prefix)sif (strcmp(str, "%(entry)s") == 0) {
+ return %(abrev)s_%(value)s;
+''',
+ prefix=prefix, entry=entry,
+ abrev=enum_abbreviation(name), value=entry.upper())
+
+ ret += mcgen('''
+ } else {
+ error_set(errp, QERR_ENUM_VALUE_INVALID, "%(name)s", str);
+ return %(abrev)s_%(value)s;
+ }
+}
+
+const char *qmp_type_%(dcc_name)s_to_str(%(name)s value, Error **errp)
+{
+''',
+ name=name, abrev=enum_abbreviation(name),
+ value=entries[0].upper(), dcc_name=de_camel_case(name))
+
+ first = True
+ for entry in entries:
+ enum = '%s_%s' % (enum_abbreviation(name), entry.upper())
+ prefix = '} else '
+ if first:
+ prefix = ''
+ first = False
+ ret += mcgen('''
+ %(prefix)sif (value == %(enum)s) {
+ return "%(entry)s";
+''',
+ entry=entry, prefix=prefix, enum=enum)
+ ret += mcgen('''
+ } else {
+ char buf[32];
+ snprintf(buf, sizeof(buf), "%%d", value);
+ error_set(errp, QERR_ENUM_VALUE_INVALID, "%(name)s", buf);
+ return NULL;
+ }
+}
+''',
+ name=name)
+ return ret
+
+def tokenize(data):
+ while len(data):
+ if data[0] in ['{', '}', ':', ',', '[', ']']:
+ yield data[0]
+ data = data[1:]
+ elif data[0] in ' \n':
+ data = data[1:]
+ elif data[0] == "'":
+ data = data[1:]
+ string = ''
+ while data[0] != "'":
+ string += data[0]
+ data = data[1:]
+ data = data[1:]
+ yield string
+
+def parse_value(tokens):
+ if tokens[0] == '{':
+ ret = OrderedDict()
+ tokens = tokens[1:]
+ while tokens[0] != '}':
+ key = tokens[0]
+ tokens = tokens[1:]
+
+ tokens = tokens[1:] # :
+
+ value, tokens = parse_value(tokens)
+
+ if tokens[0] == ',':
+ tokens = tokens[1:]
+
+ ret[key] = value
+ tokens = tokens[1:]
+ return ret, tokens
+ elif tokens[0] == '[':
+ ret = []
+ tokens = tokens[1:]
+ while tokens[0] != ']':
+ value, tokens = parse_value(tokens)
+ if tokens[0] == ',':
+ tokens = tokens[1:]
+ ret.append(value)
+ tokens = tokens[1:]
+ return ret, tokens
+ else:
+ return tokens[0], tokens[1:]
+
+def ordered_eval(string):
+ return parse_value(map(lambda x: x, tokenize(string)))[0]
+# return eval(string)
+
+def generate(kind):
+ global enum_types
+ global event_types
+ global indent_level
+
+ enum_types = []
+ event_types = {}
+ indent_level = 0
+
+ ret = mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT EDIT */
+''')
+
+ if kind == 'types-header':
+ ret += mcgen('''
+#ifndef QMP_TYPES_H
+#define QMP_TYPES_H
+
+#include "qmp-types-core.h"
+''')
+ elif kind == 'types-body':
+ ret += mcgen('''
+#include "qmp-types.h"
+#include "qmp-marshal-types.h"
+''')
+
+ exprs = []
+ expr = ''
+
+ for line in sys.stdin:
+ if line.startswith('#') or line == '\n':
+ continue
+
+ if line.startswith(' '):
+ expr += line
+ elif expr:
+ s = ordered_eval(expr)
+ exprs.append(s)
+ expr = line
+ else:
+ expr += line
+
+ if expr:
+ s = ordered_eval(expr)
+ exprs.append(s)
+
+ for s in exprs:
+ if s.has_key('type'):
+ name = s['type']
+ data = s['data']
+
+ if kind == 'types-body':
+ ret += gen_type_definition(name, data)
+ elif kind == 'types-header':
+ ret += gen_type_declaration(name, data)
+ elif s.has_key('enum'):
+ name = s['enum']
+ data = s['data']
+
+ enum_types.append(s['enum'])
+ if kind == 'types-header':
+ ret += gen_enum_declaration(name, data)
+ elif kind == 'types-body':
+ ret += gen_enum_definition(name, data)
+ elif s.has_key('event'):
+ name = s['event']
+ data = {}
+ if s.has_key('data'):
+ data = s['data']
+
+ event_types[name] = data
+ if kind == 'types-header':
+ ret += gen_type_declaration(name, data)
+ elif s.has_key('command'):
+ name = s['command']
+ options = {}
+ if s.has_key('data'):
+ options = s['data']
+ retval = 'none'
+ if s.has_key('returns'):
+ retval = s['returns']
+
+ if kind.endswith('header'):
+ ret += cgen('#endif')
+
+ return ret
+
+def main(args):
+ if len(args) != 1:
+ return 1
+ if not args[0].startswith('--'):
+ return 1
+
+ kind = args[0][2:]
+
+ ret = generate(kind)
+
+ sys.stdout.write(ret)
+
+ return 0
+
+if __name__ == '__main__':
+ sys.exit(main(sys.argv[1:]))
diff --git a/qmp-schema.json b/qmp-schema.json
new file mode 100644
index 0000000..e69de29
diff --git a/qmp-types-core.h b/qmp-types-core.h
new file mode 100644
index 0000000..a018ba7
--- /dev/null
+++ b/qmp-types-core.h
@@ -0,0 +1,29 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#ifndef QMP_TYPES_CORE_H
+#define QMP_TYPES_CORE_H
+
+#include <stdint.h>
+#include <stdbool.h>
+#include "error.h"
+
+typedef struct QmpSignal QmpSignal;
+typedef struct QmpCommandState QmpCommandState;
+typedef struct QmpState QmpState;
+
+#define BUILD_ASSERT(cond) do { \
+ (void)sizeof(int[-1+!!(cond)]); \
+} while (0)
+
+#define BUILD_BUG() BUILD_ASSERT(0)
+
+#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] Re: [PATCH 01/15] qapi: add code generator for qmp-types (v2)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2) Anthony Liguori
@ 2011-03-11 23:12 ` Anthony Liguori
2011-03-12 11:29 ` [Qemu-devel] " Blue Swirl
2011-03-18 14:14 ` [Qemu-devel] " Luiz Capitulino
2 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:12 UTC (permalink / raw)
To: Anthony Liguori
Cc: Markus Armbruster, Luiz Capitulino, agl, qemu-devel, Avi Kivity
On 03/11/2011 05:05 PM, Anthony Liguori wrote:
> Only generate qmp-types.[ch]. These files contain the type definitions for
> QMP along with the alloc/free functions for these types. Functions to convert
> enum values to integers and vice versa are also included.
>
> qmp-types is used both within QEMU and within libqmp
>
> Special alloc/free functions are provided to ensure that all structures are
> padded when allocated. This makes sure that libqmp can provide a forward
> compatible interface since all additions to a structure will have a boolean
> enable flag.
>
> The free function is convenient since individual structures may have pointers
> that also require freeing.
>
> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> ---
> v1 -> v2
> - modify code generator to use multiline strings instead of prints
> - support proxy commands
> - support async commands
As a friendly hint for reviewers, qmp-gen.py reads a lot better with
syntax highlighting as the Python code stands out from the C code. I've
pushed these patches to:
http://repo.or.cz/w/qemu/aliguori.git qapi/round.1-v2
If you'd prefer to clone that and look at the code in a git tree.
This new series generates the same code as the last series (minus a
little whitespace differences) so you can refer to the previous series
to see the code output.+def qmp_event_func_to_c(name):
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2) Anthony Liguori
2011-03-11 23:12 ` [Qemu-devel] " Anthony Liguori
@ 2011-03-12 11:29 ` Blue Swirl
2011-03-12 15:00 ` Anthony Liguori
2011-03-18 14:14 ` [Qemu-devel] " Luiz Capitulino
2 siblings, 1 reply; 49+ messages in thread
From: Blue Swirl @ 2011-03-12 11:29 UTC (permalink / raw)
To: Anthony Liguori
Cc: Markus Armbruster, Avi Kivity, Luiz Capitulino, qemu-devel,
Adam Litke
On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Only generate qmp-types.[ch]. These files contain the type definitions for
> QMP along with the alloc/free functions for these types. Functions to convert
> enum values to integers and vice versa are also included.
>
> qmp-types is used both within QEMU and within libqmp
>
> Special alloc/free functions are provided to ensure that all structures are
> padded when allocated. This makes sure that libqmp can provide a forward
> compatible interface since all additions to a structure will have a boolean
> enable flag.
>
> The free function is convenient since individual structures may have pointers
> that also require freeing.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> v1 -> v2
> - modify code generator to use multiline strings instead of prints
> - support proxy commands
> - support async commands
>
> diff --git a/Makefile b/Makefile
> index 6b1d716..6b9fd69 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -4,6 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
> ifeq ($(TRACE_BACKEND),dtrace)
> GENERATED_HEADERS += trace-dtrace.h
> endif
> +GENERATED_HEADERS += qmp-types.h
>
> ifneq ($(wildcard config-host.mak),)
> # Put the all: rule here so that config-host.mak can contain dependencies.
> @@ -146,6 +147,14 @@ trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
>
> simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
>
> +qmp-types.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-body < $< > $@, " GEN $@")
> +
> +qmp-types.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-header < $< > $@, " GEN $@")
> +
> +qmp-types.o: qmp-types.c qmp-types.h
> +
> version.o: $(SRC_PATH)/version.rc config-host.mak
> $(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 69f0383..710d99f 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -15,7 +15,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
>
> block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
> block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
> -block-obj-y += error.o
> +block-obj-y += error.o qmp-types.o
> block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
> block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
>
> diff --git a/ordereddict.py b/ordereddict.py
> new file mode 100644
Please put this into scripts/.
> index 0000000..e17269f
> --- /dev/null
> +++ b/ordereddict.py
> @@ -0,0 +1,128 @@
> +# Copyright (c) 2009 Raymond Hettinger
> +#
> +# Permission is hereby granted, free of charge, to any person
> +# obtaining a copy of this software and associated documentation files
> +# (the "Software"), to deal in the Software without restriction,
> +# including without limitation the rights to use, copy, modify, merge,
> +# publish, distribute, sublicense, and/or sell copies of the Software,
> +# and to permit persons to whom the Software is furnished to do so,
> +# subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice shall be
> +# included in all copies or substantial portions of the Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> +# OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> +# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> +# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> +# OTHER DEALINGS IN THE SOFTWARE.
> +
> +from UserDict import DictMixin
> +
> +class OrderedDict(dict, DictMixin):
> +
> + def __init__(self, *args, **kwds):
> + if len(args) > 1:
> + raise TypeError('expected at most 1 arguments, got %d' % len(args))
> + try:
> + self.__end
> + except AttributeError:
> + self.clear()
> + self.update(*args, **kwds)
> +
> + def clear(self):
> + self.__end = end = []
> + end += [None, end, end] # sentinel node for doubly linked list
> + self.__map = {} # key --> [key, prev, next]
> + dict.clear(self)
> +
> + def __setitem__(self, key, value):
> + if key not in self:
> + end = self.__end
> + curr = end[1]
> + curr[2] = end[1] = self.__map[key] = [key, curr, end]
> + dict.__setitem__(self, key, value)
> +
> + def __delitem__(self, key):
> + dict.__delitem__(self, key)
> + key, prev, next = self.__map.pop(key)
> + prev[2] = next
> + next[1] = prev
> +
> + def __iter__(self):
> + end = self.__end
> + curr = end[2]
> + while curr is not end:
> + yield curr[0]
> + curr = curr[2]
> +
> + def __reversed__(self):
> + end = self.__end
> + curr = end[1]
> + while curr is not end:
> + yield curr[0]
> + curr = curr[1]
> +
> + def popitem(self, last=True):
> + if not self:
> + raise KeyError('dictionary is empty')
> + if last:
> + key = reversed(self).next()
> + else:
> + key = iter(self).next()
> + value = self.pop(key)
> + return key, value
> +
> + def __reduce__(self):
> + items = [[k, self[k]] for k in self]
> + tmp = self.__map, self.__end
> + del self.__map, self.__end
> + inst_dict = vars(self).copy()
> + self.__map, self.__end = tmp
> + if inst_dict:
> + return (self.__class__, (items,), inst_dict)
> + return self.__class__, (items,)
> +
> + def keys(self):
> + return list(self)
> +
> + setdefault = DictMixin.setdefault
> + update = DictMixin.update
> + pop = DictMixin.pop
> + values = DictMixin.values
> + items = DictMixin.items
> + iterkeys = DictMixin.iterkeys
> + itervalues = DictMixin.itervalues
> + iteritems = DictMixin.iteritems
> +
> + def __repr__(self):
> + if not self:
> + return '%s()' % (self.__class__.__name__,)
> + return '%s(%r)' % (self.__class__.__name__, self.items())
> +
> + def copy(self):
> + return self.__class__(self)
> +
> + @classmethod
> + def fromkeys(cls, iterable, value=None):
> + d = cls()
> + for key in iterable:
> + d[key] = value
> + return d
> +
> + def __eq__(self, other):
> + if isinstance(other, OrderedDict):
> + if len(self) != len(other):
> + return False
> + for p, q in zip(self.items(), other.items()):
> + if p != q:
> + return False
> + return True
> + return dict.__eq__(self, other)
> +
> + def __ne__(self, other):
> + return not self == other
> +
> diff --git a/qmp-gen.py b/qmp-gen.py
> new file mode 100644
Also this one.
> index 0000000..cded2f6
> --- /dev/null
> +++ b/qmp-gen.py
> @@ -0,0 +1,516 @@
> +##
> +# QAPI Code Generator
> +#
> +# Copyright IBM, Corp. 2011
> +#
> +# Authors:
> +# Anthony Liguori <aliguori@us.ibm.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2. See
> +# the COPYING file in the top-level directory.
> +##
> +import sys
> +from ordereddict import OrderedDict
> +
> +enum_types = []
> +event_types = {}
> +
> +def qmp_is_proxy_cmd(name):
> + return name.startswith('guest-')
> +
> +def qmp_is_async_cmd(name):
> + return name.startswith('guest-')
> +
> +def qmp_is_stateful_cmd(name):
> + return name in ['qmp_capabilities', 'put-event', 'getfd', 'closefd']
> +
> +def c_var(name):
> + return '_'.join(name.split('-'))
> +
> +def genindent(count):
> + ret = ""
> + for i in range(count):
> + ret += " "
> + return ret
> +
> +indent_level = 0
> +
> +def push_indent():
> + global indent_level
> + indent_level += 4
> +
> +def pop_indent():
> + global indent_level
> + indent_level -= 4
> +
> +def cgen(code, **kwds):
> + indent = genindent(indent_level)
> + lines = code.split('\n')
> + lines = map(lambda x: indent + x, lines)
> + return '\n'.join(lines) % kwds + '\n'
> +
> +def mcgen(code, **kwds):
> + return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> +
> +def is_dict(obj):
> + if type(obj) in [dict, OrderedDict]:
> + return True
> + return False
> +
> +def qmp_array_type_to_c(typename):
> + if type(typename) == list or is_dict(typename):
> + return qmp_type_to_c(typename)
> + elif typename == 'int':
> + return 'IntArray *'
> + elif typename == 'str':
> + return 'StringArray *'
> + elif typename == 'bool':
> + return 'BoolArray *'
> + elif typename == 'number':
> + return 'DoubleArray *'
> + else:
> + return qmp_type_to_c(typename)
> +
> +def qmp_type_should_free(typename):
> + if (type(typename) == list or
> + typename == 'str' or
> + (typename not in ['int', 'bool', 'number'] and
> + typename not in enum_types and not typename.isupper())):
> + return True
> + return False
> +
> +def qmp_free_func(typename):
> + if type(typename) == list:
> + return qmp_free_func(typename[0])
> + elif typename == 'str':
> + return 'qemu_free'
> + else:
> + return 'qmp_free_%s' % (de_camel_case(typename))
> +
> +def qmp_type_is_event(typename):
> + if type(typename) == str and typename.isupper():
> + return True
> + return False
> +
> +def qmp_type_to_c(typename, retval=False, indent=0):
> + if type(typename) == list:
> + return qmp_array_type_to_c(typename[0])
> + elif is_dict(typename):
> + string = 'struct {\n'
> + for argname, argtype, optional in parse_args(typename):
> + if optional:
> + string += "%sbool has_%s;\n" % (genindent(indent + 4), c_var(argname))
> + string += "%s%s %s;\n" % (genindent(indent + 4),
> + qmp_type_to_c(argtype, True,
> + indent=(indent + 4)),
> + c_var(argname))
> + string += "%s}" % genindent(indent)
> + return string
> + elif typename == 'int':
> + return 'int64_t'
> + elif not retval and typename == 'str':
> + return 'const char *'
> + elif retval and typename == 'str':
> + return 'char *'
> + elif typename == 'bool':
> + return 'bool'
> + elif typename == 'number':
> + return 'double'
> + elif typename == 'none':
> + return 'void'
> + elif typename in enum_types:
> + return typename
> + elif qmp_type_is_event(typename):
> + return 'struct %s *' % qmp_event_to_c(typename)
> + else:
> + return 'struct %s *' % typename
> +
> +def qmp_type_to_qobj(typename):
> + return 'qmp_marshal_type_%s' % typename
> +
> +def qmp_type_from_qobj(typename):
> + return 'qmp_unmarshal_type_%s' % typename
> +
> +def parse_args(typeinfo):
> + for member in typeinfo:
> + argname = member
> + argtype = typeinfo[member]
> + optional = False
> + if member.startswith('*'):
> + argname = member[1:]
> + optional = True
> + yield (argname, argtype, optional)
> +
> +def de_camel_case(name):
> + new_name = ''
> + for ch in name:
> + if ch.isupper() and new_name:
> + new_name += '_'
> + new_name += ch.lower()
> + return new_name
> +
> +def camel_case(name):
> + new_name = ''
> + first = True
> + for ch in name:
> + if ch in ['_', '-']:
> + first = True
> + elif first:
> + new_name += ch.upper()
> + first = False
> + else:
> + new_name += ch.lower()
> + return new_name
> +
> +def qmp_event_to_c(name):
> + return '%sEvent' % camel_case(name)
> +
> +def qmp_event_func_to_c(name):
> + return '%sFunc' % camel_case(name)
> +
> +def enum_abbreviation(name):
> + ab = ''
> + for ch in name:
> + if ch.isupper():
> + ab += ch
> + return ab
> +
> +def gen_type_declaration(name, typeinfo):
> + ret = ''
> + if type(typeinfo) == str:
> + ret += mcgen('''
> +
> +typedef %(type)s %(name)s;
> +''',
> + type=qmp_type_to_c(typeinfo),
> + name=name)
> + elif is_dict(typeinfo) and not name.isupper():
> + ret += mcgen('''
> +
> +typedef struct %(name)s %(name)s;
> +struct %(name)s {
> +''', name=name)
> + for argname, argtype, optional in parse_args(typeinfo):
> + if optional:
> + ret += cgen(' bool has_%(c_name)s;',
> + c_name=c_var(argname))
> + ret += cgen(' %(type)s %(c_name)s;',
> + type=qmp_type_to_c(argtype, True, indent=4),
> + c_name=c_var(argname))
> + ret += mcgen('''
> + %(c_name)s *next;
> +};
> +
> +%(name)s *qmp_alloc_%(dcc_name)s(void);
> +void qmp_free_%(dcc_name)s(%(name)s *obj);
> +''',
> + c_name=c_var(name), name=name,
> + dcc_name=de_camel_case(name))
> + elif is_dict(typeinfo) and name.isupper():
> + arglist = ['void *opaque']
> + for argname, argtype, optional in parse_args(typeinfo):
> + arglist.append('%s %s' % (qmp_type_to_c(argtype), argname))
> + ret += mcgen('''
> +
> +typedef void (%(event_func)s)(%(args)s);
> +
> +typedef struct %(c_event)s {
> + QmpSignal *signal;
> + %(event_func)s *func;
> +} %(c_event)s;
> +''',
> + event_func=qmp_event_func_to_c(name),
> + args=', '.join(arglist),
> + c_event=qmp_event_to_c(name))
> + return ret
> +
> +def gen_metatype_free(typeinfo, prefix):
> + ret = ''
> +
> + for argname, argtype, optional in parse_args(typeinfo):
> + if type(argtype) == list:
> + argtype = argtype[0]
> +
> + if is_dict(argtype):
> + if optional:
> + ret += cgen(' if (%(prefix)shas_%(c_name)s) {',
> + prefix=prefix, c_name=c_var(argname))
> + push_indent()
> + ret += gen_metatype_free(argtype, '%s%s.' % (prefix, argname))
> + if optional:
> + pop_indent()
> + ret += cgen(' }')
> + elif qmp_type_should_free(argtype):
> + if optional:
> + ret += mcgen('''
> + if (%(prefix)shas_%(c_name)s) {
> + %(free)s(%(prefix)s%(c_name)s);
> + }
> +''',
> + prefix=prefix, c_name=c_var(argname),
> + free=qmp_free_func(argtype))
> + else:
> + ret += mcgen('''
> + %(free)s(%(prefix)s%(c_name)s);
> +''',
> + prefix=prefix, c_name=c_var(argname),
> + free=qmp_free_func(argtype))
> +
> + return ret
> +
> +def gen_type_definition(name, typeinfo):
> + return mcgen('''
> +
> +void qmp_free_%(dcc_name)s(%(name)s *obj)
> +{
> + if (!obj) {
> + return;
> + }
> +%(type_free)s
> +
> + %(free)s(obj->next);
> + qemu_free(obj);
> +}
> +
> +%(name)s *qmp_alloc_%(dcc_name)s(void)
> +{
> + BUILD_ASSERT(sizeof(%(name)s) < 512);
> + return qemu_mallocz(512);
> +}
> +''',
> + dcc_name=de_camel_case(name), name=name,
> + free=qmp_free_func(name),
> + type_free=gen_metatype_free(typeinfo, 'obj->'))
> +
> +def gen_enum_declaration(name, entries):
> + ret = mcgen('''
> +
> +typedef enum %(name)s {
> +''', name=name)
> + i = 0
> + for entry in entries:
> + ret += cgen(' %(abrev)s_%(name)s = %(value)d,',
> + abrev=enum_abbreviation(name),
> + name=entry.upper(), value=i)
> + i += 1
> + ret += mcgen('''
> +} %(name)s;
> +
> +%(name)s qmp_type_%(dcc_name)s_from_str(const char *str, Error **errp);
> +const char *qmp_type_%(dcc_name)s_to_str(%(name)s value, Error **errp);
> +''',
> + name=name, dcc_name=de_camel_case(name))
> + return ret
> +
> +def gen_enum_definition(name, entries):
> + ret = mcgen('''
> +
> +%(name)s qmp_type_%(dcc_name)s_from_str(const char *str, Error **errp)
> +{
> +''',
> + name=name,
> + dcc_name=de_camel_case(name))
> + first = True
> + for entry in entries:
> + prefix = '} else '
> + if first:
> + prefix = ''
> + first = False
> + ret += mcgen('''
> + %(prefix)sif (strcmp(str, "%(entry)s") == 0) {
> + return %(abrev)s_%(value)s;
> +''',
> + prefix=prefix, entry=entry,
> + abrev=enum_abbreviation(name), value=entry.upper())
> +
> + ret += mcgen('''
> + } else {
> + error_set(errp, QERR_ENUM_VALUE_INVALID, "%(name)s", str);
> + return %(abrev)s_%(value)s;
> + }
> +}
> +
> +const char *qmp_type_%(dcc_name)s_to_str(%(name)s value, Error **errp)
> +{
> +''',
> + name=name, abrev=enum_abbreviation(name),
> + value=entries[0].upper(), dcc_name=de_camel_case(name))
> +
> + first = True
> + for entry in entries:
> + enum = '%s_%s' % (enum_abbreviation(name), entry.upper())
> + prefix = '} else '
> + if first:
> + prefix = ''
> + first = False
> + ret += mcgen('''
> + %(prefix)sif (value == %(enum)s) {
> + return "%(entry)s";
> +''',
> + entry=entry, prefix=prefix, enum=enum)
> + ret += mcgen('''
> + } else {
> + char buf[32];
> + snprintf(buf, sizeof(buf), "%%d", value);
> + error_set(errp, QERR_ENUM_VALUE_INVALID, "%(name)s", buf);
> + return NULL;
> + }
> +}
> +''',
> + name=name)
> + return ret
> +
> +def tokenize(data):
> + while len(data):
> + if data[0] in ['{', '}', ':', ',', '[', ']']:
> + yield data[0]
> + data = data[1:]
> + elif data[0] in ' \n':
> + data = data[1:]
> + elif data[0] == "'":
> + data = data[1:]
> + string = ''
> + while data[0] != "'":
> + string += data[0]
> + data = data[1:]
> + data = data[1:]
> + yield string
> +
> +def parse_value(tokens):
> + if tokens[0] == '{':
> + ret = OrderedDict()
> + tokens = tokens[1:]
> + while tokens[0] != '}':
> + key = tokens[0]
> + tokens = tokens[1:]
> +
> + tokens = tokens[1:] # :
> +
> + value, tokens = parse_value(tokens)
> +
> + if tokens[0] == ',':
> + tokens = tokens[1:]
> +
> + ret[key] = value
> + tokens = tokens[1:]
> + return ret, tokens
> + elif tokens[0] == '[':
> + ret = []
> + tokens = tokens[1:]
> + while tokens[0] != ']':
> + value, tokens = parse_value(tokens)
> + if tokens[0] == ',':
> + tokens = tokens[1:]
> + ret.append(value)
> + tokens = tokens[1:]
> + return ret, tokens
> + else:
> + return tokens[0], tokens[1:]
> +
> +def ordered_eval(string):
> + return parse_value(map(lambda x: x, tokenize(string)))[0]
> +# return eval(string)
> +
> +def generate(kind):
> + global enum_types
> + global event_types
> + global indent_level
> +
> + enum_types = []
> + event_types = {}
> + indent_level = 0
> +
> + ret = mcgen('''
> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT EDIT */
> +''')
> +
> + if kind == 'types-header':
> + ret += mcgen('''
> +#ifndef QMP_TYPES_H
> +#define QMP_TYPES_H
> +
> +#include "qmp-types-core.h"
> +''')
> + elif kind == 'types-body':
> + ret += mcgen('''
> +#include "qmp-types.h"
> +#include "qmp-marshal-types.h"
> +''')
> +
> + exprs = []
> + expr = ''
> +
> + for line in sys.stdin:
> + if line.startswith('#') or line == '\n':
> + continue
> +
> + if line.startswith(' '):
> + expr += line
> + elif expr:
> + s = ordered_eval(expr)
> + exprs.append(s)
> + expr = line
> + else:
> + expr += line
> +
> + if expr:
> + s = ordered_eval(expr)
> + exprs.append(s)
> +
> + for s in exprs:
> + if s.has_key('type'):
> + name = s['type']
> + data = s['data']
> +
> + if kind == 'types-body':
> + ret += gen_type_definition(name, data)
> + elif kind == 'types-header':
> + ret += gen_type_declaration(name, data)
> + elif s.has_key('enum'):
> + name = s['enum']
> + data = s['data']
> +
> + enum_types.append(s['enum'])
> + if kind == 'types-header':
> + ret += gen_enum_declaration(name, data)
> + elif kind == 'types-body':
> + ret += gen_enum_definition(name, data)
> + elif s.has_key('event'):
> + name = s['event']
> + data = {}
> + if s.has_key('data'):
> + data = s['data']
> +
> + event_types[name] = data
> + if kind == 'types-header':
> + ret += gen_type_declaration(name, data)
> + elif s.has_key('command'):
> + name = s['command']
> + options = {}
> + if s.has_key('data'):
> + options = s['data']
> + retval = 'none'
> + if s.has_key('returns'):
> + retval = s['returns']
> +
> + if kind.endswith('header'):
> + ret += cgen('#endif')
> +
> + return ret
> +
> +def main(args):
> + if len(args) != 1:
> + return 1
> + if not args[0].startswith('--'):
> + return 1
> +
> + kind = args[0][2:]
> +
> + ret = generate(kind)
> +
> + sys.stdout.write(ret)
> +
> + return 0
> +
> +if __name__ == '__main__':
> + sys.exit(main(sys.argv[1:]))
> diff --git a/qmp-schema.json b/qmp-schema.json
> new file mode 100644
> index 0000000..e69de29
> diff --git a/qmp-types-core.h b/qmp-types-core.h
> new file mode 100644
> index 0000000..a018ba7
> --- /dev/null
> +++ b/qmp-types-core.h
> @@ -0,0 +1,29 @@
> +/*
> + * QAPI
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + * Anthony Liguori <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2. See
> + * the COPYING.LIB file in the top-level directory.
> + */
> +#ifndef QMP_TYPES_CORE_H
> +#define QMP_TYPES_CORE_H
> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include "error.h"
> +
> +typedef struct QmpSignal QmpSignal;
> +typedef struct QmpCommandState QmpCommandState;
> +typedef struct QmpState QmpState;
> +
> +#define BUILD_ASSERT(cond) do { \
> + (void)sizeof(int[-1+!!(cond)]); \
Spaces around '+'.
This could be useful elsewhere, how about putting it to some generic header?
> +} while (0)
> +
> +#define BUILD_BUG() BUILD_ASSERT(0)
> +
> +#endif
> --
> 1.7.0.4
>
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2)
2011-03-12 11:29 ` [Qemu-devel] " Blue Swirl
@ 2011-03-12 15:00 ` Anthony Liguori
2011-03-18 14:18 ` Luiz Capitulino
0 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-12 15:00 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Markus Armbruster, qemu-devel, Luiz Capitulino,
Avi Kivity, Adam Litke
On 03/12/2011 05:29 AM, Blue Swirl wrote:
> On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori<aliguori@us.ibm.com> wrote:
>> Only generate qmp-types.[ch]. These files contain the type definitions for
>> QMP along with the alloc/free functions for these types. Functions to convert
>> enum values to integers and vice versa are also included.
>>
>> qmp-types is used both within QEMU and within libqmp
>>
>> Special alloc/free functions are provided to ensure that all structures are
>> padded when allocated. This makes sure that libqmp can provide a forward
>> compatible interface since all additions to a structure will have a boolean
>> enable flag.
>>
>> The free function is convenient since individual structures may have pointers
>> that also require freeing.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>> v1 -> v2
>> - modify code generator to use multiline strings instead of prints
>> - support proxy commands
>> - support async commands
>>
>> diff --git a/Makefile b/Makefile
>> index 6b1d716..6b9fd69 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -4,6 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
>> ifeq ($(TRACE_BACKEND),dtrace)
>> GENERATED_HEADERS += trace-dtrace.h
>> endif
>> +GENERATED_HEADERS += qmp-types.h
>>
>> ifneq ($(wildcard config-host.mak),)
>> # Put the all: rule here so that config-host.mak can contain dependencies.
>> @@ -146,6 +147,14 @@ trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
>>
>> simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
>>
>> +qmp-types.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
>> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-body< $< > $@, " GEN $@")
>> +
>> +qmp-types.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
>> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-header< $< > $@, " GEN $@")
>> +
>> +qmp-types.o: qmp-types.c qmp-types.h
>> +
>> version.o: $(SRC_PATH)/version.rc config-host.mak
>> $(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 69f0383..710d99f 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -15,7 +15,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
>>
>> block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
>> block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
>> -block-obj-y += error.o
>> +block-obj-y += error.o qmp-types.o
>> block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
>> block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
>>
>> diff --git a/ordereddict.py b/ordereddict.py
>> new file mode 100644
> Please put this into scripts/.
Sure.
>> +
>> +#include<stdint.h>
>> +#include<stdbool.h>
>> +#include "error.h"
>> +
>> +typedef struct QmpSignal QmpSignal;
>> +typedef struct QmpCommandState QmpCommandState;
>> +typedef struct QmpState QmpState;
>> +
>> +#define BUILD_ASSERT(cond) do { \
>> + (void)sizeof(int[-1+!!(cond)]); \
> Spaces around '+'.
>
> This could be useful elsewhere, how about putting it to some generic header?
Sure.
Regards,
Anthony Liguori
>> +} while (0)
>> +
>> +#define BUILD_BUG() BUILD_ASSERT(0)
>> +
>> +#endif
>> --
>> 1.7.0.4
>>
>>
>>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2)
2011-03-12 15:00 ` Anthony Liguori
@ 2011-03-18 14:18 ` Luiz Capitulino
0 siblings, 0 replies; 49+ messages in thread
From: Luiz Capitulino @ 2011-03-18 14:18 UTC (permalink / raw)
To: Anthony Liguori
Cc: Anthony Liguori, qemu-devel, Markus Armbruster, Blue Swirl,
Avi Kivity, Adam Litke
On Sat, 12 Mar 2011 09:00:53 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 03/12/2011 05:29 AM, Blue Swirl wrote:
> > On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori<aliguori@us.ibm.com> wrote:
> >> Only generate qmp-types.[ch]. These files contain the type definitions for
> >> QMP along with the alloc/free functions for these types. Functions to convert
> >> enum values to integers and vice versa are also included.
> >>
> >> qmp-types is used both within QEMU and within libqmp
> >>
> >> Special alloc/free functions are provided to ensure that all structures are
> >> padded when allocated. This makes sure that libqmp can provide a forward
> >> compatible interface since all additions to a structure will have a boolean
> >> enable flag.
> >>
> >> The free function is convenient since individual structures may have pointers
> >> that also require freeing.
> >>
> >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> >> ---
> >> v1 -> v2
> >> - modify code generator to use multiline strings instead of prints
> >> - support proxy commands
> >> - support async commands
> >>
> >> diff --git a/Makefile b/Makefile
> >> index 6b1d716..6b9fd69 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -4,6 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
> >> ifeq ($(TRACE_BACKEND),dtrace)
> >> GENERATED_HEADERS += trace-dtrace.h
> >> endif
> >> +GENERATED_HEADERS += qmp-types.h
> >>
> >> ifneq ($(wildcard config-host.mak),)
> >> # Put the all: rule here so that config-host.mak can contain dependencies.
> >> @@ -146,6 +147,14 @@ trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
> >>
> >> simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
> >>
> >> +qmp-types.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> >> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-body< $< > $@, " GEN $@")
> >> +
> >> +qmp-types.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> >> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-header< $< > $@, " GEN $@")
> >> +
> >> +qmp-types.o: qmp-types.c qmp-types.h
> >> +
> >> version.o: $(SRC_PATH)/version.rc config-host.mak
> >> $(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
> >>
> >> diff --git a/Makefile.objs b/Makefile.objs
> >> index 69f0383..710d99f 100644
> >> --- a/Makefile.objs
> >> +++ b/Makefile.objs
> >> @@ -15,7 +15,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
> >>
> >> block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
> >> block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
> >> -block-obj-y += error.o
> >> +block-obj-y += error.o qmp-types.o
> >> block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
> >> block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> >>
> >> diff --git a/ordereddict.py b/ordereddict.py
> >> new file mode 100644
> > Please put this into scripts/.
>
> Sure.
Our maybe create qmp/ and put everything in there.
^ permalink raw reply [flat|nested] 49+ messages in thread
* [Qemu-devel] Re: [PATCH 01/15] qapi: add code generator for qmp-types (v2)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2) Anthony Liguori
2011-03-11 23:12 ` [Qemu-devel] " Anthony Liguori
2011-03-12 11:29 ` [Qemu-devel] " Blue Swirl
@ 2011-03-18 14:14 ` Luiz Capitulino
2 siblings, 0 replies; 49+ messages in thread
From: Luiz Capitulino @ 2011-03-18 14:14 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Markus Armbruster, Adam Litke, qemu-devel, Avi Kivity
On Fri, 11 Mar 2011 17:05:31 -0600
Anthony Liguori <aliguori@us.ibm.com> wrote:
> Only generate qmp-types.[ch]. These files contain the type definitions for
> QMP along with the alloc/free functions for these types. Functions to convert
> enum values to integers and vice versa are also included.
>
> qmp-types is used both within QEMU and within libqmp
>
> Special alloc/free functions are provided to ensure that all structures are
> padded when allocated. This makes sure that libqmp can provide a forward
> compatible interface since all additions to a structure will have a boolean
> enable flag.
>
> The free function is convenient since individual structures may have pointers
> that also require freeing.
I like the way you split the code generator, makes review easier.
Two general comments:
1. It doesn't seem to do any error detection, schema syntax errors seem
to completely break the script
2. Would nice to have a test suite
More comments below.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> v1 -> v2
> - modify code generator to use multiline strings instead of prints
> - support proxy commands
> - support async commands
>
> diff --git a/Makefile b/Makefile
> index 6b1d716..6b9fd69 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -4,6 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
> ifeq ($(TRACE_BACKEND),dtrace)
> GENERATED_HEADERS += trace-dtrace.h
> endif
> +GENERATED_HEADERS += qmp-types.h
>
> ifneq ($(wildcard config-host.mak),)
> # Put the all: rule here so that config-host.mak can contain dependencies.
> @@ -146,6 +147,14 @@ trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
>
> simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
>
> +qmp-types.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-body < $< > $@, " GEN $@")
> +
> +qmp-types.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-header < $< > $@, " GEN $@")
> +
> +qmp-types.o: qmp-types.c qmp-types.h
> +
> version.o: $(SRC_PATH)/version.rc config-host.mak
> $(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 69f0383..710d99f 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -15,7 +15,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
>
> block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
> block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
> -block-obj-y += error.o
> +block-obj-y += error.o qmp-types.o
> block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
> block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
>
> diff --git a/ordereddict.py b/ordereddict.py
> new file mode 100644
> index 0000000..e17269f
> --- /dev/null
> +++ b/ordereddict.py
Would be good to mention in the log you're adding this. You've said you'd
do it iirc.
> @@ -0,0 +1,128 @@
> +# Copyright (c) 2009 Raymond Hettinger
> +#
> +# Permission is hereby granted, free of charge, to any person
> +# obtaining a copy of this software and associated documentation files
> +# (the "Software"), to deal in the Software without restriction,
> +# including without limitation the rights to use, copy, modify, merge,
> +# publish, distribute, sublicense, and/or sell copies of the Software,
> +# and to permit persons to whom the Software is furnished to do so,
> +# subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice shall be
> +# included in all copies or substantial portions of the Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> +# OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> +# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> +# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> +# OTHER DEALINGS IN THE SOFTWARE.
> +
> +from UserDict import DictMixin
> +
> +class OrderedDict(dict, DictMixin):
> +
> + def __init__(self, *args, **kwds):
> + if len(args) > 1:
> + raise TypeError('expected at most 1 arguments, got %d' % len(args))
> + try:
> + self.__end
> + except AttributeError:
> + self.clear()
> + self.update(*args, **kwds)
> +
> + def clear(self):
> + self.__end = end = []
> + end += [None, end, end] # sentinel node for doubly linked list
> + self.__map = {} # key --> [key, prev, next]
> + dict.clear(self)
> +
> + def __setitem__(self, key, value):
> + if key not in self:
> + end = self.__end
> + curr = end[1]
> + curr[2] = end[1] = self.__map[key] = [key, curr, end]
> + dict.__setitem__(self, key, value)
> +
> + def __delitem__(self, key):
> + dict.__delitem__(self, key)
> + key, prev, next = self.__map.pop(key)
> + prev[2] = next
> + next[1] = prev
> +
> + def __iter__(self):
> + end = self.__end
> + curr = end[2]
> + while curr is not end:
> + yield curr[0]
> + curr = curr[2]
> +
> + def __reversed__(self):
> + end = self.__end
> + curr = end[1]
> + while curr is not end:
> + yield curr[0]
> + curr = curr[1]
> +
> + def popitem(self, last=True):
> + if not self:
> + raise KeyError('dictionary is empty')
> + if last:
> + key = reversed(self).next()
> + else:
> + key = iter(self).next()
> + value = self.pop(key)
> + return key, value
> +
> + def __reduce__(self):
> + items = [[k, self[k]] for k in self]
> + tmp = self.__map, self.__end
> + del self.__map, self.__end
> + inst_dict = vars(self).copy()
> + self.__map, self.__end = tmp
> + if inst_dict:
> + return (self.__class__, (items,), inst_dict)
> + return self.__class__, (items,)
> +
> + def keys(self):
> + return list(self)
> +
> + setdefault = DictMixin.setdefault
> + update = DictMixin.update
> + pop = DictMixin.pop
> + values = DictMixin.values
> + items = DictMixin.items
> + iterkeys = DictMixin.iterkeys
> + itervalues = DictMixin.itervalues
> + iteritems = DictMixin.iteritems
> +
> + def __repr__(self):
> + if not self:
> + return '%s()' % (self.__class__.__name__,)
> + return '%s(%r)' % (self.__class__.__name__, self.items())
> +
> + def copy(self):
> + return self.__class__(self)
> +
> + @classmethod
> + def fromkeys(cls, iterable, value=None):
> + d = cls()
> + for key in iterable:
> + d[key] = value
> + return d
> +
> + def __eq__(self, other):
> + if isinstance(other, OrderedDict):
> + if len(self) != len(other):
> + return False
> + for p, q in zip(self.items(), other.items()):
> + if p != q:
> + return False
> + return True
> + return dict.__eq__(self, other)
> +
> + def __ne__(self, other):
> + return not self == other
> +
> diff --git a/qmp-gen.py b/qmp-gen.py
> new file mode 100644
> index 0000000..cded2f6
> --- /dev/null
> +++ b/qmp-gen.py
> @@ -0,0 +1,516 @@
> +##
> +# QAPI Code Generator
> +#
> +# Copyright IBM, Corp. 2011
> +#
> +# Authors:
> +# Anthony Liguori <aliguori@us.ibm.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2. See
> +# the COPYING file in the top-level directory.
> +##
This misses basic documentation, like all supported types, functions, etc.
This seems to exist in the qmp-schema.json file introduced by patch 07/15. If
that's the case, then it's a better idea to add that file to this this patch,
only with the doc bits.
> +import sys
> +from ordereddict import OrderedDict
> +
> +enum_types = []
> +event_types = {}
> +
> +def qmp_is_proxy_cmd(name):
> + return name.startswith('guest-')
> +
> +def qmp_is_async_cmd(name):
> + return name.startswith('guest-')
> +
> +def qmp_is_stateful_cmd(name):
> + return name in ['qmp_capabilities', 'put-event', 'getfd', 'closefd']
Care to document what this is for?
> +
> +def c_var(name):
> + return '_'.join(name.split('-'))
> +
> +def genindent(count):
> + ret = ""
> + for i in range(count):
> + ret += " "
> + return ret
> +
> +indent_level = 0
> +
> +def push_indent():
> + global indent_level
> + indent_level += 4
> +
> +def pop_indent():
> + global indent_level
> + indent_level -= 4
> +
> +def cgen(code, **kwds):
> + indent = genindent(indent_level)
> + lines = code.split('\n')
> + lines = map(lambda x: indent + x, lines)
> + return '\n'.join(lines) % kwds + '\n'
> +
> +def mcgen(code, **kwds):
> + return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> +
> +def is_dict(obj):
> + if type(obj) in [dict, OrderedDict]:
> + return True
> + return False
> +
> +def qmp_array_type_to_c(typename):
> + if type(typename) == list or is_dict(typename):
> + return qmp_type_to_c(typename)
> + elif typename == 'int':
> + return 'IntArray *'
> + elif typename == 'str':
> + return 'StringArray *'
> + elif typename == 'bool':
> + return 'BoolArray *'
> + elif typename == 'number':
> + return 'DoubleArray *'
> + else:
> + return qmp_type_to_c(typename)
> +
> +def qmp_type_should_free(typename):
> + if (type(typename) == list or
> + typename == 'str' or
> + (typename not in ['int', 'bool', 'number'] and
> + typename not in enum_types and not typename.isupper())):
> + return True
> + return False
> +
> +def qmp_free_func(typename):
> + if type(typename) == list:
> + return qmp_free_func(typename[0])
> + elif typename == 'str':
> + return 'qemu_free'
> + else:
> + return 'qmp_free_%s' % (de_camel_case(typename))
> +
> +def qmp_type_is_event(typename):
> + if type(typename) == str and typename.isupper():
> + return True
> + return False
> +
> +def qmp_type_to_c(typename, retval=False, indent=0):
> + if type(typename) == list:
> + return qmp_array_type_to_c(typename[0])
> + elif is_dict(typename):
> + string = 'struct {\n'
> + for argname, argtype, optional in parse_args(typename):
> + if optional:
> + string += "%sbool has_%s;\n" % (genindent(indent + 4), c_var(argname))
> + string += "%s%s %s;\n" % (genindent(indent + 4),
> + qmp_type_to_c(argtype, True,
> + indent=(indent + 4)),
> + c_var(argname))
> + string += "%s}" % genindent(indent)
> + return string
> + elif typename == 'int':
> + return 'int64_t'
> + elif not retval and typename == 'str':
> + return 'const char *'
> + elif retval and typename == 'str':
> + return 'char *'
> + elif typename == 'bool':
> + return 'bool'
> + elif typename == 'number':
> + return 'double'
> + elif typename == 'none':
> + return 'void'
> + elif typename in enum_types:
> + return typename
> + elif qmp_type_is_event(typename):
> + return 'struct %s *' % qmp_event_to_c(typename)
> + else:
> + return 'struct %s *' % typename
> +
> +def qmp_type_to_qobj(typename):
> + return 'qmp_marshal_type_%s' % typename
> +
> +def qmp_type_from_qobj(typename):
> + return 'qmp_unmarshal_type_%s' % typename
> +
> +def parse_args(typeinfo):
> + for member in typeinfo:
> + argname = member
> + argtype = typeinfo[member]
> + optional = False
> + if member.startswith('*'):
> + argname = member[1:]
> + optional = True
> + yield (argname, argtype, optional)
> +
> +def de_camel_case(name):
> + new_name = ''
> + for ch in name:
> + if ch.isupper() and new_name:
> + new_name += '_'
> + new_name += ch.lower()
> + return new_name
> +
> +def camel_case(name):
> + new_name = ''
> + first = True
> + for ch in name:
> + if ch in ['_', '-']:
> + first = True
> + elif first:
> + new_name += ch.upper()
> + first = False
> + else:
> + new_name += ch.lower()
> + return new_name
> +
> +def qmp_event_to_c(name):
> + return '%sEvent' % camel_case(name)
> +
> +def qmp_event_func_to_c(name):
> + return '%sFunc' % camel_case(name)
> +
> +def enum_abbreviation(name):
> + ab = ''
> + for ch in name:
> + if ch.isupper():
> + ab += ch
> + return ab
> +
> +def gen_type_declaration(name, typeinfo):
> + ret = ''
> + if type(typeinfo) == str:
> + ret += mcgen('''
> +
> +typedef %(type)s %(name)s;
> +''',
> + type=qmp_type_to_c(typeinfo),
> + name=name)
> + elif is_dict(typeinfo) and not name.isupper():
> + ret += mcgen('''
> +
> +typedef struct %(name)s %(name)s;
> +struct %(name)s {
> +''', name=name)
> + for argname, argtype, optional in parse_args(typeinfo):
> + if optional:
> + ret += cgen(' bool has_%(c_name)s;',
> + c_name=c_var(argname))
> + ret += cgen(' %(type)s %(c_name)s;',
> + type=qmp_type_to_c(argtype, True, indent=4),
> + c_name=c_var(argname))
> + ret += mcgen('''
> + %(c_name)s *next;
> +};
> +
> +%(name)s *qmp_alloc_%(dcc_name)s(void);
> +void qmp_free_%(dcc_name)s(%(name)s *obj);
> +''',
> + c_name=c_var(name), name=name,
> + dcc_name=de_camel_case(name))
> + elif is_dict(typeinfo) and name.isupper():
> + arglist = ['void *opaque']
> + for argname, argtype, optional in parse_args(typeinfo):
> + arglist.append('%s %s' % (qmp_type_to_c(argtype), argname))
> + ret += mcgen('''
> +
> +typedef void (%(event_func)s)(%(args)s);
> +
> +typedef struct %(c_event)s {
> + QmpSignal *signal;
> + %(event_func)s *func;
> +} %(c_event)s;
> +''',
> + event_func=qmp_event_func_to_c(name),
> + args=', '.join(arglist),
> + c_event=qmp_event_to_c(name))
> + return ret
> +
> +def gen_metatype_free(typeinfo, prefix):
> + ret = ''
> +
> + for argname, argtype, optional in parse_args(typeinfo):
> + if type(argtype) == list:
> + argtype = argtype[0]
> +
> + if is_dict(argtype):
> + if optional:
> + ret += cgen(' if (%(prefix)shas_%(c_name)s) {',
> + prefix=prefix, c_name=c_var(argname))
> + push_indent()
> + ret += gen_metatype_free(argtype, '%s%s.' % (prefix, argname))
> + if optional:
> + pop_indent()
> + ret += cgen(' }')
> + elif qmp_type_should_free(argtype):
> + if optional:
> + ret += mcgen('''
> + if (%(prefix)shas_%(c_name)s) {
> + %(free)s(%(prefix)s%(c_name)s);
> + }
> +''',
> + prefix=prefix, c_name=c_var(argname),
> + free=qmp_free_func(argtype))
> + else:
> + ret += mcgen('''
> + %(free)s(%(prefix)s%(c_name)s);
> +''',
> + prefix=prefix, c_name=c_var(argname),
> + free=qmp_free_func(argtype))
> +
> + return ret
> +
> +def gen_type_definition(name, typeinfo):
> + return mcgen('''
> +
> +void qmp_free_%(dcc_name)s(%(name)s *obj)
> +{
> + if (!obj) {
> + return;
> + }
> +%(type_free)s
Indentation.
> +
> + %(free)s(obj->next);
I'm not sure I understand how this is used. You free the entire list by freeing
one of them?
> + qemu_free(obj);
> +}
> +
> +%(name)s *qmp_alloc_%(dcc_name)s(void)
> +{
> + BUILD_ASSERT(sizeof(%(name)s) < 512);
> + return qemu_mallocz(512);
> +}
Why is this needed?
> +''',
> + dcc_name=de_camel_case(name), name=name,
> + free=qmp_free_func(name),
> + type_free=gen_metatype_free(typeinfo, 'obj->'))
> +
> +def gen_enum_declaration(name, entries):
> + ret = mcgen('''
> +
> +typedef enum %(name)s {
> +''', name=name)
> + i = 0
> + for entry in entries:
> + ret += cgen(' %(abrev)s_%(name)s = %(value)d,',
> + abrev=enum_abbreviation(name),
> + name=entry.upper(), value=i)
> + i += 1
> + ret += mcgen('''
> +} %(name)s;
> +
> +%(name)s qmp_type_%(dcc_name)s_from_str(const char *str, Error **errp);
> +const char *qmp_type_%(dcc_name)s_to_str(%(name)s value, Error **errp);
> +''',
> + name=name, dcc_name=de_camel_case(name))
> + return ret
> +
> +def gen_enum_definition(name, entries):
> + ret = mcgen('''
> +
> +%(name)s qmp_type_%(dcc_name)s_from_str(const char *str, Error **errp)
> +{
> +''',
> + name=name,
> + dcc_name=de_camel_case(name))
> + first = True
> + for entry in entries:
> + prefix = '} else '
> + if first:
> + prefix = ''
> + first = False
> + ret += mcgen('''
> + %(prefix)sif (strcmp(str, "%(entry)s") == 0) {
> + return %(abrev)s_%(value)s;
> +''',
> + prefix=prefix, entry=entry,
> + abrev=enum_abbreviation(name), value=entry.upper())
> +
> + ret += mcgen('''
> + } else {
> + error_set(errp, QERR_ENUM_VALUE_INVALID, "%(name)s", str);
> + return %(abrev)s_%(value)s;
> + }
This seems to return the first defined value, isn't it better to automatically
generate the first value as ENUM_ERROR and return that instead?
Also, %(abrev) seems to be "ME". Not sure where it's defined, but isn't it
better to call it "QMP"?
> +}
> +
> +const char *qmp_type_%(dcc_name)s_to_str(%(name)s value, Error **errp)
> +{
> +''',
> + name=name, abrev=enum_abbreviation(name),
> + value=entries[0].upper(), dcc_name=de_camel_case(name))
> +
> + first = True
> + for entry in entries:
> + enum = '%s_%s' % (enum_abbreviation(name), entry.upper())
> + prefix = '} else '
> + if first:
> + prefix = ''
> + first = False
> + ret += mcgen('''
> + %(prefix)sif (value == %(enum)s) {
> + return "%(entry)s";
> +''',
> + entry=entry, prefix=prefix, enum=enum)
> + ret += mcgen('''
> + } else {
> + char buf[32];
> + snprintf(buf, sizeof(buf), "%%d", value);
> + error_set(errp, QERR_ENUM_VALUE_INVALID, "%(name)s", buf);
> + return NULL;
> + }
> +}
> +''',
> + name=name)
> + return ret
> +
> +def tokenize(data):
> + while len(data):
> + if data[0] in ['{', '}', ':', ',', '[', ']']:
> + yield data[0]
> + data = data[1:]
> + elif data[0] in ' \n':
> + data = data[1:]
> + elif data[0] == "'":
> + data = data[1:]
> + string = ''
> + while data[0] != "'":
> + string += data[0]
> + data = data[1:]
> + data = data[1:]
> + yield string
> +
> +def parse_value(tokens):
> + if tokens[0] == '{':
> + ret = OrderedDict()
> + tokens = tokens[1:]
> + while tokens[0] != '}':
> + key = tokens[0]
> + tokens = tokens[1:]
> +
> + tokens = tokens[1:] # :
> +
> + value, tokens = parse_value(tokens)
> +
> + if tokens[0] == ',':
> + tokens = tokens[1:]
> +
> + ret[key] = value
> + tokens = tokens[1:]
> + return ret, tokens
> + elif tokens[0] == '[':
> + ret = []
> + tokens = tokens[1:]
> + while tokens[0] != ']':
> + value, tokens = parse_value(tokens)
> + if tokens[0] == ',':
> + tokens = tokens[1:]
> + ret.append(value)
> + tokens = tokens[1:]
> + return ret, tokens
> + else:
> + return tokens[0], tokens[1:]
> +
> +def ordered_eval(string):
> + return parse_value(map(lambda x: x, tokenize(string)))[0]
> +# return eval(string)
> +
> +def generate(kind):
> + global enum_types
> + global event_types
> + global indent_level
> +
> + enum_types = []
> + event_types = {}
> + indent_level = 0
> +
> + ret = mcgen('''
> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT EDIT */
> +''')
> +
> + if kind == 'types-header':
> + ret += mcgen('''
> +#ifndef QMP_TYPES_H
> +#define QMP_TYPES_H
> +
> +#include "qmp-types-core.h"
> +''')
> + elif kind == 'types-body':
> + ret += mcgen('''
> +#include "qmp-types.h"
> +#include "qmp-marshal-types.h"
> +''')
The "qmp-marshal-types.h" file doesn't exist, so this patch doesn't compile
breaking git bisect.
Two more comments about the hunk above:
1. The generated code doesn't have license information
2. There are several if 'types-body' elif 'types-header', better to have two
different functions instead? Maybe a class... this would kill the global
variables above
> +
Extra whitespaces, this file has plenty of them.
> + exprs = []
> + expr = ''
> +
> + for line in sys.stdin:
> + if line.startswith('#') or line == '\n':
> + continue
> +
> + if line.startswith(' '):
> + expr += line
> + elif expr:
> + s = ordered_eval(expr)
> + exprs.append(s)
> + expr = line
> + else:
> + expr += line
> +
> + if expr:
> + s = ordered_eval(expr)
> + exprs.append(s)
> +
> + for s in exprs:
> + if s.has_key('type'):
> + name = s['type']
> + data = s['data']
> +
> + if kind == 'types-body':
> + ret += gen_type_definition(name, data)
> + elif kind == 'types-header':
> + ret += gen_type_declaration(name, data)
> + elif s.has_key('enum'):
> + name = s['enum']
> + data = s['data']
> +
> + enum_types.append(s['enum'])
> + if kind == 'types-header':
> + ret += gen_enum_declaration(name, data)
> + elif kind == 'types-body':
> + ret += gen_enum_definition(name, data)
> + elif s.has_key('event'):
> + name = s['event']
> + data = {}
> + if s.has_key('data'):
> + data = s['data']
> +
> + event_types[name] = data
> + if kind == 'types-header':
> + ret += gen_type_declaration(name, data)
> + elif s.has_key('command'):
> + name = s['command']
> + options = {}
> + if s.has_key('data'):
> + options = s['data']
> + retval = 'none'
> + if s.has_key('returns'):
> + retval = s['returns']
> +
> + if kind.endswith('header'):
> + ret += cgen('#endif')
> +
> + return ret
> +
> +def main(args):
> + if len(args) != 1:
> + return 1
> + if not args[0].startswith('--'):
> + return 1
> +
> + kind = args[0][2:]
> +
> + ret = generate(kind)
> +
> + sys.stdout.write(ret)
> +
> + return 0
> +
> +if __name__ == '__main__':
> + sys.exit(main(sys.argv[1:]))
> diff --git a/qmp-schema.json b/qmp-schema.json
> new file mode 100644
> index 0000000..e69de29
> diff --git a/qmp-types-core.h b/qmp-types-core.h
> new file mode 100644
> index 0000000..a018ba7
> --- /dev/null
> +++ b/qmp-types-core.h
> @@ -0,0 +1,29 @@
> +/*
> + * QAPI
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + * Anthony Liguori <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2. See
> + * the COPYING.LIB file in the top-level directory.
> + */
> +#ifndef QMP_TYPES_CORE_H
> +#define QMP_TYPES_CORE_H
> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include "error.h"
> +
> +typedef struct QmpSignal QmpSignal;
> +typedef struct QmpCommandState QmpCommandState;
> +typedef struct QmpState QmpState;
> +
> +#define BUILD_ASSERT(cond) do { \
> + (void)sizeof(int[-1+!!(cond)]); \
> +} while (0)
> +
> +#define BUILD_BUG() BUILD_ASSERT(0)
> +
> +#endif
^ permalink raw reply [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH 02/15] qapi: add code generator for type marshallers
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2) Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-18 15:13 ` [Qemu-devel] " Luiz Capitulino
2011-03-11 23:05 ` [Qemu-devel] [PATCH 03/15] qapi: add core QMP server support (v2) Anthony Liguori
` (13 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This code will marshal a QMP type from native representation to a QObject and
vice versa.
Marshaling from a native representation to a QObject can never generate an
Error although the opposite direction may.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
- update code generator to use multiline strings
- support proxy commands
- support async commands
diff --git a/Makefile b/Makefile
index 6b9fd69..8f3a4d3 100644
--- a/Makefile
+++ b/Makefile
@@ -4,7 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
ifeq ($(TRACE_BACKEND),dtrace)
GENERATED_HEADERS += trace-dtrace.h
endif
-GENERATED_HEADERS += qmp-types.h
+GENERATED_HEADERS += qmp-types.h qmp-marshal-types.h
ifneq ($(wildcard config-host.mak),)
# Put the all: rule here so that config-host.mak can contain dependencies.
@@ -153,7 +153,14 @@ qmp-types.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
qmp-types.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
$(call quiet-command,python $(SRC_PATH)/qmp-gen.py --types-header < $< > $@, " GEN $@")
+qmp-marshal-types.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
+ $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --marshal-body < $< > $@, " GEN $@")
+
+qmp-marshal-types.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
+ $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --marshal-header < $< > $@, " GEN $@")
+
qmp-types.o: qmp-types.c qmp-types.h
+qmp-marshal-types.o: qmp-marshal-types.c qmp-marshal-types.h qmp-types.h
version.o: $(SRC_PATH)/version.rc config-host.mak
$(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
diff --git a/Makefile.objs b/Makefile.objs
index 710d99f..f51eab3 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -102,6 +102,7 @@ common-obj-y += qdev.o qdev-properties.o
common-obj-y += block-migration.o
common-obj-y += pflib.o
common-obj-y += bitmap.o bitops.o
+common-obj-y += qmp-marshal-types.o qmp-marshal-types-core.o
common-obj-$(CONFIG_BRLAPI) += baum.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/qmp-gen.py b/qmp-gen.py
index cded2f6..02084d9 100644
--- a/qmp-gen.py
+++ b/qmp-gen.py
@@ -360,6 +360,205 @@ const char *qmp_type_%(dcc_name)s_to_str(%(name)s value, Error **errp)
name=name)
return ret
+def gen_type_marshal_declaration(name, typeinfo):
+ if is_dict(typeinfo):
+ return mcgen('''
+
+QObject *qmp_marshal_type_%(name)s(%(type)s src);
+%(type)s qmp_unmarshal_type_%(name)s(QObject *src, Error **errp);
+''',
+ name=name, type=qmp_type_to_c(name))
+
+ return ''
+
+def gen_metatype_def(typeinfo, name, lhs, sep='->', level=0):
+ new_lhs = 'qmp__member%d' % level
+
+ ret = ''
+
+ if type(typeinfo) == str:
+ ret += cgen(' %(lhs)s = %(marshal)s(%(name)s);',
+ lhs=lhs, name=name,
+ marshal=qmp_type_to_qobj(typeinfo))
+ elif is_dict(typeinfo):
+ ret += mcgen('''
+ {
+ QDict *qmp__dict = qdict_new();
+ QObject *%(new_lhs)s;
+''',
+ new_lhs=new_lhs)
+ for argname, argtype, optional in parse_args(typeinfo):
+ ret += cgen('')
+ if optional:
+ ret += mcgen('''
+ if (%(name)s%(sep)shas_%(c_name)s) {
+''',
+ name=name, sep=sep,
+ c_name=c_var(argname))
+ push_indent()
+ push_indent()
+ ret += gen_metatype_def(argtype, '%s%s%s' % (name, sep, c_var(argname)), new_lhs, '.', level + 1)
+ pop_indent()
+ ret += mcgen('''
+ qdict_put_obj(qmp__dict, "%(name)s", %(new_lhs)s);
+''',
+ name=argname, new_lhs=new_lhs)
+ if optional:
+ pop_indent()
+ ret += mcgen('''
+ }
+''')
+ ret += mcgen('''
+ %(lhs)s = QOBJECT(qmp__dict);
+ }
+''',
+ lhs=lhs)
+ elif type(typeinfo) == list:
+ ret += mcgen('''
+ {
+ QList *qmp__list = qlist_new();
+ %(type)s %(new_lhs)s_i;
+
+ for (%(new_lhs)s_i = %(name)s; %(new_lhs)s_i != NULL; %(new_lhs)s_i = %(new_lhs)s_i->next) {
+ QObject *qmp__member = %(marshal)s(%(new_lhs)s_i);
+ qlist_append_obj(qmp__list, qmp__member);
+ }
+ %(lhs)s = QOBJECT(qmp__list);
+ }
+''',
+ type=qmp_type_to_c(typeinfo[0], True),
+ new_lhs=new_lhs, name=name, lhs=lhs,
+ marshal=qmp_type_to_qobj(typeinfo[0]))
+
+ return ret
+
+def gen_metatype_undef(typeinfo, name, lhs, sep='->', level=0):
+ ret = ''
+ if type(typeinfo) == str:
+ ret += mcgen('''
+ %(lhs)s = %(unmarshal)s(%(c_name)s, &qmp__err);
+ if (qmp__err) {
+ goto qmp__err_out;
+ }
+''',
+ lhs=lhs, c_name=c_var(name),
+ unmarshal=qmp_type_from_qobj(typeinfo))
+ elif is_dict(typeinfo):
+ objname = 'qmp__object%d' % level
+ ret += mcgen('''
+ {
+ QDict *qmp__dict = qobject_to_qdict(%(c_name)s);
+ QObject *%(objname)s;
+
+''',
+ c_name=c_var(name), objname=objname)
+ for argname, argtype, optional in parse_args(typeinfo):
+ if optional:
+ ret += mcgen('''
+ if (qdict_haskey(qmp__dict, "%(name)s")) {
+''',
+ name=argname)
+ push_indent()
+ ret += mcgen('''
+ %(objname)s = qdict_get(qmp__dict, "%(name)s");
+''',
+ name=argname, objname=objname)
+ push_indent()
+ ret += gen_metatype_undef(argtype, objname, '%s%s%s' % (lhs, sep, c_var(argname)), '.', level + 1)
+ pop_indent()
+ if optional:
+ pop_indent()
+ ret += mcgen('''
+ %(lhs)s%(sep)shas_%(c_name)s = true;
+ } else {
+ %(lhs)s%(sep)shas_%(c_name)s = false;
+ }
+''',
+ lhs=lhs, sep=sep, c_name=c_var(argname))
+
+ ret += mcgen('''
+ }
+''')
+
+ elif type(typeinfo) == list:
+ objname = 'qmp__object%d' % level
+ ret += mcgen('''
+ {
+ QList *qmp__list = qobject_to_qlist(%(c_name)s);
+ QListEntry *%(objname)s;
+ QLIST_FOREACH_ENTRY(qmp__list, %(objname)s) {
+ %(type)s qmp__node = %(unmarshal)s(%(objname)s->value, &qmp__err);
+ if (qmp__err) {
+ goto qmp__err_out;
+ }
+ qmp__node->next = %(lhs)s;
+ %(lhs)s = qmp__node;
+ }
+ }
+''',
+ c_name=c_var(name), objname=objname, lhs=lhs,
+ type=qmp_type_to_c(typeinfo[0], True),
+ unmarshal=qmp_type_from_qobj(typeinfo[0]))
+
+ return ret
+
+def gen_type_marshal_definition(name, typeinfo):
+ ret = mcgen('''
+
+QObject *qmp_marshal_type_%(name)s(%(type)s src)
+{
+ QObject *qmp__retval;
+
+%(marshal)s
+
+ return qmp__retval;
+}
+
+%(type)s qmp_unmarshal_type_%(name)s(QObject *src, Error **errp)
+{
+ Error *qmp__err = NULL;
+ %(type)s qmp__retval = qmp_alloc_%(dcc_name)s();
+
+%(unmarshal)s
+
+ return qmp__retval;
+
+qmp__err_out:
+ error_propagate(errp, qmp__err);
+ %(free)s(qmp__retval);
+ return NULL;
+}
+''',
+ name=name, type=qmp_type_to_c(name),
+ marshal=gen_metatype_def(typeinfo, 'src', 'qmp__retval'),
+ dcc_name=de_camel_case(name), free=qmp_free_func(name),
+ unmarshal=gen_metatype_undef(typeinfo, 'src', 'qmp__retval'))
+
+ return ret
+
+def gen_enum_marshal_declaration(name, entries):
+ return mcgen('''
+
+QObject *qmp_marshal_type_%(name)s(%(name)s value);
+%(name)s qmp_unmarshal_type_%(name)s(QObject *obj, Error **errp);
+''',
+ name=name)
+
+def gen_enum_marshal_definition(name, entries):
+ return mcgen('''
+
+QObject *qmp_marshal_type_%(name)s(%(name)s value)
+{
+ return QOBJECT(qint_from_int(value));
+}
+
+%(name)s qmp_unmarshal_type_%(name)s(QObject *obj, Error **errp)
+{
+ return (%(name)s)qint_get_int(qobject_to_qint(obj));
+}
+''',
+ name=name)
+
def tokenize(data):
while len(data):
if data[0] in ['{', '}', ':', ',', '[', ']']:
@@ -436,6 +635,18 @@ def generate(kind):
#include "qmp-types.h"
#include "qmp-marshal-types.h"
''')
+ elif kind == 'marshal-header':
+ ret += mcgen('''
+#ifndef QMP_MARSHAL_TYPES_H
+#define QMP_MARSHAL_TYPES_H
+
+#include "qmp-marshal-types-core.h"
+''')
+ elif kind == 'marshal-body':
+ ret += mcgen('''
+#include "qmp-marshal-types.h"
+#include "qerror.h"
+''')
exprs = []
expr = ''
@@ -466,6 +677,10 @@ def generate(kind):
ret += gen_type_definition(name, data)
elif kind == 'types-header':
ret += gen_type_declaration(name, data)
+ elif kind == 'marshal-body':
+ ret += gen_type_marshal_definition(name, data)
+ elif kind == 'marshal-header':
+ ret += gen_type_marshal_declaration(name, data)
elif s.has_key('enum'):
name = s['enum']
data = s['data']
@@ -475,6 +690,10 @@ def generate(kind):
ret += gen_enum_declaration(name, data)
elif kind == 'types-body':
ret += gen_enum_definition(name, data)
+ elif kind == 'marshal-header':
+ ret += gen_enum_marshal_declaration(name, data)
+ elif kind == 'marshal-body':
+ ret += gen_enum_marshal_definition(name, data)
elif s.has_key('event'):
name = s['event']
data = {}
diff --git a/qmp-marshal-types-core.c b/qmp-marshal-types-core.c
new file mode 100644
index 0000000..6a55733
--- /dev/null
+++ b/qmp-marshal-types-core.c
@@ -0,0 +1,71 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#include "qmp-marshal-types-core.h"
+#include "qerror.h"
+
+QObject *qmp_marshal_type_int(int64_t value)
+{
+ return QOBJECT(qint_from_int(value));
+}
+
+QObject *qmp_marshal_type_str(const char *value)
+{
+ return QOBJECT(qstring_from_str(value));
+}
+
+QObject *qmp_marshal_type_bool(bool value)
+{
+ return QOBJECT(qbool_from_int(value));
+}
+
+QObject *qmp_marshal_type_number(double value)
+{
+ return QOBJECT(qfloat_from_double(value));
+}
+
+int64_t qmp_unmarshal_type_int(QObject *value, Error **errp)
+{
+ if (qobject_type(value) != QTYPE_QINT) {
+ error_set(errp, QERR_INVALID_PARAMETER_TYPE, "<unknown>", "int");
+ return 0;
+ }
+ return qint_get_int(qobject_to_qint(value));
+}
+
+char *qmp_unmarshal_type_str(QObject *value, Error **errp)
+{
+ if (qobject_type(value) != QTYPE_QSTRING) {
+ error_set(errp, QERR_INVALID_PARAMETER_TYPE, "<unknown>", "string");
+ return 0;
+ }
+ return qemu_strdup(qstring_get_str(qobject_to_qstring(value)));
+}
+
+bool qmp_unmarshal_type_bool(QObject *value, Error **errp)
+{
+ if (qobject_type(value) != QTYPE_QBOOL) {
+ error_set(errp, QERR_INVALID_PARAMETER_TYPE, "<unknown>", "bool");
+ return 0;
+ }
+ return qbool_get_int(qobject_to_qbool(value));
+}
+
+double qmp_unmarshal_type_number(QObject *value, Error **errp)
+{
+ if (qobject_type(value) != QTYPE_QFLOAT) {
+ error_set(errp, QERR_INVALID_PARAMETER_TYPE, "<unknown>", "float");
+ return 0;
+ }
+ return qfloat_get_double(qobject_to_qfloat(value));
+}
+
+
diff --git a/qmp-marshal-types-core.h b/qmp-marshal-types-core.h
new file mode 100644
index 0000000..38fe696
--- /dev/null
+++ b/qmp-marshal-types-core.h
@@ -0,0 +1,31 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#ifndef QMP_MARSHAL_TYPES_CORE_H
+#define QMP_MARSHAL_TYPES_CORE_H
+
+#include "qemu-common.h"
+#include "qemu-objects.h"
+#include "qerror.h"
+#include "error.h"
+#include "qmp-types.h"
+
+QObject *qmp_marshal_type_int(int64_t value);
+QObject *qmp_marshal_type_str(const char *value);
+QObject *qmp_marshal_type_bool(bool value);
+QObject *qmp_marshal_type_number(double value);
+
+int64_t qmp_unmarshal_type_int(QObject *value, Error **errp);
+char *qmp_unmarshal_type_str(QObject *value, Error **errp);
+bool qmp_unmarshal_type_bool(QObject *value, Error **errp);
+double qmp_unmarshal_type_number(QObject *value, Error **errp);
+
+#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] Re: [PATCH 02/15] qapi: add code generator for type marshallers
2011-03-11 23:05 ` [Qemu-devel] [PATCH 02/15] qapi: add code generator for type marshallers Anthony Liguori
@ 2011-03-18 15:13 ` Luiz Capitulino
0 siblings, 0 replies; 49+ messages in thread
From: Luiz Capitulino @ 2011-03-18 15:13 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Markus Armbruster, Adam Litke, qemu-devel, Avi Kivity
On Fri, 11 Mar 2011 17:05:32 -0600
Anthony Liguori <aliguori@us.ibm.com> wrote:
> This code will marshal a QMP type from native representation to a QObject and
> vice versa.
>
> Marshaling from a native representation to a QObject can never generate an
> Error although the opposite direction may.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
So, as I reported to you by IRC, this patch doesn't compile, breaking bisect:
/home/lcapitulino/src/qmp-unstable/qmp-core.c:12:17: fatal error: qmp.h: No such file or directory
compilation terminated.
make: *** [qmp-core.o] Error 1
make: *** Waiting for unfinished jobs....
~/src/qmp-unstable/build (qapi-core-v2-review)/
Actually, I'm pretty sure I also got this problem with the series fully
applied too, but I managed to make it go away someway and I couldn't
reproduce it anymore (so I thought it was a local problem).
^ permalink raw reply [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH 03/15] qapi: add core QMP server support (v2)
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2) Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 02/15] qapi: add code generator for type marshallers Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 04/15] qapi: add signal support to core QMP server Anthony Liguori
` (12 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This is the infrastructure to register commands.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
- support async commands
diff --git a/Makefile.objs b/Makefile.objs
index f51eab3..dbdce3c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -103,6 +103,7 @@ common-obj-y += block-migration.o
common-obj-y += pflib.o
common-obj-y += bitmap.o bitops.o
common-obj-y += qmp-marshal-types.o qmp-marshal-types-core.o
+common-obj-y += qmp-core.o
common-obj-$(CONFIG_BRLAPI) += baum.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/qmp-core.c b/qmp-core.c
new file mode 100644
index 0000000..d906557
--- /dev/null
+++ b/qmp-core.c
@@ -0,0 +1,113 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#include "qmp.h"
+#include "qmp-core.h"
+#include "json-lexer.h"
+#include "json-parser.h"
+#include "json-streamer.h"
+#include "qemu_socket.h"
+#include <glib.h>
+#include "qemu-queue.h"
+#include "sysemu.h"
+
+typedef enum QmpCommandType
+{
+ QCT_NORMAL,
+ QCT_STATEFUL,
+ QCT_ASYNC,
+} QmpCommandType;
+
+typedef struct QmpCommand
+{
+ const char *name;
+ QmpCommandType type;
+ QmpCommandFunc *fn;
+ QmpStatefulCommandFunc *sfn;
+ QmpAsyncCommandFunc *afn;
+ QTAILQ_ENTRY(QmpCommand) node;
+} QmpCommand;
+
+struct QmpCommandState
+{
+ QmpState *state;
+ QObject *tag;
+};
+
+static QTAILQ_HEAD(, QmpCommand) qmp_commands =
+ QTAILQ_HEAD_INITIALIZER(qmp_commands);
+
+void qmp_register_command(const char *name, QmpCommandFunc *fn)
+{
+ QmpCommand *cmd = qemu_mallocz(sizeof(*cmd));
+
+ cmd->name = name;
+ cmd->type = QCT_NORMAL;
+ cmd->fn = fn;
+ QTAILQ_INSERT_TAIL(&qmp_commands, cmd, node);
+}
+
+void qmp_register_stateful_command(const char *name, QmpStatefulCommandFunc *fn)
+{
+ QmpCommand *cmd = qemu_mallocz(sizeof(*cmd));
+
+ cmd->name = name;
+ cmd->type = QCT_STATEFUL;
+ cmd->sfn = fn;
+ QTAILQ_INSERT_TAIL(&qmp_commands, cmd, node);
+}
+
+void qmp_register_async_command(const char *name, QmpAsyncCommandFunc *fn)
+{
+ QmpCommand *cmd = qemu_mallocz(sizeof(*cmd));
+
+ cmd->name = name;
+ cmd->type = QCT_ASYNC;
+ cmd->afn = fn;
+ QTAILQ_INSERT_TAIL(&qmp_commands, cmd, node);
+}
+
+static QmpCommand *qmp_find_command(const char *name)
+{
+ QmpCommand *i;
+
+ QTAILQ_FOREACH(i, &qmp_commands, node) {
+ if (strcmp(i->name, name) == 0) {
+ return i;
+ }
+ }
+ return NULL;
+}
+
+char *qobject_as_string(QObject *obj)
+{
+ char buffer[1024];
+
+ switch (qobject_type(obj)) {
+ case QTYPE_QINT:
+ snprintf(buffer, sizeof(buffer), "%" PRId64,
+ qint_get_int(qobject_to_qint(obj)));
+ return qemu_strdup(buffer);
+ case QTYPE_QSTRING:
+ return qemu_strdup(qstring_get_str(qobject_to_qstring(obj)));
+ case QTYPE_QFLOAT:
+ snprintf(buffer, sizeof(buffer), "%.17g",
+ qfloat_get_double(qobject_to_qfloat(obj)));
+ return qemu_strdup(buffer);
+ case QTYPE_QBOOL:
+ if (qbool_get_int(qobject_to_qbool(obj))) {
+ return qemu_strdup("on");
+ }
+ return qemu_strdup("off");
+ default:
+ return NULL;
+ }
+}
diff --git a/qmp-core.h b/qmp-core.h
new file mode 100644
index 0000000..4765b69
--- /dev/null
+++ b/qmp-core.h
@@ -0,0 +1,31 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#ifndef QMP_CORE_H
+#define QMP_CORE_H
+
+#include "monitor.h"
+#include "qmp-marshal-types.h"
+#include "error_int.h"
+
+typedef void (QmpCommandFunc)(const QDict *, QObject **, Error **);
+typedef void (QmpStatefulCommandFunc)(QmpState *qmp__sess, const QDict *, QObject **, Error **);
+typedef void (QmpAsyncCommandFunc)(const QDict *, Error **, QmpCommandState *);
+
+void qmp_register_command(const char *name, QmpCommandFunc *fn);
+void qmp_register_stateful_command(const char *name, QmpStatefulCommandFunc *fn);
+void qmp_register_async_command(const char *name, QmpAsyncCommandFunc *fn);
+
+void qmp_async_complete_command(QmpCommandState *cmd, QObject *retval, Error *err);
+
+char *qobject_as_string(QObject *obj);
+
+#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] [PATCH 04/15] qapi: add signal support to core QMP server
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (2 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 03/15] qapi: add core QMP server support (v2) Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 05/15] qapi: add QAPI module type Anthony Liguori
` (11 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/qmp-core.c b/qmp-core.c
index d906557..7f60942 100644
--- a/qmp-core.c
+++ b/qmp-core.c
@@ -42,6 +42,13 @@ struct QmpCommandState
QObject *tag;
};
+struct QmpState
+{
+ int (*add_connection)(QmpState *s, QmpConnection *conn);
+ void (*del_connection)(QmpState *s, int global_handle, Error **errp);
+ void (*event)(QmpState *s, QObject *data);
+};
+
static QTAILQ_HEAD(, QmpCommand) qmp_commands =
QTAILQ_HEAD_INITIALIZER(qmp_commands);
@@ -111,3 +118,90 @@ char *qobject_as_string(QObject *obj)
return NULL;
}
}
+
+void qmp_state_add_connection(QmpState *sess, const char *event_name, QmpSignal *obj, int handle, QmpConnection *conn)
+{
+ conn->state = sess;
+ conn->event_name = event_name;
+ conn->signal = obj;
+ conn->handle = handle;
+ conn->global_handle = sess->add_connection(sess, conn);
+}
+
+void qmp_put_event(QmpState *sess, int global_handle, Error **errp)
+{
+ sess->del_connection(sess, global_handle, errp);
+}
+
+void qmp_state_event(QmpConnection *conn, QObject *data)
+{
+ QDict *event = qdict_new();
+ qemu_timeval tv;
+ QObject *ts;
+
+ qemu_gettimeofday(&tv);
+
+ ts = qobject_from_jsonf("{ 'seconds': %" PRId64 ", "
+ "'microseconds': %" PRId64 " }",
+ (int64_t)tv.tv_sec, (int64_t)tv.tv_usec);
+ qdict_put_obj(event, "timestamp", ts);
+
+ qdict_put(event, "event", qstring_from_str(conn->event_name));
+ if (data) {
+ qobject_incref(data);
+ qdict_put_obj(event, "data", data);
+ }
+
+ qdict_put(event, "tag", qint_from_int(conn->global_handle));
+
+ conn->state->event(conn->state, QOBJECT(event));
+ QDECREF(event);
+}
+
+QmpSignal *qmp_signal_init(void)
+{
+ QmpSignal *obj = qemu_mallocz(sizeof(*obj));
+ obj->max_handle = 0;
+ obj->ref = 1;
+ QTAILQ_INIT(&obj->slots);
+ return obj;
+}
+
+void qmp_signal_ref(QmpSignal *obj)
+{
+ obj->ref++;
+}
+
+void qmp_signal_unref(QmpSignal *obj)
+{
+ if (--obj->ref) {
+ qemu_free(obj);
+ }
+}
+
+int qmp_signal_connect(QmpSignal *obj, void *func, void *opaque)
+{
+ int handle = ++obj->max_handle;
+ QmpSlot *slot = qemu_mallocz(sizeof(*slot));
+
+ slot->handle = handle;
+ slot->func = func;
+ slot->opaque = opaque;
+
+ QTAILQ_INSERT_TAIL(&obj->slots, slot, node);
+
+ return handle;
+}
+
+void qmp_signal_disconnect(QmpSignal *obj, int handle)
+{
+ QmpSlot *slot;
+
+ QTAILQ_FOREACH(slot, &obj->slots, node) {
+ if (slot->handle == handle) {
+ QTAILQ_REMOVE(&obj->slots, slot, node);
+ qemu_free(slot);
+ break;
+ }
+ }
+}
diff --git a/qmp-core.h b/qmp-core.h
index 4765b69..c9c8b63 100644
--- a/qmp-core.h
+++ b/qmp-core.h
@@ -20,6 +20,31 @@ typedef void (QmpCommandFunc)(const QDict *, QObject **, Error **);
typedef void (QmpStatefulCommandFunc)(QmpState *qmp__sess, const QDict *, QObject **, Error **);
typedef void (QmpAsyncCommandFunc)(const QDict *, Error **, QmpCommandState *);
+typedef struct QmpSlot
+{
+ int handle;
+ void *func;
+ void *opaque;
+ QTAILQ_ENTRY(QmpSlot) node;
+} QmpSlot;
+
+struct QmpSignal
+{
+ int max_handle;
+ int ref;
+ QTAILQ_HEAD(, QmpSlot) slots;
+};
+
+typedef struct QmpConnection
+{
+ QmpState *state;
+ const char *event_name;
+ QmpSignal *signal;
+ int handle;
+ int global_handle;
+ QTAILQ_ENTRY(QmpConnection) node;
+} QmpConnection;
+
void qmp_register_command(const char *name, QmpCommandFunc *fn);
void qmp_register_stateful_command(const char *name, QmpStatefulCommandFunc *fn);
void qmp_register_async_command(const char *name, QmpAsyncCommandFunc *fn);
@@ -28,4 +53,34 @@ void qmp_async_complete_command(QmpCommandState *cmd, QObject *retval, Error *er
char *qobject_as_string(QObject *obj);
+QmpSignal *qmp_signal_init(void);
+void qmp_signal_ref(QmpSignal *obj);
+void qmp_signal_unref(QmpSignal *obj);
+int qmp_signal_connect(QmpSignal *obj, void *func, void *opaque);
+void qmp_signal_disconnect(QmpSignal *obj, int handle);
+
+void qmp_state_add_connection(QmpState *sess, const char *name, QmpSignal *obj, int handle, QmpConnection *conn);
+void qmp_put_event(QmpState *sess, int global_handle, Error **errp);
+void qmp_state_event(QmpConnection *conn, QObject *data);
+
+#define signal_init(obj) do { \
+ (obj)->signal = qmp_signal_init(); \
+} while (0)
+
+#define signal_unref(obj) qmp_signal_unref((obj)->signal)
+
+#define signal_connect(obj, fn, opaque) \
+ qmp_signal_connect((obj)->signal, (obj)->func = fn, opaque)
+
+#define signal_disconnect(obj, handle) \
+ qmp_signal_disconnect((obj)->signal, handle)
+
+#define signal_notify(obj, ...) do { \
+ QmpSlot *qmp__slot; \
+ QTAILQ_FOREACH(qmp__slot, &(obj)->signal->slots, node) { \
+ (obj)->func = qmp__slot->func; \
+ (obj)->func(qmp__slot->opaque, ## __VA_ARGS__); \
+ } \
+} while(0)
+
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] [PATCH 05/15] qapi: add QAPI module type
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (3 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 04/15] qapi: add signal support to core QMP server Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 06/15] qapi: add code generators for QMP command marshaling Anthony Liguori
` (10 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This lets us register marshaling handlers using a module init function.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/module.h b/module.h
index 9263f1c..ef66730 100644
--- a/module.h
+++ b/module.h
@@ -24,12 +24,14 @@ typedef enum {
MODULE_INIT_BLOCK,
MODULE_INIT_DEVICE,
MODULE_INIT_MACHINE,
+ MODULE_INIT_QAPI,
MODULE_INIT_MAX
} module_init_type;
#define block_init(function) module_init(function, MODULE_INIT_BLOCK)
#define device_init(function) module_init(function, MODULE_INIT_DEVICE)
#define machine_init(function) module_init(function, MODULE_INIT_MACHINE)
+#define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
void register_module_init(void (*fn)(void), module_init_type type);
diff --git a/vl.c b/vl.c
index 5e007a7..d27e750 100644
--- a/vl.c
+++ b/vl.c
@@ -1962,6 +1962,8 @@ int main(int argc, char **argv, char **envp)
cyls = heads = secs = 0;
translation = BIOS_ATA_TRANSLATION_AUTO;
+ module_call_init(MODULE_INIT_QAPI);
+
for (i = 0; i < MAX_NODES; i++) {
node_mem[i] = 0;
node_cpumask[i] = 0;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] [PATCH 06/15] qapi: add code generators for QMP command marshaling
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (4 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 05/15] qapi: add QAPI module type Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 07/15] qapi: add query-version QMP command Anthony Liguori
` (9 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This generates qmp.h which contains the declarations of all of QMP functions
to be dispatched, plus a function that registers marshallers for each of
the QMP functions.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
- change code generator to use multiline
- proxy support
- async command support
diff --git a/Makefile b/Makefile
index 8f3a4d3..47a755d 100644
--- a/Makefile
+++ b/Makefile
@@ -4,7 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
ifeq ($(TRACE_BACKEND),dtrace)
GENERATED_HEADERS += trace-dtrace.h
endif
-GENERATED_HEADERS += qmp-types.h qmp-marshal-types.h
+GENERATED_HEADERS += qmp-types.h qmp-marshal-types.h qmp.h
ifneq ($(wildcard config-host.mak),)
# Put the all: rule here so that config-host.mak can contain dependencies.
@@ -159,8 +159,15 @@ qmp-marshal-types.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
qmp-marshal-types.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
$(call quiet-command,python $(SRC_PATH)/qmp-gen.py --marshal-header < $< > $@, " GEN $@")
+qmp.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
+ $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --header < $< > $@, " GEN $@")
+
+qmp-marshal.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
+ $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --body < $< > $@, " GEN $@")
+
qmp-types.o: qmp-types.c qmp-types.h
qmp-marshal-types.o: qmp-marshal-types.c qmp-marshal-types.h qmp-types.h
+qmp-marshal.o: qmp-marshal.c qmp.h qmp-types.h qmp-marshal-types.h
version.o: $(SRC_PATH)/version.rc config-host.mak
$(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
diff --git a/Makefile.objs b/Makefile.objs
index dbdce3c..5dae800 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -103,7 +103,7 @@ common-obj-y += block-migration.o
common-obj-y += pflib.o
common-obj-y += bitmap.o bitops.o
common-obj-y += qmp-marshal-types.o qmp-marshal-types-core.o
-common-obj-y += qmp-core.o
+common-obj-y += qmp-core.o qmp-marshal.o
common-obj-$(CONFIG_BRLAPI) += baum.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/qmp-gen.py b/qmp-gen.py
index 02084d9..f1bdb2f 100644
--- a/qmp-gen.py
+++ b/qmp-gen.py
@@ -559,6 +559,372 @@ QObject *qmp_marshal_type_%(name)s(%(name)s value)
''',
name=name)
+def gen_async_fn_decl(name, options, retval):
+ if retval == 'none':
+ return mcgen('''
+typedef void (%(cc_name)sCompletionFunc)(void *qmp__opaque, Error *qmp__err);
+''',
+ cc_name=camel_case(name))
+ else:
+ return mcgen('''
+typedef void (%(cc_name)sCompletionFunc)(void *qmp__opaque, %(ret_type)s qmp__retval, Error *qmp__err);
+''', cc_name=camel_case(name), ret_type=qmp_type_to_c(retval))
+
+def gen_declaration(name, options, retval, async=False, prefix='qmp'):
+ args = []
+ ret = ''
+
+ if async:
+ ret += gen_async_fn_decl(name, options, retval)
+
+ for argname, argtype, optional in parse_args(options):
+ if argtype == '**':
+ args.append('KeyValues * %s' % c_var(argname))
+ else:
+ if optional:
+ args.append('bool has_%s' % c_var(argname))
+ args.append('%s %s' % (qmp_type_to_c(argtype), c_var(argname)))
+ args.append('Error **err')
+
+ if async:
+ args.append('%sCompletionFunc *qmp__cc' % camel_case(name))
+ args.append('void *qmp__opaque')
+ qmp_retval = 'void'
+ else:
+ qmp_retval = qmp_type_to_c(retval, True)
+
+ if qmp_is_stateful_cmd(name):
+ args = ['QmpState *qmp__state'] + args
+
+ ret += cgen('%(ret_type)s %(prefix)s_%(c_name)s(%(args)s);',
+ ret_type=qmp_retval, prefix=prefix,
+ c_name=c_var(name), args=', '.join(args))
+
+ return ret
+
+def gen_async_definition(name, options, retval):
+ if retval == 'none':
+ ret = mcgen('''
+
+static void qmp_async_completion_%(c_name)s(void *qmp__opaque, Error *qmp__err)
+{
+''',
+ c_name=c_var(name))
+ else:
+ ret = mcgen('''
+
+static void qmp_async_completion_%(c_name)s(void *qmp__opaque, %(ret_type)s qmp__retval, Error *qmp__err)
+{
+''',
+ c_name=c_var(name), ret_type=qmp_type_to_c(retval))
+ if retval != 'none':
+ ret += cgen(' QObject *qmp__ret_data;')
+ ret += mcgen('''
+ QmpCommandState *qmp__cmd = qmp__opaque;
+
+ if (qmp__err) {
+ qmp_async_complete_command(qmp__cmd, NULL, qmp__err);
+ return;
+ }
+''')
+ if retval == 'none':
+ pass
+ elif type(retval) == str:
+ ret += mcgen('''
+
+ qmp__ret_data = %(marshal)s(qmp__retval);
+''',
+ marshal=qmp_type_to_qobj(retval))
+ elif type(retval) == list:
+ ret += mcgen('''
+
+ qmp__ret_data = QOBJECT(qlist_new());
+ if (qmp__retval) {
+ QList *list = qobject_to_qlist(qmp__ret_data);
+ %(type)s i;
+ for (i = qmp__retval; i != NULL; i = i->next) {
+ QObject *obj = %(marshal)s(i);
+ qlist_append_obj(list, obj);
+ }
+ }
+''',
+ type=qmp_type_to_c(retval[0], True),
+ marshal=qmp_type_to_qobj(retval[0]))
+
+ ret_data = 'qmp__ret_data'
+ if retval == 'none':
+ ret_data = 'NULL'
+ ret += mcgen('''
+
+ qmp_async_complete_command(qmp__cmd, %(ret_var)s, NULL);
+}
+''', ret_var=ret_data)
+
+ return ret
+
+def gen_definition(name, options, retval, async=False, prefix='qmp'):
+ ret = ''
+ if async:
+ ret = gen_async_definition(name, options, retval)
+
+ if qmp_type_is_event(retval):
+ arglist = ['void *opaque']
+ for argname, argtype, optional in parse_args(event_types[retval]):
+ if optional:
+ arglist.append('bool has_%s' % c_var(argname))
+ arglist.append('%s %s' % (qmp_type_to_c(argtype), c_var(argname)))
+ ret += mcgen('''
+
+static void qmp_marshal_%(c_name)s(%(args)s)
+{
+ QDict *qmp__args = qdict_new();
+ QmpConnection *qmp__conn = opaque;
+''',
+ c_name=qmp_event_to_c(retval),
+ args=', '.join(arglist))
+
+ for argname, argtype, optional in parse_args(event_types[retval]):
+ if optional:
+ ret += cgen(' if (has_%(c_name)s) {', c_name=c_var(argname))
+ push_indent()
+ ret += mcgen('''
+ qdict_put_obj(qmp__args, "%(name)s", %(marshal)s(%(c_name)s));
+''',
+ name=argname, c_name=c_var(argname),
+ marshal=qmp_type_to_qobj(argtype))
+ if optional:
+ pop_indent()
+ ret += cgen(' }')
+
+ ret += mcgen('''
+
+ qmp_state_event(qmp__conn, QOBJECT(qmp__args));
+ QDECREF(qmp__args);
+}
+
+static void qmp_marshal_%(c_name)s(QmpState *qmp__sess, const QDict *qdict, QObject **ret_data, Error **err)
+{
+ int qmp__handle;
+ QmpConnection *qmp__connection = qemu_mallocz(sizeof(QmpConnection));
+''',
+ c_name=c_var(name))
+ elif qmp_is_stateful_cmd(name):
+ ret += mcgen('''
+
+static void qmp_marshal_%(c_name)s(QmpState *qmp__sess, const QDict *qdict, QObject **ret_data, Error **err)
+{
+''',
+ c_name=c_var(name))
+ elif async:
+ ret += mcgen('''
+
+static void qmp_marshal_%(c_name)s(const QDict *qdict, Error **err, QmpCommandState *qmp__cmd)
+{
+''',
+ c_name=c_var(name))
+ else:
+ ret += mcgen('''
+
+static void qmp_marshal_%(c_name)s(const QDict *qdict, QObject **ret_data, Error **err)
+{
+''',
+ c_name=c_var(name))
+ ret += mcgen('''
+ Error *qmp__err = NULL;
+''')
+
+ for argname, argtype, optional in parse_args(options):
+ if argtype == '**':
+ ret += cgen(' KeyValues * %(c_name)s = 0;', c_name=c_var(argname))
+ else:
+ if optional:
+ ret += cgen(' bool has_%(c_name)s = false;', c_name=c_var(argname))
+ ret += cgen(' %(type)s %(c_name)s = 0;',
+ type=qmp_type_to_c(argtype, True), c_name=c_var(argname))
+
+ if retval != 'none' and not async:
+ ret += mcgen('''
+ %(type)s qmp_retval = 0;
+''',
+ type=qmp_type_to_c(retval, True))
+
+ ret += mcgen('''
+
+ (void)qmp__err;
+''')
+
+ for argname, argtype, optional in parse_args(options):
+ if argtype == '**':
+ ret += mcgen('''
+ {
+ const QDictEntry *qmp__qdict_i;
+
+ %(c_name)s = NULL;
+ for (qmp__qdict_i = qdict_first(qdict); qmp__qdict_i; qmp__qdict_i = qdict_next(qdict, qmp__qdict_i)) {
+ KeyValues *qmp__i;
+''',
+ c_name=c_var(argname))
+ for argname1, argtype1, optional1 in parse_args(options):
+ if argname1 == argname:
+ continue
+ ret += mcgen('''
+ if (strcmp(qmp__qdict_i->key, "%(name)s") == 0) {
+ continue;
+ }
+''',
+ name=argname1)
+ ret += mcgen('''
+
+ qmp__i = qmp_alloc_key_values();
+ qmp__i->key = qemu_strdup(qmp__qdict_i->key);
+ qmp__i->value = qobject_as_string(qmp__qdict_i->value);
+ qmp__i->next = %(c_name)s;
+ %(c_name)s = qmp__i;
+ }
+ }
+''',
+ c_name=c_var(argname))
+ elif optional:
+ ret += mcgen('''
+
+ if (qdict_haskey(qdict, "%(name)s")) {
+ %(c_name)s = %(unmarshal)s(qdict_get(qdict, "%(name)s"), &qmp__err);
+ if (qmp__err) {
+ if (error_is_type(qmp__err, QERR_INVALID_PARAMETER_TYPE)) {
+ error_set(err, QERR_INVALID_PARAMETER_TYPE, "%(name)s",
+ error_get_field(qmp__err, "expected"));
+ error_free(qmp__err);
+ qmp__err = NULL;
+ } else {
+ error_propagate(err, qmp__err);
+ }
+ goto qmp__out;
+ }
+ has_%(c_name)s = true;
+ }
+''',
+ name=argname, c_name=c_var(argname),
+ unmarshal=qmp_type_from_qobj(argtype))
+ else:
+ ret += mcgen('''
+
+ if (!qdict_haskey(qdict, "%(name)s")) {
+ error_set(err, QERR_MISSING_PARAMETER, "%(name)s");
+ goto qmp__out;
+ }
+
+ %(c_name)s = %(unmarshal)s(qdict_get(qdict, "%(name)s"), &qmp__err);
+ if (qmp__err) {
+ if (error_is_type(qmp__err, QERR_INVALID_PARAMETER_TYPE)) {
+ error_set(err, QERR_INVALID_PARAMETER_TYPE, "%(name)s",
+ error_get_field(qmp__err, "expected"));
+ error_free(qmp__err);
+ qmp__err = NULL;
+ } else {
+ error_propagate(err, qmp__err);
+ }
+ goto qmp__out;
+ }
+''',
+ name=argname, c_name=c_var(argname),
+ unmarshal=qmp_type_from_qobj(argtype))
+
+ args = []
+ for argname, argtype, optional in parse_args(options):
+ if optional and argtype != '**':
+ args.append('has_%s' % c_var(argname))
+ args.append(c_var(argname))
+ args.append('err')
+
+ if qmp_is_stateful_cmd(name):
+ args = ['qmp__sess'] + args
+
+ if async:
+ args.append('qmp_async_completion_%s' % c_var(name))
+ args.append('qmp__cmd')
+
+ arglist = ', '.join(args)
+ fn = '%s_%s' % (prefix, c_var(name))
+
+ if retval == 'none' or async:
+ ret += cgen(' %(fn)s(%(args)s);', fn=fn, args=arglist)
+ else:
+ ret += cgen(' qmp_retval = %(fn)s(%(args)s);', fn=fn, args=arglist)
+
+ ret += mcgen('''
+
+ if (error_is_set(err)) {
+ goto qmp__out;
+ }
+''')
+
+ if retval == 'none' or async:
+ pass
+ elif qmp_type_is_event(retval):
+ ret += mcgen('''
+ qmp__handle = signal_connect(qmp_retval, qmp_marshal_%(event_name)s, qmp__connection);
+ qmp_state_add_connection(qmp__sess, "%(ret_name)s", qmp_retval->signal, qmp__handle, qmp__connection);
+ *ret_data = QOBJECT(qint_from_int(qmp__connection->global_handle));
+ qmp__connection = NULL;
+''',
+ event_name=qmp_event_to_c(retval),
+ ret_name=retval)
+ elif type(retval) == str:
+ ret += mcgen('''
+ *ret_data = %(marshal)s(qmp_retval);
+''',
+ marshal=qmp_type_to_qobj(retval))
+ elif type(retval) == list:
+ ret += mcgen('''
+ *ret_data = QOBJECT(qlist_new());
+ if (qmp_retval) {
+ QList *list = qobject_to_qlist(*ret_data);
+ %(ret_type)s i;
+ for (i = qmp_retval; i != NULL; i = i->next) {
+ QObject *obj = %(marshal)s(i);
+ qlist_append_obj(list, obj);
+ }
+ }
+''',
+ ret_type=qmp_type_to_c(retval[0], True),
+ marshal=qmp_type_to_qobj(retval[0]))
+
+ ret += mcgen('''
+
+qmp__out:
+''')
+
+ if qmp_type_is_event(retval):
+ ret += cgen(' qemu_free(qmp__connection);')
+ ret += cgen('')
+ args = []
+ for argname, argtype, optional in parse_args(options):
+ if argtype == '**':
+ ret += cgen(' %(free)s(%(c_name)s);',
+ free=qmp_free_func('KeyValues'),
+ c_name=c_var(argname))
+ elif qmp_type_should_free(argtype):
+ if optional:
+ ret += cgen(' if (has_%(c_name)s) {', c_name=c_var(argname))
+ push_indent()
+ ret += cgen(' %(free)s(%(c_name)s);',
+ free=qmp_free_func(argtype), c_name=c_var(argname))
+ if optional:
+ pop_indent()
+ ret += cgen(' }')
+ if retval != 'none' and not async:
+ if qmp_type_should_free(retval):
+ ret += cgen(' %(free)s(%(c_name)s);',
+ free=qmp_free_func(retval), c_name='qmp_retval')
+
+ ret += mcgen('''
+
+ return;
+}
+''')
+
+ return ret
+
def tokenize(data):
while len(data):
if data[0] in ['{', '}', ':', ',', '[', ']']:
@@ -647,6 +1013,21 @@ def generate(kind):
#include "qmp-marshal-types.h"
#include "qerror.h"
''')
+ elif kind == 'header':
+ ret += mcgen('''
+#ifndef QMP_H
+#define QMP_H
+
+#include "qemu-common.h"
+#include "qemu-objects.h"
+#include "qmp-types.h"
+#include "error.h"
+''')
+ elif kind == 'body':
+ ret += mcgen('''
+#include "qmp.h"
+#include "qmp-core.h"
+''')
exprs = []
expr = ''
@@ -711,9 +1092,49 @@ def generate(kind):
retval = 'none'
if s.has_key('returns'):
retval = s['returns']
+ if kind == 'body':
+ async = qmp_is_async_cmd(name)
+ ret += gen_definition(name, options, retval, async=async)
+ elif kind == 'header':
+ async = qmp_is_async_cmd(name)
+ ret += gen_declaration(name, options, retval, async=async)
if kind.endswith('header'):
ret += cgen('#endif')
+ elif kind == 'body':
+ ret += mcgen('''
+
+static void qmp_init_marshal(void)
+{
+''')
+ for s in exprs:
+ if not s.has_key('command'):
+ continue
+ name = s['command']
+ retval = 'none'
+ if s.has_key('returns'):
+ retval = s['returns']
+
+ if qmp_type_is_event(retval) or qmp_is_stateful_cmd(name):
+ ret += mcgen('''
+ qmp_register_stateful_command("%(name)s", &qmp_marshal_%(c_name)s);
+''',
+ name=name, c_name=c_var(name))
+ elif qmp_is_async_cmd(name):
+ ret += mcgen('''
+ qmp_register_async_command("%(name)s", &qmp_marshal_%(c_name)s);
+''',
+ name=name, c_name=c_var(name))
+ else:
+ ret += mcgen('''
+ qmp_register_command("%(name)s", &qmp_marshal_%(c_name)s);
+''',
+ name=name, c_name=c_var(name))
+ ret += mcgen('''
+}
+
+qapi_init(qmp_init_marshal);
+''')
return ret
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] [PATCH 07/15] qapi: add query-version QMP command
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (5 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 06/15] qapi: add code generators for QMP command marshaling Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-12 11:19 ` Blue Swirl
2011-03-11 23:05 ` [Qemu-devel] [PATCH 08/15] qapi: add new QMP server that uses CharDriverState (v2) Anthony Liguori
` (8 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This is used internally by QMP. It's also a pretty good example of a typical
command conversion.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/Makefile.objs b/Makefile.objs
index 5dae800..e1a2756 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -103,7 +103,7 @@ common-obj-y += block-migration.o
common-obj-y += pflib.o
common-obj-y += bitmap.o bitops.o
common-obj-y += qmp-marshal-types.o qmp-marshal-types-core.o
-common-obj-y += qmp-core.o qmp-marshal.o
+common-obj-y += qmp-core.o qmp-marshal.o qmp.o
common-obj-$(CONFIG_BRLAPI) += baum.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/qmp-schema.json b/qmp-schema.json
index e69de29..e72bc18 100644
--- a/qmp-schema.json
+++ b/qmp-schema.json
@@ -0,0 +1,90 @@
+# *-*- Mode: Python -*-*
+
+##
+# QMP Schema
+#
+# This file defines the types, commands, and events used by QMP. It should
+# fully describe the interface used by QMP.
+#
+# This file is designed to be loosely based on JSON although it's technical
+# executable Python. While dictionaries are used, they are parsed as
+# OrderedDicts so that ordering is preserved.
+#
+# There are two basic syntaxes used. The first syntax defines a type and is
+# represented by a dictionary. There are three kinds of types that are
+# supported.
+#
+# A complex type is a dictionary containing a single key who's value is a
+# dictionary. This corresponds to a struct in C or an Object in JSON. An
+# example of a complex type is:
+#
+# { 'type': 'MyType',
+# 'data' { 'member1': 'str', 'member2': 'int', '*member3': 'str } }
+#
+# The use of '*' as a prefix to the name means the member is optional. Optional
+# members should always be added to the end of the dictionary to preserve
+# backwards compatibility.
+#
+# An enumeration type is a dictionary containing a single key who's value is a
+# list of strings. An example enumeration is:
+#
+# { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
+#
+# A signal is similar in form to a complex type except the single key is
+# entirely upper case instead of CamelCase. An example signal is:
+#
+# { 'event': 'MY_SIGNAL', 'data': { 'arg1': 'str', 'arg2': 'str' } }
+#
+# Generally speaking, complex types and enums should always use CamelCase for
+# the type names.
+#
+# Commands are defined by using a list containing three members. The first
+# member is the command name, the second member is a dictionary containing
+# arguments, and the third member is the return type.
+#
+# An example command is:
+#
+# { 'command': 'my-command',
+# 'data': { 'arg1': 'str', '*arg2': 'str' },
+# 'returns': 'str' ]
+#
+# Command names should be all lower case with words separated by a hyphen.
+
+### 0.14.0 commands. Do not modify. ###
+
+##
+# @VersionInfo:
+#
+# A description of QEMU's version.
+#
+# @qemu.major: The major version of QEMU
+#
+# @qemu.minor: The minor version of QEMU
+#
+# @qemu.micro: The micro version of QEMU. By current convention, a micro
+# version of 50 signifies a development branch. A micro version
+# greater than or equal to 90 signifies a release candidate for
+# the next minor version. A micro version of less than 50
+# signifies a stable release.
+#
+# @package: QEMU will always set this field to an empty string. Downstream
+# versions of QEMU should set this to a non-empty string. The
+# exact format depends on the downstream however it highly
+# recommended that a unique name is used.
+#
+# Since: 0.14.0
+##
+{ 'type': 'VersionInfo',
+ 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
+ 'package': 'str'} }
+
+##
+# @query-version:
+#
+# Returns the current version of QEMU.
+#
+# Returns: A @VersionInfo object describing the current version of QEMU.
+#
+# Since: 0.14.0
+##
+{ 'command': 'query-version', 'returns': 'VersionInfo' }
diff --git a/qmp.c b/qmp.c
new file mode 100644
index 0000000..7b626f5
--- /dev/null
+++ b/qmp.c
@@ -0,0 +1,31 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#include "qemu-common.h"
+#include "qmp-core.h"
+#include "qmp.h"
+
+VersionInfo *qmp_query_version(Error **err)
+{
+ VersionInfo *info = qmp_alloc_version_info();
+ const char *version = QEMU_VERSION;
+ char *tmp;
+
+ info->qemu.major = strtol(version, &tmp, 10);
+ tmp++;
+ info->qemu.minor = strtol(tmp, &tmp, 10);
+ tmp++;
+ info->qemu.micro = strtol(tmp, &tmp, 10);
+ info->package = qemu_strdup(QEMU_PKGVERSION);
+
+ return info;
+}
+
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [Qemu-devel] [PATCH 07/15] qapi: add query-version QMP command
2011-03-11 23:05 ` [Qemu-devel] [PATCH 07/15] qapi: add query-version QMP command Anthony Liguori
@ 2011-03-12 11:19 ` Blue Swirl
2011-03-12 15:06 ` Anthony Liguori
0 siblings, 1 reply; 49+ messages in thread
From: Blue Swirl @ 2011-03-12 11:19 UTC (permalink / raw)
To: Anthony Liguori
Cc: Markus Armbruster, Avi Kivity, Luiz Capitulino, qemu-devel,
Adam Litke
On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> This is used internally by QMP. It's also a pretty good example of a typical
> command conversion.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 5dae800..e1a2756 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -103,7 +103,7 @@ common-obj-y += block-migration.o
> common-obj-y += pflib.o
> common-obj-y += bitmap.o bitops.o
> common-obj-y += qmp-marshal-types.o qmp-marshal-types-core.o
> -common-obj-y += qmp-core.o qmp-marshal.o
> +common-obj-y += qmp-core.o qmp-marshal.o qmp.o
>
> common-obj-$(CONFIG_BRLAPI) += baum.o
> common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
> diff --git a/qmp-schema.json b/qmp-schema.json
> index e69de29..e72bc18 100644
> --- a/qmp-schema.json
> +++ b/qmp-schema.json
> @@ -0,0 +1,90 @@
> +# *-*- Mode: Python -*-*
> +
> +##
> +# QMP Schema
> +#
> +# This file defines the types, commands, and events used by QMP. It should
> +# fully describe the interface used by QMP.
> +#
> +# This file is designed to be loosely based on JSON although it's technical
> +# executable Python. While dictionaries are used, they are parsed as
> +# OrderedDicts so that ordering is preserved.
> +#
> +# There are two basic syntaxes used. The first syntax defines a type and is
> +# represented by a dictionary. There are three kinds of types that are
> +# supported.
> +#
> +# A complex type is a dictionary containing a single key who's value is a
> +# dictionary. This corresponds to a struct in C or an Object in JSON. An
> +# example of a complex type is:
> +#
> +# { 'type': 'MyType',
> +# 'data' { 'member1': 'str', 'member2': 'int', '*member3': 'str } }
> +#
> +# The use of '*' as a prefix to the name means the member is optional. Optional
> +# members should always be added to the end of the dictionary to preserve
> +# backwards compatibility.
> +#
> +# An enumeration type is a dictionary containing a single key who's value is a
> +# list of strings. An example enumeration is:
> +#
> +# { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
> +#
> +# A signal is similar in form to a complex type except the single key is
> +# entirely upper case instead of CamelCase. An example signal is:
> +#
> +# { 'event': 'MY_SIGNAL', 'data': { 'arg1': 'str', 'arg2': 'str' } }
> +#
> +# Generally speaking, complex types and enums should always use CamelCase for
> +# the type names.
> +#
> +# Commands are defined by using a list containing three members. The first
> +# member is the command name, the second member is a dictionary containing
> +# arguments, and the third member is the return type.
> +#
> +# An example command is:
> +#
> +# { 'command': 'my-command',
> +# 'data': { 'arg1': 'str', '*arg2': 'str' },
> +# 'returns': 'str' ]
> +#
> +# Command names should be all lower case with words separated by a hyphen.
> +
> +### 0.14.0 commands. Do not modify. ###
> +
> +##
> +# @VersionInfo:
> +#
> +# A description of QEMU's version.
> +#
> +# @qemu.major: The major version of QEMU
> +#
> +# @qemu.minor: The minor version of QEMU
> +#
> +# @qemu.micro: The micro version of QEMU. By current convention, a micro
> +# version of 50 signifies a development branch. A micro version
> +# greater than or equal to 90 signifies a release candidate for
> +# the next minor version. A micro version of less than 50
> +# signifies a stable release.
> +#
> +# @package: QEMU will always set this field to an empty string. Downstream
> +# versions of QEMU should set this to a non-empty string. The
> +# exact format depends on the downstream however it highly
> +# recommended that a unique name is used.
> +#
> +# Since: 0.14.0
> +##
> +{ 'type': 'VersionInfo',
> + 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
> + 'package': 'str'} }
> +
> +##
> +# @query-version:
> +#
> +# Returns the current version of QEMU.
> +#
> +# Returns: A @VersionInfo object describing the current version of QEMU.
> +#
> +# Since: 0.14.0
> +##
> +{ 'command': 'query-version', 'returns': 'VersionInfo' }
> diff --git a/qmp.c b/qmp.c
> new file mode 100644
> index 0000000..7b626f5
> --- /dev/null
> +++ b/qmp.c
> @@ -0,0 +1,31 @@
> +/*
> + * QAPI
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + * Anthony Liguori <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2. See
> + * the COPYING.LIB file in the top-level directory.
> + */
> +#include "qemu-common.h"
> +#include "qmp-core.h"
> +#include "qmp.h"
> +
> +VersionInfo *qmp_query_version(Error **err)
> +{
> + VersionInfo *info = qmp_alloc_version_info();
> + const char *version = QEMU_VERSION;
> + char *tmp;
> +
> + info->qemu.major = strtol(version, &tmp, 10);
Since all fields are int64_t, these should use strtoll(), otherwise
they may fail for versions like QEMU 2147483648.0.0. ;-)
> + tmp++;
> + info->qemu.minor = strtol(tmp, &tmp, 10);
> + tmp++;
> + info->qemu.micro = strtol(tmp, &tmp, 10);
> + info->package = qemu_strdup(QEMU_PKGVERSION);
> +
> + return info;
> +}
> +
> --
> 1.7.0.4
>
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [Qemu-devel] [PATCH 07/15] qapi: add query-version QMP command
2011-03-12 11:19 ` Blue Swirl
@ 2011-03-12 15:06 ` Anthony Liguori
0 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-12 15:06 UTC (permalink / raw)
To: Blue Swirl
Cc: qemu-devel, Adam Litke, Luiz Capitulino, Markus Armbruster,
Avi Kivity
On 03/12/2011 05:19 AM, Blue Swirl wrote:
> On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori<aliguori@us.ibm.com> wrote:
>> This is used internally by QMP. It's also a pretty good example of a typical
>> command conversion.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 5dae800..e1a2756 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -103,7 +103,7 @@ common-obj-y += block-migration.o
>> common-obj-y += pflib.o
>> common-obj-y += bitmap.o bitops.o
>> common-obj-y += qmp-marshal-types.o qmp-marshal-types-core.o
>> -common-obj-y += qmp-core.o qmp-marshal.o
>> +common-obj-y += qmp-core.o qmp-marshal.o qmp.o
>>
>> common-obj-$(CONFIG_BRLAPI) += baum.o
>> common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
>> diff --git a/qmp-schema.json b/qmp-schema.json
>> index e69de29..e72bc18 100644
>> --- a/qmp-schema.json
>> +++ b/qmp-schema.json
>> @@ -0,0 +1,90 @@
>> +# *-*- Mode: Python -*-*
>> +
>> +##
>> +# QMP Schema
>> +#
>> +# This file defines the types, commands, and events used by QMP. It should
>> +# fully describe the interface used by QMP.
>> +#
>> +# This file is designed to be loosely based on JSON although it's technical
>> +# executable Python. While dictionaries are used, they are parsed as
>> +# OrderedDicts so that ordering is preserved.
>> +#
>> +# There are two basic syntaxes used. The first syntax defines a type and is
>> +# represented by a dictionary. There are three kinds of types that are
>> +# supported.
>> +#
>> +# A complex type is a dictionary containing a single key who's value is a
>> +# dictionary. This corresponds to a struct in C or an Object in JSON. An
>> +# example of a complex type is:
>> +#
>> +# { 'type': 'MyType',
>> +# 'data' { 'member1': 'str', 'member2': 'int', '*member3': 'str } }
>> +#
>> +# The use of '*' as a prefix to the name means the member is optional. Optional
>> +# members should always be added to the end of the dictionary to preserve
>> +# backwards compatibility.
>> +#
>> +# An enumeration type is a dictionary containing a single key who's value is a
>> +# list of strings. An example enumeration is:
>> +#
>> +# { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
>> +#
>> +# A signal is similar in form to a complex type except the single key is
>> +# entirely upper case instead of CamelCase. An example signal is:
>> +#
>> +# { 'event': 'MY_SIGNAL', 'data': { 'arg1': 'str', 'arg2': 'str' } }
>> +#
>> +# Generally speaking, complex types and enums should always use CamelCase for
>> +# the type names.
>> +#
>> +# Commands are defined by using a list containing three members. The first
>> +# member is the command name, the second member is a dictionary containing
>> +# arguments, and the third member is the return type.
>> +#
>> +# An example command is:
>> +#
>> +# { 'command': 'my-command',
>> +# 'data': { 'arg1': 'str', '*arg2': 'str' },
>> +# 'returns': 'str' ]
>> +#
>> +# Command names should be all lower case with words separated by a hyphen.
>> +
>> +### 0.14.0 commands. Do not modify. ###
>> +
>> +##
>> +# @VersionInfo:
>> +#
>> +# A description of QEMU's version.
>> +#
>> +# @qemu.major: The major version of QEMU
>> +#
>> +# @qemu.minor: The minor version of QEMU
>> +#
>> +# @qemu.micro: The micro version of QEMU. By current convention, a micro
>> +# version of 50 signifies a development branch. A micro version
>> +# greater than or equal to 90 signifies a release candidate for
>> +# the next minor version. A micro version of less than 50
>> +# signifies a stable release.
>> +#
>> +# @package: QEMU will always set this field to an empty string. Downstream
>> +# versions of QEMU should set this to a non-empty string. The
>> +# exact format depends on the downstream however it highly
>> +# recommended that a unique name is used.
>> +#
>> +# Since: 0.14.0
>> +##
>> +{ 'type': 'VersionInfo',
>> + 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
>> + 'package': 'str'} }
>> +
>> +##
>> +# @query-version:
>> +#
>> +# Returns the current version of QEMU.
>> +#
>> +# Returns: A @VersionInfo object describing the current version of QEMU.
>> +#
>> +# Since: 0.14.0
>> +##
>> +{ 'command': 'query-version', 'returns': 'VersionInfo' }
>> diff --git a/qmp.c b/qmp.c
>> new file mode 100644
>> index 0000000..7b626f5
>> --- /dev/null
>> +++ b/qmp.c
>> @@ -0,0 +1,31 @@
>> +/*
>> + * QAPI
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + * Anthony Liguori<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2. See
>> + * the COPYING.LIB file in the top-level directory.
>> + */
>> +#include "qemu-common.h"
>> +#include "qmp-core.h"
>> +#include "qmp.h"
>> +
>> +VersionInfo *qmp_query_version(Error **err)
>> +{
>> + VersionInfo *info = qmp_alloc_version_info();
>> + const char *version = QEMU_VERSION;
>> + char *tmp;
>> +
>> + info->qemu.major = strtol(version,&tmp, 10);
> Since all fields are int64_t, these should use strtoll(), otherwise
> they may fail for versions like QEMU 2147483648.0.0. ;-)
The fields are int64_t because that's what JSON supports. I considered
using more precise types and doing the conversions with range checking
as part of the marshaling. That way, we'd still send int64_t over the
wire, but the QEMU and libqmp interfaces could take u64, u32, s16, or
whatever.
I think this is probably a good change to make before we get much
further down this path because it will be much harder once we merge.
The only down side to doing this is that non-smart clients don't get the
benefit of the type conversions. I don't know that that's a serious
problem though.
Regards,
Anthony Liguori
>> + tmp++;
>> + info->qemu.minor = strtol(tmp,&tmp, 10);
>> + tmp++;
>> + info->qemu.micro = strtol(tmp,&tmp, 10);
>> + info->package = qemu_strdup(QEMU_PKGVERSION);
>> +
>> + return info;
>> +}
>> +
>> --
>> 1.7.0.4
>>
>>
>>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH 08/15] qapi: add new QMP server that uses CharDriverState (v2)
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (6 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 07/15] qapi: add query-version QMP command Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 09/15] vl: add a new -qmp2 option to expose experimental QMP server Anthony Liguori
` (7 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This will replace the current QMP server once all the functions are implemented.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
- support for get_fd
- support for async commands
- free request object on error path
diff --git a/qmp-core.c b/qmp-core.c
index 7f60942..22b413b 100644
--- a/qmp-core.c
+++ b/qmp-core.c
@@ -47,8 +47,18 @@ struct QmpState
int (*add_connection)(QmpState *s, QmpConnection *conn);
void (*del_connection)(QmpState *s, int global_handle, Error **errp);
void (*event)(QmpState *s, QObject *data);
+ int (*get_fd)(QmpState *s);
};
+typedef struct QmpSession
+{
+ JSONMessageParser parser;
+ QmpState state;
+ CharDriverState *chr;
+ int max_global_handle;
+ QTAILQ_HEAD(, QmpConnection) connections;
+} QmpSession;
+
static QTAILQ_HEAD(, QmpCommand) qmp_commands =
QTAILQ_HEAD_INITIALIZER(qmp_commands);
@@ -128,11 +138,16 @@ void qmp_state_add_connection(QmpState *sess, const char *event_name, QmpSignal
conn->global_handle = sess->add_connection(sess, conn);
}
-void qmp_put_event(QmpState *sess, int global_handle, Error **errp)
+void qmp_put_event(QmpState *sess, int64_t global_handle, Error **errp)
{
sess->del_connection(sess, global_handle, errp);
}
+int qmp_state_get_fd(QmpState *sess)
+{
+ return sess->get_fd(sess);
+}
+
void qmp_state_event(QmpConnection *conn, QObject *data)
{
QDict *event = qdict_new();
@@ -205,3 +220,230 @@ void qmp_signal_disconnect(QmpSignal *obj, int handle)
}
}
}
+
+static QObject *qmp_dispatch_err(QmpState *state, QList *tokens, Error **errp)
+{
+ const char *command;
+ QDict *args, *dict;
+ QObject *request;
+ QmpCommand *cmd;
+ QObject *ret = NULL;
+ Error *err = NULL;
+
+ request = json_parser_parse_err(tokens, NULL, &err);
+ if (request == NULL) {
+ if (err == NULL) {
+ error_set(errp, QERR_JSON_PARSE_ERROR, "no valid JSON object");
+ } else {
+ error_propagate(errp, err);
+ }
+ goto out;
+ }
+ if (qobject_type(request) != QTYPE_QDICT) {
+ error_set(errp, QERR_JSON_PARSE_ERROR, "request is not a dictionary");
+ goto out;
+ }
+
+ dict = qobject_to_qdict(request);
+ if (!qdict_haskey(dict, "execute")) {
+ error_set(errp, QERR_JSON_PARSE_ERROR, "no execute key");
+ goto out;
+ }
+
+ command = qdict_get_str(dict, "execute");
+ cmd = qmp_find_command(command);
+ if (cmd == NULL) {
+ error_set(errp, QERR_COMMAND_NOT_FOUND, command);
+ goto out;
+ }
+
+ if (!qdict_haskey(dict, "arguments")) {
+ args = qdict_new();
+ } else {
+ args = qdict_get_qdict(dict, "arguments");
+ QINCREF(args);
+ }
+
+ switch (cmd->type) {
+ case QCT_NORMAL:
+ cmd->fn(args, &ret, errp);
+ if (ret == NULL) {
+ ret = QOBJECT(qdict_new());
+ }
+ break;
+ case QCT_STATEFUL:
+ cmd->sfn(state, args, &ret, errp);
+ if (ret == NULL) {
+ ret = QOBJECT(qdict_new());
+ }
+ break;
+ case QCT_ASYNC: {
+ QmpCommandState *s = qemu_mallocz(sizeof(*s));
+ // FIXME save async commands and do something
+ // smart if disconnect occurs before completion
+ s->state = state;
+ s->tag = NULL;
+ if (qdict_haskey(dict, "tag")) {
+ s->tag = qdict_get(dict, "tag");
+ qobject_incref(s->tag);
+ }
+ cmd->afn(args, errp, s);
+ ret = NULL;
+ }
+ break;
+ }
+
+ QDECREF(args);
+
+out:
+ qobject_decref(request);
+
+ return ret;
+}
+
+static QObject *qmp_dispatch(QmpState *state, QList *tokens)
+{
+ Error *err = NULL;
+ QObject *ret;
+ QDict *rsp;
+
+ ret = qmp_dispatch_err(state, tokens, &err);
+
+ rsp = qdict_new();
+ if (err) {
+ qdict_put_obj(rsp, "error", error_get_qobject(err));
+ error_free(err);
+ } else if (ret) {
+ qdict_put_obj(rsp, "return", ret);
+ } else {
+ QDECREF(rsp);
+ return NULL;
+ }
+
+ return QOBJECT(rsp);
+}
+
+static void qmp_chr_parse(JSONMessageParser *parser, QList *tokens)
+{
+ QmpSession *s = container_of(parser, QmpSession, parser);
+ QObject *rsp;
+ QString *str;
+
+ rsp = qmp_dispatch(&s->state, tokens);
+
+ if (rsp) {
+ str = qobject_to_json(rsp);
+ qemu_chr_write(s->chr, (void *)str->string, str->length);
+ qemu_chr_write(s->chr, (void *)"\n", 1);
+
+ QDECREF(str);
+ qobject_decref(rsp);
+ }
+}
+
+static int qmp_chr_can_receive(void *opaque)
+{
+ return 1024;
+}
+
+static void qmp_chr_receive(void *opaque, const uint8_t *buf, int size)
+{
+ QmpSession *s = opaque;
+ json_message_parser_feed(&s->parser, (char *)buf, size);
+}
+
+static void qmp_chr_send_greeting(QmpSession *s)
+{
+ VersionInfo *info;
+ QObject *vers;
+ QObject *greeting;
+ QString *str;
+
+ info = qmp_query_version(NULL);
+ vers = qmp_marshal_type_VersionInfo(info);
+ qmp_free_version_info(info);
+
+ greeting = qobject_from_jsonf("{'QMP': {'version': %p, 'capabilities': []} }",
+ vers);
+ str = qobject_to_json(greeting);
+ qobject_decref(greeting);
+
+ qemu_chr_write(s->chr, (void *)str->string, str->length);
+ qemu_chr_write(s->chr, (void *)"\n", 1);
+ QDECREF(str);
+}
+
+static void qmp_chr_event(void *opaque, int event)
+{
+ QmpSession *s = opaque;
+ switch (event) {
+ case CHR_EVENT_OPENED:
+ // FIXME disconnect any connected signals including defaults
+ json_message_parser_init(&s->parser, qmp_chr_parse);
+ qmp_chr_send_greeting(s);
+ break;
+ case CHR_EVENT_CLOSED:
+ json_message_parser_flush(&s->parser);
+ break;
+ }
+}
+
+static int qmp_chr_add_connection(QmpState *state, QmpConnection *conn)
+{
+ QmpSession *s = container_of(state, QmpSession, state);
+
+ QTAILQ_INSERT_TAIL(&s->connections, conn, node);
+ return ++s->max_global_handle;
+}
+
+static void qmp_chr_send_event(QmpState *state, QObject *event)
+{
+ QmpSession *s = container_of(state, QmpSession, state);
+ QString *str;
+
+ str = qobject_to_json(event);
+ qemu_chr_write(s->chr, (void *)str->string, str->length);
+ qemu_chr_write(s->chr, (void *)"\n", 1);
+ QDECREF(str);
+}
+
+static void qmp_chr_del_connection(QmpState *state, int global_handle, Error **errp)
+{
+ QmpSession *s = container_of(state, QmpSession, state);
+ QmpConnection *conn;
+
+ QTAILQ_FOREACH(conn, &s->connections, node) {
+ if (conn->global_handle == global_handle) {
+ qmp_signal_disconnect(conn->signal, conn->handle);
+ QTAILQ_REMOVE(&s->connections, conn, node);
+ qemu_free(conn);
+ return;
+ }
+ }
+
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, "tag", "valid event handle");
+}
+
+static int qmp_chr_get_fd(QmpState *state)
+{
+ QmpSession *s = container_of(state, QmpSession, state);
+
+ return qemu_chr_get_msgfd(s->chr);
+}
+
+void qmp_init_chardev(CharDriverState *chr)
+{
+ QmpSession *s = qemu_mallocz(sizeof(*s));
+
+ s->chr = chr;
+ s->state.add_connection = qmp_chr_add_connection;
+ s->state.event = qmp_chr_send_event;
+ s->state.del_connection = qmp_chr_del_connection;
+ s->state.get_fd = qmp_chr_get_fd;
+
+ s->max_global_handle = 0;
+ QTAILQ_INIT(&s->connections);
+
+ qemu_chr_add_handlers(chr, qmp_chr_can_receive, qmp_chr_receive,
+ qmp_chr_event, s);
+}
diff --git a/qmp-core.h b/qmp-core.h
index c9c8b63..837ca07 100644
--- a/qmp-core.h
+++ b/qmp-core.h
@@ -60,9 +60,10 @@ int qmp_signal_connect(QmpSignal *obj, void *func, void *opaque);
void qmp_signal_disconnect(QmpSignal *obj, int handle);
void qmp_state_add_connection(QmpState *sess, const char *name, QmpSignal *obj, int handle, QmpConnection *conn);
-void qmp_put_event(QmpState *sess, int global_handle, Error **errp);
void qmp_state_event(QmpConnection *conn, QObject *data);
+int qmp_state_get_fd(QmpState *sess);
+
#define signal_init(obj) do { \
(obj)->signal = qmp_signal_init(); \
} while (0)
@@ -83,4 +84,6 @@ void qmp_state_event(QmpConnection *conn, QObject *data);
} \
} while(0)
+void qmp_init_chardev(CharDriverState *chr);
+
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] [PATCH 09/15] vl: add a new -qmp2 option to expose experimental QMP server
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (7 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 08/15] qapi: add new QMP server that uses CharDriverState (v2) Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:14 ` [Qemu-devel] " Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 10/15] qapi: add QMP quit command Anthony Liguori
` (6 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This is temporary until we implement all QMP commands.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/qemu-options.hx b/qemu-options.hx
index badb730..957d935 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1881,6 +1881,9 @@ serial port).
The default device is @code{vc} in graphical mode and @code{stdio} in
non graphical mode.
ETEXI
+DEF("qmp2", HAS_ARG, QEMU_OPTION_qmp2, \
+ "-qmp2 chardev experimental QMP server\n",
+ QEMU_ARCH_ALL)
DEF("qmp", HAS_ARG, QEMU_OPTION_qmp, \
"-qmp dev like -monitor but opens in 'control' mode\n",
QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index d27e750..1d1d536 100644
--- a/vl.c
+++ b/vl.c
@@ -160,6 +160,7 @@ int main(int argc, char **argv)
#include "qemu-queue.h"
#include "cpus.h"
#include "arch_init.h"
+#include "qmp-core.h"
#include "ui/qemu-spice.h"
@@ -1915,6 +1916,8 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
return popt;
}
+#define MAX_QMP_CHARDEVS 16
+
int main(int argc, char **argv, char **envp)
{
const char *gdbstub_dev = NULL;
@@ -1940,6 +1943,8 @@ int main(int argc, char **argv, char **envp)
int show_vnc_port = 0;
int defconfig = 1;
const char *trace_file = NULL;
+ int nb_qmp_chardevs = 0;
+ const char *qmp_chardevs[MAX_QMP_CHARDEVS];
atexit(qemu_run_exit_notifiers);
error_set_progname(argv[0]);
@@ -2387,6 +2392,13 @@ int main(int argc, char **argv, char **envp)
monitor_parse(optarg, "control");
default_monitor = 0;
break;
+ case QEMU_OPTION_qmp2:
+ if (nb_qmp_chardevs == MAX_QMP_CHARDEVS) {
+ fprintf(stderr, "-qmp: too many QMP chardevs\n");
+ exit(1);
+ }
+ qmp_chardevs[nb_qmp_chardevs++] = optarg;
+ break;
case QEMU_OPTION_mon:
opts = qemu_opts_parse(qemu_find_opts("mon"), optarg, 1);
if (!opts) {
@@ -3084,6 +3096,15 @@ int main(int argc, char **argv, char **envp)
}
#endif
+ for (i = 0; i < nb_qmp_chardevs; i++) {
+ CharDriverState *chr = qemu_chr_find(qmp_chardevs[i]);
+ if (chr == NULL) {
+ fprintf(stderr, "-qmp: unknown chardev `%s'\n", qmp_chardevs[i]);
+ exit(1);
+ }
+ qmp_init_chardev(chr);
+ }
+
/* display setup */
dpy_resize(ds);
dcl = ds->listeners;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] [PATCH 10/15] qapi: add QMP quit command
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (8 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 09/15] vl: add a new -qmp2 option to expose experimental QMP server Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 11/15] qapi: add QMP qmp_capabilities command Anthony Liguori
` (5 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This is needed by the test suite.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/qmp-schema.json b/qmp-schema.json
index e72bc18..d19cf73 100644
--- a/qmp-schema.json
+++ b/qmp-schema.json
@@ -88,3 +88,15 @@
# Since: 0.14.0
##
{ 'command': 'query-version', 'returns': 'VersionInfo' }
+
+##
+# @quit:
+#
+# This command will cause the QEMU process to exit gracefully. While every
+# attempt is made to send the QMP response before terminating, this is not
+# guaranteed. When using this interface, a premature EOF would not be
+# unexpected.
+#
+# Since: 0.14.0
+##
+{ 'command': 'quit' }
diff --git a/qmp.c b/qmp.c
index 7b626f5..837ac95 100644
--- a/qmp.c
+++ b/qmp.c
@@ -12,6 +12,7 @@
#include "qemu-common.h"
#include "qmp-core.h"
#include "qmp.h"
+#include "sysemu.h"
VersionInfo *qmp_query_version(Error **err)
{
@@ -29,3 +30,8 @@ VersionInfo *qmp_query_version(Error **err)
return info;
}
+void qmp_quit(Error **err)
+{
+ no_shutdown = 0;
+ qemu_system_shutdown_request();
+}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] [PATCH 11/15] qapi: add QMP qmp_capabilities command
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (9 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 10/15] qapi: add QMP quit command Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 12/15] qapi: add QMP put-event command Anthony Liguori
` (4 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
For now, it's a nop. In the near future, it will be used to register default
signals.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/qmp-core.c b/qmp-core.c
index 22b413b..ccc2b7d 100644
--- a/qmp-core.c
+++ b/qmp-core.c
@@ -447,3 +447,7 @@ void qmp_init_chardev(CharDriverState *chr)
qemu_chr_add_handlers(chr, qmp_chr_can_receive, qmp_chr_receive,
qmp_chr_event, s);
}
+
+void qmp_qmp_capabilities(QmpState *state, Error **errp)
+{
+}
diff --git a/qmp-schema.json b/qmp-schema.json
index d19cf73..e0789d0 100644
--- a/qmp-schema.json
+++ b/qmp-schema.json
@@ -100,3 +100,13 @@
# Since: 0.14.0
##
{ 'command': 'quit' }
+
+##
+# @qmp_capabilities:
+#
+# Currently a nop command. To communicate with older servers, this should be
+# sent first before executing new commands.
+#
+# Since: 0.14.0
+##
+{ 'command': 'qmp_capabilities' }
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] [PATCH 12/15] qapi: add QMP put-event command
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (10 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 11/15] qapi: add QMP qmp_capabilities command Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 13/15] qapi: add code generator for libqmp (v2) Anthony Liguori
` (3 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This is needed for libqmp to support events. put-event is used to disconnect
from signals.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/qmp-schema.json b/qmp-schema.json
index e0789d0..17db1cb 100644
--- a/qmp-schema.json
+++ b/qmp-schema.json
@@ -110,3 +110,18 @@
# Since: 0.14.0
##
{ 'command': 'qmp_capabilities' }
+
+##
+# @put_event:
+#
+# Disconnect a signal. This command is used to disconnect from a signal based
+# on the handle returned by a signal accessor.
+#
+# @tag: the handle returned by a signal accessor.
+#
+# Returns: Nothing on success.
+# If @tag is not a valid handle, InvalidParameterValue
+#
+# Since: 0.15.0
+##
+{ 'command': 'put-event', 'data': {'tag': 'int'} }
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] [PATCH 13/15] qapi: add code generator for libqmp (v2)
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (11 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 12/15] qapi: add QMP put-event command Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-12 11:10 ` Blue Swirl
2011-03-11 23:05 ` [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp Anthony Liguori
` (2 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
- update code generator to use multiline
- proxy command support
- async command support
diff --git a/Makefile b/Makefile
index 47a755d..5170675 100644
--- a/Makefile
+++ b/Makefile
@@ -4,7 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
ifeq ($(TRACE_BACKEND),dtrace)
GENERATED_HEADERS += trace-dtrace.h
endif
-GENERATED_HEADERS += qmp-types.h qmp-marshal-types.h qmp.h
+GENERATED_HEADERS += qmp-types.h qmp-marshal-types.h qmp.h libqmp.h
ifneq ($(wildcard config-host.mak),)
# Put the all: rule here so that config-host.mak can contain dependencies.
@@ -165,9 +165,16 @@ qmp.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
qmp-marshal.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
$(call quiet-command,python $(SRC_PATH)/qmp-gen.py --body < $< > $@, " GEN $@")
+libqmp.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
+ $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --lib-header < $< > $@, " GEN $@")
+
+libqmp.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
+ $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --lib-body < $< > $@, " GEN $@")
+
qmp-types.o: qmp-types.c qmp-types.h
qmp-marshal-types.o: qmp-marshal-types.c qmp-marshal-types.h qmp-types.h
qmp-marshal.o: qmp-marshal.c qmp.h qmp-types.h qmp-marshal-types.h
+libqmp.o: libqmp.c libqmp.h qmp-types.h
version.o: $(SRC_PATH)/version.rc config-host.mak
$(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
diff --git a/libqmp-core.c b/libqmp-core.c
new file mode 100644
index 0000000..4613d4f
--- /dev/null
+++ b/libqmp-core.c
@@ -0,0 +1,361 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#include "libqmp.h"
+#include "libqmp-internal.h"
+#include "libqmp-core.h"
+#include "json-streamer.h"
+#include "json-parser.h"
+#include "dirent.h"
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <assert.h>
+
+#ifndef container_of
+#define offset_of(type, field) \
+ ((unsigned long)(&((type *)0)->field))
+#define container_of(obj, type, field) \
+ ((type *)(((char *)obj) - offsetof(type, field)))
+#endif
+
+//#define DEBUG_LIBQMP 1
+
+typedef struct FdQmpSession
+{
+ QmpSession session;
+ JSONMessageParser parser;
+ QObject *result;
+ bool got_greeting;
+ int fd;
+ int event_count;
+} FdQmpSession;
+
+static EventTrampolineFunc *get_event_trampoline(QmpSession *sess, const char *name)
+{
+ QmpEventTrampoline *t;
+
+ QTAILQ_FOREACH(t, &sess->events, node) {
+ if (strcmp(t->name, name) == 0) {
+ return t->dispatch;
+ }
+ }
+
+ return NULL;
+}
+
+static void fd_qmp_session_process_event(FdQmpSession *fs, QDict *response)
+{
+ EventTrampolineFunc *tramp;
+ QmpSignal *signal;
+ const char *event;
+ int tag;
+
+ event = qdict_get_str(response, "event");
+ tramp = get_event_trampoline(&fs->session, event);
+
+ fs->event_count++;
+
+ if (tramp && qdict_haskey(response, "tag")) {
+ tag = qdict_get_int(response, "tag");
+
+ QTAILQ_FOREACH(signal, &fs->session.signals, node) {
+ if (signal->global_handle == tag) {
+ QmpConnection *conn;
+ QDict *args = NULL;
+ Error *err = NULL;
+
+ if (qdict_haskey(response, "data")) {
+ args = qobject_to_qdict(qdict_get(response, "data"));
+ }
+
+ QTAILQ_FOREACH(conn, &signal->connections, node) {
+ tramp(args, conn->fn, conn->opaque, &err);
+ if (err) {
+ error_free(err);
+ }
+ }
+
+ break;
+ }
+ }
+ }
+}
+
+static void fd_qmp_session_parse(JSONMessageParser *parser, QList *tokens)
+{
+ FdQmpSession *fs = container_of(parser, FdQmpSession, parser);
+ QObject *result;
+
+ result = json_parser_parse(tokens, NULL);
+ if (!fs->got_greeting) {
+ fs->got_greeting = true;
+ qobject_decref(result);
+ } else {
+ QDict *response = qobject_to_qdict(result);
+ if (qdict_haskey(response, "event")) {
+ fd_qmp_session_process_event(fs, response);
+ qobject_decref(result);
+ } else {
+ qobject_decref(fs->result);
+ fs->result = result;
+ }
+ }
+}
+
+static QDict *fd_qmp_session_read(FdQmpSession *fs)
+{
+ QDict *response;
+
+ assert(fs->result == NULL);
+ fs->result = NULL;
+ while (!fs->result) {
+ char buffer[1024];
+ ssize_t len;
+
+ len = read(fs->fd, buffer, sizeof(buffer));
+ if (len == -1 && errno == EINTR) {
+ continue;
+ }
+ if (len < 1) {
+ abort();
+ }
+
+#if defined(DEBUG_LIBQMP)
+ fwrite(buffer, len, 1, stdout);
+ fflush(stdout);
+#endif
+ json_message_parser_feed(&fs->parser, buffer, len);
+ }
+
+ response = qobject_to_qdict(fs->result);
+ fs->result = NULL;
+
+ return response;
+}
+
+static bool qmp_session_fd_wait_event(QmpSession *s, struct timeval *tv)
+{
+ FdQmpSession *fs = (FdQmpSession *)s;
+ fd_set readfds;
+ int ret;
+
+ FD_ZERO(&readfds);
+ FD_SET(fs->fd, &readfds);
+
+ do {
+ ret = select(fs->fd + 1, &readfds, NULL, NULL, tv);
+ } while (ret == -1 && errno == EINTR);
+
+ if (ret) {
+ char buffer[1024];
+ ssize_t len;
+ int saved_event_count;
+
+ do {
+ len = read(fs->fd, buffer, sizeof(buffer));
+ } while (len == -1 && errno == EINTR);
+
+ if (len < 1) {
+ abort();
+ }
+
+#if defined(DEBUG_LIBQMP)
+ fwrite(buffer, len, 1, stdout);
+ fflush(stdout);
+#endif
+ saved_event_count = fs->event_count;
+ json_message_parser_feed(&fs->parser, buffer, len);
+ return (saved_event_count != fs->event_count);
+ }
+
+ return false;
+}
+
+static QObject *qmp_session_fd_dispatch(QmpSession *s, const char *name,
+ QDict *args, Error **err)
+{
+ FdQmpSession *fs = (FdQmpSession *)s;
+ QString *str;
+ const char *buffer;
+ size_t size;
+ size_t offset;
+ QDict *request = qdict_new();
+ QDict *response;
+
+ qdict_put(request, "execute", qstring_from_str(name));
+
+ if (qdict_size(args)) {
+ QINCREF(args);
+ qdict_put(request, "arguments", args);
+ }
+
+ str = qobject_to_json(QOBJECT(request));
+ buffer = qstring_get_str(str);
+ size = str->length;
+
+ offset = 0;
+ while (offset < size) {
+ ssize_t len;
+
+ len = write(fs->fd, buffer + offset, size - offset);
+ if (len == -1 && errno == EINTR) {
+ continue;
+ }
+ if (len < 1) {
+ abort();
+ }
+
+#if defined(DEBUG_LIBQMP)
+ fwrite(buffer + offset, size - offset, 1, stdout);
+ fflush(stdout);
+#endif
+ offset += len;
+ }
+
+ QDECREF(str);
+ QDECREF(request);
+
+ response = fd_qmp_session_read(fs);
+
+ if (qdict_haskey(response, "error")) {
+ error_set_qobject(err, qdict_get(response, "error"));
+ QDECREF(response);
+ return NULL;
+ } else {
+ QObject *result = qdict_get(response, "return");
+ qobject_incref(result);
+ QDECREF(response);
+ return result;
+ }
+}
+
+void libqmp_register_event(QmpSession *sess, const char *name, EventTrampolineFunc *func)
+{
+ QmpEventTrampoline *t = qemu_mallocz(sizeof(*t));
+ t->name = name;
+ t->dispatch = func;
+ QTAILQ_INSERT_TAIL(&sess->events, t, node);
+}
+
+QmpSession *qmp_session_new(int fd)
+{
+ FdQmpSession *s = qemu_mallocz(sizeof(*s));
+
+ s->fd = fd;
+ s->session.dispatch = qmp_session_fd_dispatch;
+ s->session.wait_event = qmp_session_fd_wait_event;
+ s->got_greeting = false;
+
+ QTAILQ_INIT(&s->session.events);
+ QTAILQ_INIT(&s->session.signals);
+ json_message_parser_init(&s->parser, fd_qmp_session_parse);
+
+ libqmp_init_events(&s->session);
+ libqmp_qmp_capabilities(&s->session, NULL);
+
+ return &s->session;
+}
+
+void qmp_session_destroy(QmpSession *s)
+{
+ FdQmpSession *fs = container_of(s, FdQmpSession, session);
+
+ while (!QTAILQ_EMPTY(&s->events)) {
+ QmpEventTrampoline *t = QTAILQ_FIRST(&s->events);
+ QTAILQ_REMOVE(&s->events, t, node);
+ qemu_free(t);
+ }
+ if (fs->result) {
+ qobject_decref(fs->result);
+ fs->result = NULL;
+ }
+ json_message_parser_destroy(&fs->parser);
+ close(fs->fd);
+ qemu_free(fs);
+}
+
+typedef struct GenericSignal
+{
+ QmpSignal *signal;
+} GenericSignal;
+
+void *libqmp_signal_new(QmpSession *s, size_t size, int global_handle)
+{
+ GenericSignal *obj;
+ obj = qemu_mallocz(size);
+ obj->signal = qemu_mallocz(sizeof(QmpSignal));
+ obj->signal->sess = s;
+ obj->signal->global_handle = global_handle;
+ // FIXME validate there isn't another global_handle
+ QTAILQ_INIT(&obj->signal->connections);
+ QTAILQ_INSERT_TAIL(&s->signals, obj->signal, node);
+ return obj;
+}
+
+int libqmp_signal_connect(QmpSignal *obj, void *func, void *opaque)
+{
+ QmpConnection *conn;
+
+ conn = qemu_mallocz(sizeof(*conn));
+ conn->fn = func;
+ conn->opaque = opaque;
+ conn->handle = ++obj->max_handle;
+ QTAILQ_INSERT_TAIL(&obj->connections, conn, node);
+ return conn->handle;
+}
+
+void libqmp_signal_disconnect(QmpSignal *obj, int handle)
+{
+ QmpConnection *conn;
+
+ QTAILQ_FOREACH(conn, &obj->connections, node) {
+ if (conn->handle == handle) {
+ break;
+ }
+ }
+ if (conn) {
+ QTAILQ_REMOVE(&obj->connections, conn, node);
+ qemu_free(conn);
+ }
+}
+
+void libqmp_signal_free(void *base, QmpSignal *obj)
+{
+ QTAILQ_REMOVE(&obj->sess->signals, obj, node);
+
+ libqmp_put_event(obj->sess, obj->global_handle, NULL);
+ while (!QTAILQ_EMPTY(&obj->connections)) {
+ QmpConnection *conn = QTAILQ_FIRST(&obj->connections);
+ QTAILQ_REMOVE(&obj->connections, conn, node);
+ qemu_free(conn);
+ }
+ qemu_free(obj);
+ qemu_free(base);
+}
+
+bool libqmp_wait_event(QmpSession *s, struct timeval *tv)
+{
+ return s->wait_event(s, tv);
+}
+
+bool libqmp_poll_event(QmpSession *s)
+{
+ struct timeval tv = { 0, 0 };
+ bool got_event = false;
+ bool ret;
+
+ do {
+ ret = libqmp_wait_event(s, &tv);
+ got_event |= ret;
+ } while (ret);
+
+ return got_event;
+}
diff --git a/libqmp-core.h b/libqmp-core.h
new file mode 100644
index 0000000..c1063e6
--- /dev/null
+++ b/libqmp-core.h
@@ -0,0 +1,44 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#ifndef LIBQMP_CORE_H
+#define LIBQMP_CORE_H
+
+#include <sys/types.h>
+#include "qmp-types.h"
+#include "error.h"
+
+typedef struct QmpSession QmpSession;
+
+QmpSession *qmp_session_new(int fd);
+void qmp_session_destroy(QmpSession *s);
+
+bool libqmp_poll_event(QmpSession *s);
+bool libqmp_wait_event(QmpSession *s, struct timeval *tv);
+
+void *libqmp_signal_new(QmpSession *s, size_t size, int global_handle);
+int libqmp_signal_connect(QmpSignal *obj, void *func, void *opaque);
+void libqmp_signal_disconnect(QmpSignal *obj, int handle);
+void libqmp_signal_free(void *base, QmpSignal *obj);
+
+#define libqmp_signal_init(s, type, global_handle) \
+ ((type *)libqmp_signal_new(s, sizeof(type), global_handle))
+
+#define signal_connect(obj, fn, opaque) \
+ libqmp_signal_connect((obj)->signal, (obj)->func = fn, opaque)
+
+#define signal_disconnect(obj, handle) \
+ libqmp_signal_disconnect((obj)->signal, handle)
+
+#define signal_unref(obj) \
+ libqmp_signal_free((obj), (obj)->signal)
+
+#endif
diff --git a/libqmp-internal.h b/libqmp-internal.h
new file mode 100644
index 0000000..01a3dd8
--- /dev/null
+++ b/libqmp-internal.h
@@ -0,0 +1,56 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#ifndef LIBQMP_INTERNAL_H
+#define LIBQMP_INTERNAL_H
+
+#include "qemu-objects.h"
+#include "qmp-marshal-types.h"
+#include "error_int.h"
+
+typedef void (EventTrampolineFunc)(QDict *qmp__args, void *qmp__fn, void *qmp__opaque, Error **qmp__errp);
+
+typedef struct QmpEventTrampoline
+{
+ const char *name;
+ EventTrampolineFunc *dispatch;
+ QTAILQ_ENTRY(QmpEventTrampoline) node;
+} QmpEventTrampoline;
+
+typedef struct QmpConnection
+{
+ void *fn;
+ void *opaque;
+ int handle;
+ QTAILQ_ENTRY(QmpConnection) node;
+} QmpConnection;
+
+struct QmpSignal
+{
+ QmpSession *sess;
+ int global_handle;
+ int max_handle;
+ QTAILQ_HEAD(, QmpConnection) connections;
+ QTAILQ_ENTRY(QmpSignal) node;
+};
+
+struct QmpSession
+{
+ QObject *(*dispatch)(QmpSession *session, const char *name, QDict *args, Error **err);
+ bool (*wait_event)(QmpSession *session, struct timeval *tv);
+ QTAILQ_HEAD(, QmpEventTrampoline) events;
+ QTAILQ_HEAD(, QmpSignal) signals;
+};
+
+void libqmp_init_events(QmpSession *sess);
+void libqmp_register_event(QmpSession *sess, const char *name, EventTrampolineFunc *func);
+
+#endif
diff --git a/qmp-gen.py b/qmp-gen.py
index f1bdb2f..35f64cf 100644
--- a/qmp-gen.py
+++ b/qmp-gen.py
@@ -925,6 +925,321 @@ qmp__out:
return ret
+
+def gen_lib_decl(name, options, retval, suffix='', proxy=False, async=False):
+ if proxy:
+ async=True
+
+ if proxy:
+ args = []
+ else:
+ args = ['QmpSession *qmp__session']
+ for argname, argtype, optional in parse_args(options):
+ if argtype == '**':
+ args.append('KeyValues * %s' % c_var(argname))
+ else:
+ if optional:
+ args.append('bool has_%s' % c_var(argname))
+ args.append('%s %s' % (qmp_type_to_c(argtype), c_var(argname)))
+
+ args.append('Error **qmp__err')
+
+ if proxy:
+ prefix = 'qmp'
+ else:
+ prefix = 'libqmp'
+
+ if proxy:
+ qmp_retval = 'void'
+ args.append('%sCompletionFunc *qmp__cc' % camel_case(name))
+ args.append('void *qmp__opaque')
+ else:
+ qmp_retval = qmp_type_to_c(retval, True)
+
+ return mcgen('''
+%(ret)s %(prefix)s_%(name)s(%(args)s)%(suffix)s
+''', ret=qmp_retval, prefix=prefix, name=c_var(name), args=', '.join(args), suffix=suffix)
+
+def gen_lib_declaration(name, options, retval):
+ return gen_lib_decl(name, options, retval, ';')
+
+def gen_lib_event_definition(name, typeinfo):
+ args = ''
+ for argname, argtype, optional in parse_args(typeinfo):
+ if optional:
+ args += cgen(' bool has_%(name)s;', name=c_var(argname))
+ args += cgen(' %(type)s %(name)s = 0;', type=qmp_type_to_c(argtype, True), name=c_var(argname))
+
+ ret = mcgen('''
+
+static void libqmp_notify_%(fn_name)s(QDict *qmp__args, void *qmp__fn, void *qmp__opaque, Error **qmp__errp)
+{
+ %(fn_ret)s *qmp__native_fn = qmp__fn;
+ Error *qmp__err = NULL;
+%(args)s
+
+ (void)qmp__err;
+''',
+ fn_name=de_camel_case(qmp_event_to_c(name)),
+ fn_ret=qmp_event_func_to_c(name), args=args)
+
+ for argname, argtype, optional in parse_args(typeinfo):
+ if optional:
+ ret += cgen(' BUILD_BUG()')
+ ret += mcgen('''
+
+ if (!qdict_haskey(qmp__args, "%(name)s")) {
+ error_set(qmp__errp, QERR_MISSING_PARAMETER, "%(name)s");
+ goto qmp__out;
+ }
+
+ %(c_name)s = %(unmarshal)s(qdict_get(qmp__args, "%(name)s"), &qmp__err);
+ if (qmp__err) {
+ if (error_is_type(qmp__err, QERR_INVALID_PARAMETER_TYPE)) {
+ error_set(qmp__errp, QERR_INVALID_PARAMETER_TYPE, "%(name)s",
+ error_get_field(qmp__err, "expected"));
+ error_free(qmp__err);
+ qmp__err = NULL;
+ } else {
+ error_propagate(qmp__errp, qmp__err);
+ }
+ goto qmp__out;
+ }
+''', name=argname, c_name=c_var(argname), unmarshal=qmp_type_from_qobj(argtype))
+
+ arglist = ['qmp__opaque']
+ for argname, argtype, optional in parse_args(typeinfo):
+ arglist.append(c_var(argname))
+ ret += mcgen('''
+
+ qmp__native_fn(%(args)s);
+''', args=', '.join(arglist))
+
+ has_label = False
+ for argname, argtype, optional in parse_args(typeinfo):
+ if not has_label:
+ ret += mcgen('''
+qmp__out:
+''')
+ has_label = True
+
+ if qmp_type_should_free(argtype):
+ ret += cgen(' %(free)s(%(name)s);', free=qmp_free_func(argtype), name=c_var(argname))
+ ret += mcgen('''
+ return;
+}
+''')
+ return ret
+
+def gen_async_lib_definition(name, options, retval):
+ ret = mcgen('''
+
+typedef struct %(cc_name)sCompletionCB
+{
+ %(cc_name)sCompletionFunc *cb;
+ void *opaque;
+} %(cc_name)sCompletionCB;
+
+static void qmp_%(c_name)s_cb(void *qmp__opaque, QObject *qmp__retval, Error *qmp__err)
+{
+ %(cc_name)sCompletionCB *qmp__cb = qmp__opaque;
+''',
+ cc_name=camel_case(name), c_name=c_var(name))
+
+ if retval != 'none':
+ ret += cgen(' %(ret_type)s qmp__native_retval = 0;',
+ ret_type=qmp_type_to_c(retval, True))
+
+ if type(retval) == list:
+ ret += mcgen('''
+
+ if (!qmp__err) {
+ QList *qmp__list_retval = qobject_to_qlist(qmp__retval);
+ QListEntry *qmp__i;
+ QLIST_FOREACH_ENTRY(qmp__list_retval, qmp__i) {
+ %(ret_type)s qmp__native_i = %(unmarshal)s(qmp__i->value, &qmp__err);
+ if (qmp__err) {
+ %(free)s(qmp__native_retval);
+ break;
+ }
+ qmp__native_i->next = qmp__native_retval;
+ qmp__native_retval = qmp__native_i;
+ }
+ }
+''',
+ ret_type=qmp_type_to_c(retval[0], True),
+ unmarshal=qmp_type_from_qobj(retval[0]),
+ free=qmp_free_func(retval[0]))
+ elif is_dict(retval):
+ ret += mcgen('''
+ // FIXME (using an anonymous dict as return value)')
+ BUILD_BUG();
+''')
+ elif retval != 'none':
+ ret += mcgen('''
+
+ if (!qmp__err) {
+ qmp__native_retval = %(unmarshal)s(qmp__retval, &qmp__err);
+ }
+''',
+ unmarshal=qmp_type_from_qobj(retval))
+ ret += cgen('')
+ if retval == 'none':
+ ret += cgen(' qmp__cb->cb(qmp__cb->opaque, qmp__err);')
+ else:
+ ret += cgen(' qmp__cb->cb(qmp__cb->opaque, qmp__native_retval, qmp__err);')
+ ret += cgen('}')
+
+ return ret
+
+def gen_lib_definition(name, options, retval, proxy=False, async=False):
+ if proxy:
+ async = True
+
+ ret = ''
+ if proxy:
+ ret += gen_async_lib_definition(name, options, retval)
+
+ fn_decl = gen_lib_decl(name, options, retval, proxy=proxy, async=async)
+ ret += mcgen('''
+
+%(fn_decl)s
+{
+ QDict *qmp__args = qdict_new();
+''',
+ fn_decl=fn_decl)
+ if async:
+ ret += mcgen('''
+ %(cc_name)sCompletionCB *qmp__cb = qemu_mallocz(sizeof(*qmp__cb));
+
+ qmp__cb->cb = qmp__cc;
+ qmp__cb->opaque = qmp__opaque;
+''',
+ cc_name=camel_case(name))
+ else:
+ ret += mcgen('''
+ Error *qmp__local_err = NULL;
+ QObject *qmp__retval = NULL;
+''')
+ if retval != 'none':
+ ret += cgen(' %(ret_type)s qmp__native_retval = 0;',
+ ret_type=qmp_type_to_c(retval, True))
+ if qmp_type_is_event(retval):
+ ret += cgen(' int qmp__global_handle = 0;')
+ ret += cgen('')
+
+ for argname, argtype, optional in parse_args(options):
+ if argtype == '**':
+ ret += mcgen('''
+ {
+ KeyValues *qmp__i;
+ for (qmp__i = %(name)s; qmp__i; qmp__i = qmp__i->next) {
+ qdict_put(qmp__args, qmp__i->key, qstring_from_str(qmp__i->value));
+ }
+ }
+''',
+ name=c_var(argname))
+ else:
+ if optional:
+ ret += mcgen('''
+ if (has_%(c_name)s) {
+''',
+ c_name=c_var(argname))
+ push_indent()
+ ret += mcgen('''
+ qdict_put_obj(qmp__args, "%(name)s", %(marshal)s(%(c_name)s));
+''',
+ name=argname, c_name=c_var(argname),
+ marshal=qmp_type_to_qobj(argtype))
+ if optional:
+ pop_indent()
+ ret += mcgen('''
+ }
+''')
+
+ if proxy:
+ ret += mcgen('''
+ qmp_guest_dispatch("%(name)s", qmp__args, qmp__err, qmp_%(c_name)s_cb, qmp__cb);
+''',
+ name=name, c_name=c_var(name))
+ else:
+ ret += mcgen('''
+ qmp__retval = qmp__session->dispatch(qmp__session, "%(name)s", qmp__args, &qmp__local_err);
+''',
+ name=name)
+ ret += mcgen('''
+
+ QDECREF(qmp__args);
+''')
+
+ if async:
+ pass
+ elif type(retval) == list:
+ ret += mcgen('''
+
+ if (!qmp__local_err) {
+ QList *qmp__list_retval = qobject_to_qlist(qmp__retval);
+ QListEntry *qmp__i;
+ QLIST_FOREACH_ENTRY(qmp__list_retval, qmp__i) {
+ %(type)s qmp__native_i = %(unmarshal)s(qmp__i->value, &qmp__local_err);
+ if (qmp__local_err) {
+ %(free)s(qmp__native_retval);
+ break;
+ }
+ qmp__native_i->next = qmp__native_retval;
+ qmp__native_retval = qmp__native_i;
+ }
+ qobject_decref(qmp__retval);
+ }
+ error_propagate(qmp__err, qmp__local_err);
+ return qmp__native_retval;
+''',
+ type=qmp_type_to_c(retval[0], True),
+ unmarshal=qmp_type_from_qobj(retval[0]),
+ free=qmp_free_func(retval[0]))
+ elif is_dict(retval):
+ ret += mcgen('''
+ // FIXME (using an anonymous dict as return value)
+ BUILD_BUG();
+''')
+ elif qmp_type_is_event(retval):
+ if proxy:
+ ret += cgen(' BUILD_BUG();')
+ ret += mcgen('''
+ if (!qmp__local_err) {
+ qmp__global_handle = %(unmarshal)s(qmp__retval, &qmp__local_err);
+ qobject_decref(qmp__retval);
+ qmp__retval = NULL;
+ }
+ if (!qmp__local_err) {
+ qmp__native_retval = libqmp_signal_init(qmp__session, %(type)s, qmp__global_handle);
+ }
+ error_propagate(qmp__err, qmp__local_err);
+ return qmp__native_retval;
+''',
+ unmarshal=qmp_type_from_qobj('int'),
+ type=qmp_event_to_c(retval))
+ elif retval != 'none':
+ ret += mcgen('''
+
+ if (!qmp__local_err) {
+ qmp__native_retval = %(unmarshal)s(qmp__retval, &qmp__local_err);
+ qobject_decref(qmp__retval);
+ }
+ error_propagate(qmp__err, qmp__local_err);
+ return qmp__native_retval;
+''',
+ unmarshal=qmp_type_from_qobj(retval))
+ else:
+ ret += mcgen('''
+ qobject_decref(qmp__retval);
+ error_propagate(qmp__err, qmp__local_err);
+''')
+
+ ret += cgen('}')
+
+ return ret
+
def tokenize(data):
while len(data):
if data[0] in ['{', '}', ':', ',', '[', ']']:
@@ -1028,6 +1343,18 @@ def generate(kind):
#include "qmp.h"
#include "qmp-core.h"
''')
+ elif kind == 'lib-header':
+ ret += mcgen('''
+#ifndef LIBQMP_H
+#define LIBQMP_H
+
+#include "libqmp-core.h"
+''')
+ elif kind == 'lib-body':
+ ret += mcgen('''
+#include "libqmp.h"
+#include "libqmp-internal.h"
+''')
exprs = []
expr = ''
@@ -1084,6 +1411,8 @@ def generate(kind):
event_types[name] = data
if kind == 'types-header':
ret += gen_type_declaration(name, data)
+ elif kind == 'lib-body':
+ ret += gen_lib_event_definition(name, data)
elif s.has_key('command'):
name = s['command']
options = {}
@@ -1094,10 +1423,17 @@ def generate(kind):
retval = s['returns']
if kind == 'body':
async = qmp_is_async_cmd(name)
+ proxy = qmp_is_proxy_cmd(name)
+ if proxy:
+ ret += gen_lib_definition(name, options, retval, proxy=True)
ret += gen_definition(name, options, retval, async=async)
elif kind == 'header':
async = qmp_is_async_cmd(name)
ret += gen_declaration(name, options, retval, async=async)
+ elif kind == 'lib-body':
+ ret += gen_lib_definition(name, options, retval)
+ elif kind == 'lib-header':
+ ret += gen_lib_declaration(name, options, retval)
if kind.endswith('header'):
ret += cgen('#endif')
@@ -1135,6 +1471,21 @@ static void qmp_init_marshal(void)
qapi_init(qmp_init_marshal);
''')
+ elif kind == 'lib-body':
+ ret += mcgen('''
+
+void libqmp_init_events(QmpSession *sess)
+{
+''')
+ for event in event_types:
+ ret += mcgen('''
+ libqmp_register_event(sess, "%(name)s", &libqmp_notify_%(c_event_name)s);
+''',
+ name=event,
+ c_event_name=de_camel_case(qmp_event_to_c(event)))
+ ret += mcgen('''
+}
+''')
return ret
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [Qemu-devel] [PATCH 13/15] qapi: add code generator for libqmp (v2)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 13/15] qapi: add code generator for libqmp (v2) Anthony Liguori
@ 2011-03-12 11:10 ` Blue Swirl
2011-03-12 14:53 ` Anthony Liguori
0 siblings, 1 reply; 49+ messages in thread
From: Blue Swirl @ 2011-03-12 11:10 UTC (permalink / raw)
To: Anthony Liguori
Cc: Markus Armbruster, Avi Kivity, Luiz Capitulino, qemu-devel,
Adam Litke
On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> v1 -> v2
> - update code generator to use multiline
> - proxy command support
> - async command support
>
> diff --git a/Makefile b/Makefile
> index 47a755d..5170675 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -4,7 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
> ifeq ($(TRACE_BACKEND),dtrace)
> GENERATED_HEADERS += trace-dtrace.h
> endif
> -GENERATED_HEADERS += qmp-types.h qmp-marshal-types.h qmp.h
> +GENERATED_HEADERS += qmp-types.h qmp-marshal-types.h qmp.h libqmp.h
>
> ifneq ($(wildcard config-host.mak),)
> # Put the all: rule here so that config-host.mak can contain dependencies.
> @@ -165,9 +165,16 @@ qmp.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> qmp-marshal.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --body < $< > $@, " GEN $@")
>
> +libqmp.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --lib-header < $< > $@, " GEN $@")
> +
> +libqmp.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --lib-body < $< > $@, " GEN $@")
> +
> qmp-types.o: qmp-types.c qmp-types.h
> qmp-marshal-types.o: qmp-marshal-types.c qmp-marshal-types.h qmp-types.h
> qmp-marshal.o: qmp-marshal.c qmp.h qmp-types.h qmp-marshal-types.h
> +libqmp.o: libqmp.c libqmp.h qmp-types.h
>
> version.o: $(SRC_PATH)/version.rc config-host.mak
> $(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
> diff --git a/libqmp-core.c b/libqmp-core.c
> new file mode 100644
> index 0000000..4613d4f
> --- /dev/null
> +++ b/libqmp-core.c
> @@ -0,0 +1,361 @@
> +/*
> + * QAPI
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + * Anthony Liguori <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2. See
> + * the COPYING.LIB file in the top-level directory.
> + */
> +#include "libqmp.h"
> +#include "libqmp-internal.h"
> +#include "libqmp-core.h"
> +#include "json-streamer.h"
> +#include "json-parser.h"
> +#include "dirent.h"
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <assert.h>
> +
> +#ifndef container_of
> +#define offset_of(type, field) \
> + ((unsigned long)(&((type *)0)->field))
> +#define container_of(obj, type, field) \
> + ((type *)(((char *)obj) - offsetof(type, field)))
> +#endif
Why not use the existing definitions?
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH 13/15] qapi: add code generator for libqmp (v2)
2011-03-12 11:10 ` Blue Swirl
@ 2011-03-12 14:53 ` Anthony Liguori
0 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-12 14:53 UTC (permalink / raw)
To: Blue Swirl
Cc: qemu-devel, Adam Litke, Luiz Capitulino, Markus Armbruster,
Avi Kivity
On 03/12/2011 05:10 AM, Blue Swirl wrote:
> On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori<aliguori@us.ibm.com> wrote:
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>> v1 -> v2
>> - update code generator to use multiline
>> - proxy command support
>> - async command support
>>
>> diff --git a/Makefile b/Makefile
>> index 47a755d..5170675 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -4,7 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
>> ifeq ($(TRACE_BACKEND),dtrace)
>> GENERATED_HEADERS += trace-dtrace.h
>> endif
>> -GENERATED_HEADERS += qmp-types.h qmp-marshal-types.h qmp.h
>> +GENERATED_HEADERS += qmp-types.h qmp-marshal-types.h qmp.h libqmp.h
>>
>> ifneq ($(wildcard config-host.mak),)
>> # Put the all: rule here so that config-host.mak can contain dependencies.
>> @@ -165,9 +165,16 @@ qmp.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
>> qmp-marshal.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
>> $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --body< $< > $@, " GEN $@")
>>
>> +libqmp.h: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
>> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --lib-header< $< > $@, " GEN $@")
>> +
>> +libqmp.c: $(SRC_PATH)/qmp-schema.json $(SRC_PATH)/qmp-gen.py
>> + $(call quiet-command,python $(SRC_PATH)/qmp-gen.py --lib-body< $< > $@, " GEN $@")
>> +
>> qmp-types.o: qmp-types.c qmp-types.h
>> qmp-marshal-types.o: qmp-marshal-types.c qmp-marshal-types.h qmp-types.h
>> qmp-marshal.o: qmp-marshal.c qmp.h qmp-types.h qmp-marshal-types.h
>> +libqmp.o: libqmp.c libqmp.h qmp-types.h
>>
>> version.o: $(SRC_PATH)/version.rc config-host.mak
>> $(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@")
>> diff --git a/libqmp-core.c b/libqmp-core.c
>> new file mode 100644
>> index 0000000..4613d4f
>> --- /dev/null
>> +++ b/libqmp-core.c
>> @@ -0,0 +1,361 @@
>> +/*
>> + * QAPI
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + * Anthony Liguori<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2. See
>> + * the COPYING.LIB file in the top-level directory.
>> + */
>> +#include "libqmp.h"
>> +#include "libqmp-internal.h"
>> +#include "libqmp-core.h"
>> +#include "json-streamer.h"
>> +#include "json-parser.h"
>> +#include "dirent.h"
>> +#include<sys/socket.h>
>> +#include<sys/un.h>
>> +#include<assert.h>
>> +
>> +#ifndef container_of
>> +#define offset_of(type, field) \
>> + ((unsigned long)(&((type *)0)->field))
>> +#define container_of(obj, type, field) \
>> + ((type *)(((char *)obj) - offsetof(type, field)))
>> +#endif
> Why not use the existing definitions?
libqmp builds outside of the normal QEMU code base (it's a client library).
libqmp wants to be LGPL. It's not clear what the license of the stuff
in osdep is.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (12 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 13/15] qapi: add code generator for libqmp (v2) Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-12 11:23 ` Blue Swirl
2011-03-11 23:05 ` [Qemu-devel] [PATCH 15/15] qapi: generate HTML report for test-libqmp Anthony Liguori
2011-03-16 14:34 ` [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Luiz Capitulino
15 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
This provides a glib-test based testing framework for QMP
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/Makefile b/Makefile
index 5170675..1d363d7 100644
--- a/Makefile
+++ b/Makefile
@@ -72,6 +72,8 @@ defconfig:
-include config-all-devices.mak
+TOOLS += test-libqmp
+
build-all: $(DOCS) $(TOOLS) recurse-all
config-host.h: config-host.h-timestamp
@@ -205,6 +207,15 @@ check-qlist: check-qlist.o qlist.o qint.o $(CHECK_PROG_DEPS)
check-qfloat: check-qfloat.o qfloat.o $(CHECK_PROG_DEPS)
check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o $(CHECK_PROG_DEPS)
+LIBQMP_OBJS := qmp-types.o libqmp.o error.o libqmp-core.o
+LIBQMP_OBJS += qmp-marshal-types-core.o qmp-marshal-types.o
+LIBQMP_OBJS += qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o
+LIBQMP_OBJS += qerror.o
+LIBQMP_OBJS += json-streamer.o json-lexer.o json-parser.o
+LIBQMP_OBJS += $(oslib-obj-y) $(trace-obj-y) qemu-malloc.o
+
+test-libqmp: test-libqmp.o $(LIBQMP_OBJS) qemu-timer-common.o
+
clean:
# avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
diff --git a/test-libqmp.c b/test-libqmp.c
new file mode 100644
index 0000000..9b73987
--- /dev/null
+++ b/test-libqmp.c
@@ -0,0 +1,170 @@
+/*
+ * QAPI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+#include <stdio.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <netinet/tcp.h>
+#include <arpa/inet.h>
+#include <sys/un.h>
+#include <stdlib.h>
+#include <glib.h>
+#include <sys/wait.h>
+#include "config-host.h"
+#include "libqmp.h"
+#include "qerror.h"
+
+#define g_assert_noerr(err) g_assert(err == NULL);
+#define g_assert_anyerr(err) g_assert(err != NULL);
+#define g_assert_cmperr(err, op, type) do { \
+ g_assert_anyerr(err); \
+ g_assert_cmpstr(error_get_field(err, "class"), op, type); \
+} while (0)
+
+static pid_t last_qemu_pid = -1;
+
+static QmpSession *qemu(const char *fmt, ...)
+{
+ char buffer0[4096];
+ char buffer1[4096];
+ const char *pid_filename = "/tmp/test-libqmp-qemu.pid";
+ const char *path = "/tmp/test-libqmp-qemu.sock";
+ struct sockaddr_un addr;
+ va_list ap;
+ int ret;
+ int fd;
+
+ va_start(ap, fmt);
+ vsnprintf(buffer0, sizeof(buffer0), fmt, ap);
+ va_end(ap);
+
+ snprintf(buffer1, sizeof(buffer1),
+ "i386-softmmu/qemu "
+ "-enable-kvm "
+ "-name test-libqmp "
+ "-qmp2 qmp "
+ "-chardev socket,id=qmp,path=%s,server=on,wait=off "
+ "-vnc none "
+ "-daemonize "
+ "-pidfile %s "
+ "%s", path, pid_filename, buffer0);
+ g_test_message("Executing %s\n", buffer1);
+ ret = system(buffer1);
+ g_assert(ret != -1);
+
+ {
+ FILE *f;
+ char buffer[1024];
+ char *ptr;
+
+ f = fopen(pid_filename, "r");
+ g_assert(f != NULL);
+
+ ptr = fgets(buffer, sizeof(buffer), f);
+ g_assert(ptr != NULL);
+
+ fclose(f);
+
+ last_qemu_pid = atoi(buffer);
+ }
+
+ fd = socket(PF_UNIX, SOCK_STREAM, 0);
+ g_assert(fd != -1);
+
+ addr.sun_family = AF_UNIX;
+ snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
+ ret = connect(fd, (struct sockaddr *)&addr, sizeof(addr));
+ g_assert(ret != -1);
+
+ return qmp_session_new(fd);
+}
+
+static void wait_for_pid_exit(pid_t pid)
+{
+ FILE *f = NULL;
+
+ /* This is ugly but I don't know of a better way */
+ do {
+ char buffer[1024];
+
+ if (f) {
+ fclose(f);
+ usleep(10000);
+ }
+
+ snprintf(buffer, sizeof(buffer), "/proc/%d/stat", pid);
+ f = fopen(buffer, "r");
+ } while (f);
+}
+
+static void qemu_destroy(QmpSession *sess)
+{
+ wait_for_pid_exit(last_qemu_pid);
+ last_qemu_pid = -1;
+ qmp_session_destroy(sess);
+}
+
+static void test_version(void)
+{
+ QmpSession *sess = NULL;
+ VersionInfo *info;
+ char version[1024];
+ char *ptr, *end;
+ int major, minor, micro;
+
+ /* Even though we use the same string as the source input, we do parse it
+ * a little bit different for no other reason that to make sure we catch
+ * potential bugs.
+ */
+ snprintf(version, sizeof(version), "%s", QEMU_VERSION);
+ ptr = version;
+
+ end = strchr(ptr, '.');
+ g_assert(end != NULL);
+ *end = 0;
+ major = atoi(ptr);
+ ptr = end + 1;
+
+ end = strchr(ptr, '.');
+ g_assert(end != NULL);
+ *end = 0;
+ minor = atoi(ptr);
+ ptr = end + 1;
+
+ micro = atoi(ptr);
+ while (g_ascii_isdigit(*ptr)) ptr++;
+
+ sess = qemu("-S");
+
+ info = libqmp_query_version(sess, NULL);
+
+ g_assert_cmpint(major, ==, info->qemu.major);
+ g_assert_cmpint(minor, ==, info->qemu.minor);
+ g_assert_cmpint(micro, ==, info->qemu.micro);
+ g_assert_cmpstr(ptr, ==, info->package);
+
+ qmp_free_version_info(info);
+
+ libqmp_quit(sess, NULL);
+
+ qemu_destroy(sess);
+}
+
+int main(int argc, char **argv)
+{
+ g_test_init(&argc, &argv, NULL);
+
+ g_test_add_func("/0.14/misc/version", test_version);
+
+ g_test_run();
+
+ return 0;
+}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp
2011-03-11 23:05 ` [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp Anthony Liguori
@ 2011-03-12 11:23 ` Blue Swirl
2011-03-12 14:59 ` Anthony Liguori
0 siblings, 1 reply; 49+ messages in thread
From: Blue Swirl @ 2011-03-12 11:23 UTC (permalink / raw)
To: Anthony Liguori
Cc: Markus Armbruster, Avi Kivity, Luiz Capitulino, qemu-devel,
Adam Litke
On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> This provides a glib-test based testing framework for QMP
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
> diff --git a/Makefile b/Makefile
> index 5170675..1d363d7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -72,6 +72,8 @@ defconfig:
>
> -include config-all-devices.mak
>
> +TOOLS += test-libqmp
> +
> build-all: $(DOCS) $(TOOLS) recurse-all
>
> config-host.h: config-host.h-timestamp
> @@ -205,6 +207,15 @@ check-qlist: check-qlist.o qlist.o qint.o $(CHECK_PROG_DEPS)
> check-qfloat: check-qfloat.o qfloat.o $(CHECK_PROG_DEPS)
> check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o $(CHECK_PROG_DEPS)
>
> +LIBQMP_OBJS := qmp-types.o libqmp.o error.o libqmp-core.o
> +LIBQMP_OBJS += qmp-marshal-types-core.o qmp-marshal-types.o
> +LIBQMP_OBJS += qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o
> +LIBQMP_OBJS += qerror.o
> +LIBQMP_OBJS += json-streamer.o json-lexer.o json-parser.o
> +LIBQMP_OBJS += $(oslib-obj-y) $(trace-obj-y) qemu-malloc.o
> +
> +test-libqmp: test-libqmp.o $(LIBQMP_OBJS) qemu-timer-common.o
> +
> clean:
> # avoid old build problems by removing potentially incorrect old files
> rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
> diff --git a/test-libqmp.c b/test-libqmp.c
> new file mode 100644
I'd put this to tests/.
> index 0000000..9b73987
> --- /dev/null
> +++ b/test-libqmp.c
> @@ -0,0 +1,170 @@
> +/*
> + * QAPI
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + * Anthony Liguori <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2. See
> + * the COPYING.LIB file in the top-level directory.
> + */
> +#include <stdio.h>
> +#include <sys/socket.h>
> +#include <netinet/in.h>
> +#include <netinet/tcp.h>
> +#include <arpa/inet.h>
> +#include <sys/un.h>
> +#include <stdlib.h>
> +#include <glib.h>
> +#include <sys/wait.h>
> +#include "config-host.h"
> +#include "libqmp.h"
> +#include "qerror.h"
> +
> +#define g_assert_noerr(err) g_assert(err == NULL);
> +#define g_assert_anyerr(err) g_assert(err != NULL);
> +#define g_assert_cmperr(err, op, type) do { \
> + g_assert_anyerr(err); \
> + g_assert_cmpstr(error_get_field(err, "class"), op, type); \
> +} while (0)
> +
> +static pid_t last_qemu_pid = -1;
> +
> +static QmpSession *qemu(const char *fmt, ...)
> +{
> + char buffer0[4096];
> + char buffer1[4096];
> + const char *pid_filename = "/tmp/test-libqmp-qemu.pid";
> + const char *path = "/tmp/test-libqmp-qemu.sock";
Very insecure filenames.
> + struct sockaddr_un addr;
> + va_list ap;
> + int ret;
> + int fd;
> +
> + va_start(ap, fmt);
> + vsnprintf(buffer0, sizeof(buffer0), fmt, ap);
> + va_end(ap);
> +
> + snprintf(buffer1, sizeof(buffer1),
> + "i386-softmmu/qemu "
> + "-enable-kvm "
> + "-name test-libqmp "
> + "-qmp2 qmp "
> + "-chardev socket,id=qmp,path=%s,server=on,wait=off "
> + "-vnc none "
> + "-daemonize "
> + "-pidfile %s "
> + "%s", path, pid_filename, buffer0);
> + g_test_message("Executing %s\n", buffer1);
> + ret = system(buffer1);
> + g_assert(ret != -1);
> +
> + {
> + FILE *f;
> + char buffer[1024];
> + char *ptr;
> +
> + f = fopen(pid_filename, "r");
> + g_assert(f != NULL);
> +
> + ptr = fgets(buffer, sizeof(buffer), f);
> + g_assert(ptr != NULL);
> +
> + fclose(f);
> +
> + last_qemu_pid = atoi(buffer);
> + }
> +
> + fd = socket(PF_UNIX, SOCK_STREAM, 0);
> + g_assert(fd != -1);
> +
> + addr.sun_family = AF_UNIX;
> + snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
> + ret = connect(fd, (struct sockaddr *)&addr, sizeof(addr));
> + g_assert(ret != -1);
> +
> + return qmp_session_new(fd);
> +}
> +
> +static void wait_for_pid_exit(pid_t pid)
> +{
> + FILE *f = NULL;
> +
> + /* This is ugly but I don't know of a better way */
man waitpid?
> + do {
> + char buffer[1024];
> +
> + if (f) {
> + fclose(f);
> + usleep(10000);
> + }
> +
> + snprintf(buffer, sizeof(buffer), "/proc/%d/stat", pid);
> + f = fopen(buffer, "r");
> + } while (f);
> +}
> +
> +static void qemu_destroy(QmpSession *sess)
> +{
> + wait_for_pid_exit(last_qemu_pid);
> + last_qemu_pid = -1;
> + qmp_session_destroy(sess);
> +}
> +
> +static void test_version(void)
> +{
> + QmpSession *sess = NULL;
> + VersionInfo *info;
> + char version[1024];
> + char *ptr, *end;
> + int major, minor, micro;
> +
> + /* Even though we use the same string as the source input, we do parse it
> + * a little bit different for no other reason that to make sure we catch
> + * potential bugs.
> + */
> + snprintf(version, sizeof(version), "%s", QEMU_VERSION);
> + ptr = version;
> +
> + end = strchr(ptr, '.');
> + g_assert(end != NULL);
> + *end = 0;
> + major = atoi(ptr);
strtoll(), also below.
> + ptr = end + 1;
> +
> + end = strchr(ptr, '.');
> + g_assert(end != NULL);
> + *end = 0;
> + minor = atoi(ptr);
> + ptr = end + 1;
> +
> + micro = atoi(ptr);
> + while (g_ascii_isdigit(*ptr)) ptr++;
> +
> + sess = qemu("-S");
> +
> + info = libqmp_query_version(sess, NULL);
> +
> + g_assert_cmpint(major, ==, info->qemu.major);
> + g_assert_cmpint(minor, ==, info->qemu.minor);
> + g_assert_cmpint(micro, ==, info->qemu.micro);
> + g_assert_cmpstr(ptr, ==, info->package);
> +
> + qmp_free_version_info(info);
> +
> + libqmp_quit(sess, NULL);
> +
> + qemu_destroy(sess);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + g_test_init(&argc, &argv, NULL);
> +
> + g_test_add_func("/0.14/misc/version", test_version);
> +
> + g_test_run();
> +
> + return 0;
> +}
> --
> 1.7.0.4
>
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp
2011-03-12 11:23 ` Blue Swirl
@ 2011-03-12 14:59 ` Anthony Liguori
0 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-12 14:59 UTC (permalink / raw)
To: Blue Swirl
Cc: qemu-devel, Adam Litke, Luiz Capitulino, Markus Armbruster,
Avi Kivity
On 03/12/2011 05:23 AM, Blue Swirl wrote:
> On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori<aliguori@us.ibm.com> wrote:
>> This provides a glib-test based testing framework for QMP
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>
>> diff --git a/Makefile b/Makefile
>> index 5170675..1d363d7 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -72,6 +72,8 @@ defconfig:
>>
>> -include config-all-devices.mak
>>
>> +TOOLS += test-libqmp
>> +
>> build-all: $(DOCS) $(TOOLS) recurse-all
>>
>> config-host.h: config-host.h-timestamp
>> @@ -205,6 +207,15 @@ check-qlist: check-qlist.o qlist.o qint.o $(CHECK_PROG_DEPS)
>> check-qfloat: check-qfloat.o qfloat.o $(CHECK_PROG_DEPS)
>> check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o $(CHECK_PROG_DEPS)
>>
>> +LIBQMP_OBJS := qmp-types.o libqmp.o error.o libqmp-core.o
>> +LIBQMP_OBJS += qmp-marshal-types-core.o qmp-marshal-types.o
>> +LIBQMP_OBJS += qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o
>> +LIBQMP_OBJS += qerror.o
>> +LIBQMP_OBJS += json-streamer.o json-lexer.o json-parser.o
>> +LIBQMP_OBJS += $(oslib-obj-y) $(trace-obj-y) qemu-malloc.o
>> +
>> +test-libqmp: test-libqmp.o $(LIBQMP_OBJS) qemu-timer-common.o
>> +
>> clean:
>> # avoid old build problems by removing potentially incorrect old files
>> rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
>> diff --git a/test-libqmp.c b/test-libqmp.c
>> new file mode 100644
> I'd put this to tests/.
tests/ lives outside of the QEMU build system today. It's also very TCG
specific.
How about taking the current contents of tests/ and moving it to
tests/tcg and moving test-libqmp and check-*.c to tests/?
>> index 0000000..9b73987
>> --- /dev/null
>> +++ b/test-libqmp.c
>> @@ -0,0 +1,170 @@
>> +/*
>> + * QAPI
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + * Anthony Liguori<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2. See
>> + * the COPYING.LIB file in the top-level directory.
>> + */
>> +#include<stdio.h>
>> +#include<sys/socket.h>
>> +#include<netinet/in.h>
>> +#include<netinet/tcp.h>
>> +#include<arpa/inet.h>
>> +#include<sys/un.h>
>> +#include<stdlib.h>
>> +#include<glib.h>
>> +#include<sys/wait.h>
>> +#include "config-host.h"
>> +#include "libqmp.h"
>> +#include "qerror.h"
>> +
>> +#define g_assert_noerr(err) g_assert(err == NULL);
>> +#define g_assert_anyerr(err) g_assert(err != NULL);
>> +#define g_assert_cmperr(err, op, type) do { \
>> + g_assert_anyerr(err); \
>> + g_assert_cmpstr(error_get_field(err, "class"), op, type); \
>> +} while (0)
>> +
>> +static pid_t last_qemu_pid = -1;
>> +
>> +static QmpSession *qemu(const char *fmt, ...)
>> +{
>> + char buffer0[4096];
>> + char buffer1[4096];
>> + const char *pid_filename = "/tmp/test-libqmp-qemu.pid";
>> + const char *path = "/tmp/test-libqmp-qemu.sock";
> Very insecure filenames.
This disappears in round 3 when I introduce discovery support in libqmp.
Even so, I don't think security is a major concern here.
>> +static void wait_for_pid_exit(pid_t pid)
>> +{
>> + FILE *f = NULL;
>> +
>> + /* This is ugly but I don't know of a better way */
> man waitpid?
waitpid only works for child processes. Since we launch with
-daemonize, that the QEMU instance is no longer a child process.
We use -daemonize because it avoids the race condition where we try to
connect to a QMP socket but QEMU hasn't created the socket yet. It also
means that we can just use system() to invoke QEMU which makes life a
whole lot simpler.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH 15/15] qapi: generate HTML report for test-libqmp
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (13 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp Anthony Liguori
@ 2011-03-11 23:05 ` Anthony Liguori
2011-03-16 14:34 ` [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Luiz Capitulino
15 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-11 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: Adam Litke, Anthony Liguori, Luiz Capitulino, Avi Kivity,
Markus Armbruster
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/Makefile b/Makefile
index 1d363d7..c5a4820 100644
--- a/Makefile
+++ b/Makefile
@@ -216,6 +216,15 @@ LIBQMP_OBJS += $(oslib-obj-y) $(trace-obj-y) qemu-malloc.o
test-libqmp: test-libqmp.o $(LIBQMP_OBJS) qemu-timer-common.o
+check: test-libqmp
+ $(call quiet-command, ./test-libqmp, " CHECK $@")
+
+test-report.html: test-report.log
+ $(call quiet-command, gtester-report $< > $@, " GEN $@")
+
+test-report.log: test-libqmp
+ $(call quiet-command, gtester -k -o $@ ./test-libqmp 2>/dev/null >/dev/null || true, " TEST $<")
+
clean:
# avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
--
1.7.0.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
` (14 preceding siblings ...)
2011-03-11 23:05 ` [Qemu-devel] [PATCH 15/15] qapi: generate HTML report for test-libqmp Anthony Liguori
@ 2011-03-16 14:34 ` Luiz Capitulino
2011-03-16 14:49 ` Paolo Bonzini
2011-03-16 15:59 ` Anthony Liguori
15 siblings, 2 replies; 49+ messages in thread
From: Luiz Capitulino @ 2011-03-16 14:34 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Markus Armbruster, Adam Litke, qemu-devel, Avi Kivity
On Fri, 11 Mar 2011 17:05:30 -0600
Anthony Liguori <aliguori@us.ibm.com> wrote:
> For more information about the background of QAPI, see
> http://wiki.qemu.org/Features/QAPI
>
> This series depends on 'QAPI Round 0' which I posted earlier.
>
> Since v2, the major changes are:
>
> - Switch to a multiline code emitter to ease readability
> - Use named parameters for escape sequences
> - Add support for proxy commands
> - Add support for asynchronous commands
>
> This version still adds a -qmp2 option. This is the only practical way I know
> to have testable code while not merging 200 patches all at once.
I've started reviewing this and my first impression is that this seems to be
real good. However, there's a lot of code here and some parts of it are a bit
complicated, so I need more time to do a thorough review and testing.
Having said that, my only immediate concern is weather this will have any
negative side effects on the wire protocol, today or in the future.
I mean, a C library has different extensibility constraints and functionality
requirements than a high-level protocol and tying/mixing the two can have
bad side effects, like this small one (patch 12/15):
+##
+# @put_event:
+#
+# Disconnect a signal. This command is used to disconnect from a signal based
+# on the handle returned by a signal accessor.
+#
+# @tag: the handle returned by a signal accessor.
+#
+# Returns: Nothing on success.
+# If @tag is not a valid handle, InvalidParameterValue
+#
+# Since: 0.15.0
The name 'signal' (at least today) doesn't make sense on the wire protocol,
'put_event' probably doesn't make sense in the C library, nor does 'event'.
Another detail is that, event extension is more important than command
extension, because it's probably going to happen. I think it would be very
bad to add new events just because we wanted to add a new field.
Most of these problems seems to go away just by making libqmp internal
to QEMU, then I think all this work would rock big time :-)
^ permalink raw reply [flat|nested] 49+ messages in thread* [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 14:34 ` [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Luiz Capitulino
@ 2011-03-16 14:49 ` Paolo Bonzini
2011-03-16 15:00 ` Luiz Capitulino
2011-03-16 16:03 ` Anthony Liguori
2011-03-16 15:59 ` Anthony Liguori
1 sibling, 2 replies; 49+ messages in thread
From: Paolo Bonzini @ 2011-03-16 14:49 UTC (permalink / raw)
To: Luiz Capitulino
Cc: qemu-devel, Anthony Liguori, Avi Kivity, Markus Armbruster,
Adam Litke
On 03/16/2011 03:34 PM, Luiz Capitulino wrote:
> +##
> +# @put_event:
> +#
> +# Disconnect a signal. This command is used to disconnect from a signal based
> +# on the handle returned by a signal accessor.
> +#
> +# @tag: the handle returned by a signal accessor.
> +#
> +# Returns: Nothing on success.
> +# If @tag is not a valid handle, InvalidParameterValue
> +#
> +# Since: 0.15.0
>
> The name 'signal' (at least today) doesn't make sense on the wire protocol,
> 'put_event' probably doesn't make sense in the C library, nor does 'event'.
>
> Another detail is that, event extension is more important than command
> extension, because it's probably going to happen. I think it would be very
> bad to add new events just because we wanted to add a new field.
What if events were always passed a single struct, with the first field
being a bitmask saying which (or how many) fields have been filled?
It is quite ugly to work that way when calling functions, but it's not
too bad when you are writing the callees. And it's the code generator
that writes the function calls in the case of libqmp...
Paolo
^ permalink raw reply [flat|nested] 49+ messages in thread
* [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 14:49 ` Paolo Bonzini
@ 2011-03-16 15:00 ` Luiz Capitulino
2011-03-16 16:06 ` Anthony Liguori
2011-03-16 16:03 ` Anthony Liguori
1 sibling, 1 reply; 49+ messages in thread
From: Luiz Capitulino @ 2011-03-16 15:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Anthony Liguori, Avi Kivity, Markus Armbruster,
Adam Litke
On Wed, 16 Mar 2011 15:49:59 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 03/16/2011 03:34 PM, Luiz Capitulino wrote:
> > +##
> > +# @put_event:
> > +#
> > +# Disconnect a signal. This command is used to disconnect from a signal based
> > +# on the handle returned by a signal accessor.
> > +#
> > +# @tag: the handle returned by a signal accessor.
> > +#
> > +# Returns: Nothing on success.
> > +# If @tag is not a valid handle, InvalidParameterValue
> > +#
> > +# Since: 0.15.0
> >
> > The name 'signal' (at least today) doesn't make sense on the wire protocol,
> > 'put_event' probably doesn't make sense in the C library, nor does 'event'.
> >
> > Another detail is that, event extension is more important than command
> > extension, because it's probably going to happen. I think it would be very
> > bad to add new events just because we wanted to add a new field.
>
> What if events were always passed a single struct, with the first field
> being a bitmask saying which (or how many) fields have been filled?
>
> It is quite ugly to work that way when calling functions, but it's not
> too bad when you are writing the callees. And it's the code generator
> that writes the function calls in the case of libqmp...
I was also wondering if it's possible to only make the most recent version
available in the wire protocol and all existing ones in libqmp.
But I need to read more code in order to know that.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 15:00 ` Luiz Capitulino
@ 2011-03-16 16:06 ` Anthony Liguori
0 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-16 16:06 UTC (permalink / raw)
To: Luiz Capitulino
Cc: Anthony Liguori, qemu-devel, Markus Armbruster, Avi Kivity,
Adam Litke, Paolo Bonzini
On 03/16/2011 10:00 AM, Luiz Capitulino wrote:
> On Wed, 16 Mar 2011 15:49:59 +0100
> Paolo Bonzini<pbonzini@redhat.com> wrote:
>
>> On 03/16/2011 03:34 PM, Luiz Capitulino wrote:
>>> +##
>>> +# @put_event:
>>> +#
>>> +# Disconnect a signal. This command is used to disconnect from a signal based
>>> +# on the handle returned by a signal accessor.
>>> +#
>>> +# @tag: the handle returned by a signal accessor.
>>> +#
>>> +# Returns: Nothing on success.
>>> +# If @tag is not a valid handle, InvalidParameterValue
>>> +#
>>> +# Since: 0.15.0
>>>
>>> The name 'signal' (at least today) doesn't make sense on the wire protocol,
>>> 'put_event' probably doesn't make sense in the C library, nor does 'event'.
>>>
>>> Another detail is that, event extension is more important than command
>>> extension, because it's probably going to happen. I think it would be very
>>> bad to add new events just because we wanted to add a new field.
>> What if events were always passed a single struct, with the first field
>> being a bitmask saying which (or how many) fields have been filled?
>>
>> It is quite ugly to work that way when calling functions, but it's not
>> too bad when you are writing the callees. And it's the code generator
>> that writes the function calls in the case of libqmp...
> I was also wondering if it's possible to only make the most recent version
> available in the wire protocol and all existing ones in libqmp.
I don't quite follow.
libqmp works with all previous versions of QMP. New functions will
return a CommandNotFound error.
In fact, you can run test-libqmp -p /0.14 and it will run the
regressions against the QMP server shipped in 0.14[1]. This is why I
used version prefixes :-)
[1] test-libqmp right now doesn't have a way to select which method you
use for connecting to the QMP server and right now assumes a unix socket
at a well known location. It doesn't add the right parameter to QEMU to
launch this way because in my tree, this is all implicit so you need to
patch test-libqmp to explicitly add that qmp socket.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 14:49 ` Paolo Bonzini
2011-03-16 15:00 ` Luiz Capitulino
@ 2011-03-16 16:03 ` Anthony Liguori
2011-03-16 16:31 ` Paolo Bonzini
1 sibling, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-16 16:03 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Anthony Liguori, Markus Armbruster, qemu-devel, Avi Kivity,
Adam Litke, Luiz Capitulino
On 03/16/2011 09:49 AM, Paolo Bonzini wrote:
> On 03/16/2011 03:34 PM, Luiz Capitulino wrote:
>> +##
>> +# @put_event:
>> +#
>> +# Disconnect a signal. This command is used to disconnect from a
>> signal based
>> +# on the handle returned by a signal accessor.
>> +#
>> +# @tag: the handle returned by a signal accessor.
>> +#
>> +# Returns: Nothing on success.
>> +# If @tag is not a valid handle, InvalidParameterValue
>> +#
>> +# Since: 0.15.0
>>
>> The name 'signal' (at least today) doesn't make sense on the wire
>> protocol,
>> 'put_event' probably doesn't make sense in the C library, nor does
>> 'event'.
>>
>> Another detail is that, event extension is more important than command
>> extension, because it's probably going to happen. I think it would be
>> very
>> bad to add new events just because we wanted to add a new field.
>
> What if events were always passed a single struct, with the first
> field being a bitmask saying which (or how many) fields have been filled?
Complex JSON types are represented as structs in QAPI. For fields that
are marked as optional (which any added field has to be), a boolean flag
is always included. Since we always pad structures when we allocate
them (and zero fill), a structure defaults to having optional fields not
specified.
So for an event, you just need to do:
{ 'signal': 'vnc-connected', 'data': { 'client': 'VncClientInfo' } }
And you can add new fields to the VncClientInfo structure as much as
you'd like without worrying about breaking the C ABI.
> It is quite ugly to work that way when calling functions, but it's not
> too bad when you are writing the callees. And it's the code generator
> that writes the function calls in the case of libqmp...
Yes, the code generator deals with all of this stuff. Writing it by
hand would be horrible (which is why we need a libqmp if we want to
write C unit tests).
Regards,
Anthony Liguori
> Paolo
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 16:03 ` Anthony Liguori
@ 2011-03-16 16:31 ` Paolo Bonzini
2011-03-16 18:06 ` Anthony Liguori
0 siblings, 1 reply; 49+ messages in thread
From: Paolo Bonzini @ 2011-03-16 16:31 UTC (permalink / raw)
To: Anthony Liguori
Cc: Anthony Liguori, Markus Armbruster, qemu-devel, Avi Kivity,
Adam Litke, Luiz Capitulino
On 03/16/2011 05:03 PM, Anthony Liguori wrote:
> So for an event, you just need to do:
>
> { 'signal': 'vnc-connected', 'data': { 'client': 'VncClientInfo' } }
>
> And you can add new fields to the VncClientInfo structure as much as
> you'd like without worrying about breaking the C ABI.
So why couldn't you automatically wrap the events data field in a
structure (e.g. vnc-connected events receive a VncConnectedEventData*)?
Paolo
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 16:31 ` Paolo Bonzini
@ 2011-03-16 18:06 ` Anthony Liguori
0 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-16 18:06 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Luiz Capitulino, Avi Kivity, Adam Litke, Markus Armbruster,
qemu-devel
On 03/16/2011 11:31 AM, Paolo Bonzini wrote:
> On 03/16/2011 05:03 PM, Anthony Liguori wrote:
>> So for an event, you just need to do:
>>
>> { 'signal': 'vnc-connected', 'data': { 'client': 'VncClientInfo' } }
>>
>> And you can add new fields to the VncClientInfo structure as much as
>> you'd like without worrying about breaking the C ABI.
>
> So why couldn't you automatically wrap the events data field in a
> structure (e.g. vnc-connected events receive a VncConnectedEventData*)?
You could, but is it really necessary or even useful?
I don't see signals as being any different than function calls. You
could do the same thing for function calls.
A good example is the vnc-connected event. I didn't post the full
event, it's reall:
{ ' signal': 'vnc-connected', 'data': { 'client': 'VncClientInfo',
'server': 'VncServerInfo' } }
You could add an automatic structure around these two members but that
makes the C interface quite a bit more awkward for no obvious gain.
Regards,
Anthony Liguori
> Paolo
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 14:34 ` [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Luiz Capitulino
2011-03-16 14:49 ` Paolo Bonzini
@ 2011-03-16 15:59 ` Anthony Liguori
2011-03-16 18:09 ` Luiz Capitulino
2011-03-17 12:21 ` Kevin Wolf
1 sibling, 2 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-16 15:59 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel, Avi Kivity, Markus Armbruster, Adam Litke
On 03/16/2011 09:34 AM, Luiz Capitulino wrote:
> On Fri, 11 Mar 2011 17:05:30 -0600
> Anthony Liguori<aliguori@us.ibm.com> wrote:
>
>> For more information about the background of QAPI, see
>> http://wiki.qemu.org/Features/QAPI
>>
>> This series depends on 'QAPI Round 0' which I posted earlier.
>>
>> Since v2, the major changes are:
>>
>> - Switch to a multiline code emitter to ease readability
>> - Use named parameters for escape sequences
>> - Add support for proxy commands
>> - Add support for asynchronous commands
>>
>> This version still adds a -qmp2 option. This is the only practical way I know
>> to have testable code while not merging 200 patches all at once.
> I've started reviewing this and my first impression is that this seems to be
> real good. However, there's a lot of code here and some parts of it are a bit
> complicated, so I need more time to do a thorough review and testing.
>
> Having said that, my only immediate concern is weather this will have any
> negative side effects on the wire protocol, today or in the future.
>
> I mean, a C library has different extensibility constraints and functionality
> requirements than a high-level protocol and tying/mixing the two can have
> bad side effects, like this small one (patch 12/15):
C library is not quite as important as C interface. I want QMP to be an
interface that we consume internally because that will make QMP a strong
external interface.
A fundamental design characteristic for me is that first and foremost,
QMP should be a good C interface and that the wire representation should
be easy to support in a good C interface.
This is a shift in our direction but the good news is that the practical
impact is small. But I don't think there's a lot of value of focusing
on non-C consumers because any non-C consumer is capable of consuming a
good C interface (but the inverse is not true).
> +##
> +# @put_event:
> +#
> +# Disconnect a signal. This command is used to disconnect from a signal based
> +# on the handle returned by a signal accessor.
> +#
> +# @tag: the handle returned by a signal accessor.
> +#
> +# Returns: Nothing on success.
> +# If @tag is not a valid handle, InvalidParameterValue
> +#
> +# Since: 0.15.0
>
> The name 'signal' (at least today) doesn't make sense on the wire protocol,
> 'put_event' probably doesn't make sense in the C library, nor does 'event'.
I tried very hard to make events useful without changing the wire
protocol significantly but I've failed there.
I've got a new proposal for handling events that introduces the concept
of a signal on the wire and the notion of connecting and disconnecting
from signals.
> Another detail is that, event extension is more important than command
> extension, because it's probably going to happen. I think it would be very
> bad to add new events just because we wanted to add a new field.
The way this is typically handled is that signals tend to pass
structures instead of lots of fields. For instance, most of the GDK
events just pass a structure for the event (like GdkButtonEvent).
> Most of these problems seems to go away just by making libqmp internal
> to QEMU, then I think all this work would rock big time :-)
For 0.15.0, libqmp is internal to QEMU. We need to think very hard
before making it an external interface.
But the same sort of compatibility considerations apply to using QMP
within QEMU. If you add a new field to a function call, we need to
modify any internal usage of the API. If we add a field to a structure,
as long as we use feature flags (we do), only the places that care to
set that field need to worry about it.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 15:59 ` Anthony Liguori
@ 2011-03-16 18:09 ` Luiz Capitulino
2011-03-16 18:32 ` Anthony Liguori
2011-03-17 12:21 ` Kevin Wolf
1 sibling, 1 reply; 49+ messages in thread
From: Luiz Capitulino @ 2011-03-16 18:09 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Avi Kivity, Markus Armbruster, Adam Litke
On Wed, 16 Mar 2011 10:59:33 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 03/16/2011 09:34 AM, Luiz Capitulino wrote:
> > On Fri, 11 Mar 2011 17:05:30 -0600
> > Anthony Liguori<aliguori@us.ibm.com> wrote:
> >
> >> For more information about the background of QAPI, see
> >> http://wiki.qemu.org/Features/QAPI
> >>
> >> This series depends on 'QAPI Round 0' which I posted earlier.
> >>
> >> Since v2, the major changes are:
> >>
> >> - Switch to a multiline code emitter to ease readability
> >> - Use named parameters for escape sequences
> >> - Add support for proxy commands
> >> - Add support for asynchronous commands
> >>
> >> This version still adds a -qmp2 option. This is the only practical way I know
> >> to have testable code while not merging 200 patches all at once.
> > I've started reviewing this and my first impression is that this seems to be
> > real good. However, there's a lot of code here and some parts of it are a bit
> > complicated, so I need more time to do a thorough review and testing.
> >
> > Having said that, my only immediate concern is weather this will have any
> > negative side effects on the wire protocol, today or in the future.
> >
> > I mean, a C library has different extensibility constraints and functionality
> > requirements than a high-level protocol and tying/mixing the two can have
> > bad side effects, like this small one (patch 12/15):
>
> C library is not quite as important as C interface. I want QMP to be an
> interface that we consume internally because that will make QMP a strong
> external interface.
Agreed.
> A fundamental design characteristic for me is that first and foremost,
> QMP should be a good C interface and that the wire representation should
> be easy to support in a good C interface.
Agreed.
> This is a shift in our direction but the good news is that the practical
> impact is small. But I don't think there's a lot of value of focusing
> on non-C consumers because any non-C consumer is capable of consuming a
> good C interface (but the inverse is not true).
I disagree. To access a C interface from a high-level language you usually
have to write bindings. Using something like QMP instead of writing bindings
is a lot easier.
Also, what's the problem with C consumers using QMP? Libvirt is C, and it
does it just fine.
So, my personal position on shifting the direction is: I think it's if
we treat the C interface as something internal to QEMU.
> > +##
> > +# @put_event:
> > +#
> > +# Disconnect a signal. This command is used to disconnect from a signal based
> > +# on the handle returned by a signal accessor.
> > +#
> > +# @tag: the handle returned by a signal accessor.
> > +#
> > +# Returns: Nothing on success.
> > +# If @tag is not a valid handle, InvalidParameterValue
> > +#
> > +# Since: 0.15.0
> >
> > The name 'signal' (at least today) doesn't make sense on the wire protocol,
> > 'put_event' probably doesn't make sense in the C library, nor does 'event'.
>
> I tried very hard to make events useful without changing the wire
> protocol significantly but I've failed there.
>
> I've got a new proposal for handling events that introduces the concept
> of a signal on the wire and the notion of connecting and disconnecting
> from signals.
Ok.
>
> > Another detail is that, event extension is more important than command
> > extension, because it's probably going to happen. I think it would be very
> > bad to add new events just because we wanted to add a new field.
>
> The way this is typically handled is that signals tend to pass
> structures instead of lots of fields. For instance, most of the GDK
> events just pass a structure for the event (like GdkButtonEvent).
>
> > Most of these problems seems to go away just by making libqmp internal
> > to QEMU, then I think all this work would rock big time :-)
>
> For 0.15.0, libqmp is internal to QEMU. We need to think very hard
> before making it an external interface.
Ok.
> But the same sort of compatibility considerations apply to using QMP
> within QEMU. If you add a new field to a function call, we need to
> modify any internal usage of the API.
What's the problem of doing this?
> If we add a field to a structure,
> as long as we use feature flags (we do), only the places that care to
> set that field need to worry about it.
Why do we need this in an internal interface?
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 18:09 ` Luiz Capitulino
@ 2011-03-16 18:32 ` Anthony Liguori
2011-03-16 19:27 ` Luiz Capitulino
0 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-16 18:32 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: Markus Armbruster, Adam Litke, qemu-devel, Avi Kivity
On 03/16/2011 01:09 PM, Luiz Capitulino wrote:
>> This is a shift in our direction but the good news is that the practical
>> impact is small. But I don't think there's a lot of value of focusing
>> on non-C consumers because any non-C consumer is capable of consuming a
>> good C interface (but the inverse is not true).
> I disagree. To access a C interface from a high-level language you usually
> have to write bindings. Using something like QMP instead of writing bindings
> is a lot easier.
I think you're confusing multiple points.
A C friendly interface has the following properties:
1) arguments have a well defined order
2) variant arrays are not used
3) variant types are not used
4) structures only have items added to the end of them with some sort of
way to determine at run time whether the argument is present
5) functions don't increase their number of arguments
An API that follows these rules is a Good Library interface in C. It
also happens to be a Good Library interface in Python and just about
every other language.
You can design interfaces in Python that rely on variant arrays or
types, or that add keyword values to arguments, but the absence of those
does not make a Bad Library in Python.
This has nothing to do with the need for bindings. The need for
bindings is entirely based on whether the RPC being used is
self-describing. JSON is.
That said, I think we made a critical mistake in QMP that practically
means that we need bindings for QMP. There is no argument ordering. So
I can write a Python class that does:
import qmp
qmp.eject_device(device='ide0-hd0')
But I cannot write a library today that does:
qmp.eject_device('ide0-hd0')
Without using a library that has knowledge of the QMP schema. This is
somewhat unfortunate and I've been thinking about adding another code
generation mode in qmp-gen.py to generate wrappers that would enable the
more natural usage in Python.
> Also, what's the problem with C consumers using QMP? Libvirt is C, and it
> does it just fine.
I was going to cut and paste libvirt's code that handles query-pci to
show you how complex it is to use vs. just using libqmp but it turns out
libvirt doesn't implement query-pci in QMP mode and falls back to the
human monitor. I think that might be as good of a way to show my point
though :-)
>> But the same sort of compatibility considerations apply to using QMP
>> within QEMU. If you add a new field to a function call, we need to
>> modify any internal usage of the API.
> What's the problem of doing this?
If we have a bunch of code in QEMU that relies on doing
qmp_set_vnc_password(password, &err);
And then we add a new parameter to set_vnc_password(), we now need to
touch ever place we call this.
Good external interfaces also happen to be good internal interfaces and
encourage better modularity within QEMU itself. It also means that for
subsystems that we can convert to be largely implemented in terms of
QMP, we have the option to move them into an external process which is
good from a security PoV.
>> If we add a field to a structure,
>> as long as we use feature flags (we do), only the places that care to
>> set that field need to worry about it.
> Why do we need this in an internal interface?
It's about eating our own dog food. It helps us ensure that we're
really providing high quality external interfaces.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 18:32 ` Anthony Liguori
@ 2011-03-16 19:27 ` Luiz Capitulino
2011-03-16 20:00 ` Anthony Liguori
0 siblings, 1 reply; 49+ messages in thread
From: Luiz Capitulino @ 2011-03-16 19:27 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Markus Armbruster, Adam Litke, qemu-devel, Avi Kivity
On Wed, 16 Mar 2011 13:32:50 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 03/16/2011 01:09 PM, Luiz Capitulino wrote:
> >> This is a shift in our direction but the good news is that the practical
> >> impact is small. But I don't think there's a lot of value of focusing
> >> on non-C consumers because any non-C consumer is capable of consuming a
> >> good C interface (but the inverse is not true).
> > I disagree. To access a C interface from a high-level language you usually
> > have to write bindings. Using something like QMP instead of writing bindings
> > is a lot easier.
>
> I think you're confusing multiple points.
>
> A C friendly interface has the following properties:
>
> 1) arguments have a well defined order
> 2) variant arrays are not used
> 3) variant types are not used
> 4) structures only have items added to the end of them with some sort of
> way to determine at run time whether the argument is present
> 5) functions don't increase their number of arguments
>
> An API that follows these rules is a Good Library interface in C. It
> also happens to be a Good Library interface in Python and just about
> every other language.
>
> You can design interfaces in Python that rely on variant arrays or
> types, or that add keyword values to arguments, but the absence of those
> does not make a Bad Library in Python.
>
> This has nothing to do with the need for bindings.
I mentioned bindings because you put so much emphasis on C consumers
that sometimes I wonder if all you want is libqmp and libqmp only.
> The need for
> bindings is entirely based on whether the RPC being used is
> self-describing. JSON is.
>
> That said, I think we made a critical mistake in QMP that practically
> means that we need bindings for QMP. There is no argument ordering.
I'm sorry? Critical mistake? Didn't _we_ consciously choose a dictionary
for this?
> So
> I can write a Python class that does:
>
> import qmp
>
> qmp.eject_device(device='ide0-hd0')
>
> But I cannot write a library today that does:
>
> qmp.eject_device('ide0-hd0')
>
> Without using a library that has knowledge of the QMP schema. This is
> somewhat unfortunate and I've been thinking about adding another code
> generation mode in qmp-gen.py to generate wrappers that would enable the
> more natural usage in Python.
I don't get it. Having knowledge of the schema is needed anyway, isn't it?
> > Also, what's the problem with C consumers using QMP? Libvirt is C, and it
> > does it just fine.
>
> I was going to cut and paste libvirt's code that handles query-pci to
> show you how complex it is to use vs. just using libqmp
Does this suggest you think libvirt should switch from QMP to libqmp?
And, if you don't want to focus on non-C consumers, why having QMP at all?
(I'm *not* saying I'm ok in dropping it).
> but it turns out
> libvirt doesn't implement query-pci in QMP mode and falls back to the
> human monitor. I think that might be as good of a way to show my point
> though :-)
This comment assumes two things: 1. query-pci is good in its current form,
2. libvirt doesn't use it because it's complex.
We already had changes proposal to query-pci and I can't tell why libvirt
doesn't use it. If it's because it's complex, then we need to know why it's
complex: is it the command or is it accessing JSON from C?
> >> But the same sort of compatibility considerations apply to using QMP
> >> within QEMU. If you add a new field to a function call, we need to
> >> modify any internal usage of the API.
> > What's the problem of doing this?
>
> If we have a bunch of code in QEMU that relies on doing
>
> qmp_set_vnc_password(password, &err);
>
> And then we add a new parameter to set_vnc_password(), we now need to
> touch ever place we call this.
>
> Good external interfaces also happen to be good internal interfaces and
> encourage better modularity within QEMU itself. It also means that for
> subsystems that we can convert to be largely implemented in terms of
> QMP, we have the option to move them into an external process which is
> good from a security PoV.
I agree with the security PoV, but moving things to a separate process
is not an immediate step. IOW, there's no need to impose strict compatibilities
rules for something that's going to be internal for some (unknown) time.
> >> If we add a field to a structure,
> >> as long as we use feature flags (we do), only the places that care to
> >> set that field need to worry about it.
> > Why do we need this in an internal interface?
>
> It's about eating our own dog food. It helps us ensure that we're
> really providing high quality external interfaces.
We'll be eating our food just fine by using the internal interface
as an internal interface.
>
> Regards,
>
> Anthony Liguori
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 19:27 ` Luiz Capitulino
@ 2011-03-16 20:00 ` Anthony Liguori
2011-03-18 14:10 ` Luiz Capitulino
0 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-16 20:00 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel, Avi Kivity, Markus Armbruster, Adam Litke
On 03/16/2011 02:27 PM, Luiz Capitulino wrote:
>
>> You can design interfaces in Python that rely on variant arrays or
>> types, or that add keyword values to arguments, but the absence of those
>> does not make a Bad Library in Python.
>>
>> This has nothing to do with the need for bindings.
> I mentioned bindings because you put so much emphasis on C consumers
> that sometimes I wonder if all you want is libqmp and libqmp only.
The transports (libqmp and QMP) are less important than the interface
itself. My emphasis is on the API, not the transports or
materialization of it.
The terminology gets confusing and I probably am not consistent enough
in how I use it.
>> The need for
>> bindings is entirely based on whether the RPC being used is
>> self-describing. JSON is.
>>
>> That said, I think we made a critical mistake in QMP that practically
>> means that we need bindings for QMP. There is no argument ordering.
> I'm sorry? Critical mistake? Didn't _we_ consciously choose a dictionary
> for this?
Yes, we did. In fact, I'm fairly sure that Avi and/or I strongly
advocated it.
But hindsight is always 20/20 and if our goal is to have an API that
doesn't require special support in Python beyond a simple transport
class, using dictionaries and not allowing unnamed positional parameters
was a mistake.
But we makes lots of mistakes. That's part of the development process.
>> So
>> I can write a Python class that does:
>>
>> import qmp
>>
>> qmp.eject_device(device='ide0-hd0')
>>
>> But I cannot write a library today that does:
>>
>> qmp.eject_device('ide0-hd0')
>>
>> Without using a library that has knowledge of the QMP schema. This is
>> somewhat unfortunate and I've been thinking about adding another code
>> generation mode in qmp-gen.py to generate wrappers that would enable the
>> more natural usage in Python.
> I don't get it. Having knowledge of the schema is needed anyway, isn't it?
If we allowed positional arguments, it wouldn't be needed. For
instance, with xmlrpclib in python, you can do:
srv = ServerProxy('http://localhost:8000/RPC2')
srv.eject_device('ide0-hd0')
And it Just Works. The transport doesn't have to know anything about
the schema because the server does all of the parameter validation.
We could fix this by adding another way of specifying unnamed parameters
in QMP. Something like:
import qmp
srv = qmp.ServerProxy('/tmp/qmp.sock')
srv.eject_device('ide0-hd0')
Becomes:
{ 'execute': 'eject_device', 'ordered_arguments': [ 'ide0-hd0' ],
arguments: {} }
I'm not sure it's really worth it though. I don't think it's a terrible
thing for us to generate a Python consumable schema. I don't think it's
all that important for us to worry about QMP as an RPC mechanism being
consumable outside of QEMU which means that we can (and should) be
providing high level language libraries for it.
>>> Also, what's the problem with C consumers using QMP? Libvirt is C, and it
>>> does it just fine.
>> I was going to cut and paste libvirt's code that handles query-pci to
>> show you how complex it is to use vs. just using libqmp
> Does this suggest you think libvirt should switch from QMP to libqmp?
I think they eventually should, yes. Why duplicate all of the
marshalling and unmarshalling code unnecessarily especially when dealing
with complex data structures?
> And, if you don't want to focus on non-C consumers, why having QMP at all?
> (I'm *not* saying I'm ok in dropping it).
No matter what, we need an RPC. You can't drop QMP because something
needs to take it's place.
The question is whether we expect projects to write directly to the
protocol or use libraries provided by use to handle it. I don't think
it's practical to expect everyone to write directly to the protocol
(particularly when dealing with C).
>> but it turns out
>> libvirt doesn't implement query-pci in QMP mode and falls back to the
>> human monitor. I think that might be as good of a way to show my point
>> though :-)
> This comment assumes two things: 1. query-pci is good in its current form,
I think it's one of our better commands actually.
> 2. libvirt doesn't use it because it's complex.
>
> We already had changes proposal to query-pci and I can't tell why libvirt
> doesn't use it. If it's because it's complex, then we need to know why it's
> complex: is it the command or is it accessing JSON from C?
It returns a complex data structure. There's nothing wrong with that,
but it's very hard to interface with at the JSON level. Is there really
any doubt about that?
>>>> If we add a field to a structure,
>>>> as long as we use feature flags (we do), only the places that care to
>>>> set that field need to worry about it.
>>> Why do we need this in an internal interface?
>> It's about eating our own dog food. It helps us ensure that we're
>> really providing high quality external interfaces.
> We'll be eating our food just fine by using the internal interface
> as an internal interface.
What I struggle with it what it means for you to say that libqmp is an
internal interface. libqmp is a C library that implements QMP. QEMU
would never use libqmp directly (it doesn't need to). If it cannot
maintain a stable ABI, than that means it's impossible to write a QMP
client that maintains a stable C ABI unless you totally redefine the
interfaces.
Is that really desirable?
Regards,
Anthony Liguori
>> Regards,
>>
>> Anthony Liguori
>>
>>
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 20:00 ` Anthony Liguori
@ 2011-03-18 14:10 ` Luiz Capitulino
2011-03-18 14:22 ` Anthony Liguori
0 siblings, 1 reply; 49+ messages in thread
From: Luiz Capitulino @ 2011-03-18 14:10 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Avi Kivity, Markus Armbruster, Adam Litke
On Wed, 16 Mar 2011 15:00:10 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:
> >> That said, I think we made a critical mistake in QMP that practically
> >> means that we need bindings for QMP. There is no argument ordering.
> > I'm sorry? Critical mistake? Didn't _we_ consciously choose a dictionary
> > for this?
>
> Yes, we did. In fact, I'm fairly sure that Avi and/or I strongly
> advocated it.
>
> But hindsight is always 20/20 and if our goal is to have an API that
> doesn't require special support in Python beyond a simple transport
> class, using dictionaries and not allowing unnamed positional parameters
> was a mistake.
>
> But we makes lots of mistakes. That's part of the development process.
IMO, what's happening here is that you want (or your focus is in) a
different thing.
Since the beginning we (Markus and I) have focused on having a flexible
wire interface. In its most part, this requirement came from Avi (please
Avi, correct me if I'm wrong), but of course that I agreed with it.
You've said many times that you don't value non-C consumers that much,
while our focus until today has been on higher level clients.
There's a clear conflict/contradiction here, and I don't feel I can keep
discussing this anymore. So, apart from trying to help stabilizing the QAPI,
the best I can do is to pass QMP maintenance over to you and you and Avi
(as QEMU maintainers) decide what to do from here on.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-18 14:10 ` Luiz Capitulino
@ 2011-03-18 14:22 ` Anthony Liguori
0 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-18 14:22 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: Markus Armbruster, Adam Litke, qemu-devel, Avi Kivity
On 03/18/2011 09:10 AM, Luiz Capitulino wrote:
>
> IMO, what's happening here is that you want (or your focus is in) a
> different thing.
>
> Since the beginning we (Markus and I) have focused on having a flexible
> wire interface. In its most part, this requirement came from Avi (please
> Avi, correct me if I'm wrong), but of course that I agreed with it.
>
> You've said many times that you don't value non-C consumers that much,
> while our focus until today has been on higher level clients.
I realize that I've not been careful enough in my language. The
problems I'm trying to address are just as applicable to non-C clients.
In fact, I've written a Python client library that makes uses code
generation:
http://repo.or.cz/w/qemu/aliguori.git/commit/a101de2ff0781795f4f7c64ece6e3d48b5783627
It's much simpler than the C interfaces because Python has a bit more
awesomeness than C.
The only properties of the Python interface that are different than the
C is that adding optional parameters to a command handler works okay,
but adding optional parameters to a signal still will not work.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-16 15:59 ` Anthony Liguori
2011-03-16 18:09 ` Luiz Capitulino
@ 2011-03-17 12:21 ` Kevin Wolf
2011-03-17 12:46 ` Anthony Liguori
1 sibling, 1 reply; 49+ messages in thread
From: Kevin Wolf @ 2011-03-17 12:21 UTC (permalink / raw)
To: Anthony Liguori
Cc: Markus Armbruster, Avi Kivity, Adam Litke, qemu-devel,
Luiz Capitulino
Am 16.03.2011 16:59, schrieb Anthony Liguori:
> On 03/16/2011 09:34 AM, Luiz Capitulino wrote:
>> On Fri, 11 Mar 2011 17:05:30 -0600
>> Anthony Liguori<aliguori@us.ibm.com> wrote:
>>
>>> For more information about the background of QAPI, see
>>> http://wiki.qemu.org/Features/QAPI
>>>
>>> This series depends on 'QAPI Round 0' which I posted earlier.
>>>
>>> Since v2, the major changes are:
>>>
>>> - Switch to a multiline code emitter to ease readability
>>> - Use named parameters for escape sequences
>>> - Add support for proxy commands
>>> - Add support for asynchronous commands
>>>
>>> This version still adds a -qmp2 option. This is the only practical way I know
>>> to have testable code while not merging 200 patches all at once.
>> I've started reviewing this and my first impression is that this seems to be
>> real good. However, there's a lot of code here and some parts of it are a bit
>> complicated, so I need more time to do a thorough review and testing.
>>
>> Having said that, my only immediate concern is weather this will have any
>> negative side effects on the wire protocol, today or in the future.
>>
>> I mean, a C library has different extensibility constraints and functionality
>> requirements than a high-level protocol and tying/mixing the two can have
>> bad side effects, like this small one (patch 12/15):
>
> C library is not quite as important as C interface. I want QMP to be an
> interface that we consume internally because that will make QMP a strong
> external interface.
>
> A fundamental design characteristic for me is that first and foremost,
> QMP should be a good C interface and that the wire representation should
> be easy to support in a good C interface.
>
> This is a shift in our direction but the good news is that the practical
> impact is small. But I don't think there's a lot of value of focusing
> on non-C consumers because any non-C consumer is capable of consuming a
> good C interface (but the inverse is not true).
>
>> +##
>> +# @put_event:
>> +#
>> +# Disconnect a signal. This command is used to disconnect from a signal based
>> +# on the handle returned by a signal accessor.
>> +#
>> +# @tag: the handle returned by a signal accessor.
>> +#
>> +# Returns: Nothing on success.
>> +# If @tag is not a valid handle, InvalidParameterValue
>> +#
>> +# Since: 0.15.0
>>
>> The name 'signal' (at least today) doesn't make sense on the wire protocol,
>> 'put_event' probably doesn't make sense in the C library, nor does 'event'.
>
> I tried very hard to make events useful without changing the wire
> protocol significantly but I've failed there.
>
> I've got a new proposal for handling events that introduces the concept
> of a signal on the wire and the notion of connecting and disconnecting
> from signals.
>
>> Another detail is that, event extension is more important than command
>> extension, because it's probably going to happen. I think it would be very
>> bad to add new events just because we wanted to add a new field.
>
> The way this is typically handled is that signals tend to pass
> structures instead of lots of fields. For instance, most of the GDK
> events just pass a structure for the event (like GdkButtonEvent).
Can we do that with existing events or would we break the external
interface because we'd have to nest everything one level deeper?
Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-17 12:21 ` Kevin Wolf
@ 2011-03-17 12:46 ` Anthony Liguori
2011-03-17 13:15 ` Kevin Wolf
0 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-17 12:46 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-devel, Luiz Capitulino, Adam Litke, Markus Armbruster,
Avi Kivity
On 03/17/2011 07:21 AM, Kevin Wolf wrote:
>>
>>> Another detail is that, event extension is more important than command
>>> extension, because it's probably going to happen. I think it would be very
>>> bad to add new events just because we wanted to add a new field.
>> The way this is typically handled is that signals tend to pass
>> structures instead of lots of fields. For instance, most of the GDK
>> events just pass a structure for the event (like GdkButtonEvent).
> Can we do that with existing events or would we break the external
> interface because we'd have to nest everything one level deeper?
We have to introduce new versions of existing events anyway so we can
make sure to nest the structures appropriately. I think BLOCK_IO_ERROR
is the only one that isn't doing this today FWIW.
Regards,
Anthony Liguori
> Kevin
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-17 12:46 ` Anthony Liguori
@ 2011-03-17 13:15 ` Kevin Wolf
2011-03-17 13:28 ` Anthony Liguori
0 siblings, 1 reply; 49+ messages in thread
From: Kevin Wolf @ 2011-03-17 13:15 UTC (permalink / raw)
To: Anthony Liguori
Cc: qemu-devel, Luiz Capitulino, Adam Litke, Markus Armbruster,
Avi Kivity
Am 17.03.2011 13:46, schrieb Anthony Liguori:
> On 03/17/2011 07:21 AM, Kevin Wolf wrote:
>>>
>>>> Another detail is that, event extension is more important than command
>>>> extension, because it's probably going to happen. I think it would be very
>>>> bad to add new events just because we wanted to add a new field.
>>> The way this is typically handled is that signals tend to pass
>>> structures instead of lots of fields. For instance, most of the GDK
>>> events just pass a structure for the event (like GdkButtonEvent).
>> Can we do that with existing events or would we break the external
>> interface because we'd have to nest everything one level deeper?
>
> We have to introduce new versions of existing events anyway so we can
> make sure to nest the structures appropriately. I think BLOCK_IO_ERROR
> is the only one that isn't doing this today FWIW.
But then we must always send both events in order to maintain
compatibility, right? That sucks.
If I understand right, the problem with the current events isn't even on
the protocol level, which would be visible externally, but just that it
doesn't map to the C interface in the way you like. Is there a reason to
change the events from a wire protocol perspective?
Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-17 13:15 ` Kevin Wolf
@ 2011-03-17 13:28 ` Anthony Liguori
2011-03-17 14:04 ` Kevin Wolf
0 siblings, 1 reply; 49+ messages in thread
From: Anthony Liguori @ 2011-03-17 13:28 UTC (permalink / raw)
To: Kevin Wolf
Cc: Markus Armbruster, Avi Kivity, Adam Litke, qemu-devel,
Luiz Capitulino
On 03/17/2011 08:15 AM, Kevin Wolf wrote:
> Am 17.03.2011 13:46, schrieb Anthony Liguori:
>> On 03/17/2011 07:21 AM, Kevin Wolf wrote:
>>>>> Another detail is that, event extension is more important than command
>>>>> extension, because it's probably going to happen. I think it would be very
>>>>> bad to add new events just because we wanted to add a new field.
>>>> The way this is typically handled is that signals tend to pass
>>>> structures instead of lots of fields. For instance, most of the GDK
>>>> events just pass a structure for the event (like GdkButtonEvent).
>>> Can we do that with existing events or would we break the external
>>> interface because we'd have to nest everything one level deeper?
>> We have to introduce new versions of existing events anyway so we can
>> make sure to nest the structures appropriately. I think BLOCK_IO_ERROR
>> is the only one that isn't doing this today FWIW.
> But then we must always send both events in order to maintain
> compatibility, right? That sucks.
No, it's more complicated than that unfortunately. The old events are
"broadcast events". The new event/signal model requires explicit
registration. There is a capabilities negotiation feature that let's us
disable broadcast events.
So from the wire perspective, a newer client will never see/care about
broadcast events.
> If I understand right, the problem with the current events isn't even on
> the protocol level, which would be visible externally,
No, the problem with the old events is that they aren't
registered/maskable. So even if you don't care about BLOCK_IO_ERROR,
you're getting the notification. Plus, we'd like to add the ability to
add a tag to events when we register them.
The other problem is that events are all global today. BLOCK_IO_ERROR
is a good example of this. It's really an error that's specific to a
block device and it passes the name of the block device that it's
specific to as an argument. But if we have a masking mechanism it could
only globally enable/disable BLOCK_IO_ERROR for all devices.
It would be much nicer to be able to enable the event for specific block
devices. This requires some protocol visible changes. I'm still
writing up these changes but hope to have something for review soon.
> but just that it
> doesn't map to the C interface in the way you like.
I think I've maybe been using "C interface" to much. The current event
wire protocol doesn't map to any client interface well.
Regards,
Anthony Liguori
> Is there a reason to
> change the events from a wire protocol perspective?
>
> Kevin
>
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-17 13:28 ` Anthony Liguori
@ 2011-03-17 14:04 ` Kevin Wolf
2011-03-17 15:49 ` Anthony Liguori
0 siblings, 1 reply; 49+ messages in thread
From: Kevin Wolf @ 2011-03-17 14:04 UTC (permalink / raw)
To: Anthony Liguori
Cc: Markus Armbruster, Avi Kivity, Adam Litke, qemu-devel,
Luiz Capitulino
Am 17.03.2011 14:28, schrieb Anthony Liguori:
> On 03/17/2011 08:15 AM, Kevin Wolf wrote:
>> Am 17.03.2011 13:46, schrieb Anthony Liguori:
>>> On 03/17/2011 07:21 AM, Kevin Wolf wrote:
>>>>>> Another detail is that, event extension is more important than command
>>>>>> extension, because it's probably going to happen. I think it would be very
>>>>>> bad to add new events just because we wanted to add a new field.
>>>>> The way this is typically handled is that signals tend to pass
>>>>> structures instead of lots of fields. For instance, most of the GDK
>>>>> events just pass a structure for the event (like GdkButtonEvent).
>>>> Can we do that with existing events or would we break the external
>>>> interface because we'd have to nest everything one level deeper?
>>> We have to introduce new versions of existing events anyway so we can
>>> make sure to nest the structures appropriately. I think BLOCK_IO_ERROR
>>> is the only one that isn't doing this today FWIW.
>> But then we must always send both events in order to maintain
>> compatibility, right? That sucks.
>
> No, it's more complicated than that unfortunately. The old events are
> "broadcast events". The new event/signal model requires explicit
> registration. There is a capabilities negotiation feature that let's us
> disable broadcast events.
>
> So from the wire perspective, a newer client will never see/care about
> broadcast events.
Right, it can disable them (i.e. some events are registered by default
and there's a command/capability that unregisters all events). But
what's the problem if the client re-enables one of these events by
registering for it?
Adding new events means that we need to have code to generate (that's
what I meant when I said "send") both events for a single action.
Especially if we happen to do it again in the future, this is going to
get really ugly.
>> If I understand right, the problem with the current events isn't even on
>> the protocol level, which would be visible externally,
>
> No, the problem with the old events is that they aren't
> registered/maskable. So even if you don't care about BLOCK_IO_ERROR,
> you're getting the notification. Plus, we'd like to add the ability to
> add a tag to events when we register them.
What's the problem with registering them? If you want to stop client
from doing this you must introduce a special case for obsolete events
that cannot be registered. Do we gain anything from this?
> The other problem is that events are all global today. BLOCK_IO_ERROR
> is a good example of this. It's really an error that's specific to a
> block device and it passes the name of the block device that it's
> specific to as an argument. But if we have a masking mechanism it could
> only globally enable/disable BLOCK_IO_ERROR for all devices.
>
> It would be much nicer to be able to enable the event for specific block
> devices. This requires some protocol visible changes. I'm still
> writing up these changes but hope to have something for review soon.
I wonder if the old, more generic event couldn't be generated
automatically if the more specific signal is triggered in the block code.
>> but just that it
>> doesn't map to the C interface in the way you like.
>
> I think I've maybe been using "C interface" to much. The current event
> wire protocol doesn't map to any client interface well.
If you mean their broadcast style, that's not really related to nesting
or struct vs. argument.
Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
2011-03-17 14:04 ` Kevin Wolf
@ 2011-03-17 15:49 ` Anthony Liguori
0 siblings, 0 replies; 49+ messages in thread
From: Anthony Liguori @ 2011-03-17 15:49 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-devel, Luiz Capitulino, Adam Litke, Markus Armbruster,
Avi Kivity
On 03/17/2011 09:04 AM, Kevin Wolf wrote:
>
>> No, the problem with the old events is that they aren't
>> registered/maskable. So even if you don't care about BLOCK_IO_ERROR,
>> you're getting the notification. Plus, we'd like to add the ability to
>> add a tag to events when we register them.
> What's the problem with registering them?
You could add a simple mask/unmask event to the current protocol. But
that doesn't let you filter events for a given object. As I mentioned
in the previous note, I would like to be able to only receive
BLOCK_IO_ERROR for certain devices.
> If you want to stop client
> from doing this you must introduce a special case for obsolete events
> that cannot be registered. Do we gain anything from this?
Actually, the old style events are distinct from the new signals. They
cannot be registered/unregistered. My intention is for new clients to
just ignore that they exist.
>> The other problem is that events are all global today. BLOCK_IO_ERROR
>> is a good example of this. It's really an error that's specific to a
>> block device and it passes the name of the block device that it's
>> specific to as an argument. But if we have a masking mechanism it could
>> only globally enable/disable BLOCK_IO_ERROR for all devices.
>>
>> It would be much nicer to be able to enable the event for specific block
>> devices. This requires some protocol visible changes. I'm still
>> writing up these changes but hope to have something for review soon.
> I wonder if the old, more generic event couldn't be generated
> automatically if the more specific signal is triggered in the block code.
Yes, this is what I'm going to try to do. This is already how the
patches I sent out work but now that I have the new signal model, it
requires a little more tweaking.
But this is the advantage of being able to consume interfaces within
QEMU. As long as the block layer exposes a signal mechanism that
provides the same information, we can hide the old style events entirely
within the QMP layer by registering for the new style signals and
created old style events.
>>> but just that it
>>> doesn't map to the C interface in the way you like.
>> I think I've maybe been using "C interface" to much. The current event
>> wire protocol doesn't map to any client interface well.
> If you mean their broadcast style, that's not really related to nesting
> or struct vs. argument.
I meant structs vs. arguments too.
I'd like to do:
import qmp
def io_error(blockdev, operation, action):
if operation == 'stop':
....
srv = qmp.ServerProxy(...)
srv.connect('IO_ERROR', io_error)
Python has the same problem re: arguments. If we add a new argument to
a signal, then it breaks the Python client library. At the same time,
when possible, it's nice to use argument syntax because the alternative is:
import qmp
def io_error(args):
if args['action'] == 'stop':
....
Which isn't quite as nice to me. So I don't want to preclude having the
ability to have a signal with a few arguments but we just have to be
careful that we're not goign to need to add more arguments later. If we
do, then we should make sure there's a structure there too.
Regards,
Anthony Liguori
> Kevin
>
^ permalink raw reply [flat|nested] 49+ messages in thread