[PATCH 0/4] block: update inserted/removed nodes from BlockRAMRegistrar

[PATCH 0/4] block: update inserted/removed nodes from BlockRAMRegistrar

[Stefan Hajnoczi]

This patch series fixes a bug in BlockRAMRegistrar: it currently doesn't react
to block graph changes and newly inserted nodes lack RAMBlock information
needed to map I/O buffers. This is important for vdpa-blk devices because they
rely on the ability to map I/O buffers.

Stefan Hajnoczi (4):
  block: add BdrvChildClass->propagate_attach/detach() callbacks
  block: add blk_add_attach/detach_notifier() APIs
  block: rename RAMBlockRegistrar->notifier field
  block: update inserted/removed nodes from BlockRAMRegistrar

 include/block/block_int-common.h            |  11 ++
 include/system/block-backend-global-state.h |   9 ++
 include/system/block-ram-registrar.h        |   4 +-
 block.c                                     |  56 +++++--
 block/block-backend.c                       |  44 ++++++
 block/block-ram-registrar.c                 |  75 ++++++++-
 tests/unit/test-block-backend.c             | 164 ++++++++++++++++++++
 7 files changed, 344 insertions(+), 19 deletions(-)

-- 
2.51.0

[PATCH 1/4] block: add BdrvChildClass->propagate_attach/detach() callbacks

[Stefan Hajnoczi]

bdrv_replace_child_noperm() is the heart of block graph changes. It is
called when a node is inserted, removed, or replaced. These changes
happen without notifying parents in the graph, so it is currently not
possible to monitor changes.

Add BdrvChildClass callbacks to propagate attach/detach operations to
the roots of the graph. The next commit will introduce a BlockBackend
API for monitoring changes using this mechanism.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/block_int-common.h | 11 +++++++
 block.c                          | 56 +++++++++++++++++++++++++-------
 2 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 034c0634c8..0368a3191c 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -963,6 +963,17 @@ struct BdrvChildClass {
     void GRAPH_RDLOCK_PTR (*activate)(BdrvChild *child, Error **errp);
     int GRAPH_RDLOCK_PTR (*inactivate)(BdrvChild *child);
 
+    /*
+     * Optional callbacks when a descendant (child, grandchild, etc) attaches
+     * or detaches a BlockDriverState. Allows monitoring changes to the graph.
+     *
+     * Called after ->attach() and before ->detach().
+     */
+    void GRAPH_WRLOCK_PTR (*propagate_attach)(BdrvChild *self,
+                                              BdrvChild *descendant);
+    void GRAPH_WRLOCK_PTR (*propagate_detach)(BdrvChild *self,
+                                              BdrvChild *descendant);
+
     void GRAPH_WRLOCK_PTR (*attach)(BdrvChild *child);
     void GRAPH_WRLOCK_PTR (*detach)(BdrvChild *child);
 
diff --git a/block.c b/block.c
index 8848e9a7ed..e1480dda04 100644
--- a/block.c
+++ b/block.c
@@ -1497,6 +1497,32 @@ static void GRAPH_WRLOCK bdrv_child_cb_detach(BdrvChild *child)
     }
 }
 
+static void GRAPH_WRLOCK
+bdrv_child_cb_propagate_attach(BdrvChild *self, BdrvChild *descendant)
+{
+    BlockDriverState *bs = self->opaque;
+    BdrvChild *c;
+
+    QLIST_FOREACH(c, &bs->parents, next_parent) {
+        if (c->klass->propagate_attach) {
+            c->klass->propagate_attach(c, descendant);
+        }
+    }
+}
+
+static void GRAPH_WRLOCK
+bdrv_child_cb_propagate_detach(BdrvChild *self, BdrvChild *descendant)
+{
+    BlockDriverState *bs = self->opaque;
+    BdrvChild *c;
+
+    QLIST_FOREACH(c, &bs->parents, next_parent) {
+        if (c->klass->propagate_detach) {
+            c->klass->propagate_detach(c, descendant);
+        }
+    }
+}
+
 static int bdrv_child_cb_update_filename(BdrvChild *c, BlockDriverState *base,
                                          const char *filename,
                                          bool backing_mask_protocol,
@@ -1519,17 +1545,19 @@ AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c)
 }
 
 const BdrvChildClass child_of_bds = {
-    .parent_is_bds   = true,
-    .get_parent_desc = bdrv_child_get_parent_desc,
-    .inherit_options = bdrv_inherited_options,
-    .drained_begin   = bdrv_child_cb_drained_begin,
-    .drained_poll    = bdrv_child_cb_drained_poll,
-    .drained_end     = bdrv_child_cb_drained_end,
-    .attach          = bdrv_child_cb_attach,
-    .detach          = bdrv_child_cb_detach,
-    .inactivate      = bdrv_child_cb_inactivate,
-    .change_aio_ctx  = bdrv_child_cb_change_aio_ctx,
-    .update_filename = bdrv_child_cb_update_filename,
+    .parent_is_bds          = true,
+    .get_parent_desc        = bdrv_child_get_parent_desc,
+    .inherit_options        = bdrv_inherited_options,
+    .drained_begin          = bdrv_child_cb_drained_begin,
+    .drained_poll           = bdrv_child_cb_drained_poll,
+    .drained_end            = bdrv_child_cb_drained_end,
+    .attach                 = bdrv_child_cb_attach,
+    .detach                 = bdrv_child_cb_detach,
+    .propagate_attach       = bdrv_child_cb_propagate_attach,
+    .propagate_detach       = bdrv_child_cb_propagate_detach,
+    .inactivate             = bdrv_child_cb_inactivate,
+    .change_aio_ctx         = bdrv_child_cb_change_aio_ctx,
+    .update_filename        = bdrv_child_cb_update_filename,
     .get_parent_aio_context = child_of_bds_get_parent_aio_context,
 };
 
@@ -2967,6 +2995,9 @@ bdrv_replace_child_noperm(BdrvChild *child, BlockDriverState *new_bs)
     }
 
     if (old_bs) {
+        if (child->klass->propagate_detach) {
+            child->klass->propagate_detach(child, child);
+        }
         if (child->klass->detach) {
             child->klass->detach(child);
         }
@@ -2980,6 +3011,9 @@ bdrv_replace_child_noperm(BdrvChild *child, BlockDriverState *new_bs)
         if (child->klass->attach) {
             child->klass->attach(child);
         }
+        if (child->klass->propagate_attach) {
+            child->klass->propagate_attach(child, child);
+        }
     }
 
     /*
-- 
2.51.0

[PATCH 2/4] block: add blk_add_attach/detach_notifier() APIs

[Stefan Hajnoczi]

Add an API to receive a callback when a BlockDriverState is attached or
detached from the block graph. This allows callers to track insert,
remove, and replace operations.

Note that this API might be able to replace the
blk_add_remove_bs_notifier() API if the callback checks that the root
BDS is being removed and ignores all other calls. I think this is a bit
messy though, so I kept blk_add_remove_bs_notifier().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/system/block-backend-global-state.h |   9 ++
 block/block-backend.c                       |  44 ++++++
 tests/unit/test-block-backend.c             | 164 ++++++++++++++++++++
 3 files changed, 217 insertions(+)

diff --git a/include/system/block-backend-global-state.h b/include/system/block-backend-global-state.h
index c3849640df..e828f271ee 100644
--- a/include/system/block-backend-global-state.h
+++ b/include/system/block-backend-global-state.h
@@ -97,6 +97,15 @@ void blk_remove_aio_context_notifier(BlockBackend *blk,
                                      void (*detach_aio_context)(void *),
                                      void *opaque);
 void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify);
+
+typedef struct {
+    BlockBackend *blk;
+    BlockDriverState *bs;
+} BlockBackendAttachDetachArgs;
+
+void blk_add_attach_notifier(BlockBackend *blk, Notifier *notify);
+void blk_add_detach_notifier(BlockBackend *blk, Notifier *notify);
+
 BlockBackendRootState *blk_get_root_state(BlockBackend *blk);
 void blk_update_root_state(BlockBackend *blk);
 bool blk_get_detect_zeroes_from_root_state(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index f8d6ba65c1..45e31f0079 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -76,6 +76,7 @@ struct BlockBackend {
 
     /* Protected by BQL */
     NotifierList remove_bs_notifiers, insert_bs_notifiers;
+    NotifierList attach_notifiers, detach_notifiers;
     QLIST_HEAD(, BlockBackendAioNotifier) aio_notifiers;
 
     int quiesce_counter; /* atomic: written under BQL, read by other threads */
@@ -280,6 +281,30 @@ static int GRAPH_RDLOCK blk_root_inactivate(BdrvChild *child)
     return 0;
 }
 
+static void GRAPH_WRLOCK
+blk_root_propagate_attach(BdrvChild *self, BdrvChild *descendant)
+{
+    BlockBackend *blk = self->opaque;
+    BlockBackendAttachDetachArgs args = {
+        .blk = blk,
+        .bs = descendant->bs,
+    };
+
+    notifier_list_notify(&blk->attach_notifiers, &args);
+}
+
+static void GRAPH_WRLOCK
+blk_root_propagate_detach(BdrvChild *self, BdrvChild *descendant)
+{
+    BlockBackend *blk = self->opaque;
+    BlockBackendAttachDetachArgs args = {
+        .blk = blk,
+        .bs = descendant->bs,
+    };
+
+    notifier_list_notify(&blk->detach_notifiers, &args);
+}
+
 static void blk_root_attach(BdrvChild *child)
 {
     BlockBackend *blk = child->opaque;
@@ -333,6 +358,9 @@ static const BdrvChildClass child_root = {
     .activate           = blk_root_activate,
     .inactivate         = blk_root_inactivate,
 
+    .propagate_attach   = blk_root_propagate_attach,
+    .propagate_detach   = blk_root_propagate_detach,
+
     .attach             = blk_root_attach,
     .detach             = blk_root_detach,
 
@@ -374,6 +402,8 @@ BlockBackend *blk_new(AioContext *ctx, uint64_t perm, uint64_t shared_perm)
     qemu_co_queue_init(&blk->queued_requests);
     notifier_list_init(&blk->remove_bs_notifiers);
     notifier_list_init(&blk->insert_bs_notifiers);
+    notifier_list_init(&blk->attach_notifiers);
+    notifier_list_init(&blk->detach_notifiers);
     QLIST_INIT(&blk->aio_notifiers);
 
     QTAILQ_INSERT_TAIL(&block_backends, blk, link);
@@ -492,6 +522,8 @@ static void blk_delete(BlockBackend *blk)
     }
     assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers));
     assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers));
+    assert(QLIST_EMPTY(&blk->attach_notifiers.notifiers));
+    assert(QLIST_EMPTY(&blk->detach_notifiers.notifiers));
     assert(QLIST_EMPTY(&blk->aio_notifiers));
     assert(qemu_co_queue_empty(&blk->queued_requests));
     qemu_mutex_destroy(&blk->queued_requests_lock);
@@ -2512,6 +2544,18 @@ void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify)
     notifier_list_add(&blk->remove_bs_notifiers, notify);
 }
 
+void blk_add_attach_notifier(BlockBackend *blk, Notifier *notify)
+{
+    GLOBAL_STATE_CODE();
+    notifier_list_add(&blk->attach_notifiers, notify);
+}
+
+void blk_add_detach_notifier(BlockBackend *blk, Notifier *notify)
+{
+    GLOBAL_STATE_CODE();
+    notifier_list_add(&blk->detach_notifiers, notify);
+}
+
 BlockAcctStats *blk_get_stats(BlockBackend *blk)
 {
     IO_CODE();
diff --git a/tests/unit/test-block-backend.c b/tests/unit/test-block-backend.c
index 4257b3f815..693383f029 100644
--- a/tests/unit/test-block-backend.c
+++ b/tests/unit/test-block-backend.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "block/block.h"
+#include "block/block_int.h"
 #include "system/block-backend.h"
 #include "qapi/error.h"
 #include "qemu/main-loop.h"
@@ -70,6 +71,167 @@ static void test_drain_all_aio_error(void)
     blk_unref(blk);
 }
 
+static int bdrv_test_co_change_backing_file(BlockDriverState *bs,
+                                            const char *backing_file,
+                                            const char *backing_fmt)
+{
+    return 0; /* just return success so bdrv_set_backing_hd() works */
+}
+
+static BlockDriver bdrv_test = {
+    .format_name = "test",
+    .supports_backing = true,
+    .bdrv_child_perm  = bdrv_default_perms,
+    .bdrv_co_change_backing_file = bdrv_test_co_change_backing_file,
+};
+
+typedef struct {
+    Notifier attach_notifier;
+    Notifier detach_notifier;
+    GArray *notifications;
+} AttachDetach;
+
+typedef enum {
+    NOTIFY_END = 0,
+    NOTIFY_ATTACH,
+    NOTIFY_DETACH,
+} NotificationType;
+
+typedef struct {
+    NotificationType type;
+    BlockDriverState *bs;
+} Notification;
+
+static void attach_detach_append(AttachDetach *ad, NotificationType type,
+                                 BlockBackendAttachDetachArgs *args)
+{
+    Notification n = {
+        .type = type,
+        .bs = args->bs,
+    };
+
+    g_array_append_vals(ad->notifications, &n, 1);
+}
+
+static void attach_notify(Notifier *notifier, void *data)
+{
+    AttachDetach *ad = container_of(notifier, AttachDetach, attach_notifier);
+    attach_detach_append(ad, NOTIFY_ATTACH, data);
+}
+
+static void detach_notify(Notifier *notifier, void *data)
+{
+    AttachDetach *ad = container_of(notifier, AttachDetach, attach_notifier);
+    attach_detach_append(ad, NOTIFY_DETACH, data);
+}
+
+static void attach_detach_init(AttachDetach *ad, BlockBackend *blk)
+{
+    ad->attach_notifier.notify = attach_notify;
+    ad->detach_notifier.notify = detach_notify;
+    ad->notifications = g_array_new(true, true, sizeof(Notification));
+
+    blk_add_attach_notifier(blk, &ad->attach_notifier);
+    blk_add_detach_notifier(blk, &ad->detach_notifier);
+}
+
+static void attach_detach_cleanup(AttachDetach *ad)
+{
+    g_array_free(ad->notifications, true);
+    notifier_remove(&ad->detach_notifier);
+    notifier_remove(&ad->attach_notifier);
+}
+
+/*
+ * Check that the expected notifications occurred. @expected is terminated by a
+ * NOTIFY_END element.
+ */
+static void attach_detach_expect(AttachDetach *ad, const Notification *expected)
+{
+    GArray *array = ad->notifications;
+
+    /* The array is zero terminated so there is at least one element */
+    Notification *actual = (Notification *)array->data;
+
+    while (expected->type != NOTIFY_END) {
+        g_assert_cmpint(actual->type, ==, expected->type);
+        g_assert(actual->bs == expected->bs);
+        expected++;
+        actual++;
+    }
+
+    g_assert_cmpint(actual->type, ==, NOTIFY_END);
+
+    g_array_remove_range(array, 0, array->len);
+}
+
+static void test_attach_detach_notifier(void)
+{
+    AttachDetach ad;
+    BlockDriverState *format;
+    BlockDriverState *file;
+    BlockDriverState *file2;
+    BlockBackend *blk = blk_new(qemu_get_aio_context(),
+                                BLK_PERM_ALL, BLK_PERM_ALL);
+
+    attach_detach_init(&ad, blk);
+
+    format = bdrv_new_open_driver(&bdrv_test, "format", BDRV_O_RDWR,
+                                  &error_abort);
+    file = bdrv_new_open_driver(&bdrv_test, "file", BDRV_O_RDWR, &error_abort);
+    file2 = bdrv_new_open_driver(&bdrv_test, "file2", BDRV_O_RDWR,
+                                 &error_abort);
+
+    bdrv_graph_wrlock_drained();
+    bdrv_attach_child(format, file, "file", &child_of_bds,
+                      BDRV_CHILD_PRIMARY | BDRV_CHILD_DATA, &error_abort);
+    bdrv_graph_wrunlock();
+
+    /* Insert format -> file */
+    blk_insert_bs(blk, format, &error_abort);
+    attach_detach_expect(&ad, (Notification[]){
+        (Notification){NOTIFY_ATTACH, format},
+        {},
+    });
+
+    /* Replace format -> file with file2 */
+    blk_replace_bs(blk, file2, &error_abort);
+    attach_detach_expect(&ad, (Notification[]){
+        (Notification){NOTIFY_DETACH, format},
+        (Notification){NOTIFY_ATTACH, file2},
+        {},
+    });
+
+    /* Remove file2 */
+    blk_remove_bs(blk);
+    attach_detach_expect(&ad, (Notification[]){
+        (Notification){NOTIFY_DETACH, file2},
+        {},
+    });
+
+    /* These BDSes were unrefed so we need new instances */
+    file = bdrv_new_open_driver(&bdrv_test, "file", BDRV_O_RDWR, &error_abort);
+    file2 = bdrv_new_open_driver(&bdrv_test, "file2", BDRV_O_RDWR,
+                                 &error_abort);
+
+    /* Replace a non-root node */
+    bdrv_graph_wrlock_drained();
+    bdrv_attach_child(format, file, "file", &child_of_bds,
+                      BDRV_CHILD_PRIMARY | BDRV_CHILD_DATA, &error_abort);
+    bdrv_replace_node(file, file2, &error_abort);
+    bdrv_graph_wrunlock();
+    attach_detach_expect(&ad, (Notification[]){
+        (Notification){NOTIFY_ATTACH, file},
+        (Notification){NOTIFY_DETACH, file},
+        (Notification){NOTIFY_ATTACH, file2},
+        {},
+    });
+
+    attach_detach_cleanup(&ad);
+    blk_unref(blk);
+    bdrv_unref(format);
+}
+
 int main(int argc, char **argv)
 {
     bdrv_init();
@@ -80,6 +242,8 @@ int main(int argc, char **argv)
     g_test_add_func("/block-backend/drain_aio_error", test_drain_aio_error);
     g_test_add_func("/block-backend/drain_all_aio_error",
                     test_drain_all_aio_error);
+    g_test_add_func("/block-backend/attach_detach_notifier",
+                    test_attach_detach_notifier);
 
     return g_test_run();
 }
-- 
2.51.0

[PATCH 3/4] block: rename RAMBlockRegistrar->notifier field

[Stefan Hajnoczi]

The 'notifier' field name will be confusing when the BlockBackend attach
and detach notifiers are added in the next commit. Rename the field so
it's clear that this is the RAMBlock notifier.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/system/block-ram-registrar.h |  2 +-
 block/block-ram-registrar.c          | 14 ++++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/system/block-ram-registrar.h b/include/system/block-ram-registrar.h
index d8b2f7942b..76c157bd54 100644
--- a/include/system/block-ram-registrar.h
+++ b/include/system/block-ram-registrar.h
@@ -21,7 +21,7 @@
  */
 typedef struct {
     BlockBackend *blk;
-    RAMBlockNotifier notifier;
+    RAMBlockNotifier ram_block_notifier;
     bool ok;
 } BlockRAMRegistrar;
 
diff --git a/block/block-ram-registrar.c b/block/block-ram-registrar.c
index fcda2b86af..d5b84667a1 100644
--- a/block/block-ram-registrar.c
+++ b/block/block-ram-registrar.c
@@ -12,7 +12,8 @@
 static void ram_block_added(RAMBlockNotifier *n, void *host, size_t size,
                             size_t max_size)
 {
-    BlockRAMRegistrar *r = container_of(n, BlockRAMRegistrar, notifier);
+    BlockRAMRegistrar *r =
+        container_of(n, BlockRAMRegistrar, ram_block_notifier);
     Error *err = NULL;
 
     if (!r->ok) {
@@ -21,7 +22,7 @@ static void ram_block_added(RAMBlockNotifier *n, void *host, size_t size,
 
     if (!blk_register_buf(r->blk, host, max_size, &err)) {
         error_report_err(err);
-        ram_block_notifier_remove(&r->notifier);
+        ram_block_notifier_remove(n);
         r->ok = false;
     }
 }
@@ -29,14 +30,15 @@ static void ram_block_added(RAMBlockNotifier *n, void *host, size_t size,
 static void ram_block_removed(RAMBlockNotifier *n, void *host, size_t size,
                               size_t max_size)
 {
-    BlockRAMRegistrar *r = container_of(n, BlockRAMRegistrar, notifier);
+    BlockRAMRegistrar *r =
+        container_of(n, BlockRAMRegistrar, ram_block_notifier);
     blk_unregister_buf(r->blk, host, max_size);
 }
 
 void blk_ram_registrar_init(BlockRAMRegistrar *r, BlockBackend *blk)
 {
     r->blk = blk;
-    r->notifier = (RAMBlockNotifier){
+    r->ram_block_notifier = (RAMBlockNotifier){
         .ram_block_added = ram_block_added,
         .ram_block_removed = ram_block_removed,
 
@@ -47,12 +49,12 @@ void blk_ram_registrar_init(BlockRAMRegistrar *r, BlockBackend *blk)
     };
     r->ok = true;
 
-    ram_block_notifier_add(&r->notifier);
+    ram_block_notifier_add(&r->ram_block_notifier);
 }
 
 void blk_ram_registrar_destroy(BlockRAMRegistrar *r)
 {
     if (r->ok) {
-        ram_block_notifier_remove(&r->notifier);
+        ram_block_notifier_remove(&r->ram_block_notifier);
     }
 }
-- 
2.51.0

[PATCH 4/4] block: update inserted/removed nodes from BlockRAMRegistrar

[Stefan Hajnoczi]

BlockRAMRegistrar ensures that RAMBlocks are registered with
BlockDriverStates. This is essential for vdpa-blk because they need to
know the memory mappings of I/O buffers. However, BlockRAMRegistrar is
currently unaware of changes to the block graph and newly inserted nodes
have no RAMBlocks registered.

Use the new blk_add_attach_notifier() and blk_add_detach_notifier() APIs
to bring nodes up to speed when the graph changes. This fixes vdpa-blk
across mirror and other operations that modify the block graph.
Previously I/O would not succeed after a new node was inserted due to
missing memory mappings.

Buglink: https://issues.redhat.com/browse/RHEL-88175
Reported-by: Yingshun Cui <yicui@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/system/block-ram-registrar.h |  2 +
 block/block-ram-registrar.c          | 63 +++++++++++++++++++++++++++-
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/include/system/block-ram-registrar.h b/include/system/block-ram-registrar.h
index 76c157bd54..292c197d1c 100644
--- a/include/system/block-ram-registrar.h
+++ b/include/system/block-ram-registrar.h
@@ -22,6 +22,8 @@
 typedef struct {
     BlockBackend *blk;
     RAMBlockNotifier ram_block_notifier;
+    Notifier blk_attach_notifier;
+    Notifier blk_detach_notifier;
     bool ok;
 } BlockRAMRegistrar;
 
diff --git a/block/block-ram-registrar.c b/block/block-ram-registrar.c
index d5b84667a1..a46b5a723f 100644
--- a/block/block-ram-registrar.c
+++ b/block/block-ram-registrar.c
@@ -7,7 +7,9 @@
 #include "qemu/osdep.h"
 #include "system/block-backend.h"
 #include "system/block-ram-registrar.h"
+#include "system/ramblock.h"
 #include "qapi/error.h"
+#include "trace.h"
 
 static void ram_block_added(RAMBlockNotifier *n, void *host, size_t size,
                             size_t max_size)
@@ -22,8 +24,8 @@ static void ram_block_added(RAMBlockNotifier *n, void *host, size_t size,
 
     if (!blk_register_buf(r->blk, host, max_size, &err)) {
         error_report_err(err);
-        ram_block_notifier_remove(n);
-        r->ok = false;
+        blk_ram_registrar_destroy(r);
+        return;
     }
 }
 
@@ -35,6 +37,52 @@ static void ram_block_removed(RAMBlockNotifier *n, void *host, size_t size,
     blk_unregister_buf(r->blk, host, max_size);
 }
 
+static void blk_attached(Notifier *n, void *data)
+{
+    BlockRAMRegistrar *r =
+        container_of(n, BlockRAMRegistrar, blk_attach_notifier);
+    BlockBackendAttachDetachArgs *args = data;
+    BlockDriverState *bs = args->bs;
+    Error *err = NULL;
+
+    WITH_RCU_READ_LOCK_GUARD() {
+        RAMBlock *rb;
+
+        RAMBLOCK_FOREACH(rb) {
+            ram_addr_t max_size = qemu_ram_get_max_length(rb);
+            void *host = qemu_ram_get_host_addr(rb);
+
+            if (!bdrv_register_buf(bs, host, max_size, &err)) {
+                goto err;
+            }
+        }
+    }
+
+    return;
+
+err:
+    error_report_err(err);
+    blk_ram_registrar_destroy(r);
+}
+
+static void blk_detached(Notifier *n, void *data)
+{
+    BlockRAMRegistrar *r =
+        container_of(n, BlockRAMRegistrar, blk_attach_notifier);
+    BlockBackendAttachDetachArgs *args = data;
+    BlockDriverState *bs = args->bs;
+    RAMBlock *rb;
+
+    RCU_READ_LOCK_GUARD();
+
+    RAMBLOCK_FOREACH(rb) {
+        ram_addr_t max_size = qemu_ram_get_max_length(rb);
+        void *host = qemu_ram_get_host_addr(rb);
+
+        bdrv_unregister_buf(bs, host, max_size);
+    }
+}
+
 void blk_ram_registrar_init(BlockRAMRegistrar *r, BlockBackend *blk)
 {
     r->blk = blk;
@@ -47,14 +95,25 @@ void blk_ram_registrar_init(BlockRAMRegistrar *r, BlockBackend *blk)
          * value that does not change across resize.
          */
     };
+    r->blk_attach_notifier = (Notifier){
+        .notify = blk_attached,
+    };
+    r->blk_detach_notifier = (Notifier){
+        .notify = blk_detached,
+    };
     r->ok = true;
 
     ram_block_notifier_add(&r->ram_block_notifier);
+    blk_add_attach_notifier(blk, &r->blk_attach_notifier);
+    blk_add_detach_notifier(blk, &r->blk_detach_notifier);
 }
 
 void blk_ram_registrar_destroy(BlockRAMRegistrar *r)
 {
     if (r->ok) {
+        notifier_remove(&r->blk_detach_notifier);
+        notifier_remove(&r->blk_attach_notifier);
         ram_block_notifier_remove(&r->ram_block_notifier);
+        r->ok = false;
     }
 }
-- 
2.51.0

Re: [PATCH 0/4] block: update inserted/removed nodes from BlockRAMRegistrar

[Eric Blake]

On Tue, Sep 30, 2025 at 04:45:51PM -0400, Stefan Hajnoczi wrote:
> This patch series fixes a bug in BlockRAMRegistrar: it currently doesn't react
> to block graph changes and newly inserted nodes lack RAMBlock information
> needed to map I/O buffers. This is important for vdpa-blk devices because they
> rely on the ability to map I/O buffers.
> 
> Stefan Hajnoczi (4):
>   block: add BdrvChildClass->propagate_attach/detach() callbacks
>   block: add blk_add_attach/detach_notifier() APIs
>   block: rename RAMBlockRegistrar->notifier field
>   block: update inserted/removed nodes from BlockRAMRegistrar

Makes sense to me.  For the series,

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
>  include/block/block_int-common.h            |  11 ++
>  include/system/block-backend-global-state.h |   9 ++
>  include/system/block-ram-registrar.h        |   4 +-
>  block.c                                     |  56 +++++--
>  block/block-backend.c                       |  44 ++++++
>  block/block-ram-registrar.c                 |  75 ++++++++-
>  tests/unit/test-block-backend.c             | 164 ++++++++++++++++++++
>  7 files changed, 344 insertions(+), 19 deletions(-)
> 
> -- 
> 2.51.0
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 0/4] block: update inserted/removed nodes from BlockRAMRegistrar

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]

On Tue, Sep 30, 2025 at 04:45:51PM -0400, Stefan Hajnoczi wrote:
> This patch series fixes a bug in BlockRAMRegistrar: it currently doesn't react
> to block graph changes and newly inserted nodes lack RAMBlock information
> needed to map I/O buffers. This is important for vdpa-blk devices because they
> rely on the ability to map I/O buffers.
> 
> Stefan Hajnoczi (4):
>   block: add BdrvChildClass->propagate_attach/detach() callbacks
>   block: add blk_add_attach/detach_notifier() APIs
>   block: rename RAMBlockRegistrar->notifier field
>   block: update inserted/removed nodes from BlockRAMRegistrar
> 
>  include/block/block_int-common.h            |  11 ++
>  include/system/block-backend-global-state.h |   9 ++
>  include/system/block-ram-registrar.h        |   4 +-
>  block.c                                     |  56 +++++--
>  block/block-backend.c                       |  44 ++++++
>  block/block-ram-registrar.c                 |  75 ++++++++-
>  tests/unit/test-block-backend.c             | 164 ++++++++++++++++++++
>  7 files changed, 344 insertions(+), 19 deletions(-)

I need to send a v2 because I forgot to squash in a compiler warning
fix.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]