[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 06/14] block: Add backing_blocker in BlockDr
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v14 06/14] block: Add backing_blocker in BlockDriverState |
Date: |
Thu, 20 Feb 2014 16:28:56 +0800 |
User-agent: |
Mutt/1.5.22 (2013-10-16) |
On Thu, 02/20 00:08, Jeff Cody wrote:
> On Thu, Feb 20, 2014 at 01:01:38PM +0800, Fam Zheng wrote:
> > On Wed, 02/19 16:17, Jeff Cody wrote:
> > > On Wed, Feb 19, 2014 at 09:42:23PM +0800, Fam Zheng wrote:
> > > > This makes use of op_blocker and blocks all the operations except for
> > > > commit target, on each BlockDriverState->backing_hd.
> > > >
> > > > The asserts for op_blocker in bdrv_swap are removed because with this
> > > > change, the target of block commit has at least the backing blocker of
> > > > its child, so the assertion is not true. Callers should do their check.
> > > >
> > > > Signed-off-by: Fam Zheng <address@hidden>
> > > > ---
> > > > block.c | 19 +++++++++++++++----
> > > > include/block/block_int.h | 3 +++
> > > > 2 files changed, 18 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/block.c b/block.c
> > > > index dec44d4..95d8c1f 100644
> > > > --- a/block.c
> > > > +++ b/block.c
> > > > @@ -1044,19 +1044,33 @@ fail:
> > > > void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState
> > > > *backing_hd)
> > > > {
> > > > if (bs->backing_hd) {
> > > > + assert(error_is_set(&bs->backing_blocker));
> > >
> > > When I run block-commit, on either the active or non-active layer, I
> > > get an assertion here. The qemu-iotests do not catch it, and I
> > > presume it is because happens a couple of seconds or so after the
> > > success message is returned over QMP.
> > >
> >
> > I can't reproduce this, could you give some specific steps? Thanks.
> >
>
> Sure - I am guessing the key is performing some live block snapshots
> first. Here is what I did (this is from memory, but I think the steps
> are right):
>
> Nothing special really about the cmdline:
> qemu-system-x86_64 -drive file=/home/jtc/test.qcow2,if=virtio -qmp stdio ...
>
> The QMP commands:
>
> For the non-active layer case:
>
> { "execute": "qmp_capabilities" }
> { "execute": "blockdev-snapshot-sync", "arguments": { "device":
> "virtio0","snapshot-file":"/tmp/snap1.qcow2","format": "qcow2" } }
> { "execute": "blockdev-snapshot-sync", "arguments": { "device":
> "virtio0","snapshot-file":"/tmp/snap2.qcow2","format": "qcow2" } }
> { "execute": "block-commit", "arguments": { "device": "virtio0", "top":
> "/tmp/snap1.qcow2" } }
>
>
> For the active layer case (I think I still had 2 snapshots here, not
> entirely positive):
>
> { "execute": "qmp_capabilities" }
> { "execute": "blockdev-snapshot-sync", "arguments": { "device":
> "virtio0","snapshot-file":"/tmp/snap1.qcow2","format": "qcow2" } }
> { "execute": "blockdev-snapshot-sync", "arguments": { "device":
> "virtio0","snapshot-file":"/tmp/snap2.qcow2","format": "qcow2" } }
> { "execute": "block-commit", "arguments": { "device": "virtio0", "top":
> "/tmp/snap2.qcow2" } }
> { "execute": "block-job-complete", "arguments": { "device": "virtio0" }}
>
Yes. I forgot to use bdrv_set_backing_hd in bdrv_append.
Could you try if the below patch fixes it? Thanks.
Fam
---
diff --git a/block.c b/block.c
index 1af43b9..66a8e35 100644
--- a/block.c
+++ b/block.c
@@ -1978,7 +1978,7 @@ void bdrv_append(BlockDriverState *bs_new,
BlockDriverState *bs_top)
/* The contents of 'tmp' will become bs_top, as we are
* swapping bs_new and bs_top contents. */
- bs_top->backing_hd = bs_new;
+ bdrv_set_backing_hd(bs_top, bs_new);
bs_top->open_flags &= ~BDRV_O_NO_BACKING;
pstrcpy(bs_top->backing_file, sizeof(bs_top->backing_file),
bs_new->filename);
- Re: [Qemu-devel] [PATCH v14 03/14] block: Replace in_use with operation blocker, (continued)
[Qemu-devel] [PATCH v14 07/14] block: Parse "backing" option to reference existing BDS, Fam Zheng, 2014/02/19
[Qemu-devel] [PATCH v14 08/14] block: Support dropping active in bdrv_drop_intermediate, Fam Zheng, 2014/02/19
[Qemu-devel] [PATCH v14 09/14] stream: Use bdrv_drop_intermediate and drop close_unused_images, Fam Zheng, 2014/02/19