Re: [Qemu-devel] [PATCH] block: Ensure correct order when dropping intermediate images

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1754 bytes --]

On Fri, Feb 27, 2015 at 10:13:49AM +0200, Alberto Garcia wrote:
> Given a chain of images like [A] <- [B] <- [C] <- [D] <- [E], where
> [E] is the active (topmost) image, if we want to remove [B] and [C] we
> have to do it before linking [A] and [D].
> 
> When [A] is set as the backing image of [D] all its operations are
> blocked, but removing [B] afterwards would undo those changes, since
> [B] would still be connected to [A].
> 
> This would result in a situation where the backing image [A] has all
> its operations unblocked.

Op blockers are linked to a unique Error object.  When B's backing_hd is
set to NULL only B->backing_blocker is unblocked but
new_top_bs->backing_blocker is still in place.  This means operations
are still blocked.

How do you reproduce this problem?  Do you have a test case that can be
added to tests/qemu-iotests/?

> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 9b707e3..2c36351 100644
> --- a/block.c
> +++ b/block.c
> @@ -2624,13 +2624,14 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
>      if (ret) {
>          goto exit;
>      }
> -    bdrv_set_backing_hd(new_top_bs, base_bs);
> +    bdrv_set_backing_hd(new_top_bs, NULL);
>  
>      QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) {
>          /* so that bdrv_close() does not recursively close the chain */
>          bdrv_set_backing_hd(intermediate_state->bs, NULL);
>          bdrv_unref(intermediate_state->bs);
>      }
> +    bdrv_set_backing_hd(new_top_bs, base_bs);
>      ret = 0;
>  
>  exit:
> -- 
> 2.1.4
> 
> 

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]