On 15.05.2015 08:04, Fam Zheng wrote:
virtio-blk now listens to op blocker change of the associated block
backend.
Up on setting op blocker on BLOCK_OP_TYPE_DEVICE_IO:
non-dataplane:
1) Set VirtIOBlock.paused
2) In virtio_blk_handle_output, do nothing if VirtIOBlock.paused
dataplane:
1) Clear the host event notifier
2) In handle_notify, do nothing if VirtIOBlock.paused
Up on removing the op blocker:
non-dataplane:
1) Clear VirtIOBlock.paused
2) Schedule a BH on the AioContext of the backend, which calls
virtio_blk_handle_output, so that the previous unhandled kicks can
make progress
dataplane:
1) Set the host event notifier
2) Notify the host event notifier so that unhandled events are
processed
Signed-off-by: Fam Zheng <f...@redhat.com>
---
hw/block/dataplane/virtio-blk.c | 25 +++++++++++++++-
hw/block/virtio-blk.c | 63 +++++++++++++++++++++++++++++++++++++++--
include/hw/virtio/virtio-blk.h | 8 +++++-
3 files changed, 92 insertions(+), 4 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index e287e09..a5e8e35 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -87,8 +87,28 @@ static void complete_request_vring(VirtIOBlockReq *req,
unsigned char status)
qemu_bh_schedule(s->bh);
}
+static void virtio_blk_data_plane_pause(VirtIOBlock *vblk)
+{
+ VirtIOBlockDataPlane *s = vblk->dataplane;
+
+ event_notifier_test_and_clear(&s->host_notifier);
+ aio_set_event_notifier(s->ctx, &s->host_notifier, NULL);
+}
+
+static void handle_notify(EventNotifier *e);
+static void virtio_blk_data_plane_resume(VirtIOBlock *vblk)
+{
+ VirtIOBlockDataPlane *s = vblk->dataplane;
+
+ aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify);
+
+ event_notifier_set(&s->host_notifier);
+}
+
static const VirtIOBlockOps virtio_blk_data_plane_ops = {
- .complete_request = complete_request_vring,
+ .complete_request = complete_request_vring,
+ .pause = virtio_blk_data_plane_pause,
+ .resume = virtio_blk_data_plane_resume,
};
static void handle_notify(EventNotifier *e)
@@ -98,6 +118,9 @@ static void handle_notify(EventNotifier *e)
VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
event_notifier_test_and_clear(&s->host_notifier);
+ if (vblk->paused) {
+ return;
+ }
blk_io_plug(s->conf->conf.blk);
for (;;) {
MultiReqBuffer mrb = {};
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index f4a9d19..4bc1b2a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -59,8 +59,38 @@ static void virtio_blk_complete_request(VirtIOBlockReq *req,
virtio_notify(vdev, s->vq);
}
+typedef struct {
+ QEMUBH *bh;
+ VirtIOBlock *s;
+} VirtIOBlockResumeData;
+
+static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq);
+static void virtio_blk_resume_bh_cb(void *opaque)
+{
+ VirtIOBlockResumeData *data = opaque;
+ qemu_bh_delete(data->bh);
+ virtio_blk_handle_output(VIRTIO_DEVICE(data->s), data->s->vq);
+}
+
+static void virtio_blk_pause(VirtIOBlock *vblk)
+{
+ /* TODO: stop ioeventfd */
+}
+
+static void virtio_blk_resume(VirtIOBlock *vblk)
+{
+ VirtIOBlockResumeData *data = g_new(VirtIOBlockResumeData, 1);
+ data->bh = aio_bh_new(blk_get_aio_context(vblk->blk),
+ virtio_blk_resume_bh_cb, data);
+ data->s = vblk;
+ data->s->paused = false;
+ qemu_bh_schedule(data->bh);
+}
+
static const VirtIOBlockOps virtio_blk_ops = (VirtIOBlockOps) {
- .complete_request = virtio_blk_complete_request,
+ .complete_request = virtio_blk_complete_request,
+ .pause = virtio_blk_pause,
+ .resume = virtio_blk_resume,
};
static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status)
@@ -597,6 +627,9 @@ static void virtio_blk_handle_output(VirtIODevice *vdev,
VirtQueue *vq)
VirtIOBlockReq *req;
MultiReqBuffer mrb = {};
+ if (s->paused) {
+ return;
+ }
/* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
* dataplane here instead of waiting for .set_status().
*/
@@ -787,7 +820,7 @@ static void virtio_blk_save(QEMUFile *f, void *opaque)
virtio_save(vdev, f);
}
-
+
static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f)
{
VirtIOBlock *s = VIRTIO_BLK(vdev);
@@ -875,6 +908,29 @@ static void virtio_blk_migration_state_changed(Notifier
*notifier, void *data)
}
}
+static void virtio_blk_op_blocker_changed(Notifier *notifier, void *opaque)
+{
+ BlockOpEvent *event = opaque;
+ VirtIOBlock *s = container_of(notifier, VirtIOBlock,
+ op_blocker_notifier);
+ bool pause;
+
+ if (event->type != BLOCK_OP_TYPE_DEVICE_IO) {
+ return;
+ }
+ pause = event->blocking || blk_op_is_blocked(s->blk,
+ BLOCK_OP_TYPE_DEVICE_IO,
+ NULL);
Indentation is off if you intended to indent based on the opening
parenthesis.
Different question: Since event->type == BLOCK_OP_TYPE_DEVICE_IO, when
will event->blocking ever be not equal to blk_op_is_blocked()?
+ if (pause == s->paused) {
+ return;
+ }
+ if (pause) {
+ s->ops->pause(s);
+ } else {
+ s->ops->resume(s);
+ }
+}
+
static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
{
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -926,6 +982,9 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size);
blk_iostatus_enable(s->blk);
+
+ s->op_blocker_notifier.notify = virtio_blk_op_blocker_changed;
+ blk_op_blocker_add_notifier(s->blk, &s->op_blocker_notifier);
This is indeed a way to make sure the notifier is not leaked. *cough* I
don't know why I never think of that...
Apart from the event->blocking question above, looks good to me
(although that's a pretty weak statement, considering I don't know the
virtio code that well...).
Max
}
static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 28b3436..aa15fea 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -42,12 +42,16 @@ struct VirtIOBlkConf
};
struct VirtIOBlockDataPlane;
-
+struct VirtIOBlock;
struct VirtIOBlockReq;
typedef struct {
/* Function to push to vq and notify guest */
void (*complete_request)(struct VirtIOBlockReq *req, unsigned char
status);
+
+ /* Functions to pause/resume request handling */
+ void (*pause)(struct VirtIOBlock *vblk);
+ void (*resume)(struct VirtIOBlock *vblk);
} VirtIOBlockOps;
typedef struct VirtIOBlock {
@@ -62,6 +66,8 @@ typedef struct VirtIOBlock {
VMChangeStateEntry *change;
const VirtIOBlockOps *ops;
Notifier migration_state_notifier;
+ Notifier op_blocker_notifier;
+ bool paused;
struct VirtIOBlockDataPlane *dataplane;
} VirtIOBlock;
