On 2/4/20 11:08 AM, Max Reitz wrote:
Signed-off-by: Max Reitz <mre...@redhat.com>
Another sparse commit message (a recurring theme of this series). The
subject line says 'what', and the patch appears to be faithful to that,
but if a future bisection lands here, even a one-sentence 'why' would be
handy; maybe:
This is part of a larger series of unifying block device relationships
via child_of_bds.
to at least hint that searching nearby commits gives a better why.
---
block.c | 26 ++++++++++++++++++++------
block/backup-top.c | 2 +-
block/vvfat.c | 3 ++-
tests/test-bdrv-drain.c | 13 +++++++------
4 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/block.c b/block.c
index 77755f0c6c..6b705ee23a 100644
--- a/block.c
+++ b/block.c
@@ -2748,6 +2748,20 @@ static bool
bdrv_inherits_from_recursive(BlockDriverState *child,
return child != NULL;
}
+/*
+ * Return the BdrvChildRole for @bs's backing child. bs->backing is
+ * mostly used for COW backing children (role = COW), but also for
+ * filtered children (role = FILTERED | PRIMARY).
+ */
+static BdrvChildRole bdrv_backing_role(BlockDriverState *bs)
+{
+ if (bs->drv && bs->drv->is_filter) {
+ return BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY;
And here's the first point (that I've spotted at least) in this series
where you are definitely returning a non-BdrvChildRole through a return
type of BdrvChildRole, rather than me just guessing you might (the
integer formed by bitwise-or of two enum values is not itself an enum
value). Repeating what I said earlier, the C language is loose enough
to allow your usage, and your usage is somewhat better self-documenting
than using an unsigned int; but it would not fly in other languages.
So I won't insist you change it, but at least think about it. (And the
latter has already happened if you read my paragraph - so can we call
that enough thought on the matter? ;)
+ } else {
+ return BDRV_CHILD_COW;
+ }
+}
+
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
