Re: [Qemu-devel] [PATCH v14 06/14] block: Add backing_blocker in BlockDriverState

[Jeff Cody]

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" }}