From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58126) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEug0-00029K-KJ for qemu-devel@nongnu.org; Thu, 29 Aug 2013 01:21:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VEufv-0001lp-0i for qemu-devel@nongnu.org; Thu, 29 Aug 2013 01:21:48 -0400 Received: from cantor2.suse.de ([195.135.220.15]:40452 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEufu-0001lf-NY for qemu-devel@nongnu.org; Thu, 29 Aug 2013 01:21:42 -0400 Message-ID: <521EDA62.4010803@suse.de> Date: Thu, 29 Aug 2013 07:21:38 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chen Fan Cc: Igor Mammedov , qemu-devel@nongnu.org, Paolo Bonzini Am 29.08.2013 04:09, schrieb Chen Fan: > Implement cpu interface 'cpu_common_unrealizefn' for emiting vcpu-remov= e > notifier to ACPI, then ACPI could send sci interrupt to OS for hot-remo= ve > vcpu. >=20 > Signed-off-by: Chen Fan > --- > hw/i386/pc.c | 19 ++++++++++++++++++- > qom/cpu.c | 13 +++++++++++++ > 2 files changed, 31 insertions(+), 1 deletion(-) >=20 > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 75fc9bb..9a87ac0 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -960,7 +960,24 @@ void pc_hot_add_cpu(const int64_t id, Error **errp= ) > =20 > void pc_hot_del_cpu(const int64_t id, Error **errp) > { > - /* TODO: hot remove VCPU. */ > + CPUState *s =3D NULL; "cs" since this is in PC code, to free the "s" identifier. > + X86CPU *cpu =3D NULL; > + DeviceState *ds =3D NULL; "dev" please for consistency. > + DeviceClass *dc =3D NULL; > + > + s =3D qemu_get_cpu(id); Same question as on 2/6, whether id =3D=3D cpu_index here. > + if (s =3D=3D NULL) { > + error_setg(errp, "Unable to find cpu-index: %" PRIi64 > + ",it non-exists or has been deleted.", id); Space after comma please in English language and "doesn't exist". > + return; > + } > + > + cpu =3D X86_CPU(s); > + ds =3D DEVICE(cpu); > + dc =3D DEVICE_GET_CLASS(ds); > + if (dc->unrealize) { > + dc->unrealize(ds, errp); > + } Never call this directly, this is missing important device cleanups such as VMState. What you want is: object_property_set_bool(OBJECT(cs), false, "realized", errp); > } > =20 > void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) > diff --git a/qom/cpu.c b/qom/cpu.c > index 3439c5d..d2b0c9e 100644 > --- a/qom/cpu.c > +++ b/qom/cpu.c > @@ -239,6 +239,18 @@ static void cpu_common_realizefn(DeviceState *dev,= Error **errp) > } > } > =20 > +static void cpu_common_unrealizefn(DeviceState *dev, Error **errp) > +{ > + CPUNotifier notifier; > + > + notifier.dev =3D dev; > + notifier.type =3D UNPLUG; > + > + if (dev->hotplugged) { > + notifier_list_notify(&cpu_hotplug_notifiers, ¬ifier); > + } Apart from the misindentation this looks wrong. If we initialize a machine with -smp 2 we should be allowed to hot-unplug CPU 1 IMO. Are there any reasons not to allow that? I would simply drop the dev->hotplugged check and always emit the notification. When QEMU is shut down, the unrealizefn and finalizers are not called, I believe. Regards, Andreas > +} > + > static void cpu_common_initfn(Object *obj) > { > CPUState *cpu =3D CPU(obj); > @@ -269,6 +281,7 @@ static void cpu_class_init(ObjectClass *klass, void= *data) > k->gdb_read_register =3D cpu_common_gdb_read_register; > k->gdb_write_register =3D cpu_common_gdb_write_register; > dc->realize =3D cpu_common_realizefn; > + dc->unrealize =3D cpu_common_unrealizefn; > dc->no_user =3D 1; > } > =20 >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg