ShuriZma
/
xemu
mirror of https://github.com/xemu-project/xemu.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

block/snapshot: Clarify goto fallback behavior 

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>
This commit is contained in:
Max Reitz 2021-05-03 02:54:18 -07:00
parent b22726abdf
commit 32a9a245d7
1 changed files with 13 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
block/snapshot.c
View File

@ -275,13 +275,16 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
qobject_unref(file_options); qobject_unref(file_options);
g_free(subqdict_prefix); g_free(subqdict_prefix);
/* Force .bdrv_open() below to re-attach fallback_bs on *fallback_ptr */
qdict_put_str(options, (*fallback_ptr)->name, qdict_put_str(options, (*fallback_ptr)->name,
bdrv_get_node_name(fallback_bs)); bdrv_get_node_name(fallback_bs));
/* Now close bs, apply the snapshot on fallback_bs, and re-open bs */
if (drv->bdrv_close) { if (drv->bdrv_close) {
drv->bdrv_close(bs); drv->bdrv_close(bs);
} }
/* .bdrv_open() will re-attach it */
bdrv_unref_child(bs, *fallback_ptr); bdrv_unref_child(bs, *fallback_ptr);
*fallback_ptr = NULL; *fallback_ptr = NULL;
@ -296,7 +299,16 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
return ret < 0 ? ret : open_ret; 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); bdrv_unref(fallback_bs);
return ret; return ret;
} }