On Fri, 27 Nov 2020 12:50:39 +0100 Paolo Bonzini <pbonz...@redhat.com> wrote:
> On 27/11/20 11:50, Igor Mammedov wrote: > > it works in context of this series since > > > > + qemu_init_board(); > > + qemu_create_cli_devices(); > > + qemu_machine_creation_done(); > > > > are called within the same command qmp_x_exit_preconfig, > > if preconfig is enabled we happen to call qmp_set_numa_node() > > and if (qdev_hotplug) {error} work as expected, since qemu_init_board() > > hasn't been called yet. > > > > but I'm thinking about what happens beyond this series, when we start > > splitting qmp_x_exit_preconfig() after this series on separate stages. > > Ok, so that's the source of confusion. I don't think anymore that > x-exit-preconfig should be split in separate stages; I'm not looking > anymore at being able to do device-add from "qemu-system-x86_64 > -preconfig". Instead, I'm looking at having a completely separate > executable for QMP-only machine creation, which would not use vl.c > command line parsing at all. > > For this reason I've left MachinePhase to a separate series, which I > still plan for 6.0. But I will add it here instead. Assuming that qmp_x_exit_preconfig() won't be split: Reviewed-by: Igor Mammedov <imamm...@redhat.com> > FWIW I intend to have four parts: 1) this 2) QemuOpts->keyval switch for > -object/-M/-accel 3) making Machine's memdev property a > link<memory-backend> 4) making -smp/-boot/-m sugar for non-scalar > properties of Machine. I'll definitely need your review on part 3 too! I can review #3, #4 and your ram_size cleanup, which I'm somewhat familiar with. > > Thanks, > > Paolo > > > By using qdev_hotplug here, we practically loose dependency tracking > > on qemu_init_board() not being yet called. And if later we forget that, > > then it would allow to call qmp_set_numa_node() after qemu_init_board() > > but before qemu_machine_creation_done() > > > > So for this intermediate stage, instead of abusing qdev_hotplug adding > > a temporary is_board_created might be used. And when we introduce > > new phases you've described below, is_board_created could be replaced > > with appropriate phase check. >