* [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).