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

Reply via email to