Re: [Qemu-devel] [PATCH v3 5/9] QAPI: allow to specify valid runstates per command

[Igor Mammedov]

On Tue, 27 Feb 2018 16:10:31 -0600
Eric Blake <address@hidden> wrote:

> On 02/16/2018 06:37 AM, Igor Mammedov wrote:
> > Add optional 'runstates' parameter in QAPI command definition,
> > which will permit to specify RunState variations in which
> > a command could be exectuted via QMP monitor.  
> 
> s/exectuted/executed/
> 
> > 
> > For compatibility reasons, commands, that don't use  
> 
> s/commands,/commands/
> 
> > 'runstates' explicitly, are not permited to run in  
> 
> s/explicitly,/explicitly/
> s/permited/permitted/
> 
> > preconfig state but allowed in all other states.
> > 
> > New option will be used to allow commands, which are
> > prepared/need to run this early, to run in preconfig state.
> > It will include query-hotpluggable-cpus and new set-numa-node
> > commands. Other commands that should be able to run in
> > preconfig state, should be ammeded to not expect machine  
> 
> s/ammeded/amended/
> 
> > in initialized state.
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >   include/qapi/qmp/dispatch.h             |  5 +++-
> >   monitor.c                               | 28 +++++++++++++++++---
> >   qapi-schema.json                        | 12 +++++++--
> >   qapi/qmp-dispatch.c                     | 39 ++++++++++++++++++++++++++++
> >   qapi/qmp-registry.c                     |  4 ++-
> >   qapi/run-state.json                     |  6 ++++-
> >   scripts/qapi-commands.py                | 46 
> > ++++++++++++++++++++++++++-------
> >   scripts/qapi-introspect.py              |  2 +-
> >   scripts/qapi.py                         | 15 +++++++----
> >   scripts/qapi2texi.py                    |  2 +-
> >   tests/qapi-schema/doc-good.out          |  4 +--
> >   tests/qapi-schema/ident-with-escape.out |  2 +-
> >   tests/qapi-schema/indented-expr.out     |  4 +--
> >   tests/qapi-schema/qapi-schema-test.out  | 18 ++++++-------
> >   tests/qapi-schema/test-qapi.py          |  6 ++---
> >   15 files changed, 151 insertions(+), 42 deletions(-)  
> 
[...]

> Overall, this is adding a lot of complexity to QMP; are we sure this is 
> the interface libvirt wants to use for early NUMA configuration?  Can we 
> simplify it to just a new bool, similar to 'allow-oob'?
Being unsure if preconfig only specific knob would fly, I was trying
to introduce a generic mechanism that could be used to limit any command
to certain runstates.
 
I'd gladly to scrape it off and implement something like 'allow-oob'
if that's acceptable. It could be "preconfig-enabled" which defaults
to false and necessary commands would be explicitly marked as preconfig
enabled. Then I could just build into 'set-numa-node' C callback
an extra check to limit it only to preconfig state, like we typically
do in other handlers instead of going for more complex generic approach.
That would significantly simplify series.