| Message ID | 1425024829-1246-1-git-send-email-berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
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 > >
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
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:
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(-)