mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

call HotplugHandler->plug() as the last step in device realization

Browse Source 
When [2] was fixed it was agreed that adding and calling post_plug()
callback after device_reset() was low risk approach to hotfix issue
right before release. So it was merged instead of moving already
existing plug() callback after device_reset() is called which would
be more risky and require all plug() callbacks audit.

Looking at the current plug() callbacks, it doesn't seem that moving
plug() callback after device_reset() is breaking anything, so here
goes agreed upon [3] proper fix which essentially reverts [1][2]
and moves plug() callback after device_reset().
This way devices always comes to plug() stage, after it's been fully
initialized (including being reset), which fixes race condition [2]
without need for an extra post_plug() callback.

 1. (25e897881 "qdev: add HotplugHandler->post_plug() callback")
 2. (8449bcf94 "virtio-scsi: fix hotplug ->reset() vs event race")
 3. https://www.mail-archive.com/qemu-devel@nongnu.org/msg549915.html

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1539696820-273275-1-git-send-email-imammedo@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Pierre Morel<pmorel@linux.ibm.com>
Acked-by: Pierre Morel<pmorel@linux.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Igor Mammedov 2018-10-16 15:33:40 +02:00 committed by Paolo Bonzini
parent ac0989f53d
commit 8b5e6caf01
4 changed files with 7 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
hw/core/hotplug.c
View File

@ -35,16 +35,6 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
}
}
void hotplug_handler_post_plug(HotplugHandler *plug_handler,
DeviceState *plugged_dev)
{
HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
if (hdc->post_plug) {
hdc->post_plug(plug_handler, plugged_dev);
}
}
void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
DeviceState *plugged_dev,
Error **errp)

16
hw/core/qdev.c
View File

@ -832,14 +832,6 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
DEVICE_LISTENER_CALL(realize, Forward, dev);
if (hotplug_ctrl) {
hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
}
if (local_err != NULL) {
goto post_realize_fail;
}
/*
* always free/re-initialize here since the value cannot be cleaned up
* in device_unrealize due to its usage later on in the unplug path
@ -869,8 +861,12 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
dev->pending_deleted_event = false;
if (hotplug_ctrl) {
hotplug_handler_post_plug(hotplug_ctrl, dev);
}
hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
if (local_err != NULL) {
goto child_realize_fail;
}
}
} else if (!value && dev->realized) {
Error **local_errp = NULL;
QLIST_FOREACH(bus, &dev->child_bus, sibling) {

11
hw/scsi/virtio-scsi.c
View File

@ -797,16 +797,8 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
virtio_scsi_acquire(s);
blk_set_aio_context(sd->conf.blk, s->ctx);
virtio_scsi_release(s);
}
}
/* Announce the new device after it has been plugged */
static void virtio_scsi_post_hotplug(HotplugHandler *hotplug_dev,
DeviceState *dev)
{
VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev);
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
SCSIDevice *sd = SCSI_DEVICE(dev);
}
if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
virtio_scsi_acquire(s);
@ -976,7 +968,6 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data)
vdc->start_ioeventfd = virtio_scsi_dataplane_start;
vdc->stop_ioeventfd = virtio_scsi_dataplane_stop;
hc->plug = virtio_scsi_hotplug;
hc->post_plug = virtio_scsi_post_hotplug;
hc->unplug = virtio_scsi_hotunplug;
}

11
include/hw/hotplug.h
View File

@ -47,8 +47,6 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
* @parent: Opaque parent interface.
* @pre_plug: pre plug callback called at start of device.realize(true)
* @plug: plug callback called at end of device.realize(true).
* @post_plug: post plug callback called after device.realize(true) and device
* reset
* @unplug_request: unplug request callback.
* Used as a means to initiate device unplug for devices that
* require asynchronous unplug handling.
@ -63,7 +61,6 @@ typedef struct HotplugHandlerClass {
/* <public> */
hotplug_fn pre_plug;
hotplug_fn plug;
void (*post_plug)(HotplugHandler *plug_handler, DeviceState *plugged_dev);
hotplug_fn unplug_request;
hotplug_fn unplug;
} HotplugHandlerClass;
@ -86,14 +83,6 @@ void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
DeviceState *plugged_dev,
Error **errp);
/**
* hotplug_handler_post_plug:
*
* Call #HotplugHandlerClass.post_plug callback of @plug_handler.
*/
void hotplug_handler_post_plug(HotplugHandler *plug_handler,
DeviceState *plugged_dev);
/**
* hotplug_handler_unplug_request:
*