* [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState
@ 2013-08-23 1:14 Fam Zheng
2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 1/8] vvfat: use bdrv_new() to allocate BlockDriverState Fam Zheng
` (8 more replies)
0 siblings, 9 replies; 12+ messages in thread
From: Fam Zheng @ 2013-08-23 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, jcody, stefanha, pbonzini, xiawenc
BlockDriverState lifecycle management is needed by future features such as
image fleecing and blockdev-add. This series adds reference count to
BlockDriverState.
The first two patches clean up two odd BlockDriverState use cases, so all code
uses bdrv_new() to create BlockDriverState instance.
Then implemented bdrv_ref() and bdrv_unref() to operate on refcnt: Initially,
refcnt is 1, which means bdrv_unref is effectively a bdrv_delete() here. So
patch 04 has a search and replace to convert bdrv_delete to bdrv_unref, before
bdrv_ref is used anywhere. 05~08 patches calls bdrv_ref for device attach,
block-migration and nbd.
The rule is: Either bdrv_ref() or bdrv_new() must have a matching
bdrv_unref() call, and the last matching bdrv_unref deletes the bs.
v6:
08: Use BH to release reference, this fixes the iotest.
v5: resend v4 to a right list.
v4:
08: Added, let block job use BDS reference.
02: Fix leak of bs.opaque
v3:
03: Removed unnecessary bdrv_close() call.
v2:
05: Removed: "block: use BlockDriverState refcnt for device attach/detach"
07: Fix xen_disk blk_disconnect() as it depended on device attach refcnt.
Fam Zheng (8):
vvfat: use bdrv_new() to allocate BlockDriverState
iscsi: use bdrv_new() instead of stack structure
block: implement reference count for BlockDriverState
block: make bdrv_delete() static
migration: omit drive ref as we have bdrv_ref now
xen_disk: simplify blk_disconnect with refcnt
nbd: use BlockDriverState refcnt
block: use BDS ref for block jobs
block-migration.c | 4 +--
block.c | 44 ++++++++++++++++++++++++---------
block/backup.c | 2 +-
block/blkverify.c | 4 +--
block/cow.c | 2 +-
block/iscsi.c | 16 ++++++------
block/mirror.c | 2 +-
block/qcow.c | 2 +-
block/qcow2.c | 2 +-
block/qed.c | 2 +-
block/sheepdog.c | 6 ++---
block/snapshot.c | 2 +-
block/stream.c | 2 +-
block/vmdk.c | 10 ++++----
block/vvfat.c | 6 ++---
blockdev-nbd.c | 10 +-------
blockdev.c | 63 +++++++++++++++++------------------------------
blockjob.c | 1 +
hw/block/xen_disk.c | 13 +++++-----
include/block/block.h | 3 ++-
include/block/block_int.h | 1 +
nbd.c | 5 ++++
qemu-img.c | 26 +++++++++----------
qemu-io.c | 6 ++---
24 files changed, 119 insertions(+), 115 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v6 1/8] vvfat: use bdrv_new() to allocate BlockDriverState 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng @ 2013-08-23 1:14 ` Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 2/8] iscsi: use bdrv_new() instead of stack structure Fam Zheng ` (7 subsequent siblings) 8 siblings, 0 replies; 12+ messages in thread From: Fam Zheng @ 2013-08-23 1:14 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, famz, jcody, stefanha, pbonzini, xiawenc we need bdrv_new() to properly initialize BDS, don't allocate memory manually. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/vvfat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/vvfat.c b/block/vvfat.c index cd3b8ed..a827d91 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2943,7 +2943,7 @@ static int enable_write_target(BDRVVVFATState *s) unlink(s->qcow_filename); #endif - s->bs->backing_hd = calloc(sizeof(BlockDriverState), 1); + s->bs->backing_hd = bdrv_new(""); s->bs->backing_hd->drv = &vvfat_write_target; s->bs->backing_hd->opaque = g_malloc(sizeof(void*)); *(void**)s->bs->backing_hd->opaque = s; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 2/8] iscsi: use bdrv_new() instead of stack structure 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 1/8] vvfat: use bdrv_new() to allocate BlockDriverState Fam Zheng @ 2013-08-23 1:14 ` Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 3/8] block: implement reference count for BlockDriverState Fam Zheng ` (6 subsequent siblings) 8 siblings, 0 replies; 12+ messages in thread From: Fam Zheng @ 2013-08-23 1:14 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, famz, jcody, stefanha, pbonzini, xiawenc BlockDriverState structure needs bdrv_new() to initialize refcnt, don't allocate a local structure variable and memset to 0, becasue with coming refcnt implementation, bdrv_unref will crash if bs->refcnt not initialized to 1. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/iscsi.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index e7c1c2b..123f058 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1249,11 +1249,11 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options) { int ret = 0; int64_t total_size = 0; - BlockDriverState bs; + BlockDriverState *bs; IscsiLun *iscsilun = NULL; QDict *bs_options; - memset(&bs, 0, sizeof(BlockDriverState)); + bs = bdrv_new(""); /* Read out options */ while (options && options->name) { @@ -1263,12 +1263,12 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options) options++; } - bs.opaque = g_malloc0(sizeof(struct IscsiLun)); - iscsilun = bs.opaque; + bs->opaque = g_malloc0(sizeof(struct IscsiLun)); + iscsilun = bs->opaque; bs_options = qdict_new(); qdict_put(bs_options, "filename", qstring_from_str(filename)); - ret = iscsi_open(&bs, bs_options, 0); + ret = iscsi_open(bs, bs_options, 0); QDECREF(bs_options); if (ret != 0) { @@ -1282,7 +1282,7 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options) ret = -ENODEV; goto out; } - if (bs.total_sectors < total_size) { + if (bs->total_sectors < total_size) { ret = -ENOSPC; goto out; } @@ -1292,7 +1292,9 @@ out: if (iscsilun->iscsi != NULL) { iscsi_destroy_context(iscsilun->iscsi); } - g_free(bs.opaque); + g_free(bs->opaque); + bs->opaque = NULL; + bdrv_delete(bs); return ret; } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 3/8] block: implement reference count for BlockDriverState 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 1/8] vvfat: use bdrv_new() to allocate BlockDriverState Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 2/8] iscsi: use bdrv_new() instead of stack structure Fam Zheng @ 2013-08-23 1:14 ` Fam Zheng 2013-08-23 16:12 ` Jeff Cody 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 4/8] block: make bdrv_delete() static Fam Zheng ` (5 subsequent siblings) 8 siblings, 1 reply; 12+ messages in thread From: Fam Zheng @ 2013-08-23 1:14 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, famz, jcody, stefanha, pbonzini, xiawenc Introduce bdrv_ref/bdrv_unref to manage the lifecycle of BlockDriverState. They are unused for now but will used to replace bdrv_delete() later. Signed-off-by: Fam Zheng <famz@redhat.com> --- block.c | 21 +++++++++++++++++++++ include/block/block.h | 2 ++ include/block/block_int.h | 1 + 3 files changed, 24 insertions(+) diff --git a/block.c b/block.c index 01b66d8..fea6904 100644 --- a/block.c +++ b/block.c @@ -306,6 +306,7 @@ BlockDriverState *bdrv_new(const char *device_name) bdrv_iostatus_disable(bs); notifier_list_init(&bs->close_notifiers); notifier_with_return_list_init(&bs->before_write_notifiers); + bs->refcnt = 1; return bs; } @@ -1517,6 +1518,9 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, /* dirty bitmap */ bs_dest->dirty_bitmap = bs_src->dirty_bitmap; + /* reference count */ + bs_dest->refcnt = bs_src->refcnt; + /* job */ bs_dest->in_use = bs_src->in_use; bs_dest->job = bs_src->job; @@ -4391,6 +4395,23 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs) } } +/* Get a reference to bs */ +void bdrv_ref(BlockDriverState *bs) +{ + bs->refcnt++; +} + +/* Release a previously grabbed reference to bs. + * If after releasing, reference count is zero, the BlockDriverState is + * deleted. */ +void bdrv_unref(BlockDriverState *bs) +{ + assert(bs->refcnt > 0); + if (--bs->refcnt == 0) { + bdrv_delete(bs); + } +} + void bdrv_set_in_use(BlockDriverState *bs, int in_use) { assert(bs->in_use != in_use); diff --git a/include/block/block.h b/include/block/block.h index 742fce5..b33ef62 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -356,6 +356,8 @@ 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_ref(BlockDriverState *bs); +void bdrv_unref(BlockDriverState *bs); void bdrv_set_in_use(BlockDriverState *bs, int in_use); int bdrv_in_use(BlockDriverState *bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index e45f2a0..1f85cfb 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -294,6 +294,7 @@ struct BlockDriverState { BlockDeviceIoStatus iostatus; char device_name[32]; HBitmap *dirty_bitmap; + int refcnt; int in_use; /* users other than guest access, eg. block migration */ QTAILQ_ENTRY(BlockDriverState) list; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/8] block: implement reference count for BlockDriverState 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 3/8] block: implement reference count for BlockDriverState Fam Zheng @ 2013-08-23 16:12 ` Jeff Cody 2013-08-26 1:15 ` Fam Zheng 0 siblings, 1 reply; 12+ messages in thread From: Jeff Cody @ 2013-08-23 16:12 UTC (permalink / raw) To: Fam Zheng; +Cc: kwolf, pbonzini, qemu-devel, stefanha, xiawenc On Fri, Aug 23, 2013 at 09:14:46AM +0800, Fam Zheng wrote: > Introduce bdrv_ref/bdrv_unref to manage the lifecycle of > BlockDriverState. They are unused for now but will used to replace > bdrv_delete() later. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block.c | 21 +++++++++++++++++++++ > include/block/block.h | 2 ++ > include/block/block_int.h | 1 + > 3 files changed, 24 insertions(+) > > diff --git a/block.c b/block.c > index 01b66d8..fea6904 100644 > --- a/block.c > +++ b/block.c > @@ -306,6 +306,7 @@ BlockDriverState *bdrv_new(const char *device_name) > bdrv_iostatus_disable(bs); > notifier_list_init(&bs->close_notifiers); > notifier_with_return_list_init(&bs->before_write_notifiers); > + bs->refcnt = 1; Just a heads-up: This won't apply cleanly to master because of a conflict, at the above line, with '88266f5 block: stop relying on io_flush() in bdrv_drain_all()'. -Jeff > > return bs; > } > @@ -1517,6 +1518,9 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, > /* dirty bitmap */ > bs_dest->dirty_bitmap = bs_src->dirty_bitmap; > > + /* reference count */ > + bs_dest->refcnt = bs_src->refcnt; > + > /* job */ > bs_dest->in_use = bs_src->in_use; > bs_dest->job = bs_src->job; > @@ -4391,6 +4395,23 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs) > } > } > > +/* Get a reference to bs */ > +void bdrv_ref(BlockDriverState *bs) > +{ > + bs->refcnt++; > +} > + > +/* Release a previously grabbed reference to bs. > + * If after releasing, reference count is zero, the BlockDriverState is > + * deleted. */ > +void bdrv_unref(BlockDriverState *bs) > +{ > + assert(bs->refcnt > 0); > + if (--bs->refcnt == 0) { > + bdrv_delete(bs); > + } > +} > + > void bdrv_set_in_use(BlockDriverState *bs, int in_use) > { > assert(bs->in_use != in_use); > diff --git a/include/block/block.h b/include/block/block.h > index 742fce5..b33ef62 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -356,6 +356,8 @@ 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_ref(BlockDriverState *bs); > +void bdrv_unref(BlockDriverState *bs); > void bdrv_set_in_use(BlockDriverState *bs, int in_use); > int bdrv_in_use(BlockDriverState *bs); > > diff --git a/include/block/block_int.h b/include/block/block_int.h > index e45f2a0..1f85cfb 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -294,6 +294,7 @@ struct BlockDriverState { > BlockDeviceIoStatus iostatus; > char device_name[32]; > HBitmap *dirty_bitmap; > + int refcnt; > int in_use; /* users other than guest access, eg. block migration */ > QTAILQ_ENTRY(BlockDriverState) list; > > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/8] block: implement reference count for BlockDriverState 2013-08-23 16:12 ` Jeff Cody @ 2013-08-26 1:15 ` Fam Zheng 0 siblings, 0 replies; 12+ messages in thread From: Fam Zheng @ 2013-08-26 1:15 UTC (permalink / raw) To: Jeff Cody; +Cc: kwolf, pbonzini, qemu-devel, stefanha, xiawenc On Fri, 08/23 12:12, Jeff Cody wrote: > On Fri, Aug 23, 2013 at 09:14:46AM +0800, Fam Zheng wrote: > > Introduce bdrv_ref/bdrv_unref to manage the lifecycle of > > BlockDriverState. They are unused for now but will used to replace > > bdrv_delete() later. > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > block.c | 21 +++++++++++++++++++++ > > include/block/block.h | 2 ++ > > include/block/block_int.h | 1 + > > 3 files changed, 24 insertions(+) > > > > diff --git a/block.c b/block.c > > index 01b66d8..fea6904 100644 > > --- a/block.c > > +++ b/block.c > > @@ -306,6 +306,7 @@ BlockDriverState *bdrv_new(const char *device_name) > > bdrv_iostatus_disable(bs); > > notifier_list_init(&bs->close_notifiers); > > notifier_with_return_list_init(&bs->before_write_notifiers); > > + bs->refcnt = 1; > > Just a heads-up: This won't apply cleanly to master because of a conflict, > at the above line, with > '88266f5 block: stop relying on io_flush() in bdrv_drain_all()'. > Thanks Jeff, rebased locally, the merging is straightforward: just add this line in the new context. Since it's only a trivial conflict, let's wait for some more comments before incrementing the revision. Fam > > > > return bs; > > } > > @@ -1517,6 +1518,9 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, > > /* dirty bitmap */ > > bs_dest->dirty_bitmap = bs_src->dirty_bitmap; > > > > + /* reference count */ > > + bs_dest->refcnt = bs_src->refcnt; > > + > > /* job */ > > bs_dest->in_use = bs_src->in_use; > > bs_dest->job = bs_src->job; > > @@ -4391,6 +4395,23 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs) > > } > > } > > > > +/* Get a reference to bs */ > > +void bdrv_ref(BlockDriverState *bs) > > +{ > > + bs->refcnt++; > > +} > > + > > +/* Release a previously grabbed reference to bs. > > + * If after releasing, reference count is zero, the BlockDriverState is > > + * deleted. */ > > +void bdrv_unref(BlockDriverState *bs) > > +{ > > + assert(bs->refcnt > 0); > > + if (--bs->refcnt == 0) { > > + bdrv_delete(bs); > > + } > > +} > > + > > void bdrv_set_in_use(BlockDriverState *bs, int in_use) > > { > > assert(bs->in_use != in_use); > > diff --git a/include/block/block.h b/include/block/block.h > > index 742fce5..b33ef62 100644 > > --- a/include/block/block.h > > +++ b/include/block/block.h > > @@ -356,6 +356,8 @@ 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_ref(BlockDriverState *bs); > > +void bdrv_unref(BlockDriverState *bs); > > void bdrv_set_in_use(BlockDriverState *bs, int in_use); > > int bdrv_in_use(BlockDriverState *bs); > > > > diff --git a/include/block/block_int.h b/include/block/block_int.h > > index e45f2a0..1f85cfb 100644 > > --- a/include/block/block_int.h > > +++ b/include/block/block_int.h > > @@ -294,6 +294,7 @@ struct BlockDriverState { > > BlockDeviceIoStatus iostatus; > > char device_name[32]; > > HBitmap *dirty_bitmap; > > + int refcnt; > > int in_use; /* users other than guest access, eg. block migration */ > > QTAILQ_ENTRY(BlockDriverState) list; > > > > -- > > 1.8.3.1 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 4/8] block: make bdrv_delete() static 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng ` (2 preceding siblings ...) 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 3/8] block: implement reference count for BlockDriverState Fam Zheng @ 2013-08-23 1:14 ` Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 5/8] migration: omit drive ref as we have bdrv_ref now Fam Zheng ` (4 subsequent siblings) 8 siblings, 0 replies; 12+ messages in thread From: Fam Zheng @ 2013-08-23 1:14 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, famz, jcody, stefanha, pbonzini, xiawenc Manage BlockDriverState lifecycle with refcnt, so bdrv_delete() is no longer public and should be called by bdrv_unref() if refcnt is decreased to 0. This is an identical change because effectively, there's no multiple reference of BDS now: no caller of bdrv_ref() yet, only bdrv_new() sets bs->refcnt to 1, so all bdrv_unref() now actually delete the BDS. Signed-off-by: Fam Zheng <famz@redhat.com> --- block.c | 23 ++++++++++++----------- block/backup.c | 2 +- block/blkverify.c | 4 ++-- block/cow.c | 2 +- block/iscsi.c | 2 +- block/mirror.c | 2 +- block/qcow.c | 2 +- block/qcow2.c | 2 +- block/qed.c | 2 +- block/sheepdog.c | 6 +++--- block/snapshot.c | 2 +- block/stream.c | 2 +- block/vmdk.c | 10 +++++----- block/vvfat.c | 4 ++-- blockdev.c | 14 +++++++------- hw/block/xen_disk.c | 4 ++-- include/block/block.h | 1 - qemu-img.c | 26 +++++++++++++------------- qemu-io.c | 6 +++--- 19 files changed, 58 insertions(+), 58 deletions(-) diff --git a/block.c b/block.c index fea6904..73e9abd 100644 --- a/block.c +++ b/block.c @@ -877,7 +877,7 @@ fail: if (!bs->drv) { QDECREF(bs->options); } - bdrv_delete(bs); + bdrv_unref(bs); return ret; } @@ -928,7 +928,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options) *backing_filename ? backing_filename : NULL, options, back_flags, back_drv); if (ret < 0) { - bdrv_delete(bs->backing_hd); + bdrv_unref(bs->backing_hd); bs->backing_hd = NULL; bs->open_flags |= BDRV_O_NO_BACKING; return ret; @@ -1003,12 +1003,12 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, bs1 = bdrv_new(""); ret = bdrv_open(bs1, filename, NULL, 0, drv); if (ret < 0) { - bdrv_delete(bs1); + bdrv_unref(bs1); goto fail; } total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK; - bdrv_delete(bs1); + bdrv_unref(bs1); ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename)); if (ret < 0) { @@ -1082,7 +1082,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, } if (bs->file != file) { - bdrv_delete(file); + bdrv_unref(file); file = NULL; } @@ -1122,7 +1122,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, unlink_and_fail: if (file != NULL) { - bdrv_delete(file); + bdrv_unref(file); } if (bs->is_temporary) { unlink(filename); @@ -1383,7 +1383,7 @@ void bdrv_close(BlockDriverState *bs) if (bs->drv) { if (bs->backing_hd) { - bdrv_delete(bs->backing_hd); + bdrv_unref(bs->backing_hd); bs->backing_hd = NULL; } bs->drv->bdrv_close(bs); @@ -1407,7 +1407,7 @@ void bdrv_close(BlockDriverState *bs) bs->options = NULL; if (bs->file != NULL) { - bdrv_delete(bs->file); + bdrv_unref(bs->file); bs->file = NULL; } } @@ -1604,11 +1604,12 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) bs_new->drv ? bs_new->drv->format_name : ""); } -void bdrv_delete(BlockDriverState *bs) +static void bdrv_delete(BlockDriverState *bs) { assert(!bs->dev); assert(!bs->job); assert(!bs->in_use); + assert(!bs->refcnt); /* remove from list, if necessary */ bdrv_make_anon(bs); @@ -2124,7 +2125,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, 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_unref(intermediate_state->bs); } ret = 0; @@ -4625,7 +4626,7 @@ out: free_option_parameters(param); if (bs) { - bdrv_delete(bs); + bdrv_unref(bs); } } diff --git a/block/backup.c b/block/backup.c index 6ae8a05..586d941 100644 --- a/block/backup.c +++ b/block/backup.c @@ -338,7 +338,7 @@ static void coroutine_fn backup_run(void *opaque) hbitmap_free(job->bitmap); bdrv_iostatus_disable(target); - bdrv_delete(target); + bdrv_unref(target); block_job_completed(&job->common, ret); } diff --git a/block/blkverify.c b/block/blkverify.c index 1d58cc3..c4e961e 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -155,7 +155,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags) s->test_file = bdrv_new(""); ret = bdrv_open(s->test_file, filename, NULL, flags, NULL); if (ret < 0) { - bdrv_delete(s->test_file); + bdrv_unref(s->test_file); s->test_file = NULL; goto fail; } @@ -169,7 +169,7 @@ static void blkverify_close(BlockDriverState *bs) { BDRVBlkverifyState *s = bs->opaque; - bdrv_delete(s->test_file); + bdrv_unref(s->test_file); s->test_file = NULL; } diff --git a/block/cow.c b/block/cow.c index 1cc2e89..767639c 100644 --- a/block/cow.c +++ b/block/cow.c @@ -314,7 +314,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) } exit: - bdrv_delete(cow_bs); + bdrv_unref(cow_bs); return ret; } diff --git a/block/iscsi.c b/block/iscsi.c index 123f058..df3a52d 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1294,7 +1294,7 @@ out: } g_free(bs->opaque); bs->opaque = NULL; - bdrv_delete(bs); + bdrv_unref(bs); return ret; } diff --git a/block/mirror.c b/block/mirror.c index bed4a7e..6662f55 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -480,7 +480,7 @@ immediate_exit: bdrv_swap(s->target, s->common.bs); } bdrv_close(s->target); - bdrv_delete(s->target); + bdrv_unref(s->target); block_job_completed(&s->common, ret); } diff --git a/block/qcow.c b/block/qcow.c index 5239bd6..6b891ac 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -751,7 +751,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) g_free(tmp); ret = 0; exit: - bdrv_delete(qcow_bs); + bdrv_unref(qcow_bs); return ret; } diff --git a/block/qcow2.c b/block/qcow2.c index 3376901..c236505 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1390,7 +1390,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, ret = 0; out: - bdrv_delete(bs); + bdrv_unref(bs); return ret; } diff --git a/block/qed.c b/block/qed.c index f767b05..0e1b767 100644 --- a/block/qed.c +++ b/block/qed.c @@ -599,7 +599,7 @@ static int qed_create(const char *filename, uint32_t cluster_size, ret = 0; /* success */ out: g_free(l1_table); - bdrv_delete(bs); + bdrv_unref(bs); return ret; } diff --git a/block/sheepdog.c b/block/sheepdog.c index afe0533..9aebeb6 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1447,7 +1447,7 @@ static int sd_prealloc(const char *filename) } out: if (bs) { - bdrv_delete(bs); + bdrv_unref(bs); } g_free(buf); @@ -1526,13 +1526,13 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) if (!is_snapshot(&s->inode)) { error_report("cannot clone from a non snapshot vdi"); - bdrv_delete(bs); + bdrv_unref(bs); ret = -EINVAL; goto out; } base_vid = s->inode.vdi_id; - bdrv_delete(bs); + bdrv_unref(bs); } ret = do_sd_create(s, vdi, vdi_size, base_vid, &vid, 0); diff --git a/block/snapshot.c b/block/snapshot.c index 6c6d9de..8f61cc0 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -99,7 +99,7 @@ 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_unref(bs->file); bs->drv = NULL; return open_ret; } diff --git a/block/stream.c b/block/stream.c index 7fe9e48..97cc14b 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_unref(unused); } top->backing_hd = base; } diff --git a/block/vmdk.c b/block/vmdk.c index 346bb5c..fa3bd1c 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -216,7 +216,7 @@ static void vmdk_free_extents(BlockDriverState *bs) g_free(e->l2_cache); g_free(e->l1_backup_table); if (e->file != bs->file) { - bdrv_delete(e->file); + bdrv_unref(e->file); } } g_free(s->extents); @@ -745,7 +745,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, /* SPARSE extent */ ret = vmdk_open_sparse(bs, extent_file, bs->open_flags); if (ret) { - bdrv_delete(extent_file); + bdrv_unref(extent_file); return ret; } } else { @@ -1634,15 +1634,15 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) BlockDriverState *bs = bdrv_new(""); ret = bdrv_open(bs, backing_file, NULL, 0, NULL); if (ret != 0) { - bdrv_delete(bs); + bdrv_unref(bs); return ret; } if (strcmp(bs->drv->format_name, "vmdk")) { - bdrv_delete(bs); + bdrv_unref(bs); return -EINVAL; } parent_cid = vmdk_read_cid(bs, 0); - bdrv_delete(bs); + bdrv_unref(bs); snprintf(parent_desc_line, sizeof(parent_desc_line), "parentFileNameHint=\"%s\"", backing_file); } diff --git a/block/vvfat.c b/block/vvfat.c index a827d91..2178a13 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2894,7 +2894,7 @@ static int write_target_commit(BlockDriverState *bs, int64_t sector_num, static void write_target_close(BlockDriverState *bs) { BDRVVVFATState* s = *((BDRVVVFATState**) bs->opaque); - bdrv_delete(s->qcow); + bdrv_unref(s->qcow); g_free(s->qcow_filename); } @@ -2935,7 +2935,7 @@ 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) { - bdrv_delete(s->qcow); + bdrv_unref(s->qcow); goto err; } diff --git a/blockdev.c b/blockdev.c index 41b0a49..5b86c9d 100644 --- a/blockdev.c +++ b/blockdev.c @@ -213,7 +213,7 @@ 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); + bdrv_unref(dinfo->bdrv); g_free(dinfo->id); QTAILQ_REMOVE(&drives, dinfo, next); g_free(dinfo->serial); @@ -724,7 +724,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, err: qemu_opts_del(opts); QDECREF(bs_opts); - bdrv_delete(dinfo->bdrv); + bdrv_unref(dinfo->bdrv); g_free(dinfo->id); QTAILQ_REMOVE(&drives, dinfo, next); g_free(dinfo); @@ -994,7 +994,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_unref(state->new_bs); } } @@ -1597,7 +1597,7 @@ void qmp_drive_backup(const char *device, const char *target, target_bs = bdrv_new(""); ret = bdrv_open(target_bs, target, NULL, flags, drv); if (ret < 0) { - bdrv_delete(target_bs); + bdrv_unref(target_bs); error_setg_file_open(errp, -ret, target); return; } @@ -1605,7 +1605,7 @@ void qmp_drive_backup(const char *device, const char *target, backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error, block_job_cb, bs, &local_err); if (local_err != NULL) { - bdrv_delete(target_bs); + bdrv_unref(target_bs); error_propagate(errp, local_err); return; } @@ -1737,7 +1737,7 @@ void qmp_drive_mirror(const char *device, const char *target, target_bs = bdrv_new(""); ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv); if (ret < 0) { - bdrv_delete(target_bs); + bdrv_unref(target_bs); error_setg_file_open(errp, -ret, target); return; } @@ -1746,7 +1746,7 @@ void qmp_drive_mirror(const char *device, const char *target, on_source_error, on_target_error, block_job_cb, bs, &local_err); if (local_err != NULL) { - bdrv_delete(target_bs); + bdrv_unref(target_bs); error_propagate(errp, local_err); return; } diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 727f433..8bfa04e 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -813,7 +813,7 @@ static int blk_connect(struct XenDevice *xendev) readonly); if (bdrv_open(blkdev->bs, blkdev->filename, NULL, qflags, drv) != 0) { - bdrv_delete(blkdev->bs); + bdrv_unref(blkdev->bs); blkdev->bs = NULL; } } @@ -926,7 +926,7 @@ static void blk_disconnect(struct XenDevice *xendev) /* close/delete only if we created it ourself */ bdrv_close(blkdev->bs); bdrv_detach_dev(blkdev->bs, blkdev); - bdrv_delete(blkdev->bs); + bdrv_unref(blkdev->bs); } blkdev->bs = NULL; } diff --git a/include/block/block.h b/include/block/block.h index b33ef62..1929355 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -123,7 +123,6 @@ BlockDriverState *bdrv_new(const char *device_name); void bdrv_make_anon(BlockDriverState *bs); void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old); void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top); -void bdrv_delete(BlockDriverState *bs); int bdrv_parse_cache_flags(const char *mode, int *flags); int bdrv_parse_discard_flags(const char *mode, int *flags); int bdrv_file_open(BlockDriverState **pbs, const char *filename, diff --git a/qemu-img.c b/qemu-img.c index b9a848d..3eaf1f1 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -298,7 +298,7 @@ static BlockDriverState *bdrv_new_open(const char *filename, return bs; fail: if (bs) { - bdrv_delete(bs); + bdrv_unref(bs); } return NULL; } @@ -652,7 +652,7 @@ static int img_check(int argc, char **argv) fail: qapi_free_ImageCheck(check); - bdrv_delete(bs); + bdrv_unref(bs); return ret; } @@ -722,7 +722,7 @@ static int img_commit(int argc, char **argv) break; } - bdrv_delete(bs); + bdrv_unref(bs); if (ret) { return 1; } @@ -1104,11 +1104,11 @@ static int img_compare(int argc, char **argv) ret = 0; out: - bdrv_delete(bs2); + bdrv_unref(bs2); qemu_vfree(buf1); qemu_vfree(buf2); out2: - bdrv_delete(bs1); + bdrv_unref(bs1); out3: qemu_progress_end(); return ret; @@ -1538,12 +1538,12 @@ out: free_option_parameters(param); qemu_vfree(buf); if (out_bs) { - bdrv_delete(out_bs); + bdrv_unref(out_bs); } if (bs) { for (bs_i = 0; bs_i < bs_n; bs_i++) { if (bs[bs_i]) { - bdrv_delete(bs[bs_i]); + bdrv_unref(bs[bs_i]); } } g_free(bs); @@ -1681,7 +1681,7 @@ static ImageInfoList *collect_image_info_list(const char *filename, *last = elem; last = &elem->next; - bdrv_delete(bs); + bdrv_unref(bs); filename = fmt = NULL; if (chain) { @@ -1895,7 +1895,7 @@ static int img_snapshot(int argc, char **argv) } /* Cleanup */ - bdrv_delete(bs); + bdrv_unref(bs); if (ret) { return 1; } @@ -2170,14 +2170,14 @@ out: /* Cleanup */ if (!unsafe) { if (bs_old_backing != NULL) { - bdrv_delete(bs_old_backing); + bdrv_unref(bs_old_backing); } if (bs_new_backing != NULL) { - bdrv_delete(bs_new_backing); + bdrv_unref(bs_new_backing); } } - bdrv_delete(bs); + bdrv_unref(bs); if (ret) { return 1; } @@ -2300,7 +2300,7 @@ static int img_resize(int argc, char **argv) } out: if (bs) { - bdrv_delete(bs); + bdrv_unref(bs); } if (ret) { return 1; diff --git a/qemu-io.c b/qemu-io.c index d54dc86..71f4ff1 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -32,7 +32,7 @@ static char **cmdline; static int close_f(BlockDriverState *bs, int argc, char **argv) { - bdrv_delete(bs); + bdrv_unref(bs); qemuio_bs = NULL; return 0; } @@ -61,7 +61,7 @@ static int openfile(char *name, int flags, int growable) if (bdrv_open(qemuio_bs, name, NULL, flags, NULL) < 0) { fprintf(stderr, "%s: can't open device %s\n", progname, name); - bdrv_delete(qemuio_bs); + bdrv_unref(qemuio_bs); qemuio_bs = NULL; return 1; } @@ -422,7 +422,7 @@ int main(int argc, char **argv) bdrv_drain_all(); if (qemuio_bs) { - bdrv_delete(qemuio_bs); + bdrv_unref(qemuio_bs); } return 0; } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 5/8] migration: omit drive ref as we have bdrv_ref now 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng ` (3 preceding siblings ...) 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 4/8] block: make bdrv_delete() static Fam Zheng @ 2013-08-23 1:14 ` Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 6/8] xen_disk: simplify blk_disconnect with refcnt Fam Zheng ` (3 subsequent siblings) 8 siblings, 0 replies; 12+ messages in thread From: Fam Zheng @ 2013-08-23 1:14 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, famz, jcody, stefanha, pbonzini, xiawenc block-migration.c does not actually use DriveInfo anywhere. Hence it's safe to drive ref code, we really only care about referencing BDS. Signed-off-by: Fam Zheng <famz@redhat.com> --- block-migration.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block-migration.c b/block-migration.c index f803f20..daf9ec1 100644 --- a/block-migration.c +++ b/block-migration.c @@ -336,8 +336,8 @@ static void init_blk_migration_it(void *opaque, BlockDriverState *bs) bmds->completed_sectors = 0; 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); block_mig_state.total_sector_sum += sectors; @@ -575,7 +575,7 @@ 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_set_in_use(bmds->bs, 0); - drive_put_ref(drive_get_by_blockdev(bmds->bs)); + bdrv_unref(bmds->bs); g_free(bmds->aio_bitmap); g_free(bmds); } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 6/8] xen_disk: simplify blk_disconnect with refcnt 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng ` (4 preceding siblings ...) 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 5/8] migration: omit drive ref as we have bdrv_ref now Fam Zheng @ 2013-08-23 1:14 ` Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 7/8] nbd: use BlockDriverState refcnt Fam Zheng ` (2 subsequent siblings) 8 siblings, 0 replies; 12+ messages in thread From: Fam Zheng @ 2013-08-23 1:14 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, famz, jcody, stefanha, pbonzini, xiawenc We call bdrv_attach_dev when initializing whether or not bs is created locally, so call bdrv_detach_dev and let the refcnt handle the lifecycle. Signed-off-by: Fam Zheng <famz@redhat.com> --- hw/block/xen_disk.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 8bfa04e..668cc06 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -824,6 +824,9 @@ static int blk_connect(struct XenDevice *xendev) /* setup via qemu cmdline -> already setup for us */ xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n"); blkdev->bs = blkdev->dinfo->bdrv; + /* blkdev->bs is not create by us, we get a reference + * so we can bdrv_unref() unconditionally */ + bdrv_ref(blkdev->bs); } bdrv_attach_dev_nofail(blkdev->bs, blkdev); blkdev->file_size = bdrv_getlength(blkdev->bs); @@ -922,12 +925,8 @@ 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_unref(blkdev->bs); - } + bdrv_detach_dev(blkdev->bs, blkdev); + bdrv_unref(blkdev->bs); blkdev->bs = NULL; } xen_be_unbind_evtchn(&blkdev->xendev); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 7/8] nbd: use BlockDriverState refcnt 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng ` (5 preceding siblings ...) 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 6/8] xen_disk: simplify blk_disconnect with refcnt Fam Zheng @ 2013-08-23 1:14 ` Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 8/8] block: use BDS ref for block jobs Fam Zheng 2013-09-04 13:52 ` [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Stefan Hajnoczi 8 siblings, 0 replies; 12+ messages in thread From: Fam Zheng @ 2013-08-23 1:14 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, famz, jcody, stefanha, pbonzini, xiawenc Previously, nbd calls drive_get_ref() on the drive of bs. A BDS doesn't always have associated dinfo, which nbd doesn't care either. We already have BDS ref count, so use it to make it safe for a BDS w/o blockdev. Signed-off-by: Fam Zheng <famz@redhat.com> --- blockdev-nbd.c | 10 +--------- nbd.c | 5 +++++ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 95f10c8..922cf56 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -69,12 +69,6 @@ static void nbd_close_notifier(Notifier *n, void *data) g_free(cn); } -static void nbd_server_put_ref(NBDExport *exp) -{ - BlockDriverState *bs = nbd_export_get_blockdev(exp); - drive_put_ref(drive_get_by_blockdev(bs)); -} - void qmp_nbd_server_add(const char *device, bool has_writable, bool writable, Error **errp) { @@ -105,11 +99,9 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable, writable = false; } - exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, - nbd_server_put_ref); + exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, NULL); nbd_export_set_name(exp, device); - drive_get_ref(drive_get_by_blockdev(bs)); n = g_malloc0(sizeof(NBDCloseNotifier)); n->n.notify = nbd_close_notifier; diff --git a/nbd.c b/nbd.c index 2606403..f258cdd 100644 --- a/nbd.c +++ b/nbd.c @@ -881,6 +881,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, exp->nbdflags = nbdflags; exp->size = size == -1 ? bdrv_getlength(bs) : size; exp->close = close; + bdrv_ref(bs); return exp; } @@ -927,6 +928,10 @@ void nbd_export_close(NBDExport *exp) } nbd_export_set_name(exp, NULL); nbd_export_put(exp); + if (exp->bs) { + bdrv_unref(exp->bs); + exp->bs = NULL; + } } void nbd_export_get(NBDExport *exp) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 8/8] block: use BDS ref for block jobs 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng ` (6 preceding siblings ...) 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 7/8] nbd: use BlockDriverState refcnt Fam Zheng @ 2013-08-23 1:14 ` Fam Zheng 2013-09-04 13:52 ` [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Stefan Hajnoczi 8 siblings, 0 replies; 12+ messages in thread From: Fam Zheng @ 2013-08-23 1:14 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, famz, jcody, stefanha, pbonzini, xiawenc Block jobs used drive_get_ref(drive_get_by_blockdev(bs)) to avoid BDS being deleted. Now we have BDS reference count, and block jobs don't care about dinfo, so replace them to get cleaner code. It is also the safe way when BDS has no drive info. Signed-off-by: Fam Zheng <famz@redhat.com> --- blockdev.c | 49 +++++++++++++++---------------------------------- blockjob.c | 1 + 2 files changed, 16 insertions(+), 34 deletions(-) diff --git a/blockdev.c b/blockdev.c index 5b86c9d..d8ea1d0 100644 --- a/blockdev.c +++ b/blockdev.c @@ -235,32 +235,32 @@ void drive_get_ref(DriveInfo *dinfo) typedef struct { QEMUBH *bh; - DriveInfo *dinfo; -} DrivePutRefBH; + BlockDriverState *bs; +} BDRVPutRefBH; -static void drive_put_ref_bh(void *opaque) +static void bdrv_put_ref_bh(void *opaque) { - DrivePutRefBH *s = opaque; + BDRVPutRefBH *s = opaque; - drive_put_ref(s->dinfo); + bdrv_unref(s->bs); qemu_bh_delete(s->bh); g_free(s); } /* - * Release a drive reference in a BH + * Release a BDS reference in a BH * - * It is not possible to use drive_put_ref() from a callback function when the - * callers still need the drive. In such cases we schedule a BH to release the - * reference. + * It is not safe to use bdrv_unref() from a callback function when the callers + * still need the BlockDriverState. In such cases we schedule a BH to release + * the reference. */ -static void drive_put_ref_bh_schedule(DriveInfo *dinfo) +static void bdrv_put_ref_bh_schedule(BlockDriverState *bs) { - DrivePutRefBH *s; + BDRVPutRefBH *s; - s = g_new(DrivePutRefBH, 1); - s->bh = qemu_bh_new(drive_put_ref_bh, s); - s->dinfo = dinfo; + s = g_new(BDRVPutRefBH, 1); + s->bh = qemu_bh_new(bdrv_put_ref_bh, s); + s->bs = bs; qemu_bh_schedule(s->bh); } @@ -1395,7 +1395,7 @@ static void block_job_cb(void *opaque, int ret) } qobject_decref(obj); - drive_put_ref_bh_schedule(drive_get_by_blockdev(bs)); + bdrv_put_ref_bh_schedule(bs); } void qmp_block_stream(const char *device, bool has_base, @@ -1432,11 +1432,6 @@ void qmp_block_stream(const char *device, bool has_base, return; } - /* Grab a reference so hotplug does not delete the BlockDriverState from - * underneath us. - */ - drive_get_ref(drive_get_by_blockdev(bs)); - trace_qmp_block_stream(bs, bs->job); } @@ -1493,10 +1488,6 @@ void qmp_block_commit(const char *device, error_propagate(errp, local_err); return; } - /* Grab a reference so hotplug does not delete the BlockDriverState from - * underneath us. - */ - drive_get_ref(drive_get_by_blockdev(bs)); } void qmp_drive_backup(const char *device, const char *target, @@ -1609,11 +1600,6 @@ void qmp_drive_backup(const char *device, const char *target, error_propagate(errp, local_err); return; } - - /* Grab a reference so hotplug does not delete the BlockDriverState from - * underneath us. - */ - drive_get_ref(drive_get_by_blockdev(bs)); } #define DEFAULT_MIRROR_BUF_SIZE (10 << 20) @@ -1750,11 +1736,6 @@ void qmp_drive_mirror(const char *device, const char *target, error_propagate(errp, local_err); return; } - - /* Grab a reference so hotplug does not delete the BlockDriverState from - * underneath us. - */ - drive_get_ref(drive_get_by_blockdev(bs)); } static BlockJob *find_block_job(const char *device) diff --git a/blockjob.c b/blockjob.c index ca80df1..9fe9869 100644 --- a/blockjob.c +++ b/blockjob.c @@ -45,6 +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_ref(bs); bdrv_set_in_use(bs, 1); job = g_malloc0(job_type->instance_size); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng ` (7 preceding siblings ...) 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 8/8] block: use BDS ref for block jobs Fam Zheng @ 2013-09-04 13:52 ` Stefan Hajnoczi 8 siblings, 0 replies; 12+ messages in thread From: Stefan Hajnoczi @ 2013-09-04 13:52 UTC (permalink / raw) To: Fam Zheng; +Cc: kwolf, jcody, qemu-devel, stefanha, pbonzini, xiawenc On Fri, Aug 23, 2013 at 09:14:43AM +0800, Fam Zheng wrote: > BlockDriverState lifecycle management is needed by future features such as > image fleecing and blockdev-add. This series adds reference count to > BlockDriverState. > > The first two patches clean up two odd BlockDriverState use cases, so all code > uses bdrv_new() to create BlockDriverState instance. > > Then implemented bdrv_ref() and bdrv_unref() to operate on refcnt: Initially, > refcnt is 1, which means bdrv_unref is effectively a bdrv_delete() here. So > patch 04 has a search and replace to convert bdrv_delete to bdrv_unref, before > bdrv_ref is used anywhere. 05~08 patches calls bdrv_ref for device attach, > block-migration and nbd. > > The rule is: Either bdrv_ref() or bdrv_new() must have a matching > bdrv_unref() call, and the last matching bdrv_unref deletes the bs. > > v6: > 08: Use BH to release reference, this fixes the iotest. > > v5: resend v4 to a right list. > > v4: > 08: Added, let block job use BDS reference. > 02: Fix leak of bs.opaque > > v3: > 03: Removed unnecessary bdrv_close() call. > > v2: > 05: Removed: "block: use BlockDriverState refcnt for device attach/detach" > 07: Fix xen_disk blk_disconnect() as it depended on device attach refcnt. > > > Fam Zheng (8): > vvfat: use bdrv_new() to allocate BlockDriverState > iscsi: use bdrv_new() instead of stack structure > block: implement reference count for BlockDriverState > block: make bdrv_delete() static > migration: omit drive ref as we have bdrv_ref now > xen_disk: simplify blk_disconnect with refcnt > nbd: use BlockDriverState refcnt > block: use BDS ref for block jobs > > block-migration.c | 4 +-- > block.c | 44 ++++++++++++++++++++++++--------- > block/backup.c | 2 +- > block/blkverify.c | 4 +-- > block/cow.c | 2 +- > block/iscsi.c | 16 ++++++------ > block/mirror.c | 2 +- > block/qcow.c | 2 +- > block/qcow2.c | 2 +- > block/qed.c | 2 +- > block/sheepdog.c | 6 ++--- > block/snapshot.c | 2 +- > block/stream.c | 2 +- > block/vmdk.c | 10 ++++---- > block/vvfat.c | 6 ++--- > blockdev-nbd.c | 10 +------- > blockdev.c | 63 +++++++++++++++++------------------------------ > blockjob.c | 1 + > hw/block/xen_disk.c | 13 +++++----- > include/block/block.h | 3 ++- > include/block/block_int.h | 1 + > nbd.c | 5 ++++ > qemu-img.c | 26 +++++++++---------- > qemu-io.c | 6 ++--- > 24 files changed, 119 insertions(+), 115 deletions(-) > > -- > 1.8.3.1 > > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-09-04 13:52 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-23 1:14 [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 1/8] vvfat: use bdrv_new() to allocate BlockDriverState Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 2/8] iscsi: use bdrv_new() instead of stack structure Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 3/8] block: implement reference count for BlockDriverState Fam Zheng 2013-08-23 16:12 ` Jeff Cody 2013-08-26 1:15 ` Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 4/8] block: make bdrv_delete() static Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 5/8] migration: omit drive ref as we have bdrv_ref now Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 6/8] xen_disk: simplify blk_disconnect with refcnt Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 7/8] nbd: use BlockDriverState refcnt Fam Zheng 2013-08-23 1:14 ` [Qemu-devel] [PATCH v6 8/8] block: use BDS ref for block jobs Fam Zheng 2013-09-04 13:52 ` [Qemu-devel] [PATCH v6 0/8] Implement reference count for BlockDriverState Stefan Hajnoczi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).