Block-jobs: improve backup fleecing

-----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCgAdFiEEi5wmzbL9FHyIDoahVh8kwfGfefsFAmb6WsYACgkQVh8kwfGf
 eftECRAAgRFGeVSE8+fgibsHH4TixG41Rg/4gi09KZuhNHCDQXiVWlpNf4isnjcg
 K3PAZh5XSr+ClYFzbwqrjnbjGOZDJRpfb5KAnWK2GhhbBrwFBAussJ/njOg3IB3V
 1Qtm5cc6aKrFW4dybqWW96gZZkIxFuSOW0jAjuVh4r0ZfK03Xytm5WITYg7CLnnQ
 WqnseoQuj9HvT3F2A3HnRk4tIlgEb2eWppDhX8DiW/IO4C1ScfYpyP2cvUROCCGF
 GIJKv7/GRYosSUx4nsCCx1y7ISX0A5m1sCBlWbnQmYLIaJ/JpUYA2Tqxyfy3LTRE
 IJK/NcWcR9fCy2W1hNQJaKfIKoA7EdfqSvCFQmBHrEmtRYslejNRB3PjIUzth7Oh
 knfHxKSiXyGBdikaE5a//MUBK74pc9eC9zuIvIzxdLYxBTTGKZlTuhxG28QBoLO3
 /IcPd22loEZwh6PRgcFw7olsHo9lhfbiRua8dKwj384yJKFGU4s4QghSqRLxBHjP
 xnqQrU8uE3UOIVMY9ahsMxwUzzYGbwi0vQw98qLm26vcQBZRD1C9L97M9yCCrhQ4
 jfWyMsqdODA6V3IRpIV+JBL5KvRkC1fuoAw6Oh5R2r3K/WpX74gthATna9+/wGno
 0wPiaEvBox8y6zLKAZe4MEA6phX0tb5In6OQB2JlILHa7gds4x4=
 =LqMR
 -----END PGP SIGNATURE-----

Merge tag 'pull-block-jobs-2024-09-30' of https://gitlab.com/vsementsov/qemu into staging

Block-jobs: improve backup fleecing

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCgAdFiEEi5wmzbL9FHyIDoahVh8kwfGfefsFAmb6WsYACgkQVh8kwfGf
# eftECRAAgRFGeVSE8+fgibsHH4TixG41Rg/4gi09KZuhNHCDQXiVWlpNf4isnjcg
# K3PAZh5XSr+ClYFzbwqrjnbjGOZDJRpfb5KAnWK2GhhbBrwFBAussJ/njOg3IB3V
# 1Qtm5cc6aKrFW4dybqWW96gZZkIxFuSOW0jAjuVh4r0ZfK03Xytm5WITYg7CLnnQ
# WqnseoQuj9HvT3F2A3HnRk4tIlgEb2eWppDhX8DiW/IO4C1ScfYpyP2cvUROCCGF
# GIJKv7/GRYosSUx4nsCCx1y7ISX0A5m1sCBlWbnQmYLIaJ/JpUYA2Tqxyfy3LTRE
# IJK/NcWcR9fCy2W1hNQJaKfIKoA7EdfqSvCFQmBHrEmtRYslejNRB3PjIUzth7Oh
# knfHxKSiXyGBdikaE5a//MUBK74pc9eC9zuIvIzxdLYxBTTGKZlTuhxG28QBoLO3
# /IcPd22loEZwh6PRgcFw7olsHo9lhfbiRua8dKwj384yJKFGU4s4QghSqRLxBHjP
# xnqQrU8uE3UOIVMY9ahsMxwUzzYGbwi0vQw98qLm26vcQBZRD1C9L97M9yCCrhQ4
# jfWyMsqdODA6V3IRpIV+JBL5KvRkC1fuoAw6Oh5R2r3K/WpX74gthATna9+/wGno
# 0wPiaEvBox8y6zLKAZe4MEA6phX0tb5In6OQB2JlILHa7gds4x4=
# =LqMR
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 30 Sep 2024 09:01:10 BST
# gpg:                using RSA key 8B9C26CDB2FD147C880E86A1561F24C1F19F79FB
# gpg: Good signature from "Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>" [unknown]
# gpg:                 aka "Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 8B9C 26CD B2FD 147C 880E  86A1 561F 24C1 F19F 79FB

* tag 'pull-block-jobs-2024-09-30' of https://gitlab.com/vsementsov/qemu:
  util/co-shared-resource: Remove unused co_try_get_from_shres
  block: Remove unused aio_task_pool_empty
  block/reqlist: allow adding overlapping requests
  backup: add minimum cluster size to performance options
  copy-before-write: allow specifying minimum cluster size

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

block/aio_task.c

@@ -119,8 +119,3 @@ int aio_task_pool_status(AioTaskPool *pool)
 
     return pool->status;
 }
-
-bool aio_task_pool_empty(AioTaskPool *pool)
-{
-    return pool->busy_tasks == 0;
-}

block/backup.c

@@ -458,7 +458,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     }
 
     cbw = bdrv_cbw_append(bs, target, filter_node_name, discard_source,
-                          &bcs, errp);
+                          perf->min_cluster_size, &bcs, errp);
     if (!cbw) {
         goto error;
     }

block/block-copy.c

@@ -310,6 +310,7 @@ void block_copy_set_copy_opts(BlockCopyState *s, bool use_copy_range,
 }
 
 static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
+                                                 int64_t min_cluster_size,
                                                  Error **errp)
 {
     int ret;
@@ -319,6 +320,9 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
     GLOBAL_STATE_CODE();
     GRAPH_RDLOCK_GUARD_MAINLOOP();
 
+    min_cluster_size = MAX(min_cluster_size,
+                           (int64_t)BLOCK_COPY_CLUSTER_SIZE_DEFAULT);
+
     target_does_cow = bdrv_backing_chain_next(target);
 
     /*
@@ -329,13 +333,13 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
     ret = bdrv_get_info(target, &bdi);
     if (ret == -ENOTSUP && !target_does_cow) {
         /* Cluster size is not defined */
-        warn_report("The target block device doesn't provide "
+        warn_report("The target block device doesn't provide information about "
-                    "information about the block size and it doesn't have a "
+                    "the block size and it doesn't have a backing file. The "
-                    "backing file. The default block size of %u bytes is "
+                    "(default) block size of %" PRIi64 " bytes is used. If the "
-                    "used. If the actual block size of the target exceeds "
+                    "actual block size of the target exceeds this value, the "
-                    "this default, the backup may be unusable",
+                    "backup may be unusable",
-                    BLOCK_COPY_CLUSTER_SIZE_DEFAULT);
+                    min_cluster_size);
-        return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
+        return min_cluster_size;
     } else if (ret < 0 && !target_does_cow) {
         error_setg_errno(errp, -ret,
             "Couldn't determine the cluster size of the target image, "
@@ -345,16 +349,17 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
         return ret;
     } else if (ret < 0 && target_does_cow) {
         /* Not fatal; just trudge on ahead. */
-        return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
+        return min_cluster_size;
     }
 
-    return MAX(BLOCK_COPY_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
+    return MAX(min_cluster_size, bdi.cluster_size);
 }
 
 BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
                                      BlockDriverState *copy_bitmap_bs,
                                      const BdrvDirtyBitmap *bitmap,
                                      bool discard_source,
+                                     uint64_t min_cluster_size,
                                      Error **errp)
 {
     ERRP_GUARD();
@@ -365,7 +370,18 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
 
     GLOBAL_STATE_CODE();
 
-    cluster_size = block_copy_calculate_cluster_size(target->bs, errp);
+    if (min_cluster_size > INT64_MAX) {
+        error_setg(errp, "min-cluster-size too large: %" PRIu64 " > %" PRIi64,
+                   min_cluster_size, INT64_MAX);
+        return NULL;
+    } else if (min_cluster_size && !is_power_of_2(min_cluster_size)) {
+        error_setg(errp, "min-cluster-size needs to be a power of 2");
+        return NULL;
+    }
+
+    cluster_size = block_copy_calculate_cluster_size(target->bs,
+                                                     (int64_t)min_cluster_size,
+                                                     errp);
     if (cluster_size < 0) {
         return NULL;
     }

block/copy-before-write.c

@@ -66,7 +66,8 @@ typedef struct BDRVCopyBeforeWriteState {
 
     /*
      * @frozen_read_reqs: current read requests for fleecing user in bs->file
-     * node. These areas must not be rewritten by guest.
+     * node. These areas must not be rewritten by guest. There can be multiple
+     * overlapping read requests.
      */
     BlockReqList frozen_read_reqs;
 
@@ -417,6 +418,7 @@ static BlockdevOptions *cbw_parse_options(QDict *options, Error **errp)
     qdict_extract_subqdict(options, NULL, "bitmap");
     qdict_del(options, "on-cbw-error");
     qdict_del(options, "cbw-timeout");
+    qdict_del(options, "min-cluster-size");
 
 out:
     visit_free(v);
@@ -476,8 +478,10 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
              bs->file->bs->supported_zero_flags);
 
     s->discard_source = flags & BDRV_O_CBW_DISCARD_SOURCE;
+
     s->bcs = block_copy_state_new(bs->file, s->target, bs, bitmap,
-                                  flags & BDRV_O_CBW_DISCARD_SOURCE, errp);
+                                  flags & BDRV_O_CBW_DISCARD_SOURCE,
+                                  opts->min_cluster_size, errp);
     if (!s->bcs) {
         error_prepend(errp, "Cannot create block-copy-state: ");
         return -EINVAL;
@@ -545,6 +549,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockDriverState *target,
                                   const char *filter_node_name,
                                   bool discard_source,
+                                  uint64_t min_cluster_size,
                                   BlockCopyState **bcs,
                                   Error **errp)
 {
@@ -564,6 +569,14 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
     qdict_put_str(opts, "file", bdrv_get_node_name(source));
     qdict_put_str(opts, "target", bdrv_get_node_name(target));
 
+    if (min_cluster_size > INT64_MAX) {
+        error_setg(errp, "min-cluster-size too large: %" PRIu64 " > %" PRIi64,
+                   min_cluster_size, INT64_MAX);
+        qobject_unref(opts);
+        return NULL;
+    }
+    qdict_put_int(opts, "min-cluster-size", (int64_t)min_cluster_size);
+
     top = bdrv_insert_node(source, opts, flags, errp);
     if (!top) {
         return NULL;

block/copy-before-write.h

@@ -40,6 +40,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockDriverState *target,
                                   const char *filter_node_name,
                                   bool discard_source,
+                                  uint64_t min_cluster_size,
                                   BlockCopyState **bcs,
                                   Error **errp);
 void bdrv_cbw_drop(BlockDriverState *bs);

block/reqlist.c

@@ -20,8 +20,6 @@
 void reqlist_init_req(BlockReqList *reqs, BlockReq *req, int64_t offset,
                       int64_t bytes)
 {
-    assert(!reqlist_find_conflict(reqs, offset, bytes));
-
     *req = (BlockReq) {
         .offset = offset,
         .bytes = bytes,

blockdev.c

@@ -2655,6 +2655,9 @@ static BlockJob *do_backup_common(BackupCommon *backup,
         if (backup->x_perf->has_max_chunk) {
             perf.max_chunk = backup->x_perf->max_chunk;
         }
+        if (backup->x_perf->has_min_cluster_size) {
+            perf.min_cluster_size = backup->x_perf->min_cluster_size;
+        }
     }
 
     if ((backup->sync == MIRROR_SYNC_MODE_BITMAP) ||

include/block/aio_task.h

@@ -40,8 +40,6 @@ void aio_task_pool_free(AioTaskPool *);
 /* error code of failed task or 0 if all is OK */
 int aio_task_pool_status(AioTaskPool *pool);
 
-bool aio_task_pool_empty(AioTaskPool *pool);
-
 /* User provides filled @task, however task->pool will be set automatically */
 void coroutine_fn aio_task_pool_start_task(AioTaskPool *pool, AioTask *task);
 

include/block/block-copy.h

@@ -28,6 +28,7 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
                                      BlockDriverState *copy_bitmap_bs,
                                      const BdrvDirtyBitmap *bitmap,
                                      bool discard_source,
+                                     uint64_t min_cluster_size,
                                      Error **errp);
 
 /* Function should be called prior any actual copy request */

include/qemu/co-shared-resource.h

@@ -44,13 +44,6 @@ SharedResource *shres_create(uint64_t total);
  */
 void shres_destroy(SharedResource *s);
 
-/*
- * Try to allocate an amount of @n.  Return true on success, and false
- * if there is too little left of the collective resource to fulfill
- * the request.
- */
-bool co_try_get_from_shres(SharedResource *s, uint64_t n);
-
 /*
  * Allocate an amount of @n, and, if necessary, yield until
  * that becomes possible.

qapi/block-core.json

@@ -1551,11 +1551,16 @@
 #     it should not be less than job cluster size which is calculated
 #     as maximum of target image cluster size and 64k.  Default 0.
 #
+# @min-cluster-size: Minimum size of blocks used by copy-before-write
+#     and background copy operations.  Has to be a power of 2.  No
+#     effect if smaller than the maximum of the target's cluster size
+#     and 64 KiB.  Default 0.  (Since 9.2)
+#
 # Since: 6.0
 ##
 { 'struct': 'BackupPerf',
-  'data': { '*use-copy-range': 'bool',
+  'data': { '*use-copy-range': 'bool', '*max-workers': 'int',
-            '*max-workers': 'int', '*max-chunk': 'int64' } }
+            '*max-chunk': 'int64', '*min-cluster-size': 'size' } }
 
 ##
 # @BackupCommon:
@@ -4639,12 +4644,18 @@
 #     @on-cbw-error parameter will decide how this failure is handled.
 #     Default 0.  (Since 7.1)
 #
+# @min-cluster-size: Minimum size of blocks used by copy-before-write
+#     operations.  Has to be a power of 2.  No effect if smaller than
+#     the maximum of the target's cluster size and 64 KiB.  Default 0.
+#     (Since 9.2)
+#
 # Since: 6.2
 ##
 { 'struct': 'BlockdevOptionsCbw',
   'base': 'BlockdevOptionsGenericFormat',
   'data': { 'target': 'BlockdevRef', '*bitmap': 'BlockDirtyBitmap',
-            '*on-cbw-error': 'OnCbwError', '*cbw-timeout': 'uint32' } }
+            '*on-cbw-error': 'OnCbwError', '*cbw-timeout': 'uint32',
+            '*min-cluster-size': 'size' } }
 
 ##
 # @BlockdevOptions:

util/qemu-co-shared-resource.c

@@ -66,12 +66,6 @@ static bool co_try_get_from_shres_locked(SharedResource *s, uint64_t n)
     return false;
 }
 
-bool co_try_get_from_shres(SharedResource *s, uint64_t n)
-{
-    QEMU_LOCK_GUARD(&s->lock);
-    return co_try_get_from_shres_locked(s, n);
-}
-
 void coroutine_fn co_get_from_shres(SharedResource *s, uint64_t n)
 {
     assert(n <= s->total);