[PATCH v2] blockdev-backup: Add error handling option for copy-before-write jobs

[PATCH v2] blockdev-backup: Add error handling option for copy-before-write jobs

[Raman Dzehtsiar]

This patch extends the blockdev-backup QMP command to allow users to specify
how to behave when IO errors occur during copy-before-write operations.
Previously, the behavior was fixed and could not be controlled by the user.

The new 'on-cbw-error' option can be set to one of two values:
- 'break-guest-write': Forwards the IO error to the guest and triggers
  the on-source-error policy. This preserves snapshot integrity at the
  expense of guest IO operations.
- 'break-snapshot': Allows the guest OS to continue running normally,
  but invalidates the snapshot and aborts related jobs. This prioritizes
  guest operation over backup consistency.

This enhancement provides more flexibility for backup operations in different
environments where requirements for guest availability versus backup
consistency may vary.

The default behavior remains unchanged to maintain backward compatibility.

Signed-off-by: Raman Dzehtsiar <Raman.Dzehtsiar@gmail.com>
---
 block/backup.c                         | 3 ++-
 block/copy-before-write.c              | 2 ++
 block/copy-before-write.h              | 1 +
 block/replication.c                    | 4 +++-
 blockdev.c                             | 6 ++++++
 include/block/block_int-global-state.h | 2 ++
 qapi/block-core.json                   | 4 ++++
 7 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 79652bf57b..0151e84395 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -361,6 +361,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
                   BackupPerf *perf,
                   BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
+                  OnCbwError on_cbw_error,
                   int creation_flags,
                   BlockCompletionFunc *cb, void *opaque,
                   JobTxn *txn, Error **errp)
@@ -458,7 +459,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     }
 
     cbw = bdrv_cbw_append(bs, target, filter_node_name, discard_source,
-                          perf->min_cluster_size, &bcs, errp);
+                          perf->min_cluster_size, &bcs, on_cbw_error, errp);
     if (!cbw) {
         goto error;
     }
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index fd470f5f92..00af0b18ac 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -551,6 +551,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   bool discard_source,
                                   uint64_t min_cluster_size,
                                   BlockCopyState **bcs,
+                                  OnCbwError on_cbw_error,
                                   Error **errp)
 {
     BDRVCopyBeforeWriteState *state;
@@ -568,6 +569,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_str(opts, "on-cbw-error", OnCbwError_str(on_cbw_error));
 
     if (min_cluster_size > INT64_MAX) {
         error_setg(errp, "min-cluster-size too large: %" PRIu64 " > %" PRIi64,
diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index 2a5d4ba693..eb93364e85 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -42,6 +42,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   bool discard_source,
                                   uint64_t min_cluster_size,
                                   BlockCopyState **bcs,
+                                  OnCbwError on_cbw_error,
                                   Error **errp);
 void bdrv_cbw_drop(BlockDriverState *bs);
 
diff --git a/block/replication.c b/block/replication.c
index 0020f33843..748cf648ec 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -584,7 +584,9 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
                                 0, MIRROR_SYNC_MODE_NONE, NULL, 0, false, false,
                                 NULL, &perf,
                                 BLOCKDEV_ON_ERROR_REPORT,
-                                BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL,
+                                BLOCKDEV_ON_ERROR_REPORT,
+                                ON_CBW_ERROR_BREAK_GUEST_WRITE,
+                                JOB_INTERNAL,
                                 backup_job_completed, bs, NULL, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
diff --git a/blockdev.c b/blockdev.c
index 1d1f27cfff..818ec42511 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2641,6 +2641,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
     BdrvDirtyBitmap *bmap = NULL;
     BackupPerf perf = { .max_workers = 64 };
     int job_flags = JOB_DEFAULT;
+    OnCbwError on_cbw_error = ON_CBW_ERROR_BREAK_GUEST_WRITE;
 
     if (!backup->has_speed) {
         backup->speed = 0;
@@ -2745,6 +2746,10 @@ static BlockJob *do_backup_common(BackupCommon *backup,
         job_flags |= JOB_MANUAL_DISMISS;
     }
 
+    if (backup->has_on_cbw_error) {
+        on_cbw_error = backup->on_cbw_error;
+    }
+
     job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
                             backup->sync, bmap, backup->bitmap_mode,
                             backup->compress, backup->discard_source,
@@ -2752,6 +2757,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
                             &perf,
                             backup->on_source_error,
                             backup->on_target_error,
+                            on_cbw_error,
                             job_flags, NULL, NULL, txn, errp);
     return job;
 }
diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h
index eb2d92a226..0d93783763 100644
--- a/include/block/block_int-global-state.h
+++ b/include/block/block_int-global-state.h
@@ -179,6 +179,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
  *        all ".has_*" fields are ignored.
  * @on_source_error: The action to take upon error reading from the source.
  * @on_target_error: The action to take upon error writing to the target.
+ * @on_cbw_error: The action to take upon error in copy-before-write operations.
  * @creation_flags: Flags that control the behavior of the Job lifetime.
  *                  See @BlockJobCreateFlags
  * @cb: Completion function for the job.
@@ -198,6 +199,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
                             BackupPerf *perf,
                             BlockdevOnError on_source_error,
                             BlockdevOnError on_target_error,
+                            OnCbwError on_cbw_error,
                             int creation_flags,
                             BlockCompletionFunc *cb, void *opaque,
                             JobTxn *txn, Error **errp);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ee6eccc68c..3a7cf82b57 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1622,6 +1622,9 @@
 # @discard-source: Discard blocks on source which have already been
 #     copied to the target.  (Since 9.1)
 #
+# @on-cbw-error: optional policy defining behavior on I/O errors in
+#     copy-before-write jobs; defaults to break-guest-write.  (Since 10.0)
+#
 # @x-perf: Performance options.  (Since 6.0)
 #
 # Features:
@@ -1641,6 +1644,7 @@
             '*compress': 'bool',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError',
+            '*on-cbw-error': 'OnCbwError',
             '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
             '*filter-node-name': 'str',
             '*discard-source': 'bool',
-- 
2.43.0

Re: [PATCH v2] blockdev-backup: Add error handling option for copy-before-write jobs

[Raman Dzehtsiar]

On Tue, Mar 4, 2025 at 10:17 AM Raman Dzehtsiar
<raman.dzehtsiar@gmail.com> wrote:
>
> This patch extends the blockdev-backup QMP command to allow users to specify
> how to behave when IO errors occur during copy-before-write operations.
> Previously, the behavior was fixed and could not be controlled by the user.
>
> The new 'on-cbw-error' option can be set to one of two values:
> - 'break-guest-write': Forwards the IO error to the guest and triggers
>   the on-source-error policy. This preserves snapshot integrity at the
>   expense of guest IO operations.
> - 'break-snapshot': Allows the guest OS to continue running normally,
>   but invalidates the snapshot and aborts related jobs. This prioritizes
>   guest operation over backup consistency.
>
> This enhancement provides more flexibility for backup operations in different
> environments where requirements for guest availability versus backup
> consistency may vary.
>
> The default behavior remains unchanged to maintain backward compatibility.
>
> Signed-off-by: Raman Dzehtsiar <Raman.Dzehtsiar@gmail.com>

Hi team,

Thank you for taking the time to review my changes. I would like to
clarify if there is
anything preventing this patch from being merged, as I am eager to
have it included
in the next release.

Thanks

Re: [PATCH v2] blockdev-backup: Add error handling option for copy-before-write jobs

[Vladimir Sementsov-Ogievskiy]

On 04.03.25 12:17, Raman Dzehtsiar wrote:
> This patch extends the blockdev-backup QMP command to allow users to specify
> how to behave when IO errors occur during copy-before-write operations.
> Previously, the behavior was fixed and could not be controlled by the user.
> 
> The new 'on-cbw-error' option can be set to one of two values:
> - 'break-guest-write': Forwards the IO error to the guest and triggers
>    the on-source-error policy. This preserves snapshot integrity at the
>    expense of guest IO operations.
> - 'break-snapshot': Allows the guest OS to continue running normally,
>    but invalidates the snapshot and aborts related jobs. This prioritizes
>    guest operation over backup consistency.
> 
> This enhancement provides more flexibility for backup operations in different
> environments where requirements for guest availability versus backup
> consistency may vary.
> 
> The default behavior remains unchanged to maintain backward compatibility.
> 
> Signed-off-by: Raman Dzehtsiar<Raman.Dzehtsiar@gmail.com>


Hi Raman, sorry for a delay!

The patch looks good to me. Still, could you also provide a test for a new option?

Probably the simplest would be add a test-case to `tests/qemu-iotests/tests/copy-before-write`, where existing on-cbw-error option is tested. Or you can make a separate test.

-- 
Best regards,
Vladimir

Re: [PATCH v2] blockdev-backup: Add error handling option for copy-before-write jobs

[Raman Dzehtsiar]

Hi Vladimir,
Thank you for your review.

> The patch looks good to me. Still, could you also provide a test for a new option?
> Probably the simplest would be add a test-case to `tests/qemu-iotests/tests/copy-before-write`, where existing on-cbw-error option is tested. Or you can make a separate test.

I've added test cases to the `copy-before-write` test to cover the new
option. The changes are included in v3 of the patch.
Would appreciate it if you could take another look. Still hoping it
can make it into v10.

Thanks again and best regards,
Raman Dzehtsiar

Re: [PATCH v2] blockdev-backup: Add error handling option for copy-before-write jobs

[Vladimir Sementsov-Ogievskiy]

On 14.04.25 04:30, Raman Dzehtsiar wrote:
> Hi Vladimir,
> Thank you for your review.
> 
>> The patch looks good to me. Still, could you also provide a test for a new option?
>> Probably the simplest would be add a test-case to `tests/qemu-iotests/tests/copy-before-write`, where existing on-cbw-error option is tested. Or you can make a separate test.
> 
> I've added test cases to the `copy-before-write` test to cover the new
> option. The changes are included in v3 of the patch.
> Would appreciate it if you could take another look.

Will do

> Still hoping it
> can make it into v10.
> 

10.0 - unfortunately, impossible. It's frozen (for new features) since March 11, and should be released tomorrow or a week later. See planning here: https://wiki.qemu.org/Planning/10.0
10.1 is a target for the series.

-- 
Best regards,
Vladimir