On 08.07.20 20:24, Andrey Shinkevich wrote:
On 25.06.2020 18:21, Max Reitz wrote:
Places that use patterns like
if (bs->drv->is_filter && bs->file) {
... something about bs->file->bs ...
}
should be
BlockDriverState *filtered = bdrv_filter_bs(bs);
if (filtered) {
... something about @filtered ...
}
instead.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 31 ++++++++++++++++++++-----------
block/io.c | 7 +++++--
migration/block-dirty-bitmap.c | 8 +-------
3 files changed, 26 insertions(+), 20 deletions(-)
...
diff --git a/block/io.c b/block/io.c
index df8f2a98d4..385176b331 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3307,6 +3307,7 @@ int coroutine_fn bdrv_co_truncate(BdrvChild
*child, int64_t offset, bool exact,
Error **errp)
{
BlockDriverState *bs = child->bs;
+ BdrvChild *filtered;
BlockDriver *drv = bs->drv;
BdrvTrackedRequest req;
int64_t old_size, new_bytes;
@@ -3358,6 +3359,8 @@ int coroutine_fn bdrv_co_truncate(BdrvChild
*child, int64_t offset, bool exact,
goto out;
}
+ filtered = bdrv_filter_child(bs);
+
Isn't better to have this initialization right before the relevant
if/else block?
Hm, well, yes. In this case, though, maybe not. Patch 16 will add
another BdrvChild to be initialized here (@backing), and we need to
initialize that one here. So I felt it made sense to group them together.
They got split up when I decided to put @filtered into this patch and
@backing into its own. So now it may look a bit weird, but I feel like
after patch 16 it makes sense.
(I’m indifferent, basically.)
Max