On Fri, Sep 15, 2017 at 04:55:51PM +0100, Dr. David Alan Gilbert wrote: > * Peter Xu (pet...@redhat.com) wrote: > > Having "allow-oob" to true for a command does not mean that this command > > will always be run in out-of-band mode. The out-of-band quick path will > > only be executed if we specify the extra "run-oob" flag when sending the > > QMP request: > > > > { "execute": "command-that-allows-oob", > > "arguments": { ... }, > > "control": { "run-oob": true } } > > > > The "control" key is introduced to store this extra flag. "control" > > field is used to store arguments that are shared by all the commands, > > rather than command specific arguments. Let "run-oob" be the first. > > > > Signed-off-by: Peter Xu <pet...@redhat.com> > > I don't understand how this enforces the allowoob, that is it stops > other commands being called with run-oob=true
Here's what I thought: OOB commands are executed directly in the parser, and currently we only have one single parser/IO thread, then we can't have two oob commands in parallel, can we? Say, if we got one OOB command, we won't handle anything else (no matter OOB or non-OOB) before we finished processing that OOB command. > > > --- > > docs/devel/qapi-code-gen.txt | 10 ++++++++++ > > include/qapi/qmp/dispatch.h | 1 + > > monitor.c | 11 +++++++++++ > > qapi/qmp-dispatch.c | 34 ++++++++++++++++++++++++++++++++++ > > trace-events | 2 ++ > > 5 files changed, 58 insertions(+) > > > > diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt > > index 61fa167..47d16bb 100644 > > --- a/docs/devel/qapi-code-gen.txt > > +++ b/docs/devel/qapi-code-gen.txt > > @@ -665,6 +665,16 @@ allowed to run out-of-band can also be introspected > > using > > query-qmp-schema command. Please see the section "Client JSON > > Protocol introspection" for more information. > > > > +To execute a command in out-of-band way, we need to specify the > > +"control" field in the request, with "run-oob" set to true. Example: > > + > > + => { "execute": "command-support-oob", > > + "arguments": { ... }, > > + "control": { "run-oob": true } } > > + <= { "return": { } } > > + > > +Without it, even the commands that supports out-of-band execution will > > +still be run in-band. > > > > === Events === > > > > diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h > > index b767988..ee2b8ce 100644 > > --- a/include/qapi/qmp/dispatch.h > > +++ b/include/qapi/qmp/dispatch.h > > @@ -49,6 +49,7 @@ bool qmp_command_is_enabled(const QmpCommand *cmd); > > const char *qmp_command_name(const QmpCommand *cmd); > > bool qmp_has_success_response(const QmpCommand *cmd); > > QObject *qmp_build_error_object(Error *err); > > +bool qmp_is_oob(const QObject *request); > > > > typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque); > > > > diff --git a/monitor.c b/monitor.c > > index 599ea36..cb96204 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -3928,6 +3928,7 @@ static void monitor_qmp_bh_dispatcher(void *data) > > if (!req_obj) { > > break; > > } > > + trace_monitor_qmp_cmd_in_band(qobject_get_str(req_obj->id)); > > monitor_qmp_dispatch_one(req_obj); > > } > > } > > @@ -3963,6 +3964,16 @@ static void handle_qmp_command(JSONMessageParser > > *parser, GQueue *tokens, > > req_obj->id = id; > > req_obj->req = req; > > > > + if (qmp_is_oob(req)) { > > + /* > > + * Trigger fast-path to handle the out-of-band request, by > > + * executing the command directly in parser. > > + */ > > + trace_monitor_qmp_cmd_out_of_band(qobject_get_str(req_obj->id)); > > + monitor_qmp_dispatch_one(req_obj); > > + return; > > + } > > I wonder if there is a way to run all allowoob commands in this way; > the only difference being if they're not started with run-oob > you wiat for completion of !oob commands. > That way the allowoob commands are always run from the same > thread/context which feels like it should simplify them. Maybe I misread the comment... Even with current patch, all OOB commands will be run from the same thread/context (which is the newly created parser thread), right? Thanks, -- Peter Xu