[v1] commit: Make base read-only if there is an early failure

[Qemu-devel] [PATCH for-4.1] commit: Make base read-only if there is an early failure

Alberto Garcia posted 1 patch 5 years ago

Diff against v2 v3
Download mbox

Test docker-mingw@fedora passed

Test docker-clang@ubuntu passed

Test checkpatch passed

Test asan passed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190410152429.8076-1-berto@igalia.com

Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>

There is a newer version of this series

block/commit.c | 3 +++
1 file changed, 3 insertions(+)

Expand all Fold all

[Qemu-devel] [PATCH for-4.1] commit: Make base read-only if there is an early failure

Posted by Alberto Garcia 5 years ago

You can reproduce this by passing an invalid filter-node-name (like
"1234") to block-commit. In this case the base image is put in
read-write mode but is never reset back to read-only.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/commit.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/commit.c b/block/commit.c
index ba60fef58a..698eda1dfe 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -384,6 +384,9 @@ fail:
     if (s->top) {
         blk_unref(s->top);
     }
+    if (s->base_read_only) {
+        bdrv_reopen_set_read_only(base, true, NULL);
+    }
     job_early_fail(&s->common.job);
     /* commit_top_bs has to be replaced after deleting the block job,
      * otherwise this would fail because of lack of permissions. */
-- 
2.11.0

Re: [Qemu-devel] [PATCH for-4.1] commit: Make base read-only if there is an early failure

Posted by Eric Blake 5 years ago

On 4/10/19 10:24 AM, Alberto Garcia wrote:
> You can reproduce this by passing an invalid filter-node-name (like
> "1234") to block-commit. In this case the base image is put in
> read-write mode but is never reset back to read-only.
> 

Is it worth iotest coverage?

> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/commit.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/commit.c b/block/commit.c
> index ba60fef58a..698eda1dfe 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -384,6 +384,9 @@ fail:
>      if (s->top) {
>          blk_unref(s->top);
>      }
> +    if (s->base_read_only) {
> +        bdrv_reopen_set_read_only(base, true, NULL);
> +    }
>      job_early_fail(&s->common.job);
>      /* commit_top_bs has to be replaced after deleting the block job,
>       * otherwise this would fail because of lack of permissions. */
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-4.1] commit: Make base read-only if there is an early failure

Posted by Alberto Garcia 5 years ago

On Wed 10 Apr 2019 05:32:47 PM CEST, Eric Blake wrote:
> On 4/10/19 10:24 AM, Alberto Garcia wrote:
>> You can reproduce this by passing an invalid filter-node-name (like
>> "1234") to block-commit. In this case the base image is put in
>> read-write mode but is never reset back to read-only.
>
> Is it worth iotest coverage?

I couldn't think of any way to reproduce this that would trigger an
error on an iotest, but I think I have one now, I'll try to write
something.

Berto