Device attach, NBD server and block job can share a BlockDriverState,
this patch makes them to use refcount to grab/release a bs so they don't
interfere each other with BDS lifecycle.
Local BDS allocation/releasing don't need to use refcount to manage it,
just use bdrv_new() and bdrv_delete().
If BDS is possibly shared with other code (e.g. NBD and guest device),
use bdrv_new to allocate, then bdrv_get_ref to grab it, when job is
done, call bdrv_put_ref() to release it. Don't call bdrv_delete(), since
it's deleted automatically by the last bdrv_put_ref().
Signed-off-by: Fam Zheng <f...@redhat.com>
---
block-migration.c | 1 -
block.c | 31 ++++++++++++++++++++-----------
block/backup.c | 3 ++-
block/blkdebug.c | 1 +
block/blkverify.c | 2 ++
block/mirror.c | 4 ++--
block/snapshot.c | 3 ++-
block/stream.c | 2 +-
block/vvfat.c | 4 +++-
blockdev.c | 5 +++--
hw/block/xen_disk.c | 7 +------
include/block/block_int.h | 16 ++++++++++++++++
12 files changed, 53 insertions(+), 26 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index 2efb6c0..2154b3a 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -558,7 +558,6 @@ static void blk_mig_cleanup(void)
while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) {
QSIMPLEQ_REMOVE_HEAD(&block_mig_state.bmds_list, entry);
bdrv_get_ref(bmds->bs);
- 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 84c3181..d1ce570 100644
--- a/block.c
+++ b/block.c
@@ -740,7 +740,6 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockDriverState *file,
ret = -EINVAL;
goto free_and_fail;
}
- assert(file != NULL);
bs->file = file;
ret = drv->bdrv_open(bs, options, open_flags);
}
@@ -748,7 +747,6 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockDriverState *file,
if (ret < 0) {
goto free_and_fail;
}
-
ret = refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
goto free_and_fail;
@@ -925,6 +923,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
*options)
bs->open_flags |= BDRV_O_NO_BACKING;
return ret;
}
+ bdrv_get_ref(bs->backing_hd);
return 0;
}
@@ -1068,9 +1067,11 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, QDict *options,
if (bs->file != file) {
bdrv_delete(file);
- file = NULL;
}
+ file = NULL;
+ bdrv_get_ref(bs->file);
+
/* If there is a backing file, use it */
if ((flags & BDRV_O_NO_BACKING) == 0) {
QDict *backing_options;
@@ -1367,7 +1368,7 @@ void bdrv_close(BlockDriverState *bs)
if (bs->drv) {
if (bs->backing_hd) {
- bdrv_delete(bs->backing_hd);
+ bdrv_put_ref(bs->backing_hd);
bs->backing_hd = NULL;
}
bs->drv->bdrv_close(bs);
@@ -1391,7 +1392,7 @@ void bdrv_close(BlockDriverState *bs)
bs->options = NULL;
if (bs->file != NULL) {
- bdrv_delete(bs->file);
+ bdrv_put_ref(bs->file);
bs->file = NULL;
}
}
@@ -1536,7 +1537,7 @@ 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->refcount == 0);
+ assert(bs_new->refcount <= 1);
assert(bs_new->io_limits_enabled == false);
assert(bs_new->block_timer == NULL);
@@ -1555,7 +1556,7 @@ 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->refcount == 0);
+ assert(bs_new->refcount <= 1);
assert(bs_new->io_limits_enabled == false);
assert(bs_new->block_timer == NULL);
@@ -1609,6 +1610,7 @@ int bdrv_attach_dev(BlockDriverState *bs, void *dev)
return -EBUSY;
}
bs->dev = dev;
+ bdrv_get_ref(bs);
bdrv_iostatus_reset(bs);
return 0;
}
@@ -1626,6 +1628,7 @@ void bdrv_detach_dev(BlockDriverState *bs, void *dev)
{
assert(bs->dev == dev);
bs->dev = NULL;
+ bdrv_put_ref(bs);
bs->dev_ops = NULL;
bs->dev_opaque = NULL;
bs->buffer_alignment = 512;
@@ -2102,13 +2105,16 @@ int bdrv_drop_intermediate(BlockDriverState *active,
BlockDriverState *top,
if (ret) {
goto exit;
}
+ if (new_top_bs->backing_hd) {
+ bdrv_put_ref(new_top_bs->backing_hd);
+ }
new_top_bs->backing_hd = base_bs;
-
+ bdrv_get_ref(base_bs);
QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) {
/* so that bdrv_close() does not recursively close the chain */
intermediate_state->bs->backing_hd = NULL;
- bdrv_delete(intermediate_state->bs);
+ bdrv_put_ref(intermediate_state->bs);
}
ret = 0;
@@ -4365,7 +4371,10 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs)
void bdrv_put_ref(BlockDriverState *bs)
{
assert(bs->refcount > 0);
- bs->refcount--;
+ if (--bs->refcount == 0) {
+ bdrv_close(bs);
+ bdrv_delete(bs);
+ }
}
void bdrv_get_ref(BlockDriverState *bs)
@@ -4375,7 +4384,7 @@ void bdrv_get_ref(BlockDriverState *bs)
int bdrv_in_use(BlockDriverState *bs)
{
- return bs->refcount > 0;
+ return bs->refcount > 1;
}
void bdrv_iostatus_enable(BlockDriverState *bs)
diff --git a/block/backup.c b/block/backup.c
index 16105d4..4e9f927 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -294,7 +294,7 @@ static void coroutine_fn backup_run(void *opaque)
hbitmap_free(job->bitmap);
bdrv_iostatus_disable(target);
- bdrv_delete(target);
+ bdrv_put_ref(target);
block_job_completed(&job->common, ret);
}
@@ -332,6 +332,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState
*target,
return;
}
+ bdrv_get_ref(target);
job->on_source_error = on_source_error;
job->on_target_error = on_target_error;
job->target = target;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index ccb627a..83c4018 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -389,6 +389,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict
*options, int flags)
if (ret < 0) {
goto fail;
}
+ bdrv_get_ref(bs->file);
ret = 0;
fail:
diff --git a/block/blkverify.c b/block/blkverify.c
index 1d58cc3..fd63f2b 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -145,6 +145,8 @@ static int blkverify_open(BlockDriverState *bs, QDict
*options, int flags)
goto fail;
}
+ bdrv_get_ref(bs->file);
+
/* Open the test file */
filename = qemu_opt_get(opts, "x-image");
if (filename == NULL) {
diff --git a/block/mirror.c b/block/mirror.c
index bed4a7e..ed98313 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -300,6 +300,7 @@ static void coroutine_fn mirror_run(void *opaque)
int ret = 0;
int n;
+ bdrv_get_ref(s->target);
if (block_job_is_cancelled(&s->common)) {
goto immediate_exit;
}
@@ -479,8 +480,7 @@ immediate_exit:
}
bdrv_swap(s->target, s->common.bs);
}
- bdrv_close(s->target);
- bdrv_delete(s->target);
+ bdrv_put_ref(s->target);
block_job_completed(&s->common, ret);
}
diff --git a/block/snapshot.c b/block/snapshot.c
index 6c6d9de..2d864d4 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -99,7 +99,8 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
ret = bdrv_snapshot_goto(bs->file, snapshot_id);
open_ret = drv->bdrv_open(bs, NULL, bs->open_flags);
if (open_ret < 0) {
- bdrv_delete(bs->file);
+ bdrv_put_ref(bs->file);
+ bs->file = NULL;
bs->drv = NULL;
return open_ret;
}
diff --git a/block/stream.c b/block/stream.c
index 7fe9e48..e611f31 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -68,7 +68,7 @@ static void close_unused_images(BlockDriverState *top,
BlockDriverState *base,
unused = intermediate;
intermediate = intermediate->backing_hd;
unused->backing_hd = NULL;
- bdrv_delete(unused);
+ bdrv_put_ref(unused);
}
top->backing_hd = base;
}
diff --git a/block/vvfat.c b/block/vvfat.c
index 87b0279..a73d765 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2938,8 +2938,10 @@ static int enable_write_target(BDRVVVFATState *s)
ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow);
if (ret < 0) {
- return ret;
+ bdrv_delete(s->qcow);
+ return ret;
}
+ bdrv_get_ref(s->qcow);
#ifndef _WIN32
unlink(s->qcow_filename);
diff --git a/blockdev.c b/blockdev.c
index b3a57e0..2c2ea59 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -212,7 +212,6 @@ static void bdrv_format_print(void *opaque, const char
*name)
static void drive_uninit(DriveInfo *dinfo)
{
qemu_opts_del(dinfo->opts);
- bdrv_delete(dinfo->bdrv);
g_free(dinfo->id);
QTAILQ_REMOVE(&drives, dinfo, next);
g_free(dinfo->serial);
@@ -894,6 +893,7 @@ static void external_snapshot_prepare(BlkTransactionState
*common,
if (ret != 0) {
error_setg_file_open(errp, -ret, new_image_file);
}
+ bdrv_get_ref(state->new_bs);
}
static void external_snapshot_commit(BlkTransactionState *common)
@@ -908,6 +908,7 @@ static void external_snapshot_commit(BlkTransactionState
*common)
* don't want to abort all of them if one of them fails the reopen */
bdrv_reopen(state->new_bs, state->new_bs->open_flags & ~BDRV_O_RDWR,
NULL);
+ bdrv_put_ref(state->new_bs);
}
static void external_snapshot_abort(BlkTransactionState *common)
@@ -915,7 +916,7 @@ static void external_snapshot_abort(BlkTransactionState
*common)
ExternalSnapshotState *state =
DO_UPCAST(ExternalSnapshotState, common, common);
if (state->new_bs) {
- bdrv_delete(state->new_bs);
+ bdrv_put_ref(state->new_bs);
}
}
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 247f32f..ae17acc 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -910,12 +910,7 @@ static void blk_disconnect(struct XenDevice *xendev)
struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
if (blkdev->bs) {
- if (!blkdev->dinfo) {
- /* close/delete only if we created it ourself */
- bdrv_close(blkdev->bs);
- bdrv_detach_dev(blkdev->bs, blkdev);
- bdrv_delete(blkdev->bs);
- }
+ bdrv_detach_dev(blkdev->bs, blkdev);
blkdev->bs = NULL;
}
xen_be_unbind_evtchn(&blkdev->xendev);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1ea80e7..92acddf 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -294,6 +294,22 @@ struct BlockDriverState {
BlockDeviceIoStatus iostatus;
char device_name[32];
HBitmap *dirty_bitmap;
+
+ /* Number of global references of this BDS, increased each time when:
+ * - Attached to device
+ * - Used by block job (both source and target)
+ * - NBD exported
+ * - Referenced as backing_hd of other BDS
+ * - Shared by multiple parts of QEMU in any other way
+ * And decreased each time when one of these roles finished.
+ *
+ * Note: Local/temp BDS isn't required to get a refcount. Two ways to
+ * manage the BDS lifecycle:
+ * - If BDS is only accessable locally, use bdrv_new() and bdrv_delete(),
+ * Just forget refcount.
+ * - bdrv_new() then bdrv_get_ref()/bdrv_put_ref. DON'T call bdrv_delete()
+ * yourself.
+ */
int refcount;
QTAILQ_ENTRY(BlockDriverState) list;
--
1.8.3.1
