From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34902) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWyMD-0006Iz-LR for qemu-devel@nongnu.org; Wed, 24 Sep 2014 22:00:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWyM7-0000xg-1W for qemu-devel@nongnu.org; Wed, 24 Sep 2014 22:00:33 -0400 Received: from [59.151.112.132] (port=53756 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWyM6-0000wP-CN for qemu-devel@nongnu.org; Wed, 24 Sep 2014 22:00:26 -0400 Message-ID: <54237725.8070100@cn.fujitsu.com> Date: Thu, 25 Sep 2014 10:00:05 +0800 From: Tang Chen MIME-Version: 1.0 References: <1411559299-19042-1-git-send-email-imammedo@redhat.com> <1411559299-19042-10-git-send-email-imammedo@redhat.com> In-Reply-To: <1411559299-19042-10-git-send-email-imammedo@redhat.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 09/30] access BusState.allow_hotplug using wraper qbus_is_hotpluggable() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov , qemu-devel@nongnu.org Cc: cornelia.huck@de.ibm.com, mst@redhat.com, amit.shah@redhat.com, agraf@suse.de, borntraeger@de.ibm.com, kraxel@redhat.com, dmitry@daynix.com, pbonzini@redhat.com, rth@twiddle.net On 09/24/2014 07:47 PM, Igor Mammedov wrote: > it would allow transparently switch detection if Bus > is hotpluggable from allow_hotplug field to hotplug_handler > link and drop allow_hotplug field once all users are > converted to hotplug handler API. > > Signed-off-by: Igor Mammedov > --- > hw/core/qdev.c | 6 +++--- > hw/i386/acpi-build.c | 2 +- > hw/pci/pci-hotplug-old.c | 4 ++-- > include/hw/qdev-core.h | 5 +++++ > qdev-monitor.c | 2 +- > 5 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index fcb1638..5e5b963 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -86,7 +86,7 @@ static void bus_add_child(BusState *bus, DeviceState *child) > BusChild *kid = g_malloc0(sizeof(*kid)); > > if (qdev_hotplug) { > - assert(bus->allow_hotplug); > + assert(qbus_is_hotpluggable(bus)); > } > > kid->index = bus->max_index++; > @@ -213,7 +213,7 @@ void qdev_unplug(DeviceState *dev, Error **errp) > { > DeviceClass *dc = DEVICE_GET_CLASS(dev); > > - if (dev->parent_bus && !dev->parent_bus->allow_hotplug) { > + if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { > error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); > return; > } > @@ -925,7 +925,7 @@ static bool device_get_hotpluggable(Object *obj, Error **errp) > DeviceState *dev = DEVICE(obj); > > return dc->hotpluggable && (dev->parent_bus == NULL || > - dev->parent_bus->allow_hotplug); > + qbus_is_hotpluggable(dev->parent_bus)); > } > > static bool device_get_hotplugged(Object *obj, Error **err) > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index a313321..00be4bb 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -774,7 +774,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) > unsigned *bsel_alloc = opaque; > unsigned *bus_bsel; > > - if (bus->qbus.allow_hotplug) { > + if (qbus_is_hotpluggable(BUS(bus))) { > bus_bsel = g_malloc(sizeof *bus_bsel); > > *bus_bsel = (*bsel_alloc)++; > diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c > index cf2caeb..f9b7398 100644 > --- a/hw/pci/pci-hotplug-old.c > +++ b/hw/pci/pci-hotplug-old.c > @@ -77,7 +77,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon, > monitor_printf(mon, "Invalid PCI device address %s\n", devaddr); > return NULL; > } > - if (!((BusState*)bus)->allow_hotplug) { > + if (!qbus_is_hotpluggable(BUS(bus))) { > monitor_printf(mon, "PCI bus doesn't support hotplug\n"); > return NULL; > } > @@ -224,7 +224,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon, > monitor_printf(mon, "Invalid PCI device address %s\n", devaddr); > return NULL; > } > - if (!((BusState*)bus)->allow_hotplug) { > + if (!qbus_is_hotpluggable(BUS(bus))) { > monitor_printf(mon, "PCI bus doesn't support hotplug\n"); > return NULL; > } > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 178fee2..48a96d2 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -368,4 +368,9 @@ static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, > QDEV_HOTPLUG_HANDLER_PROPERTY, errp); > bus->allow_hotplug = 1; > } > + > +static inline bool qbus_is_hotpluggable(BusState *bus) > +{ > + return bus->allow_hotplug || bus->hotplug_handler; Why not synchronize these two members ? What is the case that bus has a hotplug_handler but allow_hotplug is false ? BTW, I think there are other places that using bus->allow_hotplug directly, right ? eg: s390_virtio_bus_init() { ...... _bus->allow_hotplug = 1; ...... } Thanks. > +} > #endif > diff --git a/qdev-monitor.c b/qdev-monitor.c > index 5ec6606..f6db461 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -515,7 +515,7 @@ DeviceState *qdev_device_add(QemuOpts *opts) > return NULL; > } > } > - if (qdev_hotplug && bus && !bus->allow_hotplug) { > + if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) { > qerror_report(QERR_BUS_NO_HOTPLUG, bus->name); > return NULL; > }