qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: peterx@redhat.com, "Prasad Pandit" <ppandit@redhat.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Fabiano Rosas" <farosas@suse.de>,
	"Cédric Le Goater" <clg@redhat.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Anthony Perard" <anthony.perard@citrix.com>,
	"Paul Durrant" <paul@xen.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"David Hildenbrand" <david@redhat.com>
Subject: [PULL 18/26] memory: Add Error** argument to .log_global_start() handler
Date: Tue, 23 Apr 2024 18:38:05 -0400	[thread overview]
Message-ID: <20240423223813.3237060-19-peterx@redhat.com> (raw)
In-Reply-To: <20240423223813.3237060-1-peterx@redhat.com>

From: Cédric Le Goater <clg@redhat.com>

Modify all .log_global_start() handlers to take an Error** parameter
and return a bool. Adapt memory_global_dirty_log_start() to interrupt
on the first error the loop on handlers. In such case, a rollback is
performed to stop dirty logging on all listeners where it was
previously enabled.

Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Paul Durrant <paul@xen.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-10-clg@redhat.com
[peterx: modify & enrich the comment for listener_add_address_space() ]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/exec/memory.h |  5 ++++-
 hw/i386/xen/xen-hvm.c |  3 ++-
 hw/vfio/common.c      |  4 +++-
 hw/virtio/vhost.c     |  3 ++-
 system/memory.c       | 41 +++++++++++++++++++++++++++++++++++++++--
 5 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8626a355b3..5555567bc4 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -998,8 +998,11 @@ struct MemoryListener {
      * active at that time.
      *
      * @listener: The #MemoryListener.
+     * @errp: pointer to Error*, to store an error if it happens.
+     *
+     * Return: true on success, else false setting @errp with error.
      */
-    void (*log_global_start)(MemoryListener *listener);
+    bool (*log_global_start)(MemoryListener *listener, Error **errp);
 
     /**
      * @log_global_stop:
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 7745cb3963..f6e9a1bc86 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -457,11 +457,12 @@ static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
                           int128_get64(section->size));
 }
 
-static void xen_log_global_start(MemoryListener *listener)
+static bool xen_log_global_start(MemoryListener *listener, Error **errp)
 {
     if (xen_enabled()) {
         xen_in_migration = true;
     }
+    return true;
 }
 
 static void xen_log_global_stop(MemoryListener *listener)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 011ceaab89..8f9cbdc026 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1066,7 +1066,8 @@ out:
     return ret;
 }
 
-static void vfio_listener_log_global_start(MemoryListener *listener)
+static bool vfio_listener_log_global_start(MemoryListener *listener,
+                                           Error **errp)
 {
     VFIOContainerBase *bcontainer = container_of(listener, VFIOContainerBase,
                                                  listener);
@@ -1083,6 +1084,7 @@ static void vfio_listener_log_global_start(MemoryListener *listener)
                      ret, strerror(-ret));
         vfio_set_migration_error(ret);
     }
+    return !ret;
 }
 
 static void vfio_listener_log_global_stop(MemoryListener *listener)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f50180e60e..4acd77e890 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1044,7 +1044,7 @@ check_dev_state:
     return r;
 }
 
-static void vhost_log_global_start(MemoryListener *listener)
+static bool vhost_log_global_start(MemoryListener *listener, Error **errp)
 {
     int r;
 
@@ -1052,6 +1052,7 @@ static void vhost_log_global_start(MemoryListener *listener)
     if (r < 0) {
         abort();
     }
+    return true;
 }
 
 static void vhost_log_global_stop(MemoryListener *listener)
diff --git a/system/memory.c b/system/memory.c
index a229a79988..86d6c33180 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -2914,9 +2914,33 @@ static unsigned int postponed_stop_flags;
 static VMChangeStateEntry *vmstate_change;
 static void memory_global_dirty_log_stop_postponed_run(void);
 
+static bool memory_global_dirty_log_do_start(Error **errp)
+{
+    MemoryListener *listener;
+
+    QTAILQ_FOREACH(listener, &memory_listeners, link) {
+        if (listener->log_global_start) {
+            if (!listener->log_global_start(listener, errp)) {
+                goto err;
+            }
+        }
+    }
+    return true;
+
+err:
+    while ((listener = QTAILQ_PREV(listener, link)) != NULL) {
+        if (listener->log_global_stop) {
+            listener->log_global_stop(listener);
+        }
+    }
+
+    return false;
+}
+
 void memory_global_dirty_log_start(unsigned int flags)
 {
     unsigned int old_flags;
+    Error *local_err = NULL;
 
     assert(flags && !(flags & (~GLOBAL_DIRTY_MASK)));
 
@@ -2936,7 +2960,13 @@ void memory_global_dirty_log_start(unsigned int flags)
     trace_global_dirty_changed(global_dirty_tracking);
 
     if (!old_flags) {
-        MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
+        if (!memory_global_dirty_log_do_start(&local_err)) {
+            global_dirty_tracking &= ~flags;
+            trace_global_dirty_changed(global_dirty_tracking);
+            error_report_err(local_err);
+            return;
+        }
+
         memory_region_transaction_begin();
         memory_region_update_pending = true;
         memory_region_transaction_commit();
@@ -3014,8 +3044,15 @@ static void listener_add_address_space(MemoryListener *listener,
         listener->begin(listener);
     }
     if (global_dirty_tracking) {
+        /*
+         * Currently only VFIO can fail log_global_start(), and it's not
+         * yet allowed to hotplug any PCI device during migration. So this
+         * should never fail when invoked, guard it with error_abort.  If
+         * it can start to fail in the future, we need to be able to fail
+         * the whole listener_add_address_space() and its callers.
+         */
         if (listener->log_global_start) {
-            listener->log_global_start(listener);
+            listener->log_global_start(listener, &error_abort);
         }
     }
 
-- 
2.44.0



  parent reply	other threads:[~2024-04-23 22:41 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
2024-04-23 22:37 ` [PULL 01/26] tests/qtest/migration: Add 'to' object into migrate_qmp() Peter Xu
2024-04-23 22:37 ` [PULL 02/26] tests/qtest/migration: Replace connect_uri and move migrate_get_socket_address inside migrate_qmp Peter Xu
2024-04-23 22:37 ` [PULL 03/26] tests/qtest/migration: Replace migrate_get_connect_uri inplace of migrate_get_socket_address Peter Xu
2024-04-23 22:37 ` [PULL 04/26] tests/qtest/migration: Add channels parameter in migrate_qmp_fail Peter Xu
2024-04-23 22:37 ` [PULL 05/26] tests/qtest/migration: Add migrate_set_ports into migrate_qmp to update migration port value Peter Xu
2024-04-23 22:37 ` [PULL 06/26] tests/qtest/migration: Add channels parameter in migrate_qmp Peter Xu
2024-04-23 22:37 ` [PULL 07/26] tests/qtest/migration: Add multifd_tcp_plain test using list of channels instead of uri Peter Xu
2024-04-23 22:37 ` [PULL 08/26] tests/qtest/migration: Add negative tests to validate migration QAPIs Peter Xu
2024-04-23 22:37 ` [PULL 09/26] tests/qtest/migration: Fix typo for vsock in SocketAddress_to_str Peter Xu
2024-04-23 22:37 ` [PULL 10/26] s390/stattrib: Add Error** argument to set_migrationmode() handler Peter Xu
2024-04-23 22:37 ` [PULL 11/26] vfio: Always report an error in vfio_save_setup() Peter Xu
2024-04-23 22:37 ` [PULL 12/26] migration: Always report an error in block_save_setup() Peter Xu
2024-04-23 22:38 ` [PULL 13/26] migration: Always report an error in ram_save_setup() Peter Xu
2024-04-23 22:38 ` [PULL 14/26] migration: Add Error** argument to vmstate_save() Peter Xu
2024-04-23 22:38 ` [PULL 15/26] migration: Add Error** argument to qemu_savevm_state_setup() Peter Xu
2024-04-23 22:38 ` [PULL 16/26] migration: Add Error** argument to .save_setup() handler Peter Xu
2024-04-23 22:38 ` [PULL 17/26] migration: Add Error** argument to .load_setup() handler Peter Xu
2024-04-23 22:38 ` Peter Xu [this message]
2024-04-23 22:38 ` [PULL 19/26] migration: Introduce ram_bitmaps_destroy() Peter Xu
2024-04-23 22:38 ` [PULL 20/26] memory: Add Error** argument to the global_dirty_log routines Peter Xu
2024-04-23 22:38 ` [PULL 21/26] migration: Add Error** argument to ram_state_init() Peter Xu
2024-04-23 22:38 ` [PULL 22/26] migration: Add Error** argument to xbzrle_init() Peter Xu
2024-04-23 22:38 ` [PULL 23/26] migration: Modify ram_init_bitmaps() to report dirty tracking errors Peter Xu
2024-04-23 22:38 ` [PULL 24/26] migration: Add Error** argument to add_bitmaps_to_list() Peter Xu
2024-04-23 22:38 ` [PULL 25/26] migration/multifd: solve zero page causing multiple page faults Peter Xu
2024-04-23 22:38 ` [PULL 26/26] migration/colo: Fix bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed Peter Xu
2024-04-24  6:50 ` [PULL 00/26] Migration 20240423 patches Richard Henderson

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=20240423223813.3237060-19-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=anthony.perard@citrix.com \
    --cc=clg@redhat.com \
    --cc=david@redhat.com \
    --cc=farosas@suse.de \
    --cc=mst@redhat.com \
    --cc=paul@xen.org \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=ppandit@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sstabellini@kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).