Re: [PATCH 03/14] memory: Add Error** argument to .log_global*() handlers

[Philippe Mathieu-Daudé]

Hi Cédric,

On 7/2/24 14:33, Cédric Le Goater wrote:

Modify memory_global_dirty_log_start() and memory_global_dirty_log_stop()
to also take an Error** parameter and report the error in the callers.
Aside from error reporting, there should be no functional changes.

Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Anthony Perard <anthony.per...@citrix.com>
Cc: Paul Durrant <p...@xen.org>
Cc: Michael S. Tsirkin <m...@redhat.com>
Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: David Hildenbrand <da...@redhat.com>
Signed-off-by: Cédric Le Goater <c...@redhat.com>
---
  include/exec/memory.h | 12 ++++++++----
  hw/i386/xen/xen-hvm.c |  8 ++++----
  hw/vfio/common.c      |  6 ++++--
  hw/virtio/vhost.c     |  4 ++--
  migration/dirtyrate.c | 24 ++++++++++++++++++++----
  migration/ram.c       | 27 +++++++++++++++++++++++----
  system/memory.c       | 37 +++++++++++++++++++++++++------------
  7 files changed, 86 insertions(+), 32 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 
177be23db709d8bab9cebfe6acbae57611073327..b348070dc8f17b3505196d3a92d8cfb2171b640f
 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -998,8 +998,9 @@ struct MemoryListener {
       * active at that time.
       *
       * @listener: The #MemoryListener.
+     * @errp: pointer to Error*, to store an error if it happens.
       */
-    void (*log_global_start)(MemoryListener *listener);
+    void (*log_global_start)(MemoryListener *listener, Error **errp);

As documented in error.h, functions taking an Error** handle
as last argument should return a boolean indicating failure...

(multiple occurrences)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 
1d2e85746fb7b10eb7f149976970f9a92125af8a..443acab7a7efbd6e9c94883363e1a827a3538292
 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -90,13 +90,19 @@ static int64_t do_calculate_dirtyrate(DirtyPageRecord 
dirty_pages,

void global_dirty_log_change(unsigned int flag, bool start)

  {
+    Error *local_err = NULL;
+
      bql_lock();
      if (start) {
-        memory_global_dirty_log_start(flag);
+        memory_global_dirty_log_start(flag, &local_err);
      } else {
-        memory_global_dirty_log_stop(flag);
+        memory_global_dirty_log_stop(flag, &local_err);
      }
      bql_unlock();
+
+    if (local_err) {

... that way we don't have to check the pointer.

+        error_report_err(local_err);
+    }
  }