Introduce refcnt_soft (soft reference) and refcnt_hard (hard reference)
to BlockDriverState, since in_use mechanism cannot provide proper
management of lifecycle when a BDS is referenced in multiple places
(e.g. pointed to by another bs's backing_hd while also used as a block
job device, in the use case of image fleecing).
The original in_use case is considered a "hard reference" in this patch,
where the bs is busy and should not be used in other tasks that require
a hard reference. (However the interface doesn't force this, caller
still need to call bdrv_in_use() to check by itself.).
A soft reference is implemented but not used yet. It will be used in
following patches to manage the lifecycle together with hard reference.
If bdrv_ref() is called on a BDS, it must be released by exactly the
same numbers of bdrv_unref() with the same "soft/hard" type, and never
call bdrv_delete() directly. If the BDS is only used locally (unnamed),
bdrv_ref/bdrv_unref can be skipped and just use bdrv_delete().
Signed-off-by: Fam Zheng <f...@redhat.com>
---
block-migration.c | 4 ++--
block.c | 40 +++++++++++++++++++++++++++++++---------
blockjob.c | 6 +++---
hw/block/dataplane/virtio-blk.c | 4 ++--
include/block/block.h | 5 +++--
include/block/block_int.h | 3 ++-
6 files changed, 43 insertions(+), 19 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index 2fd7699..d558410 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -321,7 +321,7 @@ static void init_blk_migration_it(void *opaque,
BlockDriverState *bs)
bmds->shared_base = block_mig_state.shared_base;
alloc_aio_bitmap(bmds);
drive_get_ref(drive_get_by_blockdev(bs));
- bdrv_set_in_use(bs, 1);
+ bdrv_ref(bs, true);
block_mig_state.total_sector_sum += sectors;
@@ -557,7 +557,7 @@ static void blk_mig_cleanup(void)
blk_mig_lock();
while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) {
QSIMPLEQ_REMOVE_HEAD(&block_mig_state.bmds_list, entry);
- bdrv_set_in_use(bmds->bs, 0);
+ bdrv_unref(bmds->bs, true);
drive_put_ref(drive_get_by_blockdev(bmds->bs));
g_free(bmds->aio_bitmap);
g_free(bmds);
diff --git a/block.c b/block.c
index b560241..4170ff6 100644
--- a/block.c
+++ b/block.c
@@ -1511,8 +1511,11 @@ static void bdrv_move_feature_fields(BlockDriverState
*bs_dest,
/* dirty bitmap */
bs_dest->dirty_bitmap = bs_src->dirty_bitmap;
+ /* reference count */
+ bs_dest->refcnt_soft = bs_src->refcnt_soft;
+ bs_dest->refcnt_hard = bs_src->refcnt_hard;
+
/* job */
- bs_dest->in_use = bs_src->in_use;
bs_dest->job = bs_src->job;
/* keep the same entry in bdrv_states */
@@ -1542,7 +1545,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState
*bs_old)
assert(bs_new->dirty_bitmap == NULL);
assert(bs_new->job == NULL);
assert(bs_new->dev == NULL);
- assert(bs_new->in_use == 0);
assert(bs_new->io_limits_enabled == false);
assert(bs_new->block_timer == NULL);
@@ -1561,7 +1563,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState
*bs_old)
/* Check a few fields that should remain attached to the device */
assert(bs_new->dev == NULL);
assert(bs_new->job == NULL);
- assert(bs_new->in_use == 0);
assert(bs_new->io_limits_enabled == false);
assert(bs_new->block_timer == NULL);
@@ -1598,7 +1599,6 @@ void bdrv_delete(BlockDriverState *bs)
{
assert(!bs->dev);
assert(!bs->job);
- assert(!bs->in_use);
/* remove from list, if necessary */
bdrv_make_anon(bs);
@@ -4374,15 +4374,37 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs)
}
}
-void bdrv_set_in_use(BlockDriverState *bs, int in_use)
+/* Get a soft or hard reference to bs */
+void bdrv_ref(BlockDriverState *bs, bool is_hard)
+{
+ if (is_hard) {
+ bs->refcnt_hard++;
+ } else {
+ bs->refcnt_soft++;
+ }
+}
+
+/* Release a previously grabbed reference to bs, need to specify if it is hard
+ * or soft. If after this releasing, both soft and hard reference counts are
+ * zero, the BlockDriverState is deleted. */
+void bdrv_unref(BlockDriverState *bs, bool is_hard)
{
- assert(bs->in_use != in_use);
- bs->in_use = in_use;
+ if (is_hard) {
+ assert(bs->refcnt_hard > 0);
+ bs->refcnt_hard--;
+ } else {
+ assert(bs->refcnt_soft > 0);
+ bs->refcnt_soft--;
+ }
+ if (bs->refcnt_hard == 0 && bs->refcnt_soft == 0) {
+ bdrv_close(bs);
+ bdrv_delete(bs);
+ }
}
-int bdrv_in_use(BlockDriverState *bs)
+bool bdrv_in_use(BlockDriverState *bs)
{
- return bs->in_use;
+ return bs->refcnt_hard > 0;
}
void bdrv_iostatus_enable(BlockDriverState *bs)
diff --git a/blockjob.c b/blockjob.c
index ca80df1..24f07f9 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -45,7 +45,7 @@ void *block_job_create(const BlockJobType *job_type,
BlockDriverState *bs,
error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
return NULL;
}
- bdrv_set_in_use(bs, 1);
+ bdrv_ref(bs, true);
job = g_malloc0(job_type->instance_size);
job->job_type = job_type;
@@ -63,7 +63,7 @@ void *block_job_create(const BlockJobType *job_type,
BlockDriverState *bs,
if (error_is_set(&local_err)) {
bs->job = NULL;
g_free(job);
- bdrv_set_in_use(bs, 0);
+ bdrv_unref(bs, true);
error_propagate(errp, local_err);
return NULL;
}
@@ -79,7 +79,7 @@ void block_job_completed(BlockJob *job, int ret)
job->cb(job->opaque, ret);
bs->job = NULL;
g_free(job);
- bdrv_set_in_use(bs, 0);
+ bdrv_unref(bs, true);
}
void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 0356665..3075e84 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -431,7 +431,7 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev,
VirtIOBlkConf *blk,
s->blk = blk;
/* Prevent block operations that conflict with data plane thread */
- bdrv_set_in_use(blk->conf.bs, 1);
+ bdrv_ref(blk->conf.bs, true);
error_setg(&s->migration_blocker,
"x-data-plane does not support migration");
@@ -450,7 +450,7 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
virtio_blk_data_plane_stop(s);
migrate_del_blocker(s->migration_blocker);
error_free(s->migration_blocker);
- bdrv_set_in_use(s->blk->conf.bs, 0);
+ bdrv_unref(s->blk->conf.bs, true);
g_free(s);
}
diff --git a/include/block/block.h b/include/block/block.h
index b6b9014..8df8794 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -354,8 +354,9 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs);
void bdrv_enable_copy_on_read(BlockDriverState *bs);
void bdrv_disable_copy_on_read(BlockDriverState *bs);
-void bdrv_set_in_use(BlockDriverState *bs, int in_use);
-int bdrv_in_use(BlockDriverState *bs);
+void bdrv_ref(BlockDriverState *bs, bool is_hard);
+void bdrv_unref(BlockDriverState *bs, bool is_hard);
+bool bdrv_in_use(BlockDriverState *bs);
#ifdef CONFIG_LINUX_AIO
int raw_get_aio_fd(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index c6ac871..9de5a8f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -294,7 +294,8 @@ struct BlockDriverState {
BlockDeviceIoStatus iostatus;
char device_name[32];
HBitmap *dirty_bitmap;
- int in_use; /* users other than guest access, eg. block migration */
+ int refcnt_soft;
+ int refcnt_hard;
QTAILQ_ENTRY(BlockDriverState) list;
QLIST_HEAD(, BdrvTrackedRequest) tracked_requests;
--
1.8.3.2
