Re: [PATCH v4 2/4] hw/i386: Attach CPUs to machine

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 7 Feb 2022 09:14:37 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

> On Sat,  5 Feb 2022 13:45:24 +0100
> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
> > Previously CPUs were exposed in the QOM tree at a path
> > 
> >   /machine/unattached/device[nn]
> > 
> > where the 'nn' of the first CPU is usually zero, but can
> > vary depending on what devices were already created.
> > 
> > With this change the CPUs are now at
> > 
> >   /machine/cpu[nn]
> > 
> > where the 'nn' of the first CPU is always zero.  
> 
> Could you add to commit message the reason behind the change?

regardless, it looks like unwarranted movement to me
prompted by livirt accessing/expecting a QOM patch which is
not stable ABI. I'd rather get it fixed on libvirt side.

If libvirt needs for some reason access a CPU instance,
it should use @query-hotpluggable-cpus to get a list of CPUs
(which includes QOM path of already present CPUs) instead of
hard-codding some 'well-known' path as there is no any guarantee 
that it will stay stable whatsoever.
 
> > Note: This (intentionally) breaks compatibility with current
> > libvirt code that looks for "/machine/unattached/device[0]"
> > in the assumption it is the first CPU.  
> Why libvirt does this in the first place?
> 
>  
> > Cc: libvir-list@redhat.com
> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >  hw/i386/x86.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > index b84840a1bb9..50bf249c700 100644
> > --- a/hw/i386/x86.c
> > +++ b/hw/i386/x86.c
> > @@ -108,6 +108,7 @@ void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
> >  {
> >      Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
> >  
> > +    object_property_add_child(OBJECT(x86ms), "cpu[*]", OBJECT(cpu));  
> 
> that will take in account only initial cpus, -device/device_add cpus
> will still go to wherever device_add attaches them (see qdev_set_id)
> 
> >      if (!object_property_set_uint(cpu, "apic-id", apic_id, errp)) {
> >          goto out;
> >      }  
>