| Message ID | 1332169763-30665-8-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 03/19/2012 09:09 AM, Anthony Liguori wrote: > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > qapi-schema.json | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > qemu-config.c | 51 +++++++++++++++++++++++++++++++++++++++++ > qmp-commands.hx | 5 ++++ > 3 files changed, 122 insertions(+), 0 deletions(-) > > diff --git a/qapi-schema.json b/qapi-schema.json > index 04fa84f..2a80ef6 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1663,3 +1663,69 @@ > { 'command': 'qom-list-types', > 'data': { '*implements': 'str', '*abstract': 'bool' }, > 'returns': [ 'ObjectTypeInfo' ] } > + > +## > +# @ConfigOptionType > +# > +# An enumeration describing the type of a configuration file entry. > +# > +# @string: a UTF-8 string > +# > +# @bool: either 'on' or 'off' > +# > +# @number: an integer > +# > +# @size: an integer followed by either 'K', 'M', 'G', or 'T'. Not quite - I found three different parsers, but none of them match this description. That is, qemu-option.c:parse_option_size() supports 'k' and 'b', as well as omitting a suffix, but does not support 'm'. Then there's cmd.c:cvtnum(), which is case-insensitive, and adds 'p' and 'e', but omits 'b'. Then there's the 'o' type in monitor.c that defers to cutils.c:strtosz(), which defaults to bytes but is case-insensitive and supports 'b' but not 'p'. Why are we using three different parsers, anyway? Please be sure that your documentation exactly matches the code used by the particular parser used for 'size', since it is is already confusing as-is. > +# > +# @help: an optional help description. This should not be parsed or relied upon > +# in any way. Its content's may change in future versions of QEMU. s/content's/contents/
On 03/19/2012 03:10 PM, Eric Blake wrote: > On 03/19/2012 09:09 AM, Anthony Liguori wrote: >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >> --- >> qapi-schema.json | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> qemu-config.c | 51 +++++++++++++++++++++++++++++++++++++++++ >> qmp-commands.hx | 5 ++++ >> 3 files changed, 122 insertions(+), 0 deletions(-) >> >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 04fa84f..2a80ef6 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -1663,3 +1663,69 @@ >> { 'command': 'qom-list-types', >> 'data': { '*implements': 'str', '*abstract': 'bool' }, >> 'returns': [ 'ObjectTypeInfo' ] } >> + >> +## >> +# @ConfigOptionType >> +# >> +# An enumeration describing the type of a configuration file entry. >> +# >> +# @string: a UTF-8 string >> +# >> +# @bool: either 'on' or 'off' >> +# >> +# @number: an integer >> +# >> +# @size: an integer followed by either 'K', 'M', 'G', or 'T'. > > Not quite - I found three different parsers, but none of them match this > description. Heh, I was going by the documentation in the code: QEMU_OPT_SIZE, /* size, accepts (K)ilo, (M)ega, (G)iga, (T)era postfix */ > That is, qemu-option.c:parse_option_size() supports 'k' > and 'b', as well as omitting a suffix, but does not support 'm'. TBH, I'd prefer to not document 'k' and 'b' and explicitly document that the suffix is optional and bytes is implied (which I thought was implied in my documentation, but I can see that it may not be). I'd rather we support officially support one way of doing things and if we support more, do it under the mantra of being liberal in what we accept. > Then > there's cmd.c:cvtnum(), which is case-insensitive, and adds 'p' and 'e', > but omits 'b'. Then there's the 'o' type in monitor.c that defers to > cutils.c:strtosz(), which defaults to bytes but is case-insensitive and > supports 'b' but not 'p'. Why are we using three different parsers, anyway? Yes, I am aware of that, and yes, I'm going to fix it. One bit at a time though. > Please be sure that your documentation exactly matches the code used by > the particular parser used for 'size', since it is is already confusing > as-is. Let's be careful about documenting what is the officially supported interface and what is possible with the given state of the code. >> +# >> +# @help: an optional help description. This should not be parsed or relied upon >> +# in any way. Its content's may change in future versions of QEMU. > > s/content's/contents/ Ack. Regards, Anthony Liguori
On 03/19/2012 02:19 PM, Anthony Liguori wrote: >>> +# @size: an integer followed by either 'K', 'M', 'G', or 'T'. >> >> Not quite - I found three different parsers, but none of them match this >> description. > > Heh, I was going by the documentation in the code: > > QEMU_OPT_SIZE, /* size, accepts (K)ilo, (M)ega, (G)iga, > (T)era postfix */ > > >> That is, qemu-option.c:parse_option_size() supports 'k' >> and 'b', as well as omitting a suffix, but does not support 'm'. > > TBH, I'd prefer to not document 'k' and 'b' and explicitly document that > the suffix is optional and bytes is implied (which I thought was implied > in my documentation, but I can see that it may not be). > > I'd rather we support officially support one way of doing things and if > we support more, do it under the mantra of being liberal in what we accept. Okay, I can live with documenting less than we accept, but we definitely need to mention that the suffix is optional, and that when omitted, the unit is bytes (as originally written, I read it that the suffix was mandatory, and that there was no way to request bytes). I'm not sure whether documenting an explicit 'b' for bytes helps or hurts, and I'm not sure whether adding support for 'p' and 'e' as documented suffixes makes sense. > >> Then >> there's cmd.c:cvtnum(), which is case-insensitive, and adds 'p' and 'e', >> but omits 'b'. Then there's the 'o' type in monitor.c that defers to >> cutils.c:strtosz(), which defaults to bytes but is case-insensitive and >> supports 'b' but not 'p'. Why are we using three different parsers, >> anyway? > > Yes, I am aware of that, and yes, I'm going to fix it. One bit at a > time though. Fair enough :) It's just that I recently fixed a bug in libvirt where it was using a human interface with no suffix and getting 'M' behavior, where an explicit 'B' behavior fixed things to use bytes, and I was quite put off by the inconsistencies and lack of documentation when I finally discovered that a 'B' suffix did what I wanted. I'm glad that QMP defaults to 'B', not 'M'. (Libvirt had its own fair share of inconsistent scaling routines, where some interfaces default to 'k' instead of bytes, and I recently consolidated libvirt to use a single scaling routing for much the same reasons.)
On 03/19/2012 03:31 PM, Eric Blake wrote: > On 03/19/2012 02:19 PM, Anthony Liguori wrote: >>>> +# @size: an integer followed by either 'K', 'M', 'G', or 'T'. >>> >>> Not quite - I found three different parsers, but none of them match this >>> description. >> >> Heh, I was going by the documentation in the code: >> >> QEMU_OPT_SIZE, /* size, accepts (K)ilo, (M)ega, (G)iga, >> (T)era postfix */ >> >> >>> That is, qemu-option.c:parse_option_size() supports 'k' >>> and 'b', as well as omitting a suffix, but does not support 'm'. >> >> TBH, I'd prefer to not document 'k' and 'b' and explicitly document that >> the suffix is optional and bytes is implied (which I thought was implied >> in my documentation, but I can see that it may not be). >> >> I'd rather we support officially support one way of doing things and if >> we support more, do it under the mantra of being liberal in what we accept. > > Okay, I can live with documenting less than we accept, but we definitely > need to mention that the suffix is optional, and that when omitted, the > unit is bytes (as originally written, I read it that the suffix was > mandatory, and that there was no way to request bytes). I'm not sure > whether documenting an explicit 'b' for bytes helps or hurts, and I'm > not sure whether adding support for 'p' and 'e' as documented suffixes > makes sense. I'm not really concerned about 'p' and 'e' for now. 'T' still seems novel enough to me :-) >>> Then >>> there's cmd.c:cvtnum(), which is case-insensitive, and adds 'p' and 'e', >>> but omits 'b'. Then there's the 'o' type in monitor.c that defers to >>> cutils.c:strtosz(), which defaults to bytes but is case-insensitive and >>> supports 'b' but not 'p'. Why are we using three different parsers, >>> anyway? >> >> Yes, I am aware of that, and yes, I'm going to fix it. One bit at a >> time though. > > Fair enough :) It's just that I recently fixed a bug in libvirt where > it was using a human interface with no suffix and getting 'M' behavior, > where an explicit 'B' behavior fixed things to use bytes, and I was > quite put off by the inconsistencies and lack of documentation when I > finally discovered that a 'B' suffix did what I wanted. Yup, and hopefully qapi-schema.json is fixing the documentation problem here. For what it's worth, my aim is to refactor the QemuOpts definitions to be part of qapi-schema.json too such that they carry the same level of documentation (and self consistency). This is the advantage of having a single centralized schema. I think it has really helped make sure everything is consistently documented and specified. Having disjoint definitions and descriptions has led to a lot of inconsistency over time. > I'm glad that > QMP defaults to 'B', not 'M'. (Libvirt had its own fair share of > inconsistent scaling routines, where some interfaces default to 'k' > instead of bytes, and I recently consolidated libvirt to use a single > scaling routing for much the same reasons.) You can thank Xen for the kb default :-) That goes back to the libvir days. Regards, Anthony Liguori
diff --git a/qapi-schema.json b/qapi-schema.json index 04fa84f..2a80ef6 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1663,3 +1663,69 @@ { 'command': 'qom-list-types', 'data': { '*implements': 'str', '*abstract': 'bool' }, 'returns': [ 'ObjectTypeInfo' ] } + +## +# @ConfigOptionType +# +# An enumeration describing the type of a configuration file entry. +# +# @string: a UTF-8 string +# +# @bool: either 'on' or 'off' +# +# @number: an integer +# +# @size: an integer followed by either 'K', 'M', 'G', or 'T'. +# +# Since: 1.1 +## +{ 'enum': 'ConfigOptionType' + 'data': [ 'string', 'bool', 'number', 'size' ] } + +## +# @ConfigOption +# +# An option within a configuration section. +# +# @name: the name of the configuration option +# +# @type: the type of the configuration option +# +# @help: an optional help description. This should not be parsed or relied upon +# in any way. Its content's may change in future versions of QEMU. +# +# Since: 1.1 +## +{ 'type': 'ConfigOption', + 'data': { 'name': 'str', + 'type': 'ConfigOptionType', + '*help': 'str' } } + +## +# @ConfigSection +# +# A section within a configuration file. +# +# @name: the section name +# +# @parameters: a list of supported parameters +# +# Since: 1.1 +## +{ 'type': 'ConfigSection', + 'data': { 'name': 'str', + 'parameters': [ 'ConfigOption' ] } } + +## +# @query-config-capabilities +# +# Returns the current set of capabilities supported by the various QEMU commands +# that work with configuration files. +# +# Returns: a list of @ConfigSections describe the currently supported +# configuration sections. +# +# Since: 1.1 +## +{ 'command': 'query-config-capabilities', + 'returns': [ 'ConfigSection' ] } diff --git a/qemu-config.c b/qemu-config.c index 5856e5a..447aad6 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -2,6 +2,7 @@ #include "qemu-error.h" #include "qemu-option.h" #include "qemu-config.h" +#include "qmp-commands.h" #include "hw/qdev.h" #include "gdbstub.h" #include "net.h" @@ -866,3 +867,53 @@ int qemu_read_config_file(const char *filename) { return qemu_config_parse(vm_config_groups, filename); } + +ConfigSectionList *qmp_query_config_capabilities(Error **errp) +{ + ConfigSectionList *lst = NULL; + int i; + + for (i = 0; vm_config_groups[i]; i++) { + QemuOptsList *olist = vm_config_groups[i]; + ConfigSection *s = g_malloc0(sizeof(*s)); + ConfigSectionList *e = g_malloc0(sizeof(*e)); + QemuOptDesc *desc; + + s->name = g_strdup(olist->name); + for (desc = olist->desc; desc->name; desc++) { + ConfigOption *o = g_malloc0(sizeof(*o)); + ConfigOptionList *oe = g_malloc0(sizeof(*oe)); + + o->name = g_strdup(desc->name); + switch (desc->type) { + case QEMU_OPT_STRING: + o->type = CONFIG_OPTION_TYPE_STRING; + break; + case QEMU_OPT_BOOL: + o->type = CONFIG_OPTION_TYPE_BOOL; + break; + case QEMU_OPT_NUMBER: + o->type = CONFIG_OPTION_TYPE_NUMBER; + break; + case QEMU_OPT_SIZE: + o->type = CONFIG_OPTION_TYPE_SIZE; + break; + } + + if (desc->help) { + o->has_help = true; + o->help = g_strdup(desc->help); + } + + oe->value = o; + oe->next = s->parameters; + s->parameters = oe; + } + + e->value = s; + e->next = lst; + lst = e; + } + + return lst; +} diff --git a/qmp-commands.hx b/qmp-commands.hx index dfe8a5b..8024995 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2125,3 +2125,8 @@ EQMP .args_type = "implements:s?,abstract:b?", .mhandler.cmd_new = qmp_marshal_input_qom_list_types, }, + { + .name = "query-config-capabilities", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_query_config_capabilities, + },
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- qapi-schema.json | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ qemu-config.c | 51 +++++++++++++++++++++++++++++++++++++++++ qmp-commands.hx | 5 ++++ 3 files changed, 122 insertions(+), 0 deletions(-)