23.09.2020 19:18, Alberto Garcia wrote:
On Wed 16 Sep 2020 02:20:05 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
- for (p = backing_bs(bs); p != base; p = backing_bs(p)) {
+ for (p = backing_bs(bs); include_base || p != base; p = backing_bs(p)) {
ret = bdrv_co_block_status(p, want_zero, offset, bytes, pnum, map,
file);
if (ret < 0) {
@@ -2420,6 +2422,11 @@ bdrv_co_common_block_status_above(BlockDriverState *bs,
break;
}
+ if (p == base) {
+ assert(include_base);
+ break;
+ }
+
Another option is something like:
BlockDriverState *last_bs = include_base ? base : backing_bs(base);
hmm, in case when include_base is false, last bs is not backing_bs(base) but
the parent of base.
and you get a simpler 'for' loop.
But why do we need include_base at all? Can't the caller just pass
backing_bs(base) instead? I'm talking also about the existing case of
bdrv_is_allocated_above().
include_base was introduced for the case when caller doesn't own
backing_bs(base), and therefore shouldn't do operations that may yield
(block_status can) dependent on backing_bs(base). In particular, in block
stream, where link to base is not frozen.
--
Best regards,
Vladimir