| Message ID | 1299460984-15849-15-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Mar 7, 2011 at 1:22 AM, Anthony Liguori <aliguori@us.ibm.com> wrote: > diff --git a/qmp-schema.json b/qmp-schema.json > index e69de29..b343f5e 100644 > --- a/qmp-schema.json > +++ b/qmp-schema.json > @@ -0,0 +1,38 @@ > +# *-*- Mode: Python -*-* By the way JSON does not seem to support comments (neither /* */ nor #). So this comment feature you're using is an extension to JSON. I also can't see how to do multi-line strings in JSON so "doc-strings" aren't in the cards either. Stefan
On 03/07/2011 07:35 AM, Stefan Hajnoczi wrote: > On Mon, Mar 7, 2011 at 1:22 AM, Anthony Liguori<aliguori@us.ibm.com> wrote: > >> diff --git a/qmp-schema.json b/qmp-schema.json >> index e69de29..b343f5e 100644 >> --- a/qmp-schema.json >> +++ b/qmp-schema.json >> @@ -0,0 +1,38 @@ >> +# *-*- Mode: Python -*-* >> > By the way JSON does not seem to support comments (neither /* */ nor > #). So this comment feature you're using is an extension to JSON. Yeah, it's only loosely JSON as I don't use a JSON parser. Regards, Anthony Liguori > I > also can't see how to do multi-line strings in JSON so "doc-strings" > aren't in the cards either. > > Stefan > >
On 03/07/2011 03:41 PM, Anthony Liguori wrote: > On 03/07/2011 07:35 AM, Stefan Hajnoczi wrote: >> On Mon, Mar 7, 2011 at 1:22 AM, Anthony Liguori<aliguori@us.ibm.com> >> wrote: >>> diff --git a/qmp-schema.json b/qmp-schema.json >>> index e69de29..b343f5e 100644 >>> --- a/qmp-schema.json >>> +++ b/qmp-schema.json >>> @@ -0,0 +1,38 @@ >>> +# *-*- Mode: Python -*-* >> By the way JSON does not seem to support comments (neither /* */ nor >> #). So this comment feature you're using is an extension to JSON. > > Yeah, it's only loosely JSON as I don't use a JSON parser. Goes kind of against all the buzzwords you're letting fly here...
On 03/07/2011 03:22 AM, Anthony Liguori wrote: > This is used internally by QMP. It's also a pretty good example of a typical > command conversion. > > +## > +{ 'VersionInfo': {'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 > +## > +[ 'query-version', {}, {}, 'VersionInfo' ] (just picking on a patch that has a bit of schema in it) Something that can be added to the schema are default values for newly added parameters. This way we can avoid an explosion of commands where adding an optional parameter suffices; should be easier for the user to pick the right command and easier for us to document and support.
On 03/09/2011 07:28 AM, Avi Kivity wrote: > On 03/07/2011 03:41 PM, Anthony Liguori wrote: >> On 03/07/2011 07:35 AM, Stefan Hajnoczi wrote: >>> On Mon, Mar 7, 2011 at 1:22 AM, Anthony >>> Liguori<aliguori@us.ibm.com> wrote: >>>> diff --git a/qmp-schema.json b/qmp-schema.json >>>> index e69de29..b343f5e 100644 >>>> --- a/qmp-schema.json >>>> +++ b/qmp-schema.json >>>> @@ -0,0 +1,38 @@ >>>> +# *-*- Mode: Python -*-* >>> By the way JSON does not seem to support comments (neither /* */ nor >>> #). So this comment feature you're using is an extension to JSON. >> >> Yeah, it's only loosely JSON as I don't use a JSON parser. > > Goes kind of against all the buzzwords you're letting fly here... The schema defines arguments in a dictionary because in QMP, the argument order doesn't matter. But the argument order matters in C so I need to use a custom parser to preserve dictionary order. There's no way to do commenting in JSON and I really wanted to have inline documentation. But otherwise, it's valid JSON. Regards, Anthony Liguori
On 03/09/2011 03:44 PM, Anthony Liguori wrote: >>> Yeah, it's only loosely JSON as I don't use a JSON parser. >> >> Goes kind of against all the buzzwords you're letting fly here... > > > The schema defines arguments in a dictionary because in QMP, the > argument order doesn't matter. But the argument order matters in C so > I need to use a custom parser to preserve dictionary order. We could extend our parser to annotate the dictionary with the original order. Not worth it though. > > There's no way to do commenting in JSON and I really wanted to have > inline documentation. > > But otherwise, it's valid JSON. > We should then have a transformation that generates a valid json for clients to use. We could even include the documentation as a 'doc': key.
On 03/09/2011 07:36 AM, Avi Kivity wrote: > On 03/07/2011 03:22 AM, Anthony Liguori wrote: >> This is used internally by QMP. It's also a pretty good example of a >> typical >> command conversion. >> >> +## >> +{ 'VersionInfo': {'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 >> +## >> +[ 'query-version', {}, {}, 'VersionInfo' ] > > (just picking on a patch that has a bit of schema in it) If you want to see more of the schema in action http://repo.or.cz/w/qemu/aliguori.git/blob/refs/heads/glib:/qmp-schema.json > Something that can be added to the schema are default values for newly > added parameters. This way we can avoid an explosion of commands > where adding an optional parameter suffices; should be easier for the > user to pick the right command and easier for us to document and support. Adding a parameter to a command, even if the schema specifies a default value, is going to break the C library ABI so it's something we should strongly discourage. We definitely want to systematically document defaults but the question is whether that belongs in the documentation for the command or the schema itself. Since a default doesn't affect the wire protocol in any way, shape, or form, I think there a pretty strong argument that it belongs in the documentation and not the schema. gtk-doc typically uses a tag to identify defaults. I think it's something like #default although that is purely a marking concept (the value isn't parsed out or anything). Whether we'd want to automatically parse the value following the #default tag and change the internal signature is probably a discussion we can defer. Since this only really should apply to structures, I suspect it's not a huge win. Regards, Anthony Liguori
On 03/09/2011 07:51 AM, Avi Kivity wrote: > On 03/09/2011 03:44 PM, Anthony Liguori wrote: >>>> Yeah, it's only loosely JSON as I don't use a JSON parser. >>> >>> Goes kind of against all the buzzwords you're letting fly here... >> >> >> The schema defines arguments in a dictionary because in QMP, the >> argument order doesn't matter. But the argument order matters in C >> so I need to use a custom parser to preserve dictionary order. > > We could extend our parser to annotate the dictionary with the > original order. Not worth it though. > >> >> There's no way to do commenting in JSON and I really wanted to have >> inline documentation. >> >> But otherwise, it's valid JSON. >> > > We should then have a transformation that generates a valid json for > clients to use. We could even include the documentation as a 'doc': key. Yes, as I mentioned on the call, that's my plan for 0.15. I hadn't thought about the doc bit though. Regards, Anthony Liguori
On 03/09/2011 04:11 PM, Anthony Liguori wrote: >> (just picking on a patch that has a bit of schema in it) > > > If you want to see more of the schema in action > http://repo.or.cz/w/qemu/aliguori.git/blob/refs/heads/glib:/qmp-schema.json Thanks. Something a little worrying is the reliance on capitalization and punctuation ( {} vs [] ) do distinguish among the different types of declarations. It's not immediately clear if something is a type, event, or command. We could do [ { 'type': 'MyType', fields: [['a', 'str'], ['b', 'int'], ['c', 'AnotherType']] } { 'event': 'MY_EVENT', 'arguments': [ ... ] } { 'command': 'my-command', 'arguments': [ ... ], 'return': ... } ] which leaves us room for additional metainformation. The concern is more about non-qemu consumers of the schema. > >> Something that can be added to the schema are default values for >> newly added parameters. This way we can avoid an explosion of >> commands where adding an optional parameter suffices; should be >> easier for the user to pick the right command and easier for us to >> document and support. > > Adding a parameter to a command, even if the schema specifies a > default value, is going to break the C library ABI so it's something > we should strongly discourage. We could add compatibility signatures when we extend a command: { 'command': 'x', arguments: [['a', 'str']], return: ..., 'signatures': { 'x': [], 'x2': ['a'] } } That lets the wire protocol extend x without introducing a new command, but for libqmp it adds a new x2() API with the new parameter. > > We definitely want to systematically document defaults but the > question is whether that belongs in the documentation for the command > or the schema itself. Since a default doesn't affect the wire > protocol in any way, shape, or form, I think there a pretty strong > argument that it belongs in the documentation and not the schema. Agree. > > gtk-doc typically uses a tag to identify defaults. I think it's > something like #default although that is purely a marking concept (the > value isn't parsed out or anything). Whether we'd want to > automatically parse the value following the #default tag and change > the internal signature is probably a discussion we can defer. Since > this only really should apply to structures, I suspect it's not a huge > win.
On 03/09/2011 08:37 AM, Avi Kivity wrote: > On 03/09/2011 04:11 PM, Anthony Liguori wrote: >>> (just picking on a patch that has a bit of schema in it) >> >> >> If you want to see more of the schema in action >> http://repo.or.cz/w/qemu/aliguori.git/blob/refs/heads/glib:/qmp-schema.json > > Thanks. Something a little worrying is the reliance on capitalization > and punctuation ( {} vs [] ) do distinguish among the different types > of declarations. It's not immediately clear if something is a type, > event, or command. > > We could do > > [ > > { 'type': 'MyType', fields: [['a', 'str'], ['b', 'int'], ['c', > 'AnotherType']] } > { 'event': 'MY_EVENT', 'arguments': [ ... ] } > { 'command': 'my-command', 'arguments': [ ... ], 'return': ... } > > ] > > which leaves us room for additional metainformation. > > The concern is more about non-qemu consumers of the schema. Yeah, I dislike it too and I've been leaning towards changing it. My preference would be: { 'type': 'MyType', 'fields': { 'a': 'str', 'b': 'int', 'c': 'AnotherType' } } { 'event': 'MY_EVENT', 'arguments': {...} } { 'command': 'my-command', 'arguments': {...}, 'returns': 'int' } I do prefer the dictionary syntax for arguments over a list because a list implies order. Plus I think the syntax is just awkward and a whole lot easier to get wrong (too many/few elements in list). I don't think I want to make this sort of change just yet. Also note that the schema that will be exposed over the wire is not directly related to the schema we use for code generation. >>> Something that can be added to the schema are default values for >>> newly added parameters. This way we can avoid an explosion of >>> commands where adding an optional parameter suffices; should be >>> easier for the user to pick the right command and easier for us to >>> document and support. >> >> Adding a parameter to a command, even if the schema specifies a >> default value, is going to break the C library ABI so it's something >> we should strongly discourage. > > We could add compatibility signatures when we extend a command: > > > { 'command': 'x', arguments: [['a', 'str']], return: ..., > 'signatures': { 'x': [], 'x2': ['a'] } } > > That lets the wire protocol extend x without introducing a new > command, but for libqmp it adds a new x2() API with the new parameter. I'd rather just not add arguments. Treating QMP as a C API makes it easier for us to maintain compatibility both internally and externally. Regards, Anthony Liguori
On 03/09/2011 04:47 PM, Anthony Liguori wrote: >> [ >> >> { 'type': 'MyType', fields: [['a', 'str'], ['b', 'int'], ['c', >> 'AnotherType']] } >> { 'event': 'MY_EVENT', 'arguments': [ ... ] } >> { 'command': 'my-command', 'arguments': [ ... ], 'return': ... } >> >> ] >> >> which leaves us room for additional metainformation. >> >> The concern is more about non-qemu consumers of the schema. > > > Yeah, I dislike it too and I've been leaning towards changing it. > > My preference would be: > > { 'type': 'MyType', 'fields': { 'a': 'str', 'b': 'int', 'c': > 'AnotherType' } } > { 'event': 'MY_EVENT', 'arguments': {...} } > { 'command': 'my-command', 'arguments': {...}, 'returns': 'int' } > > I do prefer the dictionary syntax for arguments over a list because a > list implies order. Plus I think the syntax is just awkward and a > whole lot easier to get wrong (too many/few elements in list). Yeah. We can rationalize it by saying that most dynamic consumers of the schema will not care about argument order, and that if they do, they can implement a custom parser. > I don't think I want to make this sort of change just yet. Also note > that the schema that will be exposed over the wire is not directly > related to the schema we use for code generation. Right, we have to nail down the format for the former, though.
On 03/10/2011 02:41 PM, Avi Kivity wrote: >> I don't think I want to make this sort of change just yet. Also note >> that the schema that will be exposed over the wire is not directly >> related to the schema we use for code generation. > > Right, we have to nail down the format for the former, though. > btw. Back in the day when json was proposed vs. a custom text-line oriented protocol, it beat established RPCs because they were all so cumbersome with IDLs and code generation and general heavyweightness. And now we're making our json-rpc exactly like that. There's a moral in there somewhere.
On 03/10/2011 06:41 AM, Avi Kivity wrote: >> My preference would be: >> >> { 'type': 'MyType', 'fields': { 'a': 'str', 'b': 'int', 'c': >> 'AnotherType' } } >> { 'event': 'MY_EVENT', 'arguments': {...} } >> { 'command': 'my-command', 'arguments': {...}, 'returns': 'int' } >> >> I do prefer the dictionary syntax for arguments over a list because a >> list implies order. Plus I think the syntax is just awkward and a >> whole lot easier to get wrong (too many/few elements in list). > > > Yeah. We can rationalize it by saying that most dynamic consumers of > the schema will not care about argument order, and that if they do, > they can implement a custom parser. > >> I don't think I want to make this sort of change just yet. Also note >> that the schema that will be exposed over the wire is not directly >> related to the schema we use for code generation. > > Right, we have to nail down the format for the former, though. There's also enum syntax which I guess would be: { 'enum': 'MyEnum', values: [ 'value1', 'value2', 'value3' ] } The other thing that I'm delaying until post 0.15 is errors. I'd like to define errors as part of the schema. Something like: { 'error': 'FileNotFound', 'arguments': { 'filename': 'str' } } Regards, Anthony Liguori
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..b343f5e 100644 --- a/qmp-schema.json +++ b/qmp-schema.json @@ -0,0 +1,38 @@ +# *-*- Mode: Python -*-* +### 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 +## +{ 'VersionInfo': {'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 +## +[ 'query-version', {}, {}, '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; +} +
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>