From: "Cédric Le Goater" <clg@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Peter Xu" <peterx@redhat.com>, "Fabiano Rosas" <farosas@suse.de>,
"Alex Williamson" <alex.williamson@redhat.com>,
"Avihai Horon" <avihaih@nvidia.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"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: [PATCH v2 08/21] memory: Add Error** argument to .log_global*() handlers
Date: Tue, 27 Feb 2024 19:03:32 +0100 [thread overview]
Message-ID: <20240227180345.548960-9-clg@redhat.com> (raw)
In-Reply-To: <20240227180345.548960-1-clg@redhat.com>
Modify all log_global*() handlers to take an Error** parameter and
return a bool. A new MEMORY_LISTENER_CALL_LOG_GLOBAL macro looping on
the listeners is introduced to handle a possible error, which will
would interrupt the loop if necessary.
To be noted a change in memory_global_dirty_log_start() behavior as it
will return as soon as an error is detected.
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>
---
include/exec/memory.h | 15 ++++++--
hw/i386/xen/xen-hvm.c | 6 ++--
hw/vfio/common.c | 8 +++--
hw/virtio/vhost.c | 6 ++--
system/memory.c | 83 +++++++++++++++++++++++++++++++++++++------
system/physmem.c | 5 +--
6 files changed, 101 insertions(+), 22 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8626a355b310ed7b1a1db7978ba4b394032c2f15..4bc146c5ebdd377cd14a4e462f32cc945db5a0a8 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:
@@ -1009,8 +1012,11 @@ struct MemoryListener {
* the address space.
*
* @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_stop)(MemoryListener *listener);
+ bool (*log_global_stop)(MemoryListener *listener, Error **errp);
/**
* @log_global_after_sync:
@@ -1019,8 +1025,11 @@ struct MemoryListener {
* for any #MemoryRegionSection.
*
* @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_after_sync)(MemoryListener *listener);
+ bool (*log_global_after_sync)(MemoryListener *listener, Error **errp);
/**
* @eventfd_add:
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f42621e6742552035122ea58092c91c3458338ff..925a207b494b4eed52d5f360b554f18ac8a9806d 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -446,16 +446,18 @@ 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)
+static bool xen_log_global_stop(MemoryListener *listener, Error **errp)
{
xen_in_migration = false;
+ return true;
}
static const MemoryListener xen_memory_listener = {
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 059bfdc07a85e2eb908df828c1f42104d683e911..8bba95ba6a2010b78cae54c6905857686bbb6309 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1075,7 +1075,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);
@@ -1092,9 +1093,11 @@ 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)
+static bool vfio_listener_log_global_stop(MemoryListener *listener,
+ Error **errp)
{
VFIOContainerBase *bcontainer = container_of(listener, VFIOContainerBase,
listener);
@@ -1111,6 +1114,7 @@ static void vfio_listener_log_global_stop(MemoryListener *listener)
ret, strerror(-ret));
vfio_set_migration_error(ret);
}
+ return !!ret;
}
static int vfio_device_dma_logging_report(VFIODevice *vbasedev, hwaddr iova,
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 2c9ac794680ea9b65eba6cc22e70cf141e90aa73..7a555f941934991a72a2817e5505fe0ce6d6fc64 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,9 +1052,10 @@ static void vhost_log_global_start(MemoryListener *listener)
if (r < 0) {
abort();
}
+ return true;
}
-static void vhost_log_global_stop(MemoryListener *listener)
+static bool vhost_log_global_stop(MemoryListener *listener, Error **errp)
{
int r;
@@ -1062,6 +1063,7 @@ static void vhost_log_global_stop(MemoryListener *listener)
if (r < 0) {
abort();
}
+ return true;
}
static void vhost_log_start(MemoryListener *listener,
diff --git a/system/memory.c b/system/memory.c
index a229a79988fce2aa3cb77e3a130db4c694e8cd49..af06157ead5b1272548e87f79ab9fb3036055328 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -127,6 +127,35 @@ enum ListenerDirection { Forward, Reverse };
} \
} while (0)
+#define MEMORY_LISTENER_CALL_LOG_GLOBAL(_callback, _direction, _errp, \
+ _args...) \
+ do { \
+ MemoryListener *_listener; \
+ \
+ switch (_direction) { \
+ case Forward: \
+ QTAILQ_FOREACH(_listener, &memory_listeners, link) { \
+ if (_listener->_callback) { \
+ if (!_listener->_callback(_listener, _errp, ##_args)) { \
+ break; \
+ } \
+ } \
+ } \
+ break; \
+ case Reverse: \
+ QTAILQ_FOREACH_REVERSE(_listener, &memory_listeners, link) { \
+ if (_listener->_callback) { \
+ if (!_listener->_callback(_listener, _errp, ##_args)) { \
+ break; \
+ } \
+ } \
+ } \
+ break; \
+ default: \
+ abort(); \
+ }; \
+ } while (0)
+
#define MEMORY_LISTENER_CALL(_as, _callback, _direction, _section, _args...) \
do { \
MemoryListener *_listener; \
@@ -2903,7 +2932,13 @@ void memory_global_dirty_log_sync(bool last_stage)
void memory_global_after_dirty_log_sync(void)
{
- MEMORY_LISTENER_CALL_GLOBAL(log_global_after_sync, Forward);
+ Error *local_err = NULL;
+
+ MEMORY_LISTENER_CALL_LOG_GLOBAL(log_global_after_sync, Forward,
+ &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ }
}
/*
@@ -2912,18 +2947,22 @@ void memory_global_after_dirty_log_sync(void)
*/
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_stop_postponed_run(Error **errp);
void memory_global_dirty_log_start(unsigned int flags)
{
unsigned int old_flags;
+ Error *local_err = NULL;
assert(flags && !(flags & (~GLOBAL_DIRTY_MASK)));
if (vmstate_change) {
/* If there is postponed stop(), operate on it first */
postponed_stop_flags &= ~flags;
- memory_global_dirty_log_stop_postponed_run();
+ if (!memory_global_dirty_log_stop_postponed_run(&local_err)) {
+ error_report_err(local_err);
+ return;
+ }
}
flags &= ~global_dirty_tracking;
@@ -2936,15 +2975,22 @@ 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);
+ MEMORY_LISTENER_CALL_LOG_GLOBAL(log_global_start, Forward,
+ &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ return;
+ }
memory_region_transaction_begin();
memory_region_update_pending = true;
memory_region_transaction_commit();
}
}
-static void memory_global_dirty_log_do_stop(unsigned int flags)
+static bool memory_global_dirty_log_do_stop(unsigned int flags, Error **errp)
{
+ ERRP_GUARD();
+
assert(flags && !(flags & (~GLOBAL_DIRTY_MASK)));
assert((global_dirty_tracking & flags) == flags);
global_dirty_tracking &= ~flags;
@@ -2955,39 +3001,49 @@ static void memory_global_dirty_log_do_stop(unsigned int flags)
memory_region_transaction_begin();
memory_region_update_pending = true;
memory_region_transaction_commit();
- MEMORY_LISTENER_CALL_GLOBAL(log_global_stop, Reverse);
+ MEMORY_LISTENER_CALL_LOG_GLOBAL(log_global_stop, Reverse, errp);
}
+ return !*errp;
}
/*
* Execute the postponed dirty log stop operations if there is, then reset
* everything (including the flags and the vmstate change hook).
*/
-static void memory_global_dirty_log_stop_postponed_run(void)
+static bool memory_global_dirty_log_stop_postponed_run(Error **errp)
{
+ bool ret = true;
+
/* This must be called with the vmstate handler registered */
assert(vmstate_change);
/* Note: postponed_stop_flags can be cleared in log start routine */
if (postponed_stop_flags) {
- memory_global_dirty_log_do_stop(postponed_stop_flags);
+ ret = memory_global_dirty_log_do_stop(postponed_stop_flags, errp);
postponed_stop_flags = 0;
}
qemu_del_vm_change_state_handler(vmstate_change);
vmstate_change = NULL;
+ return ret;
}
static void memory_vm_change_state_handler(void *opaque, bool running,
RunState state)
{
+ Error *local_err = NULL;
+
if (running) {
- memory_global_dirty_log_stop_postponed_run();
+ if (!memory_global_dirty_log_stop_postponed_run(&local_err)) {
+ error_report_err(local_err);
+ }
}
}
void memory_global_dirty_log_stop(unsigned int flags)
{
+ Error *local_err = NULL;
+
if (!runstate_is_running()) {
/* Postpone the dirty log stop, e.g., to when VM starts again */
if (vmstate_change) {
@@ -3001,7 +3057,9 @@ void memory_global_dirty_log_stop(unsigned int flags)
return;
}
- memory_global_dirty_log_do_stop(flags);
+ if (!memory_global_dirty_log_do_stop(flags, &local_err)) {
+ error_report_err(local_err);
+ }
}
static void listener_add_address_space(MemoryListener *listener,
@@ -3009,13 +3067,16 @@ static void listener_add_address_space(MemoryListener *listener,
{
FlatView *view;
FlatRange *fr;
+ Error *local_err = NULL;
if (listener->begin) {
listener->begin(listener);
}
if (global_dirty_tracking) {
if (listener->log_global_start) {
- listener->log_global_start(listener);
+ if (!listener->log_global_start(listener, &local_err)) {
+ error_report_err(local_err);
+ }
}
}
diff --git a/system/physmem.c b/system/physmem.c
index e3ebc19eefd8050a1dee16e3d1449f0c144f751f..9adbf9aea847cd80bdac6dca466fb476844ac048 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -148,7 +148,7 @@ typedef struct subpage_t {
static void io_mem_init(void);
static void memory_map_init(void);
-static void tcg_log_global_after_sync(MemoryListener *listener);
+static bool tcg_log_global_after_sync(MemoryListener *listener, Error **errp);
static void tcg_commit(MemoryListener *listener);
/**
@@ -2475,7 +2475,7 @@ static void do_nothing(CPUState *cpu, run_on_cpu_data d)
{
}
-static void tcg_log_global_after_sync(MemoryListener *listener)
+static bool tcg_log_global_after_sync(MemoryListener *listener, Error **errp)
{
CPUAddressSpace *cpuas;
@@ -2507,6 +2507,7 @@ static void tcg_log_global_after_sync(MemoryListener *listener)
cpuas = container_of(listener, CPUAddressSpace, tcg_as_listener);
run_on_cpu(cpuas->cpu, do_nothing, RUN_ON_CPU_NULL);
}
+ return true;
}
static void tcg_commit_cpu(CPUState *cpu, run_on_cpu_data data)
--
2.43.2
next prev parent reply other threads:[~2024-02-27 20:48 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-27 18:03 [PATCH v2 00/21] migration: Improve error reporting Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 01/21] migration: Report error when shutdown fails Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 02/21] migration: Remove SaveStateHandler and LoadStateHandler typedefs Cédric Le Goater
2024-02-29 1:21 ` Fabiano Rosas
2024-02-29 3:55 ` Peter Xu
2024-02-27 18:03 ` [PATCH v2 03/21] migration: Add documentation for SaveVMHandlers Cédric Le Goater
2024-02-29 4:10 ` Peter Xu
2024-02-29 13:19 ` Cédric Le Goater
2024-03-04 9:05 ` Avihai Horon
2024-03-04 10:29 ` Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 04/21] migration: Do not call PRECOPY_NOTIFY_SETUP notifiers in case of error Cédric Le Goater
2024-02-29 4:12 ` Peter Xu
2024-02-27 18:03 ` [PATCH v2 05/21] migration: Add Error** argument to qemu_savevm_state_setup() Cédric Le Goater
2024-02-29 4:19 ` Peter Xu
2024-02-27 18:03 ` [PATCH v2 06/21] migration: Add Error** argument to .save_setup() handler Cédric Le Goater
2024-02-29 6:32 ` Markus Armbruster
2024-02-29 7:20 ` Vladimir Sementsov-Ogievskiy
2024-02-29 10:35 ` Thomas Huth
2024-02-29 13:21 ` Markus Armbruster
2024-03-01 14:44 ` Vladimir Sementsov-Ogievskiy
2024-03-01 14:52 ` Vladimir Sementsov-Ogievskiy
2024-02-29 13:34 ` Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 07/21] migration: Add Error** argument to .load_setup() handler Cédric Le Goater
2024-02-29 4:21 ` Peter Xu
2024-02-27 18:03 ` Cédric Le Goater [this message]
2024-02-29 5:29 ` [PATCH v2 08/21] memory: Add Error** argument to .log_global*() handlers Peter Xu
2024-03-04 13:38 ` Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 09/21] memory: Add Error** argument to the global_dirty_log routines Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 10/21] migration: Modify ram_init_bitmaps() to report dirty tracking errors Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 11/21] migration: Fix migration termination Cédric Le Goater
2024-02-29 5:34 ` Peter Xu
2024-02-29 15:13 ` Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 12/21] vfio: Add Error** argument to .set_dirty_page_tracking() handler Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 13/21] vfio: Add Error** argument to vfio_devices_dma_logging_start() Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 14/21] vfio: Add Error** argument to vfio_devices_dma_logging_stop() Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 15/21] vfio: Use new Error** argument in vfio_save_setup() Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 16/21] vfio: Add Error** argument to .vfio_save_config() handler Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 17/21] vfio: Reverse test on vfio_get_dirty_bitmap() Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 18/21] memory: Add Error** argument to memory_get_xlat_addr() Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 19/21] vfio: Add Error** argument to .get_dirty_bitmap() handler Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 20/21] vfio: Also trace event failures in vfio_save_complete_precopy() Cédric Le Goater
2024-02-27 18:03 ` [PATCH v2 21/21] vfio: Extend vfio_set_migration_error() with Error* argument Cédric Le Goater
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=20240227180345.548960-9-clg@redhat.com \
--to=clg@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=anthony.perard@citrix.com \
--cc=avihaih@nvidia.com \
--cc=david@redhat.com \
--cc=farosas@suse.de \
--cc=mst@redhat.com \
--cc=paul@xen.org \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=philmd@linaro.org \
--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).