Re: [PATCH] qmp: Stabilize preconfig

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 11/11/21 07:11, Markus Armbruster wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>>> On 11/3/21 09:02, Markus Armbruster wrote:
>>>>> I wonder whether we really have to step through three states
>>>>>
>>>>>            x-exit-preconfig  cont
>>>>>       preconfig ---> pre run ---> run
>>>>>
>>>>> and not two
>>>>>
>>>>>               cont
>>>>>       pre run ---> run
>>>
>>> Devices would be hotplugged between x-exit-preconfig and cont, and
>> 
>> Cold plugged!
>
> Nope, hotplugged!  After x-exit-preconfig, the machine is ready to
> start, and that means that the machine will have completed 
> initialization via their machine_init_done notifiers.
>
> For example, fw_cfg will have set the bootorder.  Any device created
> after x-exit-preconfig will not be in the bootorder.

After re-reading this a couple of times, and checking the code, I now
see what I missed.

There has to be a point where we transition from cold to hot plug.  It
obviously can be no later than VM starting to run, i.e. first cont.

We actually moved it from there back to x-exit-preconfig.  I missed /
forgot that.

Why does it have to be moved back?  Let's see below.

>>> part of the machine until x-exit-preconfig; so there is a need for
>>> something like x-exit-preconfig.
>> 
>> Can you briefly explain why device_add doesn't work before
>> x-exit-preconfig and does after?
>
> The answer to this question is basically the verbose version of the
> coldplug/hotplug thing from above.  There are five stages in the
> startup of QEMU (marked by different values of the MachineInitPhase
> enum):
>
> 1) PHASE_NO_MACHINE - backends can already be created here, but no
> machine exists yet
>
> 2) PHASE_MACHINE_CREATED - the machine object has been created.  It's
> not initialized, but it's there.
>
> 3) PHASE_ACCEL_CREATED - the accelerator object has been created.  The
> accelerator needs the machine object, because for example KVM might
> not support all machine types.  So the accelerator queries the machine 
> object and fails creation in case of incompatibility.  This enables
> e.g. fallback to TCG.  -preconfig starts the monitor here.

We should be able to start monitors first, if we put in the work.

> 4) PHASE_MACHINE_INIT - machine initialization consists mostly in
> creating the onboard devices.  For this to happen, the machine needs
> to learn about the accelerator, because onboard devices include CPUs
> and other accelerator-dependent devices.  Devices plugged in this
> phase are cold-plugged.
>
> 5) PHASE_MACHINE_READY - machine init done notifiers have been called
> and the VM is ready.  Devices plugged in this phase already count as 
> hot-plugged.  -S starts the monitor here.

Remind us: what work is done in the machine init done notifiers?

What exactly necessitates "count as hot-plugged"?  Is it something done
in these notifiers?

> x-exit-preconfig goes straight from PHASE_ACCEL_CREATED to
> PHASE_MACHINE_READY.  Devices can only be created after 
> PHASE_MACHINE_INIT, so device_add cannot be enabled at preconfig
> stage.

Now I am confused again.  Can you cold plug devices with device_add in
presence of -preconfig, and if yes, how?

Related question: when exactly in these phases do we create devices
specified with -device?

> stage.   Why does preconfig start at PHASE_ACCEL_CREATED?  Well, the
> phases were not as easy to identify in qemu_init() when it was
> introduced, so I suppose it just seemed like a good place. :)  These
> days, qemu_init() is just a hundred lines of code apart from the huge
> command line parsing switch statement, so we have a clearer idea of
> the steps and you can look deeper at what happens in each phase if you
> want.  phase_advance() is your friend.
>
>
> With a pure-QMP configuration flow, PHASE_MACHINE_CREATED would be
> reached with a machine-set command (corresponding to the
> non-deprecated parts of -machine) and PHASE_ACCEL_CREATED would be
> reached with an accel-set command (corresponding to -accel).

I don't think this depends on "pure-QMP configuration flow".  -machine
and -accel could advance the phase just like their buddies machine-set
and accel-set.

State transition diagram:

    PHASE_NO_MACHINE (initial state)
            |
            |  -machine or machine-set
            v
    PHASE_MACHINE_CREATED
            |
            |  -accel or accel-set
            v
    PHASE_ACCEL_CREATED
            |
            |  ???
            v
    PHASE_MACHINE_INIT
            |
            |  ???
            v
    PHASE_MACHINE_READY
            |
            |  cont
            v
           ???

Can you fill in the ??? blanks?

> I haven't yet thought hard enough whether accel-set could go directly
> from PHASE_MACHINE_CREATED to PHASE_MACHINE_INIT.  It probably depends 
> on how CPUs would be configured in the QMP flow; if accel-set must
> return at PHASE_ACCEL_CREATED, a separate command is needed to reach 
> PHASE_MACHINE_INIT.  But either way, there the monitor would be
> accessible at PHASE_MACHINE_INIT, where device_add works and
> cold-plugs the devices.

The earlier the monitor becomes available, the better.

Ideally, we'd process the command line strictly left to right, and fail
options that are "out of phase".  Make the monitor available right when
we process its -mon.  The -chardev for its character device must precede
it.

Likewise, we'd fail QMP commands that are "out of phase".
@allow-preconfig is a crutch that only exists because we're afraid (with
reason) of hidden assumptions in QMP commands.