Re: [PATCH 26/36] remove preconfig state

[Igor Mammedov]

On Fri, 27 Nov 2020 12:50:39 +0100
Paolo Bonzini <pbonzini@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 <imammedo@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.  
>