qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 0/1] Block patch
@ 2021-06-24 11:43 Max Reitz
  2021-06-24 11:43 ` [PULL 1/1] block/snapshot: Clarify goto fallback behavior Max Reitz
  2021-06-25 16:04 ` [PULL 0/1] Block patch Peter Maydell
  0 siblings, 2 replies; 3+ messages in thread
From: Max Reitz @ 2021-06-24 11:43 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

The following changes since commit b22726abdfa54592d6ad88f65b0297c0e8b363e2:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.1-pull-request' into staging (2021-06-22 16:07:53 +0100)

are available in the Git repository at:

  https://github.com/XanClic/qemu.git tags/pull-block-2021-06-24

for you to fetch changes up to 32a9a245d719a883eef2cbf07d2cf89efa0206d0:

  block/snapshot: Clarify goto fallback behavior (2021-06-24 09:49:04 +0200)

----------------------------------------------------------------
Block patch:
- Fix Coverity complaint in block/snapshot.c

----------------------------------------------------------------
Max Reitz (1):
  block/snapshot: Clarify goto fallback behavior

 block/snapshot.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

-- 
2.31.1



^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PULL 1/1] block/snapshot: Clarify goto fallback behavior
  2021-06-24 11:43 [PULL 0/1] Block patch Max Reitz
@ 2021-06-24 11:43 ` Max Reitz
  2021-06-25 16:04 ` [PULL 0/1] Block patch Peter Maydell
  1 sibling, 0 replies; 3+ messages in thread
From: Max Reitz @ 2021-06-24 11:43 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

In the bdrv_snapshot_goto() fallback code, we work with a pointer to
either bs->file or bs->backing.  We detach that child, close the node
(with .bdrv_close()), apply the snapshot on the child node, and then
re-open the node (with .bdrv_open()).

In order for .bdrv_open() to attach the same child node that we had
before, we pass "file={child-node}" or "backing={child-node}" to it.
Therefore, when .bdrv_open() has returned success, we can assume that
bs->file or bs->backing (respectively) points to our original child
again.  This is verified by an assertion.

All of this is not immediately clear from a quick glance at the code,
so add a comment to the assertion what it is for, and why it is valid.
It certainly confused Coverity.

Reported-by: Coverity (CID 1452774)
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210503095418.31521-1-mreitz@redhat.com>
[mreitz: s/close/detach/]
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/snapshot.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index 6702c75e42..ccacda8bd5 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -275,13 +275,16 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
         qobject_unref(file_options);
         g_free(subqdict_prefix);
 
+        /* Force .bdrv_open() below to re-attach fallback_bs on *fallback_ptr */
         qdict_put_str(options, (*fallback_ptr)->name,
                       bdrv_get_node_name(fallback_bs));
 
+        /* Now close bs, apply the snapshot on fallback_bs, and re-open bs */
         if (drv->bdrv_close) {
             drv->bdrv_close(bs);
         }
 
+        /* .bdrv_open() will re-attach it */
         bdrv_unref_child(bs, *fallback_ptr);
         *fallback_ptr = NULL;
 
@@ -296,7 +299,16 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
             return ret < 0 ? ret : open_ret;
         }
 
-        assert(fallback_bs == (*fallback_ptr)->bs);
+        /*
+         * fallback_ptr is &bs->file or &bs->backing.  *fallback_ptr
+         * was closed above and set to NULL, but the .bdrv_open() call
+         * has opened it again, because we set the respective option
+         * (with the qdict_put_str() call above).
+         * Assert that .bdrv_open() has attached some child on
+         * *fallback_ptr, and that it has attached the one we wanted
+         * it to (i.e., fallback_bs).
+         */
+        assert(*fallback_ptr && fallback_bs == (*fallback_ptr)->bs);
         bdrv_unref(fallback_bs);
         return ret;
     }
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PULL 0/1] Block patch
  2021-06-24 11:43 [PULL 0/1] Block patch Max Reitz
  2021-06-24 11:43 ` [PULL 1/1] block/snapshot: Clarify goto fallback behavior Max Reitz
@ 2021-06-25 16:04 ` Peter Maydell
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Maydell @ 2021-06-25 16:04 UTC (permalink / raw)
  To: Max Reitz; +Cc: Kevin Wolf, QEMU Developers, Qemu-block

On Thu, 24 Jun 2021 at 12:43, Max Reitz <mreitz@redhat.com> wrote:
>
> The following changes since commit b22726abdfa54592d6ad88f65b0297c0e8b363e2:
>
>   Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.1-pull-request' into staging (2021-06-22 16:07:53 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/XanClic/qemu.git tags/pull-block-2021-06-24
>
> for you to fetch changes up to 32a9a245d719a883eef2cbf07d2cf89efa0206d0:
>
>   block/snapshot: Clarify goto fallback behavior (2021-06-24 09:49:04 +0200)
>
> ----------------------------------------------------------------
> Block patch:
> - Fix Coverity complaint in block/snapshot.c


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-25 16:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-24 11:43 [PULL 0/1] Block patch Max Reitz
2021-06-24 11:43 ` [PULL 1/1] block/snapshot: Clarify goto fallback behavior Max Reitz
2021-06-25 16:04 ` [PULL 0/1] Block patch Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).