Re: [Qemu-devel] [PATCH v19 07/16] block: Add backing_blocker in BlockDriverState

[Fam Zheng]

On Mon, 05/19 22:23, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
> > On 05/11/2014 07:35 PM, 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                   | 24 ++++++++++++++++++++----
> >>  block/mirror.c            |  1 +
> >>  include/block/block_int.h |  3 +++
> >>  3 files changed, 24 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/block.c b/block.c
> >> index ec26a2b..8155e68 100644
> >> --- a/block.c
> >> +++ b/block.c
> >> @@ -1097,14 +1097,31 @@ fail:
> >>  void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState 
> >> *backing_hd)
> >>  {
> >>  
> >> +    if (bs->backing_hd) {
> >> +        assert(error_is_set(&bs->backing_blocker));
> >
> > error_is_set() is going away.  Please don't use it.
> > https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03801.html
> >
> > This can just be assert(bs->backing_blocker).
> 
> Yes.  See commit 0fb6395.  Another instance in PATCH 14.

Yes.

> 
> >> +        bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
> >> +    } else if (backing_hd) {
> >> +        error_setg(&bs->backing_blocker,
> >> +                   "device is used as backing hd of '%s'",
> >> +                   bs->device_name);
> >> +    }
> >> +
> >>      bs->backing_hd = backing_hd;
> >>      if (!backing_hd) {
> >> +        if (error_is_set(&bs->backing_blocker)) {
> >> +            error_free(bs->backing_blocker);
> >
> > if (bs->backing_blocker) {
> 
> Actually, unconditional error_free(bs->backing_blocker) is fine.
> 

Thanks, will update.

Fam