Re: [RFC PATCH 6/9] qapi: Implement 'advance-machine-phase' command

[Kevin Wolf <kwolf@redhat.com>]

Am 13.05.2021 um 10:25 hat Mirela Grujic geschrieben:
> The command takes the target initialization phase as the argument
> and triggers QEMU to advance the machine to the target phase (i.e.
> execute all initialization steps required to enter the target phase).
> 
> This command would be used as an alternative to 'next-machine-phase'
> if it's more convenient to jump to a target initialization phase than
> to single-step through phases.
> 
> The command is used in combination with the -preconfig CLI option.
> 
> Note: advancing the machine to the 'ready' phase has the same effect as
> executing the 'x-exit-preconfig' command when the machine is in
> 'accel-created' phase.
> 
> Signed-off-by: Mirela Grujic <mirela.grujic@greensocs.com>

I think this command is preferable, not just because it is more
convenient if you don't have anything to do in some phase, but also
because it is more explicit and doesn't change its behaviour depending
on the current state.

We probably need to expect that this is a command that might often be
used in quickly hacked up shell scripts, which are error prone (Did I
really count the number of 'next-machine-phase' command right? Which
phase are we switching to again in this line?) and may lack proper error
handling, so the least amount of implicit magic will make sure that
users don't get more surprises than necessary.

> diff --git a/qapi/machine.json b/qapi/machine.json
> index 968d67dd95..31872aae72 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1352,3 +1352,29 @@
>  #
>  ##
>  { 'command': 'next-machine-phase', 'allow-preconfig': true }
> +
> +##
> +# @advance-machine-phase:
> +#
> +# Advance machine initialization phase to the target phase
> +#
> +# @phase: target machine initialization phase
> +#
> +# Since: #FIXME
> +#
> +# Returns: If successful, nothing
> +#
> +# Notes: This command will trigger QEMU to execute initialization steps
> +#        that are required to enter the target machine initialization phase.
> +#        If the target phase is the final initialization phase, the guest will
> +#        start running immediately unless the -S option is used. The command
> +#        is available only if the -preconfig command line option was passed.
> +#
> +# Example:
> +#
> +# -> { "execute": "advance-machine-phase", "arguments": { "phase": "ready" } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'advance-machine-phase', 'data' : {'phase': 'MachineInitPhase'},
> +             'allow-preconfig': true }
> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index 8aa743d59b..6b21a3fdd5 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
> @@ -219,3 +219,13 @@ void qmp_next_machine_phase(Error **errp)
>  
>      qemu_machine_enter_phase(target_phase, errp);
>  }
> +
> +void qmp_advance_machine_phase(MachineInitPhase phase, Error **errp)
> +{
> +    if (phase_get() == phase) {
> +        error_setg(errp, "Machine is already in the target phase");
> +        return;
> +    }

Another option would be making it set-machine-phase, which doesn't fail
if you're setting the phase that you're already in. It would only fail
if you're trying to go backwards. But this is a minor detail.

> +    qemu_machine_enter_phase(phase, errp);
> +}

Kevin