Re: [Qemu-devel] [PATCH 05/16] introduce CPU hot-plug notifier

[Gleb Natapov]

On Mon, Apr 22, 2013 at 01:09:19PM +0200, Igor Mammedov wrote:
> On Mon, 22 Apr 2013 14:00:01 +0300
> Gleb Natapov <address@hidden> wrote:
> 
> > On Tue, Apr 16, 2013 at 12:12:45AM +0200, Igor Mammedov wrote:
> > > hot-added CPU will be distributed to acpi_piix4 and rtc_cmos
> > > 
> > > Signed-off-by: Igor Mammedov <address@hidden>
> > > Reviewed-by: Paolo Bonzini <address@hidden>
> > > Reviewed-by: Eduardo Habkost <address@hidden>
> > > ---
> > > v2:
> > >   * move notifier to qom/cpu.c and call it from CPUClass.realize() on
> > > hotplug
> > >   * remove get_firmware_id() since it belong to other patch
> > > ---
> > >  include/sysemu/sysemu.h |  3 +++
> > >  qom/cpu.c               | 12 ++++++++++++
> > >  2 files changed, 15 insertions(+)
> > > 
> > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > > index 6578782..a8c3de1 100644
> > > --- a/include/sysemu/sysemu.h
> > > +++ b/include/sysemu/sysemu.h
> > > @@ -152,6 +152,9 @@ void do_pci_device_hot_remove(Monitor *mon, const
> > > QDict *qdict); /* generic hotplug */
> > >  void drive_hot_add(Monitor *mon, const QDict *qdict);
> > >  
> > > +/* CPU hotplug */
> > > +void qemu_register_cpu_added_notifier(Notifier *notifier);
> > > +
> > >  /* pcie aer error injection */
> > >  void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
> > >  int do_pcie_aer_inject_error(Monitor *mon,
> > > diff --git a/qom/cpu.c b/qom/cpu.c
> > > index 40a4259..819986e 100644
> > > --- a/qom/cpu.c
> > > +++ b/qom/cpu.c
> > > @@ -21,6 +21,17 @@
> > >  #include "qom/cpu.h"
> > >  #include "qemu-common.h"
> > >  #include "sysemu/kvm.h"
> > > +#include "qemu/notify.h"
> > > +#include "sysemu/sysemu.h"
> > > +
> > > +/* CPU hot-plug notifiers */
> > > +static NotifierList cpu_added_notifiers =
> > > +    NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
> > > +
> > > +void qemu_register_cpu_added_notifier(Notifier *notifier)
> > > +{
> > > +    notifier_list_add(&cpu_added_notifiers, notifier);
> > > +}
> > >  
> > >  void cpu_reset_interrupt(CPUState *cpu, int mask)
> > >  {
> > > @@ -61,6 +72,7 @@ static void cpu_common_realizefn(DeviceState *dev,
> > > Error **errp) if (dev->hotplugged) {
> > >          cpu_synchronize_post_init(CPU(dev));
> > >          resume_vcpu(CPU(dev));
> > > +        notifier_list_notify(&cpu_added_notifiers, dev);
> > Why do it after vcpu is running? May be notifier want to do something
> > that should be done before vcpu is running.
> currently there is not need for adding_cpu notifier.
> This one signals that there is a new fully functional CPU available.
> 
You are using it to update cmos information for BIOS. It would be strange
for newly added vcpu to see incorrect number of cpus in cmos if, for
some reason, it will check it. The "new cpu is added and ready to tun,
do your last things before it is launched" callback seams more reasonable
then "new cpu is running wild" one.

--
                        Gleb.