| Message ID | 1607536336-24701-4-git-send-email-alejandro.j.jimenez@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add a new -action parameter | expand |
On 09/12/20 18:52, Alejandro Jimenez wrote: > +# Set the action that will be taken by the emulator in response to a guest > +# event. > +# > +# @pair: a @RunStateAction type that describes an event|action pair. > +# > +# Returns: Nothing on success. > +# > +# Since: 6.0 > +# > +# Example: > +# > +# -> { "execute": "set-action", > +# "arguments": { "pair": { > +# "event": "shutdown", > +# "action": "pause" } } } > +# <- { "return": {} } > +## > +{ 'command': 'set-action', 'data' : {'pair': 'RunStateAction'} } > + > +## > # @GUEST_PANICKED: > # > # Emitted when guest OS panic is detected > diff --git a/softmmu/runstate-action.c b/softmmu/runstate-action.c > index a644d80..7877e7e 100644 > --- a/softmmu/runstate-action.c > +++ b/softmmu/runstate-action.c > @@ -80,6 +80,35 @@ static void panic_set_action(PanicAction action, Error **errp) > } > > /* > + * Receives a RunStateAction type which represents an event|action pair > + * and sets the internal state as requested. > + */ > +void qmp_set_action(RunStateAction *pair, Error **errp) > +{ > + switch (pair->event) { > + case RUN_STATE_EVENT_TYPE_REBOOT: > + reboot_set_action(pair->u.reboot.action, NULL); > + break; > + case RUN_STATE_EVENT_TYPE_SHUTDOWN: > + shutdown_set_action(pair->u.shutdown.action, NULL); > + break; > + case RUN_STATE_EVENT_TYPE_PANIC: > + panic_set_action(pair->u.panic.action, NULL); > + break; > + case RUN_STATE_EVENT_TYPE_WATCHDOG: > + qmp_watchdog_set_action(pair->u.watchdog.action, NULL); > + break; > + default: > + /* > + * The fields in the RunStateAction argument are validated > + * by the QMP marshalling code before this function is called. > + * This case is unreachable unless new variants are added. > + */ > + g_assert_not_reached(); > + } > +} > + Any reason not to have the multiple optional arguments as discussed in v1 (no reply usually means you agree)? The implementation would be nice, like if (actions->has_reboot) { reboot_set_action(actions->reboot); } etc. ? Note that, in patches 1-2, you don't need to add an Error** argument to functions that cannot fail. Thanks, Paolo
On 12/9/2020 4:43 PM, Paolo Bonzini wrote: > On 09/12/20 18:52, Alejandro Jimenez wrote: >> +# Set the action that will be taken by the emulator in response to a >> guest >> +# event. >> +# >> +# @pair: a @RunStateAction type that describes an event|action pair. >> +# >> +# Returns: Nothing on success. >> +# >> +# Since: 6.0 >> +# >> +# Example: >> +# >> +# -> { "execute": "set-action", >> +# "arguments": { "pair": { >> +# "event": "shutdown", >> +# "action": "pause" } } } >> +# <- { "return": {} } >> +## >> +{ 'command': 'set-action', 'data' : {'pair': 'RunStateAction'} } >> + >> +## >> # @GUEST_PANICKED: >> # >> # Emitted when guest OS panic is detected >> diff --git a/softmmu/runstate-action.c b/softmmu/runstate-action.c >> index a644d80..7877e7e 100644 >> --- a/softmmu/runstate-action.c >> +++ b/softmmu/runstate-action.c >> @@ -80,6 +80,35 @@ static void panic_set_action(PanicAction action, >> Error **errp) >> } >> /* >> + * Receives a RunStateAction type which represents an event|action pair >> + * and sets the internal state as requested. >> + */ >> +void qmp_set_action(RunStateAction *pair, Error **errp) >> +{ >> + switch (pair->event) { >> + case RUN_STATE_EVENT_TYPE_REBOOT: >> + reboot_set_action(pair->u.reboot.action, NULL); >> + break; >> + case RUN_STATE_EVENT_TYPE_SHUTDOWN: >> + shutdown_set_action(pair->u.shutdown.action, NULL); >> + break; >> + case RUN_STATE_EVENT_TYPE_PANIC: >> + panic_set_action(pair->u.panic.action, NULL); >> + break; >> + case RUN_STATE_EVENT_TYPE_WATCHDOG: >> + qmp_watchdog_set_action(pair->u.watchdog.action, NULL); >> + break; >> + default: >> + /* >> + * The fields in the RunStateAction argument are validated >> + * by the QMP marshalling code before this function is called. >> + * This case is unreachable unless new variants are added. >> + */ >> + g_assert_not_reached(); >> + } >> +} >> + > > Any reason not to have the multiple optional arguments as discussed in > v1 (no reply usually means you agree)? The implementation would be > nice, like > > if (actions->has_reboot) { > reboot_set_action(actions->reboot); > } > etc. > > ? I misunderstood your request in v1. I'll try to be explicit to avoid more confusion. Are you expecting a command of the form: { 'command': 'set-action', 'data' : { '*reboot': 'RebootAction', '*shutdown': 'ShutdownAction', '*panic': 'PanicAction', '*watchdog': 'WatchdogAction' } } ? Or is it better to encapsulate all of those optional fields inside a new struct definition (RunStateActions?) so that the command would be: { 'command': 'set-action', 'data': 'actions' : 'RunStateActions' } which is what the "actions->has_reboot" example seems to suggest? Or is it something else that I am not understanding yet? > > Note that, in patches 1-2, you don't need to add an Error** argument > to functions that cannot fail. This was left over from the initial patch. I'll fix it. Alejandro > > Thanks, > > Paolo >
On 10/12/20 04:21, Alejandro Jimenez wrote: > I misunderstood your request in v1. Oh ypu're right, in v1 you had multiple commands. My fault then. > > { 'command': 'set-action', > 'data' : { > '*reboot': 'RebootAction', > '*shutdown': 'ShutdownAction', > '*panic': 'PanicAction', > '*watchdog': 'WatchdogAction' } } > ? > > Or is it better to encapsulate all of those optional fields inside a new > struct definition (RunStateActions?) so that the command would be: > > { 'command': 'set-action', 'data': 'actions' : 'RunStateActions' } Any of the two is fine; the QMP stream is the same. I used actions->reboot because that's what you did in v2. While at it, you might add 'allow-preconfig': true, as well. (Right now there are relatively few allow-preconfig commands, but I'm in the process of adding it to all commands where it makes sense). Thanks, Paolo
diff --git a/qapi/run-state.json b/qapi/run-state.json index 27b62ce..ead9dab 100644 --- a/qapi/run-state.json +++ b/qapi/run-state.json @@ -451,6 +451,28 @@ { 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} } ## +# @set-action: +# +# Set the action that will be taken by the emulator in response to a guest +# event. +# +# @pair: a @RunStateAction type that describes an event|action pair. +# +# Returns: Nothing on success. +# +# Since: 6.0 +# +# Example: +# +# -> { "execute": "set-action", +# "arguments": { "pair": { +# "event": "shutdown", +# "action": "pause" } } } +# <- { "return": {} } +## +{ 'command': 'set-action', 'data' : {'pair': 'RunStateAction'} } + +## # @GUEST_PANICKED: # # Emitted when guest OS panic is detected diff --git a/softmmu/runstate-action.c b/softmmu/runstate-action.c index a644d80..7877e7e 100644 --- a/softmmu/runstate-action.c +++ b/softmmu/runstate-action.c @@ -80,6 +80,35 @@ static void panic_set_action(PanicAction action, Error **errp) } /* + * Receives a RunStateAction type which represents an event|action pair + * and sets the internal state as requested. + */ +void qmp_set_action(RunStateAction *pair, Error **errp) +{ + switch (pair->event) { + case RUN_STATE_EVENT_TYPE_REBOOT: + reboot_set_action(pair->u.reboot.action, NULL); + break; + case RUN_STATE_EVENT_TYPE_SHUTDOWN: + shutdown_set_action(pair->u.shutdown.action, NULL); + break; + case RUN_STATE_EVENT_TYPE_PANIC: + panic_set_action(pair->u.panic.action, NULL); + break; + case RUN_STATE_EVENT_TYPE_WATCHDOG: + qmp_watchdog_set_action(pair->u.watchdog.action, NULL); + break; + default: + /* + * The fields in the RunStateAction argument are validated + * by the QMP marshalling code before this function is called. + * This case is unreachable unless new variants are added. + */ + g_assert_not_reached(); + } +} + +/* * Process an event|action pair and set the appropriate internal * state if event and action are valid. */
Add a QMP command to allow for the behaviors specified by the -action event=action command line option to be set at runtime. The new command is named set-action, and takes a single argument of type RunStateAction, which contains an event|action pair. Example: -> { "execute": "set-action", "arguments": { "pair": { "event": "shutdown", "action": "pause" } } } <- { "return": {} } Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com> --- qapi/run-state.json | 22 ++++++++++++++++++++++ softmmu/runstate-action.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+)