[PATCH RFC 00/11] vl: Explore redesign of startup

Markus Armbruster posted 11 patches 2 years, 4 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20211202070450.264743-1-armbru@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Eduardo Habkost <eduardo@habkost.net>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Michael Roth <michael.roth@amd.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eric Auger <eric.auger@redhat.com>
qapi/misc.json             |   27 -
qapi/phase.json            |   31 +
qapi/qapi-schema.json      |    1 +
include/hw/qdev-core.h     |   31 -
hw/core/machine-qmp-cmds.c |    1 +
hw/core/machine.c          |    1 +
hw/core/qdev.c             |    7 +
hw/pci/pci.c               |    1 +
hw/usb/core.c              |    1 +
hw/virtio/virtio-iommu.c   |    1 +
monitor/hmp-cmds.c         |    8 -
monitor/hmp.c              |    1 +
softmmu/qdev-monitor.c     |    3 +
softmmu/vl.c               | 2833 ++----------------------------------
ui/console.c               |    1 +
MAINTAINERS                |    1 +
hmp-commands.hx            |   18 -
qapi/meson.build           |    1 +
18 files changed, 180 insertions(+), 2788 deletions(-)
create mode 100644 qapi/phase.json
[PATCH RFC 00/11] vl: Explore redesign of startup
Posted by Markus Armbruster 2 years, 4 months ago
These patches are meant to back the memo "Redesign of QEMU startup &
initial configuration" I just posted.  Read that first, please.

My running example for initial configuration via QMP is cold plug.  It
works at the end of the series.

I'm taking a number of shortcuts:

* I hack up qemu-system-FOO instead of creating an alternate program.
  Just so I don't have to mess with Meson.

* Instead of creating QAPI/CLI infrastructure, I use QMP as CLI: each
  argument is interpreted as QMP command.  This is certainly bad CLI,
  but should suffice to demonstrate things.

* Instead of feeding the CLI's QMP commands to the main loop via a
  quasi-monitor, I send them directly to the QMP dispatcher.  Simpler,
  but I'm not sure that's going to work for all QMP commands we want.

* Phase advance is by explicit command @until-phase only.  Carelessly
  named.  We may want some other commands to advance the phase
  automatically.

* There are no safeguards.  You *can* run QMP commands in phases where
  they crash.  Data corruption is left as an exercise for the reader.

* Possibly more I can't remember right now :)

Markus Armbruster (11):
  vl: Cut off the CLI with an axe
  vl: Drop x-exit-preconfig
  vl: Hardcode a QMP monitor on stdio for now
  vl: Hardcode a VGA device for now
  vl: Demonstrate (bad) CLI wrapped around QMP
  vl: Factor qemu_until_phase() out of qemu_init()
  vl: Implement qemu_until_phase() running from arbitrary phase
  vl: Implement qemu_until_phase() running to arbitrary phase
  vl: New QMP command until-phase
  vl: Disregard lack of 'allow-preconfig': true
  vl: Enter main loop in phase @machine-initialized

 qapi/misc.json             |   27 -
 qapi/phase.json            |   31 +
 qapi/qapi-schema.json      |    1 +
 include/hw/qdev-core.h     |   31 -
 hw/core/machine-qmp-cmds.c |    1 +
 hw/core/machine.c          |    1 +
 hw/core/qdev.c             |    7 +
 hw/pci/pci.c               |    1 +
 hw/usb/core.c              |    1 +
 hw/virtio/virtio-iommu.c   |    1 +
 monitor/hmp-cmds.c         |    8 -
 monitor/hmp.c              |    1 +
 softmmu/qdev-monitor.c     |    3 +
 softmmu/vl.c               | 2833 ++----------------------------------
 ui/console.c               |    1 +
 MAINTAINERS                |    1 +
 hmp-commands.hx            |   18 -
 qapi/meson.build           |    1 +
 18 files changed, 180 insertions(+), 2788 deletions(-)
 create mode 100644 qapi/phase.json

-- 
2.31.1


Re: [PATCH RFC 00/11] vl: Explore redesign of startup
Posted by Markus Armbruster 2 years, 4 months ago
I fat-fingered Edgar's e-mail address.  Sorry for the inconvenience.


Re: [PATCH RFC 00/11] vl: Explore redesign of startup
Posted by Damien Hedde 2 years, 4 months ago
Hi Markus,

It looks promising. I did not think we could so "easily" have a new 
working startup. But I'm not so sure that I understand how we should 
progress from here.

I see 3 main parts in this:
A. introducing new binary (meson, ...)
B. startup api: phase related stuff (maybe more)
C. cli to qmp parser

I think if we want to add a new binary (instead of replace it), there 
will be some common api and every startup will have to support/implement 
it. Probably some part of vl.c will have to go in some common code.
In practice, we probably should introduce/extract this before 
introducing the new binary.

One central part of this api is the phase mechanism (even if legacy 
startup can only support it partially or not-at-all).

I think we have 2 choices:
+ we have to use until_phase explicitly
+ we make qmp commands implicitly advances phases when needed.

I think it's better to go the implicit way as much as possible: it means 
we focus on commands and not on some artificial phases we set up because 
of legacy.

Either way, we probably should put the phase info in qapi so that we 
don't have to hardcode that in every command in order to have common 
error handling. One thing we could do is replace "allow-preconfig" in 
qapi by some phase requirement entry(entries?) and make qmp call 
qemu_until_phase() or some qemu_phase_check() function.

We also maybe need to sort out if we want to merge the phases into the 
runstate.

Thanks for making the effort to do this rfc,
--
Damien

On 12/2/21 08:04, Markus Armbruster wrote:
> These patches are meant to back the memo "Redesign of QEMU startup &
> initial configuration" I just posted.  Read that first, please.
> 
> My running example for initial configuration via QMP is cold plug.  It
> works at the end of the series.
> 
> I'm taking a number of shortcuts:
> 
> * I hack up qemu-system-FOO instead of creating an alternate program.
>    Just so I don't have to mess with Meson.
> 
> * Instead of creating QAPI/CLI infrastructure, I use QMP as CLI: each
>    argument is interpreted as QMP command.  This is certainly bad CLI,
>    but should suffice to demonstrate things.
> 
> * Instead of feeding the CLI's QMP commands to the main loop via a
>    quasi-monitor, I send them directly to the QMP dispatcher.  Simpler,
>    but I'm not sure that's going to work for all QMP commands we want.
> 
> * Phase advance is by explicit command @until-phase only.  Carelessly
>    named.  We may want some other commands to advance the phase
>    automatically.
> 
> * There are no safeguards.  You *can* run QMP commands in phases where
>    they crash.  Data corruption is left as an exercise for the reader.
> 
> * Possibly more I can't remember right now :)
> 
> Markus Armbruster (11):
>    vl: Cut off the CLI with an axe
>    vl: Drop x-exit-preconfig
>    vl: Hardcode a QMP monitor on stdio for now
>    vl: Hardcode a VGA device for now
>    vl: Demonstrate (bad) CLI wrapped around QMP
>    vl: Factor qemu_until_phase() out of qemu_init()
>    vl: Implement qemu_until_phase() running from arbitrary phase
>    vl: Implement qemu_until_phase() running to arbitrary phase
>    vl: New QMP command until-phase
>    vl: Disregard lack of 'allow-preconfig': true
>    vl: Enter main loop in phase @machine-initialized
> 
>   qapi/misc.json             |   27 -
>   qapi/phase.json            |   31 +
>   qapi/qapi-schema.json      |    1 +
>   include/hw/qdev-core.h     |   31 -
>   hw/core/machine-qmp-cmds.c |    1 +
>   hw/core/machine.c          |    1 +
>   hw/core/qdev.c             |    7 +
>   hw/pci/pci.c               |    1 +
>   hw/usb/core.c              |    1 +
>   hw/virtio/virtio-iommu.c   |    1 +
>   monitor/hmp-cmds.c         |    8 -
>   monitor/hmp.c              |    1 +
>   softmmu/qdev-monitor.c     |    3 +
>   softmmu/vl.c               | 2833 ++----------------------------------
>   ui/console.c               |    1 +
>   MAINTAINERS                |    1 +
>   hmp-commands.hx            |   18 -
>   qapi/meson.build           |    1 +
>   18 files changed, 180 insertions(+), 2788 deletions(-)
>   create mode 100644 qapi/phase.json
> 

Re: [PATCH RFC 00/11] vl: Explore redesign of startup
Posted by Markus Armbruster 2 years, 4 months ago
Damien Hedde <damien.hedde@greensocs.com> writes:

> Hi Markus,
>
> It looks promising. I did not think we could so "easily" have a new
> working startup.

Look at this big axe I got!  ;)

>                  But I'm not so sure that I understand how we should 
> progress from here.

I neglected to explain this my cover letter.  My apologies...

> I see 3 main parts in this:
> A. introducing new binary (meson, ...)
> B. startup api: phase related stuff (maybe more)
> C. cli to qmp parser

Makes sense to me at a high level.

> I think if we want to add a new binary (instead of replace it), there
> will be some common api and every startup will have to
> support/implement it. Probably some part of vl.c will have to go in
> some common code.
> In practice, we probably should introduce/extract this before
> introducing the new binary.

I think there are two practical ways to structure such patches:

* Refactor existing code to make parts available for new code, then
  introduce new code that uses them.

* Copy, cut unwanted parts, refactor to deduplicate.

I think either way can work as patches.  The second way is how I'd start
the work myself.

> One central part of this api is the phase mechanism (even if legacy
> startup can only support it partially or not-at-all).
>
> I think we have 2 choices:
> + we have to use until_phase explicitly
> + we make qmp commands implicitly advances phases when needed.

Yes.

> I think it's better to go the implicit way as much as possible: it
> means we focus on commands and not on some artificial phases we set up
> because of legacy.

An explicit phase control command looked like the fast & easy path to
phase control to me, so that's what I picked for the RFC.

Instead of a single "advance to arbitrary phase" command, we can have
multiple "do X, which requires phase Y and advances to phase Y+1"
commands.  E.g. "create machine" goes from @no-machine to
@machine-created.

We may want additional, automatic phase advances for convenience, but I
feel it's best to get the essential stuff roughly right before talking
about convenience features.

> Either way, we probably should put the phase info in qapi so that we
> don't have to hardcode that in every command in order to have common 
> error handling. One thing we could do is replace "allow-preconfig" in
> qapi by some phase requirement entry(entries?) and make qmp call 
> qemu_until_phase() or some qemu_phase_check() function.

I'd also like some phase support from QAPI.  Manual phase checking code
in commands would be tedious and error prone.  Better to declare
required phase(s) in the schema.

One small step further: declare phase transitions in the schema, too.
Then the phase state machine definition is all *data*.  Data is easier
to reason about than code.  Extracting the complete state machine from
the schema is straightforward.  Extracting it from C code is anything
but.

> We also maybe need to sort out if we want to merge the phases into the
> runstate.

Yes.

> Thanks for making the effort to do this rfc,

Thanks for your feedback!