* Markus Armbruster (arm...@redhat.com) wrote: > Damien Hedde <damien.he...@greensocs.com> writes:
> > Patches 1, 3 and 5 miss a review. > > > > The series is organized as follow: > > > > + Patches 1 and 2 converts the MachinePhase enum to a qapi definition > > and add the 'query-machine-phase'. It allows to introspect the > > current machine phase during preconfig as we will now be able to > > reach several machine phases using QMP. > > If we fold MachinePhase into RunState, we can reuse query-status. > > Having two state machines run one after the other feels like one too > many. Be careful, the RunState is API and things watch for events on it, so any changes to it are delicate. Dave > > + Patch 3 adds the 'x-machine-init' QMP command to stop QEMU at > > machine-initialized phase during preconfig. > > + Patch 4 allows issuing device_add QMP command during the > > machine-initialized phase. > > + Patch 5 improves the doc about preconfig in consequence. > > I understand you want to make progress towards machine configuration > with QMP. However, QEMU startup is (in my educated opinion) a hole, and > we should be wary of digging deeper. > > The "timeline" you gave above illustrates this. It's a complicated > shuffling of command line options and QMP commands that basically nobody > can keep in working memory. We have reshuffled it / made it more > complicated quite a few times already to support new features. Based on > your cover letter, I figure you're making it more complicated once more. > > At some point, we need to stop digging us deeper into the hole. This is > not an objection to merging your work. It's a call to stop and think. > > Let me quote the sketch I posted to the "Stabilize preconfig" thread: > > 1. Start event loop > > 2. Feed it CLI left to right. Each option runs a handler just like each > QMP command does. > > Options that read a configuration file inject the file into the feed. > > Options that create a monitor create it suspended. > > Options may advance the phase / run state, and they may require > certain phase(s). > > 3. When we're done with CLI, resume any monitors we created. > > 4. Monitors now feed commands to the event loop. Commands may advance > the phase / run state, and they may require certain phase(s). > > Users can do as much or as little with the CLI as they want. You'd > probably want to set up a QMP monitor and no more. > > device_add becomes possible at a certain state of the phase / run state > machine. It changes from cold to hot plug at a certain later state. > > > [1]: > > https://lore.kernel.org/qemu-devel/b31f442d28920447690a6b8cee865bdbacde1283.1635160056.git.mpriv...@redhat.com > > > > Thanks for your feedback. > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
