On Fri, Apr 27, 2018 at 05:05:30PM -0500, Eric Blake wrote:
[...]
> > diff --git a/monitor.c b/monitor.c
> > index 0ffdf1d..e5e60dc 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1183,7 +1183,7 @@ static void monitor_init_qmp_commands(void)
> >
> > qmp_register_command(&qmp_commands, "query-qmp-schema",
> > qmp_query_qmp_schema,
> > - QCO_NO_OPTIONS);
> > + QCO_ALLOWED_IN_PRECONFIG);
>
> I understand why query-qmp-schema is special cased (because it uses
> 'gen':false in QAPI), but...
>
> > qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
> > QCO_NO_OPTIONS);
> > qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add,
> > @@ -1193,7 +1193,8 @@ static void monitor_init_qmp_commands(void)
> >
> > QTAILQ_INIT(&qmp_cap_negotiation_commands);
> > qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
> > - qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS);
> > + qmp_marshal_qmp_capabilities,
> > + QCO_ALLOWED_IN_PRECONFIG);
>
> ...why are we still special-casing the registration of qmp_capabilities
> here...
[1]
>
> > }
> >
> > static bool qmp_cap_enabled(Monitor *mon, QMPCapability cap)
> > diff --git a/qapi/introspect.json b/qapi/introspect.json
> > index c7f67b7..8036154 100644
> > --- a/qapi/introspect.json
> > +++ b/qapi/introspect.json
> > @@ -262,13 +262,16 @@
> > # @allow-oob: whether the command allows out-of-band execution.
> > # (Since: 2.12)
> > #
> > +# @allowed-in-preconfig: command can be executed in preconfig runstate,
> > +# default: 'false' (Since 2.13)
> > +#
>
> If we like the bikeshedding on the QAPI spelling, I think it also
> applies to the introspection spelling.
>
>
> > +++ b/qapi/misc.json
> > @@ -35,7 +35,8 @@
> > #
> > ##
> > { 'command': 'qmp_capabilities',
> > - 'data': { '*enable': [ 'QMPCapability' ] } }
> > + 'data': { '*enable': [ 'QMPCapability' ] },
> > + 'allowed-in-preconfig': true }
>
> ...should't this be good enough to get qmp_capabilities registered? Hmm
> - maybe that's an independent cleanup (and it might be missed fallout
> from Peter Xu's OOB work).
My understanding is that we have two lists:
QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
And here it only registers with qmp_commands list via:
qmp_init_marshal(&qmp_commands);
But not for the other one, which is explicitly registered at [1]. So
it seems that [1] is still needed?
Thanks,
--
Peter Xu
