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] [RFC qemu 6/7] block/copy-before-write: allow specifying error callback
Date: Mon, 10 Jun 2024 14:59:41 +0200	[thread overview]
Message-ID: <20240610125942.116985-7-f.ebner@proxmox.com> (raw)
In-Reply-To: <20240610125942.116985-1-f.ebner@proxmox.com>

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@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



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


  parent reply	other threads:[~2024-06-10 12:59 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-10 12:59 [pve-devel] [RFC qemu] fix #3231+#3631: PVE backup: fail backup rather than guest write when backup target cannot be reached or is too slow Fiona Ebner
2024-06-10 12:59 ` [pve-devel] [PATCH qemu 1/7] PVE backup: fleecing: properly set minimum cluster size Fiona Ebner
2024-06-10 12:59 ` [pve-devel] [RFC qemu 2/7] block/copy-before-write: allow passing additional options for bdrv_cbw_append() Fiona Ebner
2024-06-10 12:59 ` [pve-devel] [RFC qemu 3/7] block/backup: allow passing additional options for copy-before-write upon job creation Fiona Ebner
2024-07-05  9:30   ` Fabian Grünbichler
2024-06-10 12:59 ` [pve-devel] [RFC qemu 4/7] block/backup: make cbw error also fail backup that does not use fleecing Fiona Ebner
2024-06-10 12:59 ` [pve-devel] [RFC qemu 5/7] fix #3231+#3631: PVE backup: add timeout for copy-before-write operations and fail backup instead of guest writes Fiona Ebner
2024-06-10 12:59 ` Fiona Ebner [this message]
2024-06-10 12:59 ` [pve-devel] [RFC qemu 7/7] block/backup: set callback for cbw errors Fiona Ebner
2024-07-05  9:37   ` Fabian Grünbichler
2024-07-05  9:41 ` [pve-devel] [RFC qemu] fix #3231+#3631: PVE backup: fail backup rather than guest write when backup target cannot be reached or is too slow Fabian Grünbichler

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=20240610125942.116985-7-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