From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 648571FF39E
	for <inbox@lore.proxmox.com>; Mon, 10 Jun 2024 14:59:23 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 9C1AA173B3;
	Mon, 10 Jun 2024 14:59:50 +0200 (CEST)
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Mon, 10 Jun 2024 14:59:41 +0200
Message-Id: <20240610125942.116985-7-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.2
In-Reply-To: <20240610125942.116985-1-f.ebner@proxmox.com>
References: <20240610125942.116985-1-f.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.059 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
Subject: [pve-devel] [RFC qemu 6/7] block/copy-before-write: allow
 specifying error callback
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.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