diff mbox

block: fix external snapshot abort permission error

Message ID d5b487b58c2c6b3e68de0e858b84877434f05c76.1496843678.git.jcody@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeff Cody June 7, 2017, 1:55 p.m. UTC
In external_snapshot_abort(), we try to undo what was done in
external_snapshot_prepare() calling bdrv_replace_node() to swap the
nodes back.  However, we receive a permissions error as writers are
blocked on the old node, which is now the new node backing file.

An easy fix (initially suggested by Kevin Wolf) is to call
bdrv_set_backing_hd() on the new node, to set the backing node to NULL.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 blockdev.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Eric Blake June 7, 2017, 3:37 p.m. UTC | #1
On 06/07/2017 08:55 AM, Jeff Cody wrote:
> In external_snapshot_abort(), we try to undo what was done in
> external_snapshot_prepare() calling bdrv_replace_node() to swap the
> nodes back.  However, we receive a permissions error as writers are
> blocked on the old node, which is now the new node backing file.
> 
> An easy fix (initially suggested by Kevin Wolf) is to call
> bdrv_set_backing_hd() on the new node, to set the backing node to NULL.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  blockdev.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/blockdev.c b/blockdev.c
> index 892d768..6472548 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1803,7 +1803,11 @@ static void external_snapshot_abort(BlkActionState *common)
>                               DO_UPCAST(ExternalSnapshotState, common, common);
>      if (state->new_bs) {
>          if (state->overlay_appended) {
> +            bdrv_ref(state->old_bs);   /* we can't let bdrv_set_backind_hd()
> +                                          close state->old_bs; we need it */
> +            bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);
>              bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);
> +            bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */
>          }
>      }
>  }
>
Kevin Wolf June 7, 2017, 4:52 p.m. UTC | #2
Am 07.06.2017 um 15:55 hat Jeff Cody geschrieben:
> In external_snapshot_abort(), we try to undo what was done in
> external_snapshot_prepare() calling bdrv_replace_node() to swap the
> nodes back.  However, we receive a permissions error as writers are
> blocked on the old node, which is now the new node backing file.
> 
> An easy fix (initially suggested by Kevin Wolf) is to call
> bdrv_set_backing_hd() on the new node, to set the backing node to NULL.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>

Thanks, applied to the block branch.

Kevin
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 892d768..6472548 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1803,7 +1803,11 @@  static void external_snapshot_abort(BlkActionState *common)
                              DO_UPCAST(ExternalSnapshotState, common, common);
     if (state->new_bs) {
         if (state->overlay_appended) {
+            bdrv_ref(state->old_bs);   /* we can't let bdrv_set_backind_hd()
+                                          close state->old_bs; we need it */
+            bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);
             bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);
+            bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */
         }
     }
 }