Re: [PATCH v2 1/7] block/block-copy: specialcase first copy_range request

[Vladimir Sementsov-Ogievskiy]

07.02.2020 20:28, Max Reitz wrote:

On 27.11.19 19:08, Vladimir Sementsov-Ogievskiy wrote:

In block_copy_do_copy we fallback to read+write if copy_range failed.
In this case copy_size is larger than defined for buffered IO, and
there is corresponding commit. Still, backup copies data cluster by
cluster, and most of requests are limited to one cluster anyway, so the
only source of this one bad-limited request is copy-before-write
operation.

Further patch will move backup to use block_copy directly, than for
cases where copy_range is not supported, first request will be
oversized in each backup. It's not good, let's change it now.

Fix is simple: just limit first copy_range request like buffer-based
request. If it succeed, set larger copy_range limit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
  block/block-copy.c | 41 ++++++++++++++++++++++++++++++-----------
  1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index 79798a1567..8602e2cae7 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c

[...]

@@ -168,7 +170,21 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState 
*s,
              s->use_copy_range = false;
              s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
              /* Fallback to read+write with allocated buffer */
-        } else {
+        } else if (s->use_copy_range) {
+            /*
+             * Successful copy-range. Now increase copy_size.
+             * copy_range does not respect max_transfer (it's a TODO), so we
+             * factor that in here.
+             *
+             * Note: we double-check s->use_copy_range for the case when
+             * parallel block-copy request unset it during previous
+             * bdrv_co_copy_range call.

But we should still goto out anyway, shouldn’t we?

Oops yes, will fix.

+             */
+            s->copy_size =
+                    MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
+                        QEMU_ALIGN_DOWN(block_copy_max_transfer(s->source,
+                                                                s->target),
+                                        s->cluster_size));
              goto out;
          }
      }
@@ -176,7 +192,10 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState 
*s,
      /*
       * In case of failed copy_range request above, we may proceed with 
buffered
       * request larger than BLOCK_COPY_MAX_BUFFER. Still, further requests will
-     * be properly limited, so don't care too much.
+     * be properly limited, so don't care too much. Moreover the most possible

s/possible/likely/

Max

+     * case (copy_range is unsupported for the configuration, so the very first
+     * copy_range request fails) is handled by setting large copy_size only
+     * after first successful copy_range.
       */

bounce_buffer = qemu_blockalign(s->source->bs, nbytes);

--
Best regards,
Vladimir