diff mbox series

[2/2] backup: add minimum cluster size to performance options

Message ID 20240308155158.830258-3-f.ebner@proxmox.com (mailing list archive)
State New, archived
Headers show
Series backup: allow specifying minimum cluster size | expand

Commit Message

Fiona Ebner March 8, 2024, 3:51 p.m. UTC
Useful to make discard-source work in the context of backup fleecing
when the fleecing image has a larger granularity than the backup
target.

Backup/block-copy will use at least this granularity for copy operations
and in particular, discard requests to the backup source will too. If
the granularity is too small, they will just be aligned down in
cbw_co_pdiscard_snapshot() and thus effectively ignored.

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 block/backup.c            | 2 +-
 block/copy-before-write.c | 2 ++
 block/copy-before-write.h | 1 +
 blockdev.c                | 3 +++
 qapi/block-core.json      | 9 +++++++--
 5 files changed, 14 insertions(+), 3 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy March 29, 2024, 10:15 a.m. UTC | #1
On 08.03.24 18:51, Fiona Ebner wrote:
> Useful to make discard-source work in the context of backup fleecing
> when the fleecing image has a larger granularity than the backup
> target.
> 
> Backup/block-copy will use at least this granularity for copy operations
> and in particular, discard requests to the backup source will too. If
> the granularity is too small, they will just be aligned down in
> cbw_co_pdiscard_snapshot() and thus effectively ignored.
> 
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>   block/backup.c            | 2 +-
>   block/copy-before-write.c | 2 ++
>   block/copy-before-write.h | 1 +
>   blockdev.c                | 3 +++
>   qapi/block-core.json      | 9 +++++++--
>   5 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index 3dd2e229d2..a1292c01ec 100644
> --- a/block/backup.c
> +++ b/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;
>       }
> diff --git a/block/copy-before-write.c b/block/copy-before-write.c
> index f9896c6c1e..55a9272485 100644
> --- a/block/copy-before-write.c
> +++ b/block/copy-before-write.c
> @@ -545,6 +545,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
>                                     BlockDriverState *target,
>                                     const char *filter_node_name,
>                                     bool discard_source,
> +                                  int64_t min_cluster_size,

same note, suggest uint32_t

>                                     BlockCopyState **bcs,
>                                     Error **errp)
>   {
> @@ -563,6 +564,7 @@ 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));
> +    qdict_put_int(opts, "min-cluster-size", min_cluster_size);
>   
>       top = bdrv_insert_node(source, opts, flags, errp);
>       if (!top) {
> diff --git a/block/copy-before-write.h b/block/copy-before-write.h
> index 01af0cd3c4..dc6cafe7fa 100644
> --- a/block/copy-before-write.h
> +++ b/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,
> +                                  int64_t min_cluster_size,
>                                     BlockCopyState **bcs,
>                                     Error **errp);
>   void bdrv_cbw_drop(BlockDriverState *bs);
> diff --git a/blockdev.c b/blockdev.c
> index daceb50460..8e6bdbc94a 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2653,6 +2653,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) ||
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 85c8f88f6e..ba0836892f 100644
> --- a/qapi/block-core.json
> +++ b/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.0)

9.1

> +#
>   # Since: 6.0
>   ##
>   { 'struct': 'BackupPerf',
> -  'data': { '*use-copy-range': 'bool',
> -            '*max-workers': 'int', '*max-chunk': 'int64' } }
> +  'data': { '*use-copy-range': 'bool', '*max-workers': 'int',
> +            '*max-chunk': 'int64', '*min-cluster-size': 'uint32' } }
>   
>   ##
>   # @BackupCommon:
diff mbox series

Patch

diff --git a/block/backup.c b/block/backup.c
index 3dd2e229d2..a1292c01ec 100644
--- a/block/backup.c
+++ b/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;
     }
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index f9896c6c1e..55a9272485 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -545,6 +545,7 @@  BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockDriverState *target,
                                   const char *filter_node_name,
                                   bool discard_source,
+                                  int64_t min_cluster_size,
                                   BlockCopyState **bcs,
                                   Error **errp)
 {
@@ -563,6 +564,7 @@  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));
+    qdict_put_int(opts, "min-cluster-size", min_cluster_size);
 
     top = bdrv_insert_node(source, opts, flags, errp);
     if (!top) {
diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index 01af0cd3c4..dc6cafe7fa 100644
--- a/block/copy-before-write.h
+++ b/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,
+                                  int64_t min_cluster_size,
                                   BlockCopyState **bcs,
                                   Error **errp);
 void bdrv_cbw_drop(BlockDriverState *bs);
diff --git a/blockdev.c b/blockdev.c
index daceb50460..8e6bdbc94a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2653,6 +2653,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) ||
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 85c8f88f6e..ba0836892f 100644
--- a/qapi/block-core.json
+++ b/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.0)
+#
 # Since: 6.0
 ##
 { 'struct': 'BackupPerf',
-  'data': { '*use-copy-range': 'bool',
-            '*max-workers': 'int', '*max-chunk': 'int64' } }
+  'data': { '*use-copy-range': 'bool', '*max-workers': 'int',
+            '*max-chunk': 'int64', '*min-cluster-size': 'uint32' } }
 
 ##
 # @BackupCommon: