From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38959) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XX8pc-0005ap-2I for qemu-devel@nongnu.org; Thu, 25 Sep 2014 09:11:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XX8pT-0008WT-V8 for qemu-devel@nongnu.org; Thu, 25 Sep 2014 09:11:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:31378) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XX8pT-0008V4-NL for qemu-devel@nongnu.org; Thu, 25 Sep 2014 09:11:27 -0400 Date: Thu, 25 Sep 2014 15:11:10 +0200 From: Igor Mammedov Message-ID: <20140925151110.250dccd8@nial.usersys.redhat.com> In-Reply-To: <20140925130838.492d655d.cornelia.huck@de.ibm.com> References: <1411559299-19042-1-git-send-email-imammedo@redhat.com> <1411559299-19042-21-git-send-email-imammedo@redhat.com> <20140925130838.492d655d.cornelia.huck@de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 20/30] s390x: convert virtio-ccw to hotplug handler API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: dmitry@daynix.com, mst@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, borntraeger@de.ibm.com, kraxel@redhat.com, amit.shah@redhat.com, pbonzini@redhat.com, rth@twiddle.net On Thu, 25 Sep 2014 13:08:38 +0200 Cornelia Huck wrote: > On Wed, 24 Sep 2014 11:48:09 +0000 > Igor Mammedov wrote: >=20 > > Signed-off-by: Igor Mammedov > > --- > > hw/s390x/virtio-ccw.c | 24 ++++++++++++++++-------- > > 1 file changed, 16 insertions(+), 8 deletions(-) >=20 > Well, I think I now see what's going on here. More below... >=20 >=20 > > @@ -1620,13 +1620,13 @@ static Property virtio_ccw_properties[] =3D { > > static void virtio_ccw_device_class_init(ObjectClass *klass, void *dat= a) > > { > > DeviceClass *dc =3D DEVICE_CLASS(klass); > > + HotplugHandlerClass *hc =3D HOTPLUG_HANDLER_CLASS(klass); > >=20 > > dc->props =3D virtio_ccw_properties; > > dc->init =3D virtio_ccw_busdev_init; > > dc->exit =3D virtio_ccw_busdev_exit; > > - dc->unplug =3D virtio_ccw_busdev_unplug; >=20 > Before, this callback was invoked when a device of the virtio-ccw class > was unplugged. >=20 > > dc->bus_type =3D TYPE_VIRTUAL_CSS_BUS; > > - > > + hc->unplug =3D virtio_ccw_busdev_unplug; >=20 > Now, this callback is supposed to be invoked instead. However, the > unplugging code invokes the callback for the _parent bus_, which > means... >=20 > > } > >=20 > > static const TypeInfo virtio_ccw_device_info =3D { >=20 > > static void virtual_css_bridge_class_init(ObjectClass *klass, void *da= ta) > > { > > SysBusDeviceClass *k =3D SYS_BUS_DEVICE_CLASS(klass); > > + HotplugHandlerClass *hc =3D HOTPLUG_HANDLER_CLASS(klass); > >=20 > > k->init =3D virtual_css_bridge_init; > > + hc->unplug =3D qdev_simple_device_unplug_cb; >=20 > ...we're invoking this one, as the parent bus for virtio-ccw devices is > the virtual-css bus. >=20 > If I change this callback to the virtio-ccw one, everything works as > expected. >=20 > > } >=20 > So, to summarize, what happened before was >=20 > bridge device <--- (simple unplug invoked for dev) simple unplug should not exits for above device > -> virtual bus > -> virtio proxy device <--- virtio unplug invoked for dev > -> virtio bus > -> virtio device >=20 > which your patch changed to >=20 > bridge device > -> virtual bus <--- simple unplug invoked for virtio proxy dev > -> virtio proxy device > -> virtio bus <--- (virtio unplug invoked for virtio dev) > -> virtio device >=20 > Am I understanding this correctly? Let's try other way around: bridge device (virtual_css_bridge) - non hotpluggable sysbus device -> virtual bus (VIRTUAL_CSS_BUS) -> virtio proxy device | -> virtio bus |- virtio_ccw_device_foo composite device managed= with -> virtio device | device_add/del command internal "virtio device" is the only child of "virtio bus" and it's not sup= posed to be managed via device_add/del. So to unplug "virtio proxy device" we should call virtio_ccw_busdev_unplug stored in "bridge device" with "virtual bus" using bridge as hotplug-handle= r. does following patch work for you? --- =46rom 8f249aa4686f0a7dfa5d9636d1eee68f1d264316 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Fri, 19 Sep 2014 09:07:10 +0000 Subject: [PATCH] s390x: convert virtio-ccw to hotplug handler API Signed-off-by: Igor Mammedov --- hw/s390x/virtio-ccw.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 33a1d86..036bd20 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -230,7 +230,7 @@ VirtualCssBus *virtual_css_bus_init(void) cbus =3D VIRTUAL_CSS_BUS(bus); =20 /* Enable hotplugging */ - bus->allow_hotplug =3D 1; + qbus_set_hotplug_handler(bus, dev, &error_abort); =20 return cbus; } @@ -1590,7 +1590,8 @@ static int virtio_ccw_busdev_exit(DeviceState *dev) return _info->exit(_dev); } =20 -static int virtio_ccw_busdev_unplug(DeviceState *dev) +static void virtio_ccw_busdev_unplug(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { VirtioCcwDevice *_dev =3D (VirtioCcwDevice *)dev; SubchDev *sch =3D _dev->sch; @@ -1609,7 +1610,6 @@ static int virtio_ccw_busdev_unplug(DeviceState *dev) css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, 1, 0); =20 object_unparent(OBJECT(dev)); - return 0; } =20 static Property virtio_ccw_properties[] =3D { @@ -1624,9 +1624,7 @@ static void virtio_ccw_device_class_init(ObjectClass = *klass, void *data) dc->props =3D virtio_ccw_properties; dc->init =3D virtio_ccw_busdev_init; dc->exit =3D virtio_ccw_busdev_exit; - dc->unplug =3D virtio_ccw_busdev_unplug; dc->bus_type =3D TYPE_VIRTUAL_CSS_BUS; - } =20 static const TypeInfo virtio_ccw_device_info =3D { @@ -1636,6 +1634,10 @@ static const TypeInfo virtio_ccw_device_info =3D { .class_init =3D virtio_ccw_device_class_init, .class_size =3D sizeof(VirtIOCCWDeviceClass), .abstract =3D true, + .interfaces =3D (InterfaceInfo[]) { + { TYPE_HOTPLUG_HANDLER }, + { } + } }; =20 /***************** Virtual-css Bus Bridge Device ********************/ @@ -1650,8 +1652,10 @@ static int virtual_css_bridge_init(SysBusDevice *dev) static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) { SysBusDeviceClass *k =3D SYS_BUS_DEVICE_CLASS(klass); + HotplugHandlerClass *hc =3D HOTPLUG_HANDLER_CLASS(klass); =20 k->init =3D virtual_css_bridge_init; + hc->unplug =3D virtio_ccw_busdev_unplug; } =20 static const TypeInfo virtual_css_bridge_info =3D { @@ -1659,6 +1663,10 @@ static const TypeInfo virtual_css_bridge_info =3D { .parent =3D TYPE_SYS_BUS_DEVICE, .instance_size =3D sizeof(SysBusDevice), .class_init =3D virtual_css_bridge_class_init, + .interfaces =3D (InterfaceInfo[]) { + { TYPE_HOTPLUG_HANDLER }, + { } + } }; =20 /* virtio-ccw-bus */ @@ -1667,13 +1675,11 @@ static void virtio_ccw_bus_new(VirtioBusState *bus,= size_t bus_size, VirtioCcwDevice *dev) { DeviceState *qdev =3D DEVICE(dev); - BusState *qbus; char virtio_bus_name[] =3D "virtio-bus"; =20 qbus_create_inplace(bus, bus_size, TYPE_VIRTIO_CCW_BUS, qdev, virtio_bus_name); - qbus =3D BUS(bus); - qbus->allow_hotplug =3D 1; + qbus_set_hotplug_handler(BUS(bus), qdev, &error_abort); } =20 static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data) --=20 1.8.3.1