diff mbox series

[v4,5/5] migration: Change ram_save_queue_pages() retval to bool

Message ID 20231017202633.296756-6-peterx@redhat.com
State New
Headers show
Series migration: Better error handling in rp thread, allow failures in recover | expand

Commit Message

Peter Xu Oct. 17, 2023, 8:26 p.m. UTC 
After we have errp which contains the more detailed error message, make
ram_save_queue_pages() returns bool in its stack.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/ram.h       |  4 ++--
 migration/migration.c | 16 ++++++++--------
 migration/ram.c       | 18 +++++++++---------
 3 files changed, 19 insertions(+), 19 deletions(-)

Comments

Fabiano Rosas Oct. 19, 2023, 9:13 p.m. UTC | #1 
Peter Xu <peterx@redhat.com> writes:

> After we have errp which contains the more detailed error message, make
> ram_save_queue_pages() returns bool in its stack.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Juan Quintela Oct. 31, 2023, 2:34 p.m. UTC | #2 
Peter Xu <peterx@redhat.com> wrote:
> After we have errp which contains the more detailed error message, make
> ram_save_queue_pages() returns bool in its stack.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/ram.h       |  4 ++--
>  migration/migration.c | 16 ++++++++--------
>  migration/ram.c       | 18 +++++++++---------
>  3 files changed, 19 insertions(+), 19 deletions(-)
>

>  static bool migrate_handle_rp_recv_bitmap(MigrationState *s, char *block_name,
> @@ -1990,8 +1992,7 @@ static void *source_return_path_thread(void *opaque)
>          case MIG_RP_MSG_REQ_PAGES:
>              start = ldq_be_p(buf);
>              len = ldl_be_p(buf + 8);
> -            migrate_handle_rp_req_pages(ms, NULL, start, len, &err);
> -            if (err) {
> +            if (!migrate_handle_rp_req_pages(ms, NULL, start, len, &err)) {

I thought we were headed the other direction.
Make functions return void and just check if the error is set.

I think just change the type form returning int to return void will make
the trick.

Later, Juan.
diff mbox series

Patch

diff --git a/migration/ram.h b/migration/ram.h
index af0290f8ab..e22a6b0d1c 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -51,8 +51,8 @@  uint64_t ram_bytes_total(void);
 void mig_throttle_counter_reset(void);
 
 uint64_t ram_pagesize_summary(void);
-int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len,
-                         Error **errp);
+bool ram_save_queue_pages(const char *rbname, ram_addr_t start,
+                          ram_addr_t len, Error **errp);
 void ram_postcopy_migrated_memory_release(MigrationState *ms);
 /* For outgoing discard bitmap */
 void ram_postcopy_send_discard_bitmap(MigrationState *ms);
diff --git a/migration/migration.c b/migration/migration.c
index dfb8b48dcb..50bf8422c7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1820,8 +1820,10 @@  static struct rp_cmd_args {
  * Process a request for pages received on the return path,
  * We're allowed to send more than requested (e.g. to round to our page size)
  * and we don't need to send pages that have already been sent.
+ *
+ * Returns true if succeed, false otherwise.
  */
-static void
+static bool
 migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname,
                             ram_addr_t start, size_t len, Error **errp)
 {
@@ -1837,10 +1839,10 @@  migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname,
         !QEMU_IS_ALIGNED(len, our_host_ps)) {
         error_setg(errp, "MIG_RP_MSG_REQ_PAGES: Misaligned page request, start:"
                    RAM_ADDR_FMT " len: %zd", start, len);
-        return;
+        return false;
     }
 
-    ram_save_queue_pages(rbname, start, len, errp);
+    return ram_save_queue_pages(rbname, start, len, errp);
 }
 
 static bool migrate_handle_rp_recv_bitmap(MigrationState *s, char *block_name,
@@ -1990,8 +1992,7 @@  static void *source_return_path_thread(void *opaque)
         case MIG_RP_MSG_REQ_PAGES:
             start = ldq_be_p(buf);
             len = ldl_be_p(buf + 8);
-            migrate_handle_rp_req_pages(ms, NULL, start, len, &err);
-            if (err) {
+            if (!migrate_handle_rp_req_pages(ms, NULL, start, len, &err)) {
                 goto out;
             }
             break;
@@ -2012,9 +2013,8 @@  static void *source_return_path_thread(void *opaque)
                            header_len, expected_len);
                 goto out;
             }
-            migrate_handle_rp_req_pages(ms, (char *)&buf[13], start, len,
-                                        &err);
-            if (err) {
+            if (!migrate_handle_rp_req_pages(ms, (char *)&buf[13], start, len,
+                                             &err)) {
                 goto out;
             }
             break;
diff --git a/migration/ram.c b/migration/ram.c
index ca77444e18..aca9ae5846 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1948,15 +1948,15 @@  static void migration_page_queue_free(RAMState *rs)
  *
  * A request from postcopy destination for example.
  *
- * Returns zero on success or negative on error
+ * Returns true on success or false on error (detailed error put in @errp)
  *
  * @rbname: Name of the RAMBLock of the request. NULL means the
  *          same that last one.
  * @start: starting address from the start of the RAMBlock
  * @len: length (in bytes) to send
  */
-int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len,
-                         Error **errp)
+bool ram_save_queue_pages(const char *rbname, ram_addr_t start,
+                          ram_addr_t len, Error **errp)
 {
     RAMBlock *ramblock;
     RAMState *rs = ram_state;
@@ -1974,7 +1974,7 @@  int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len,
              * it's the 1st request.
              */
             error_setg(errp, "MIG_RP_MSG_REQ_PAGES has no previous block");
-            return -1;
+            return false;
         }
     } else {
         ramblock = qemu_ram_block_by_name(rbname);
@@ -1982,7 +1982,7 @@  int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len,
         if (!ramblock) {
             /* We shouldn't be asked for a non-existent RAMBlock */
             error_setg(errp, "MIG_RP_MSG_REQ_PAGES has no block '%s'", rbname);
-            return -1;
+            return false;
         }
         rs->last_req_rb = ramblock;
     }
@@ -1992,7 +1992,7 @@  int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len,
                    "start=" RAM_ADDR_FMT " len="
                    RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
                    start, len, ramblock->used_length);
-        return -1;
+        return false;
     }
 
     /*
@@ -2003,7 +2003,7 @@  int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len,
         ram_addr_t page_start = start >> TARGET_PAGE_BITS;
         size_t page_size = qemu_ram_pagesize(ramblock);
         PageSearchStatus *pss = &ram_state->pss[RAM_CHANNEL_POSTCOPY];
-        int ret = 0;
+        bool ret = true;
 
         qemu_mutex_lock(&rs->bitmap_mutex);
 
@@ -2026,7 +2026,7 @@  int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len,
                 error_setg(errp, "ram_save_host_page_urgent() failed: "
                            "ramblock=%s, start_addr=0x"RAM_ADDR_FMT,
                            ramblock->idstr, start);
-                ret = -1;
+                ret = false;
                 break;
             }
             /*
@@ -2057,7 +2057,7 @@  int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len,
     migration_make_urgent_request();
     qemu_mutex_unlock(&rs->src_page_req_mutex);
 
-    return 0;
+    return true;
 }
 
 static bool save_page_use_compression(RAMState *rs)