|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [PATCH v6 2/5] block/io: bdrv_common_block_status_above: support include_base |
Date: | Wed, 23 Sep 2020 20:11:57 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.2.2 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |