From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WMhRq-0000te-2V for qemu-devel@nongnu.org; Sun, 09 Mar 2014 13:23:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WMhRl-0008Dl-6l for qemu-devel@nongnu.org; Sun, 09 Mar 2014 13:23:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11855) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WMhRk-00089s-US for qemu-devel@nongnu.org; Sun, 09 Mar 2014 13:23:33 -0400 Date: Sun, 9 Mar 2014 18:55:05 +0200 From: "Michael S. Tsirkin" Message-ID: <20140309165505.GC12527@redhat.com> References: <1394215998-10830-1-git-send-email-afaerber@suse.de> <20140309164018.GB8717@redhat.com> <531C9B79.70905@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <531C9B79.70905@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: Igor Mammedov , qemu-devel@nongnu.org On Sun, Mar 09, 2014 at 05:48:57PM +0100, Andreas F=E4rber wrote: > Am 09.03.2014 17:40, schrieb Michael S. Tsirkin: > > On Fri, Mar 07, 2014 at 07:13:18PM +0100, Andreas F=E4rber wrote: > >> Commit 1a37eca107cece3ed454bae29eef0bd1fac4a244 (qdev: add > >> "hotpluggable" property to Device) added a property "hotpluggable" t= o > >> each device, with its getter accessing parent_bus->allow_hotplug. > >> > >> Add a NULL check. > >> > >> Cc: Igor Mammedov > >> Signed-off-by: Andreas F=E4rber > >=20 > > Fair enough but I'm guessing we should > > assume devices without a parent are not > > hotpluggable, should we not? > >=20 > > Without a bus what handles hotplug? >=20 > The device has to, in its realize function. > >> --- > >> hw/core/qdev.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c > >> index 749c83a..cb07863 100644 > >> --- a/hw/core/qdev.c > >> +++ b/hw/core/qdev.c > >> @@ -735,7 +735,8 @@ static bool device_get_hotpluggable(Object *obj,= Error **err) > >> DeviceClass *dc =3D DEVICE_GET_CLASS(obj); > >> DeviceState *dev =3D DEVICE(obj); > >> =20 > >> - return dc->hotpluggable && dev->parent_bus->allow_hotplug; > >> + return dc->hotpluggable && (dev->parent_bus =3D=3D NULL || > >> + dev->parent_bus->allow_hotplug); > >=20 > > So maybe this should be: > >> return dc->hotpluggable && dev->parent_bus && > >> dev->parent_bus->allow_hotplug; > >=20 > > ? >=20 > Hmm... that would render non-x86 CPUs non-hotpluggable. I guess we need > to check dc->bus_type for a NULL value and if non-NULL use your variant > and otherwise mine. > I'll give it some more thought. >=20 > Thanks, > Andreas Or clear dc->hotpluggable if there's no parent bus and we don't want to allow hotplug. I'm beginning to think your patch is the right one after all... >=20 > >=20 > >> } > >> =20 > >> static void device_initfn(Object *obj) > >> --=20 > >> 1.8.4.5 >=20 > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrn= berg