public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu v3 05/22] copy-before-write: allow specifying minimum cluster size
Date: Thu, 11 Apr 2024 11:29:26 +0200	[thread overview]
Message-ID: <20240411092943.57377-6-f.ebner@proxmox.com> (raw)
In-Reply-To: <20240411092943.57377-1-f.ebner@proxmox.com>

Useful to make discard-source work in the context of backup fleecing
when the fleecing image has a larger granularity than the backup
target.

Copy-before-write operations will use at least this granularity and in
particular, discard requests to the source node will too. If the
granularity is too small, they will just be aligned down in
cbw_co_pdiscard_snapshot() and thus effectively ignored.

The QAPI uses uint32 so the value will be non-negative, but still fit
into a uint64_t.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 block/block-copy.c         | 17 +++++++++++++----
 block/copy-before-write.c  |  3 ++-
 include/block/block-copy.h |  1 +
 qapi/block-core.json       |  8 +++++++-
 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index 3c61e52bae..c9a722a5a6 100644
--- a/block/block-copy.c
+++ b/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;
@@ -330,7 +331,7 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
                     "used. If the actual block size of the target exceeds "
                     "this default, the backup may be unusable",
                     BLOCK_COPY_CLUSTER_SIZE_DEFAULT);
-        return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
+        return MAX(min_cluster_size, BLOCK_COPY_CLUSTER_SIZE_DEFAULT);
     } else if (ret < 0 && !target_does_cow) {
         error_setg_errno(errp, -ret,
             "Couldn't determine the cluster size of the target image, "
@@ -340,16 +341,18 @@ 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 MAX(min_cluster_size, BLOCK_COPY_CLUSTER_SIZE_DEFAULT);
     }
 
-    return MAX(BLOCK_COPY_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
+    return MAX(min_cluster_size,
+               MAX(BLOCK_COPY_CLUSTER_SIZE_DEFAULT, bdi.cluster_size));
 }
 
 BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
                                      BlockDriverState *copy_bitmap_bs,
                                      const BdrvDirtyBitmap *bitmap,
                                      bool discard_source,
+                                     int64_t min_cluster_size,
                                      Error **errp)
 {
     ERRP_GUARD();
@@ -358,7 +361,13 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
     BdrvDirtyBitmap *copy_bitmap;
     bool is_fleecing;
 
-    cluster_size = block_copy_calculate_cluster_size(target->bs, errp);
+    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,
+                                                     min_cluster_size, errp);
     if (cluster_size < 0) {
         return NULL;
     }
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 3503702d71..4a8c5bdb62 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -479,7 +479,8 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int 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: ");
         ret = -EINVAL;
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index bdc703bacd..77857c6c68 100644
--- a/include/block/block-copy.h
+++ b/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,
+                                     int64_t min_cluster_size,
                                      Error **errp);
 
 /* Function should be called prior any actual copy request */
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4297e5beda..33e7e3c090 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4825,12 +4825,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 8.1)
+#
 # 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': 'uint32' } }
 
 ##
 # @BlockdevOptions:
-- 
2.39.2





  parent reply	other threads:[~2024-04-11  9:33 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-11  9:29 [pve-devel] [PATCH-SERIES v3] fix #4136: implement backup fleecing Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH qemu v3 01/22] block/copy-before-write: fix permission Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH qemu v3 02/22] block/copy-before-write: support unligned snapshot-discard Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH qemu v3 03/22] block/copy-before-write: create block_copy bitmap in filter node Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH qemu v3 04/22] qapi: blockdev-backup: add discard-source parameter Fiona Ebner
2024-04-11  9:29 ` Fiona Ebner [this message]
2024-04-11  9:29 ` [pve-devel] [PATCH qemu v3 06/22] backup: add minimum cluster size to performance options Fiona Ebner
2024-04-11 18:41   ` [pve-devel] partially-applied: " Thomas Lamprecht
2024-04-11  9:29 ` [pve-devel] [PATCH qemu v3 07/22] PVE backup: add fleecing option Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH common v3 08/22] json schema: add format description for pve-storage-id standard option Fiona Ebner
2024-04-11 17:58   ` [pve-devel] applied: " Thomas Lamprecht
2024-04-11  9:29 ` [pve-devel] [PATCH guest-common v3 09/22] vzdump: schema: add fleecing property string Fiona Ebner
2024-04-11 18:07   ` [pve-devel] applied: " Thomas Lamprecht
2024-04-12  8:38     ` Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH guest-common v3 10/22] vzdump: schema: make storage for fleecing semi-optional Fiona Ebner
2024-04-11 18:07   ` Thomas Lamprecht
2024-04-11 18:07   ` [pve-devel] applied: " Thomas Lamprecht
2024-04-11  9:29 ` [pve-devel] [RFC guest-common v3 11/22] abstract config: do not copy fleecing images entry for snapshot Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH manager v3 12/22] vzdump: have property string helpers always return the result Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH manager v3 13/22] vzdump: handle new 'fleecing' property string Fiona Ebner
2024-04-22  8:15   ` Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH manager v3 14/22] api: backup/vzdump: add permission check for fleecing storage Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH qemu-server v3 15/22] backup: disk info: also keep track of size Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH qemu-server v3 16/22] backup: implement fleecing option Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [RFC qemu-server v3 17/22] parse config: allow config keys with minus sign Fiona Ebner
2024-04-11 17:50   ` Thomas Lamprecht
2024-04-16  9:02     ` Fiona Ebner
2024-10-21 13:28       ` Thomas Lamprecht
2024-04-11  9:29 ` [pve-devel] [RFC qemu-server v3 18/22] schema: add fleecing-images config property Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [RFC qemu-server v3 19/22] vzdump: better cleanup fleecing images after hard errors Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [RFC qemu-server v3 20/22] migration: attempt to clean up potential left-over fleecing images Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [RFC qemu-server v3 21/22] destroy vm: " Fiona Ebner
2024-04-11  9:29 ` [pve-devel] [PATCH docs v3 22/22] vzdump: add section about backup fleecing Fiona Ebner
2024-04-19 15:23 ` [pve-devel] partially-applied: [PATCH-SERIES v3] fix #4136: implement " Fiona Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240411092943.57377-6-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal