| Message ID | 20200909151149.490589-11-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | monitor: Optionally run handlers in coroutines | expand |
* Kevin Wolf (kwolf@redhat.com) wrote: > Often, QMP command handlers are not only called to handle QMP commands, > but also from a corresponding HMP command handler. In order to give them > a consistent environment, optionally run HMP command handlers in a > coroutine, too. > > The implementation is a lot simpler than in QMP because for HMP, we > still block the VM while the coroutine is running. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > monitor/monitor-internal.h | 1 + > monitor/hmp.c | 37 ++++++++++++++++++++++++++++++++----- > 2 files changed, 33 insertions(+), 5 deletions(-) > > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h > index b55d6df07f..ad2e64be13 100644 > --- a/monitor/monitor-internal.h > +++ b/monitor/monitor-internal.h > @@ -74,6 +74,7 @@ typedef struct HMPCommand { > const char *help; > const char *flags; /* p=preconfig */ > void (*cmd)(Monitor *mon, const QDict *qdict); > + bool coroutine; > /* > * @sub_table is a list of 2nd level of commands. If it does not exist, > * cmd should be used. If it exists, sub_table[?].cmd should be > diff --git a/monitor/hmp.c b/monitor/hmp.c > index 4b66ca1cd6..b858b0dbde 100644 > --- a/monitor/hmp.c > +++ b/monitor/hmp.c > @@ -1056,12 +1056,26 @@ fail: > return NULL; > } > > +typedef struct HandleHmpCommandCo { > + Monitor *mon; > + const HMPCommand *cmd; > + QDict *qdict; > + bool done; > +} HandleHmpCommandCo; > + > +static void handle_hmp_command_co(void *opaque) > +{ > + HandleHmpCommandCo *data = opaque; > + data->cmd->cmd(data->mon, data->qdict); > + monitor_set_cur(qemu_coroutine_self(), NULL); > + data->done = true; > +} > + > void handle_hmp_command(MonitorHMP *mon, const char *cmdline) > { > QDict *qdict; > const HMPCommand *cmd; > const char *cmd_start = cmdline; > - Monitor *old_mon; > > trace_handle_hmp_command(mon, cmdline); > > @@ -1080,10 +1094,23 @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline) > return; > } > > - /* old_mon is non-NULL when called from qmp_human_monitor_command() */ > - old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common); > - cmd->cmd(&mon->common, qdict); > - monitor_set_cur(qemu_coroutine_self(), old_mon); > + if (!cmd->coroutine) { > + /* old_mon is non-NULL when called from qmp_human_monitor_command() */ > + Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common); > + cmd->cmd(&mon->common, qdict); > + monitor_set_cur(qemu_coroutine_self(), old_mon); > + } else { > + HandleHmpCommandCo data = { > + .mon = &mon->common, > + .cmd = cmd, > + .qdict = qdict, > + .done = false, > + }; > + Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data); > + monitor_set_cur(co, &mon->common); > + aio_co_enter(qemu_get_aio_context(), co); > + AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done); > + } > > qobject_unref(qdict); > } > -- > 2.25.4 >
Additional nitpick detail on Kevin's request. Kevin Wolf <kwolf@redhat.com> writes: > Often, QMP command handlers are not only called to handle QMP commands, > but also from a corresponding HMP command handler. In order to give them > a consistent environment, optionally run HMP command handlers in a > coroutine, too. > > The implementation is a lot simpler than in QMP because for HMP, we > still block the VM while the coroutine is running. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > monitor/monitor-internal.h | 1 + > monitor/hmp.c | 37 ++++++++++++++++++++++++++++++++----- > 2 files changed, 33 insertions(+), 5 deletions(-) > > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h > index b55d6df07f..ad2e64be13 100644 > --- a/monitor/monitor-internal.h > +++ b/monitor/monitor-internal.h > @@ -74,6 +74,7 @@ typedef struct HMPCommand { > const char *help; > const char *flags; /* p=preconfig */ > void (*cmd)(Monitor *mon, const QDict *qdict); > + bool coroutine; > /* > * @sub_table is a list of 2nd level of commands. If it does not exist, > * cmd should be used. If it exists, sub_table[?].cmd should be > diff --git a/monitor/hmp.c b/monitor/hmp.c > index 4b66ca1cd6..b858b0dbde 100644 > --- a/monitor/hmp.c > +++ b/monitor/hmp.c > @@ -1056,12 +1056,26 @@ fail: > return NULL; > } > > +typedef struct HandleHmpCommandCo { > + Monitor *mon; > + const HMPCommand *cmd; > + QDict *qdict; > + bool done; > +} HandleHmpCommandCo; > + > +static void handle_hmp_command_co(void *opaque) > +{ > + HandleHmpCommandCo *data = opaque; > + data->cmd->cmd(data->mon, data->qdict); > + monitor_set_cur(qemu_coroutine_self(), NULL); > + data->done = true; > +} > + > void handle_hmp_command(MonitorHMP *mon, const char *cmdline) > { > QDict *qdict; > const HMPCommand *cmd; > const char *cmd_start = cmdline; > - Monitor *old_mon; > > trace_handle_hmp_command(mon, cmdline); > > @@ -1080,10 +1094,23 @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline) > return; > } > > - /* old_mon is non-NULL when called from qmp_human_monitor_command() */ > - old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common); > - cmd->cmd(&mon->common, qdict); > - monitor_set_cur(qemu_coroutine_self(), old_mon); > + if (!cmd->coroutine) { > + /* old_mon is non-NULL when called from qmp_human_monitor_command() */ > + Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common); Long line. David seems fine with it, and he's the maintainer. > + cmd->cmd(&mon->common, qdict); > + monitor_set_cur(qemu_coroutine_self(), old_mon); > + } else { > + HandleHmpCommandCo data = { > + .mon = &mon->common, > + .cmd = cmd, > + .qdict = qdict, > + .done = false, > + }; > + Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data); > + monitor_set_cur(co, &mon->common); > + aio_co_enter(qemu_get_aio_context(), co); > + AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done); > + } > > qobject_unref(qdict); > }
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h index b55d6df07f..ad2e64be13 100644 --- a/monitor/monitor-internal.h +++ b/monitor/monitor-internal.h @@ -74,6 +74,7 @@ typedef struct HMPCommand { const char *help; const char *flags; /* p=preconfig */ void (*cmd)(Monitor *mon, const QDict *qdict); + bool coroutine; /* * @sub_table is a list of 2nd level of commands. If it does not exist, * cmd should be used. If it exists, sub_table[?].cmd should be diff --git a/monitor/hmp.c b/monitor/hmp.c index 4b66ca1cd6..b858b0dbde 100644 --- a/monitor/hmp.c +++ b/monitor/hmp.c @@ -1056,12 +1056,26 @@ fail: return NULL; } +typedef struct HandleHmpCommandCo { + Monitor *mon; + const HMPCommand *cmd; + QDict *qdict; + bool done; +} HandleHmpCommandCo; + +static void handle_hmp_command_co(void *opaque) +{ + HandleHmpCommandCo *data = opaque; + data->cmd->cmd(data->mon, data->qdict); + monitor_set_cur(qemu_coroutine_self(), NULL); + data->done = true; +} + void handle_hmp_command(MonitorHMP *mon, const char *cmdline) { QDict *qdict; const HMPCommand *cmd; const char *cmd_start = cmdline; - Monitor *old_mon; trace_handle_hmp_command(mon, cmdline); @@ -1080,10 +1094,23 @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline) return; } - /* old_mon is non-NULL when called from qmp_human_monitor_command() */ - old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common); - cmd->cmd(&mon->common, qdict); - monitor_set_cur(qemu_coroutine_self(), old_mon); + if (!cmd->coroutine) { + /* old_mon is non-NULL when called from qmp_human_monitor_command() */ + Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common); + cmd->cmd(&mon->common, qdict); + monitor_set_cur(qemu_coroutine_self(), old_mon); + } else { + HandleHmpCommandCo data = { + .mon = &mon->common, + .cmd = cmd, + .qdict = qdict, + .done = false, + }; + Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data); + monitor_set_cur(co, &mon->common); + aio_co_enter(qemu_get_aio_context(), co); + AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done); + } qobject_unref(qdict); }
Often, QMP command handlers are not only called to handle QMP commands, but also from a corresponding HMP command handler. In order to give them a consistent environment, optionally run HMP command handlers in a coroutine, too. The implementation is a lot simpler than in QMP because for HMP, we still block the VM while the coroutine is running. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- monitor/monitor-internal.h | 1 + monitor/hmp.c | 37 ++++++++++++++++++++++++++++++++----- 2 files changed, 33 insertions(+), 5 deletions(-)