diff mbox

[v5,01/13] block: Add op blocker type "device IO"

Message ID 1432102576-6637-2-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng May 20, 2015, 6:16 a.m. UTC
It blocks device IO.

All bdrv_op_block_all/blk_op_block_all callers are taken care of:

- virtio_blk_data_plane_create
- virtio_scsi_hotplug

  Device creation, unblock it.

- bdrv_set_backing_hd

  Backing hd is not used by device, so blocking is OK.

- backup_start

  Blocking target when backup is running, unblock it.

- mirror_complete

  Blocking s->to_replace until mirror_exit, OK.

- block_job_complete

  The block job may be long running. Unblock it.

- init_blk_migration

  The block migration may be long running, Unblock it.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockjob.c                      | 1 +
 hw/block/dataplane/virtio-blk.c | 1 +
 hw/scsi/virtio-scsi.c           | 1 +
 include/block/block.h           | 1 +
 migration/block.c               | 1 +
 5 files changed, 5 insertions(+)

Comments

Max Reitz May 23, 2015, 3:20 p.m. UTC | #1
On 20.05.2015 08:16, Fam Zheng wrote:
> It blocks device IO.
>
> All bdrv_op_block_all/blk_op_block_all callers are taken care of:
>
> - virtio_blk_data_plane_create
> - virtio_scsi_hotplug
>
>    Device creation, unblock it.
>
> - bdrv_set_backing_hd
>
>    Backing hd is not used by device, so blocking is OK.
>
> - backup_start
>
>    Blocking target when backup is running, unblock it.
>
> - mirror_complete
>
>    Blocking s->to_replace until mirror_exit, OK.
>
> - block_job_complete
>
>    The block job may be long running. Unblock it.
>
> - init_blk_migration
>
>    The block migration may be long running, Unblock it.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   blockjob.c                      | 1 +
>   hw/block/dataplane/virtio-blk.c | 1 +
>   hw/scsi/virtio-scsi.c           | 1 +
>   include/block/block.h           | 1 +
>   migration/block.c               | 1 +
>   5 files changed, 5 insertions(+)

Reviewed-by: Max Reitz <mreitz@redhat.com>
Max Reitz May 23, 2015, 3:25 p.m. UTC | #2
On 20.05.2015 08:16, Fam Zheng wrote:
> It blocks device IO.
>
> All bdrv_op_block_all/blk_op_block_all callers are taken care of:
>
> - virtio_blk_data_plane_create
> - virtio_scsi_hotplug
>
>    Device creation, unblock it.
>
> - bdrv_set_backing_hd
>
>    Backing hd is not used by device, so blocking is OK.
>
> - backup_start
>
>    Blocking target when backup is running, unblock it.

Hm, wait, here you are saying that you want to unblock the target, but 
you're actually not doing this. I think keeping it blocked is fine 
(which is why I gave the R-b), because we don't want any I/O on the 
backup target.

So something is off, maybe it's the commit message, maybe the patch, 
maybe just me having missed something. :-)

Max

> - mirror_complete
>
>    Blocking s->to_replace until mirror_exit, OK.
>
> - block_job_complete
>
>    The block job may be long running. Unblock it.
>
> - init_blk_migration
>
>    The block migration may be long running, Unblock it.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   blockjob.c                      | 1 +
>   hw/block/dataplane/virtio-blk.c | 1 +
>   hw/scsi/virtio-scsi.c           | 1 +
>   include/block/block.h           | 1 +
>   migration/block.c               | 1 +
>   5 files changed, 5 insertions(+)
>
> diff --git a/blockjob.c b/blockjob.c
> index 2755465..e39bdde 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -51,6 +51,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
>                  BlockJobType_lookup[driver->job_type]);
>       bdrv_op_block_all(bs, job->blocker);
>       bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
> +    bdrv_op_unblock(bs, BLOCK_OP_TYPE_DEVICE_IO, job->blocker);
>   
>       job->driver        = driver;
>       job->bs            = bs;
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index 3db139b..3ecc8bd 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -209,6 +209,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
>       blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker);
>       blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker);
>       blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker);
> +    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_DEVICE_IO, s->blocker);
>   
>       *dataplane = s;
>   }
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index e242fef..5e15fa6 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -772,6 +772,7 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>               return;
>           }
>           blk_op_block_all(sd->conf.blk, s->blocker);
> +        blk_op_unblock(sd->conf.blk, BLOCK_OP_TYPE_DEVICE_IO, s->blocker);
>           aio_context_acquire(s->ctx);
>           blk_set_aio_context(sd->conf.blk, s->ctx);
>           aio_context_release(s->ctx);
> diff --git a/include/block/block.h b/include/block/block.h
> index 7d1a717..906fb31 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -159,6 +159,7 @@ typedef enum BlockOpType {
>       BLOCK_OP_TYPE_RESIZE,
>       BLOCK_OP_TYPE_STREAM,
>       BLOCK_OP_TYPE_REPLACE,
> +    BLOCK_OP_TYPE_DEVICE_IO,
>       BLOCK_OP_TYPE_MAX,
>   } BlockOpType;
>   
> diff --git a/migration/block.c b/migration/block.c
> index ddb59cc..b833bac 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -379,6 +379,7 @@ static void init_blk_migration(QEMUFile *f)
>           alloc_aio_bitmap(bmds);
>           error_setg(&bmds->blocker, "block device is in use by migration");
>           bdrv_op_block_all(bs, bmds->blocker);
> +        bdrv_op_unblock(bs, BLOCK_OP_TYPE_DEVICE_IO, bmds->blocker);
>           bdrv_ref(bs);
>   
>           block_mig_state.total_sector_sum += sectors;
diff mbox

Patch

diff --git a/blockjob.c b/blockjob.c
index 2755465..e39bdde 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -51,6 +51,7 @@  void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
                BlockJobType_lookup[driver->job_type]);
     bdrv_op_block_all(bs, job->blocker);
     bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
+    bdrv_op_unblock(bs, BLOCK_OP_TYPE_DEVICE_IO, job->blocker);
 
     job->driver        = driver;
     job->bs            = bs;
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 3db139b..3ecc8bd 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -209,6 +209,7 @@  void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
     blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker);
     blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker);
     blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker);
+    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_DEVICE_IO, s->blocker);
 
     *dataplane = s;
 }
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index e242fef..5e15fa6 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -772,6 +772,7 @@  static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
             return;
         }
         blk_op_block_all(sd->conf.blk, s->blocker);
+        blk_op_unblock(sd->conf.blk, BLOCK_OP_TYPE_DEVICE_IO, s->blocker);
         aio_context_acquire(s->ctx);
         blk_set_aio_context(sd->conf.blk, s->ctx);
         aio_context_release(s->ctx);
diff --git a/include/block/block.h b/include/block/block.h
index 7d1a717..906fb31 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -159,6 +159,7 @@  typedef enum BlockOpType {
     BLOCK_OP_TYPE_RESIZE,
     BLOCK_OP_TYPE_STREAM,
     BLOCK_OP_TYPE_REPLACE,
+    BLOCK_OP_TYPE_DEVICE_IO,
     BLOCK_OP_TYPE_MAX,
 } BlockOpType;
 
diff --git a/migration/block.c b/migration/block.c
index ddb59cc..b833bac 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -379,6 +379,7 @@  static void init_blk_migration(QEMUFile *f)
         alloc_aio_bitmap(bmds);
         error_setg(&bmds->blocker, "block device is in use by migration");
         bdrv_op_block_all(bs, bmds->blocker);
+        bdrv_op_unblock(bs, BLOCK_OP_TYPE_DEVICE_IO, bmds->blocker);
         bdrv_ref(bs);
 
         block_mig_state.total_sector_sum += sectors;