[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotplu
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter |
Date: |
Mon, 10 Mar 2014 12:37:14 +0100 |
On Sun, 9 Mar 2014 18:55:05 +0200
"Michael S. Tsirkin" <address@hidden> wrote:
> On Sun, Mar 09, 2014 at 05:48:57PM +0100, Andreas Färber wrote:
> > Am 09.03.2014 17:40, schrieb Michael S. Tsirkin:
> > > On Fri, Mar 07, 2014 at 07:13:18PM +0100, Andreas Färber wrote:
> > >> Commit 1a37eca107cece3ed454bae29eef0bd1fac4a244 (qdev: add
> > >> "hotpluggable" property to Device) added a property "hotpluggable" to
> > >> each device, with its getter accessing parent_bus->allow_hotplug.
> > >>
> > >> Add a NULL check.
> > >>
> > >> Cc: Igor Mammedov <address@hidden>
> > >> Signed-off-by: Andreas Färber <address@hidden>
> > >
> > > Fair enough but I'm guessing we should
> > > assume devices without a parent are not
> > > hotpluggable, should we not?
> > >
> > > Without a bus what handles hotplug?
> >
> > 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 = DEVICE_GET_CLASS(obj);
> > >> DeviceState *dev = DEVICE(obj);
> > >>
> > >> - return dc->hotpluggable && dev->parent_bus->allow_hotplug;
> > >> + return dc->hotpluggable && (dev->parent_bus == NULL ||
> > >> + dev->parent_bus->allow_hotplug);
> > >
> > > So maybe this should be:
> > >> return dc->hotpluggable && dev->parent_bus &&
> > >> dev->parent_bus->allow_hotplug;
> > >
> > > ?
> >
> > 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.
> >
> > 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...
Before hotplug rework all devices were assumed to be hotpluggable,
the non-hotpluggable ones explicitly cleared dc->hotpluggable in
class_init(), rework hasn't changed it so Andreas's patch looks
better as to not block hotplug on bus-less devices.
Later if we implement bus-less hotplug with link<>s, we can
add link<> set hook to that will call hotplug handler in the same manner
as it's done for bus based devices.
> >
> > >
> > >> }
> > >>
> > >> static void device_initfn(Object *obj)
> > >> --
> > >> 1.8.4.5
> >
> > --
> > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg