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

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

[Alberto Garcia]

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.

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

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

[Stefan Hajnoczi]

[-- 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 --]

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

[Alberto Garcia]

On Fri, Feb 27, 2015 at 11:21:10AM +0000, Stefan Hajnoczi wrote:

> 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.

You're actually right, sorry for the noise!

Berto