Am 26.11.2013 um 06:45 hat Fam Zheng geschrieben:
> If active is top, it will be mirrored to base, (with block/mirror.c
> code), then the image is switched when user completes the block job.
>
> QMP documentation is updated.
>
> Signed-off-by: Fam Zheng <f...@redhat.com>
> ---
> block/mirror.c | 11 +++++++++++
> blockdev.c | 9 +++++++--
> qapi-schema.json | 5 +++--
> 3 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 9be741a..86ffac8 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -478,6 +478,13 @@ immediate_exit:
> bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL);
> }
> bdrv_swap(s->target, s->common.bs);
> + if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
> + /* drop the bs loop chain formed by the swap: break the loop then
> + * trigger the unref from the top one */
> + BlockDriverState *p = s->base->backing_hd;
> + s->base->backing_hd = NULL;
> + bdrv_unref(p);
I'm not sure about the refcounts...
Let's assume we have the following backing file chain, refcount in
brackets:
+------ NBD server
v
base (1) <-- sn1 (1) <-- sn2 (3) <-- guest
^
+------ something else ;-)
> + }
> }
> bdrv_unref(s->target);
> block_job_completed(&s->common, ret);
> @@ -619,6 +626,10 @@ void commit_active_start(BlockDriverState *bs,
> BlockDriverState *base,
> BlockDriverCompletionFunc *cb,
> void *opaque, Error **errp)
> {
> + if (bdrv_reopen(base, bs->open_flags, errp)) {
> + return;
> + }
> + bdrv_ref(base);
So when we start, the refcount changes as follows:
+--- commit job +------ NBD server
v v
base (2) <-- sn1 (1) <-- sn2 (3) <-- guest
^
+------ something else
Once all the data is copied over, we get o the code above, and first do
a bdrv_swap, which results in the following:
+--- commit job +------ NBD server
v v
sn2 (2) <-- sn1 (1) base (3) <-- guest
| ^ ^
+------------+ +------ something else
Break the loop (but no refcount update!):
+--- commit job +------ NBD server
v v
sn2 (2) <-- sn1 (1) base (3) <-- guest
^
+------ something else
Drop the commit job's reference:
+------ NBD server
v
sn2 (1) <-- sn1 (1) base (3) <-- guest
^
+------ something else
Okay, so writing this down made me actually understand what's wrong with
the refcount logic. You can't set s->base->backing_hd = NULL without
updating the reference count of the backing file.
Easy enough to fix. :-)
Kevin
