* [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter @ 2014-03-07 18:13 Andreas Färber 2014-03-09 16:40 ` Michael S. Tsirkin 0 siblings, 1 reply; 5+ messages in thread From: Andreas Färber @ 2014-03-07 18:13 UTC (permalink / raw) To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber, mst 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 <imammedo@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de> --- 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); } static void device_initfn(Object *obj) -- 1.8.4.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter 2014-03-07 18:13 [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter Andreas Färber @ 2014-03-09 16:40 ` Michael S. Tsirkin 2014-03-09 16:48 ` Andreas Färber 0 siblings, 1 reply; 5+ messages in thread From: Michael S. Tsirkin @ 2014-03-09 16:40 UTC (permalink / raw) To: Andreas Färber; +Cc: Igor Mammedov, qemu-devel 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 <imammedo@redhat.com> > Signed-off-by: Andreas Färber <afaerber@suse.de> 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? > --- > 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; ? > } > > static void device_initfn(Object *obj) > -- > 1.8.4.5 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter 2014-03-09 16:40 ` Michael S. Tsirkin @ 2014-03-09 16:48 ` Andreas Färber 2014-03-09 16:55 ` Michael S. Tsirkin 0 siblings, 1 reply; 5+ messages in thread From: Andreas Färber @ 2014-03-09 16:48 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Igor Mammedov, qemu-devel 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 <imammedo@redhat.com> >> Signed-off-by: Andreas Färber <afaerber@suse.de> > > 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 > >> } >> >> 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter 2014-03-09 16:48 ` Andreas Färber @ 2014-03-09 16:55 ` Michael S. Tsirkin 2014-03-10 11:37 ` Igor Mammedov 0 siblings, 1 reply; 5+ messages in thread From: Michael S. Tsirkin @ 2014-03-09 16:55 UTC (permalink / raw) To: Andreas Färber; +Cc: Igor Mammedov, qemu-devel 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 <imammedo@redhat.com> > >> Signed-off-by: Andreas Färber <afaerber@suse.de> > > > > 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... > > > > >> } > >> > >> 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter 2014-03-09 16:55 ` Michael S. Tsirkin @ 2014-03-10 11:37 ` Igor Mammedov 0 siblings, 0 replies; 5+ messages in thread From: Igor Mammedov @ 2014-03-10 11:37 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Andreas Färber, qemu-devel On Sun, 9 Mar 2014 18:55:05 +0200 "Michael S. Tsirkin" <mst@redhat.com> 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 <imammedo@redhat.com> > > >> Signed-off-by: Andreas Färber <afaerber@suse.de> > > > > > > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-10 11:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-07 18:13 [Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter Andreas Färber 2014-03-09 16:40 ` Michael S. Tsirkin 2014-03-09 16:48 ` Andreas Färber 2014-03-09 16:55 ` Michael S. Tsirkin 2014-03-10 11:37 ` Igor Mammedov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).