[pve-devel] [RFC qemu 6/7] block/copy-before-write: allow specifying error callback

Fiona Ebner f.ebner at proxmox.com
Mon Jun 10 14:59:41 CEST 2024


The callback is invoked when cbw is configured to not break the guest
write. Will be useful to be able to abort a backup job immediately.
Currently, it has to wait for the rest of the block copy operation to
finish before checking the cbw error state. Since the job will be
required for the callback and the cbw node is created before the
backup job, the error callback can only be set after the cbw node is
created. That is why a dedicated function is used for that.

Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
---
 block/copy-before-write.c | 33 ++++++++++++++++++++++++++++++++-
 block/copy-before-write.h |  7 +++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 5e0c1252f6..00ce1e957e 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -79,6 +79,9 @@ typedef struct BDRVCopyBeforeWriteState {
      * atomics.
      */
     int snapshot_error;
+
+    CbwErrorCallbackFunc error_cb;
+    void *error_cb_opaque;
 } BDRVCopyBeforeWriteState;
 
 static int coroutine_fn GRAPH_RDLOCK
@@ -140,7 +143,15 @@ static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs,
     WITH_QEMU_LOCK_GUARD(&s->lock) {
         if (ret < 0) {
             assert(s->on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT);
-            qatomic_cmpxchg(&s->snapshot_error, 0, ret);
+            int old = qatomic_cmpxchg(&s->snapshot_error, 0, ret);
+            CbwErrorCallbackFunc error_cb = qatomic_load_acquire(&s->error_cb);
+            /*
+             * Only invoke error callback the first time. There can be massive amounts of errros
+             * when the backup target cannot be reached.
+             */
+            if (!old && error_cb) {
+                error_cb(s->error_cb_opaque, ret);
+            }
         } else {
             bdrv_set_dirty_bitmap(s->done_bitmap, off, end - off);
         }
@@ -591,6 +602,26 @@ int bdrv_cbw_snapshot_error(BlockDriverState *bs)
     return qatomic_read(&s->snapshot_error);
 }
 
+void bdrv_cbw_set_error_cb(BlockDriverState *bs, CbwErrorCallbackFunc error_cb,
+                           void *error_cb_opaque)
+{
+    BDRVCopyBeforeWriteState *s = bs->opaque;
+
+    /*
+     * Supporting multiple calls requires using locking or indirection of callback+arguments into
+     * another struct whose pointer can then be set with atomics.
+     */
+    assert(!s->error_cb);
+
+    s->error_cb_opaque = error_cb_opaque;
+    /*
+     * The only user of this function is during initalization of the backup job while drained. And
+     * while there should be enough stuff happening in between to ensure the callback and argument
+     * is seen by cbw_do_copy_before_write(), play it safe.
+     */
+    qatomic_store_release(&s->error_cb, error_cb);
+}
+
 static void cbw_init(void)
 {
     bdrv_register(&bdrv_cbw_filter);
diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index 4c22bd282e..cddbd5d9a2 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -29,6 +29,8 @@
 #include "block/block_int.h"
 #include "block/block-copy.h"
 
+typedef void (*CbwErrorCallbackFunc)(void *opaque, int error);
+
 /*
  * Global state (GS) API. These functions run under the BQL.
  *
@@ -45,5 +47,10 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   Error **errp);
 void bdrv_cbw_drop(BlockDriverState *bs);
 int bdrv_cbw_snapshot_error(BlockDriverState *bs);
+/*
+ * Can only be called once for a given cbw node.
+ */
+void bdrv_cbw_set_error_cb(BlockDriverState *bs, CbwErrorCallbackFunc error_cb,
+                           void *error_cb_opaque);
 
 #endif /* COPY_BEFORE_WRITE_H */
-- 
2.39.2





More information about the pve-devel mailing list