[PULL 0/3] Migration patches for 2024-08-02

[PULL 0/3] Migration patches for 2024-08-02

[Fabiano Rosas]

The following changes since commit c4d242501a61093a8b80ee8f6dd071c5110a100c:

  Merge tag 'net-pull-request' of https://github.com/jasowang/qemu into staging (2024-08-02 15:53:54 +1000)

are available in the Git repository at:

  https://gitlab.com/farosas/qemu.git tags/migration-20240802-pull-request

for you to fetch changes up to 0bd5b9284fa94a6242a0d27a46380d93e753488b:

  migration/multifd: Fix multifd_send_setup cleanup when channel creation fails (2024-08-02 09:47:40 -0300)

----------------------------------------------------------------
Migration pull request

- Akihiko Odaki's fix for a memory leak on ppc migration
- Fabiano's fix for asserts during multifd error handling

----------------------------------------------------------------

Akihiko Odaki (1):
  migration: Free removed SaveStateEntry

Fabiano Rosas (2):
  migration: Fix cleanup of iochannel in file migration
  migration/multifd: Fix multifd_send_setup cleanup when channel
    creation fails

 migration/file.c    |  2 --
 migration/multifd.c | 26 +++++++++++++++-----------
 migration/savevm.c  |  2 ++
 3 files changed, 17 insertions(+), 13 deletions(-)

-- 
2.35.3

[PULL 1/3] migration: Free removed SaveStateEntry

[Fabiano Rosas]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

This fixes LeakSanitizer warnings.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/savevm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index deb57833f8..85958d7b09 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -874,6 +874,8 @@ int vmstate_replace_hack_for_ppc(VMStateIf *obj, int instance_id,
 
     if (se) {
         savevm_state_handler_remove(se);
+        g_free(se->compat);
+        g_free(se);
     }
     return vmstate_register(obj, instance_id, vmsd, opaque);
 }
-- 
2.35.3

[PULL 2/3] migration: Fix cleanup of iochannel in file migration

[Fabiano Rosas]

The QIOChannelFile object already has its reference decremented by
g_autoptr. Trying to unref an extra time causes:

ERROR:../qom/object.c:1241:object_unref: assertion failed: (obj->ref > 0)

Fixes: a701c03dec ("migration: Drop reference to QIOChannel if file seeking fails")
Fixes: 6d3279655a ("migration: Fix file migration with fdset")
Reported-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/file.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/migration/file.c b/migration/file.c
index db870f2cf0..6451a21c86 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -112,7 +112,6 @@ void file_start_outgoing_migration(MigrationState *s,
         error_setg_errno(errp, errno,
                          "failed to truncate migration file to offset %" PRIx64,
                          offset);
-        object_unref(OBJECT(fioc));
         return;
     }
 
@@ -120,7 +119,6 @@ void file_start_outgoing_migration(MigrationState *s,
 
     ioc = QIO_CHANNEL(fioc);
     if (offset && qio_channel_io_seek(ioc, offset, SEEK_SET, errp) < 0) {
-        object_unref(OBJECT(fioc));
         return;
     }
     qio_channel_set_name(ioc, "migration-file-outgoing");
-- 
2.35.3

[PULL 3/3] migration/multifd: Fix multifd_send_setup cleanup when channel creation fails

[Fabiano Rosas]

When a channel fails to create, the code currently just returns. This
is wrong for two reasons:

1) Channel n+1 will not get to initialize it's semaphores, leading to
   an assert when terminate_threads tries to post to it:

 qemu-system-x86_64: ../util/qemu-thread-posix.c:92:
 qemu_mutex_lock_impl: Assertion `mutex->initialized' failed.

2) (theoretical) If channel n-1 already started creation it will
   defeat the purpose of the channels_created logic which is in place
   to avoid migrate_fd_cleanup() to run while channels are still being
   created.

   This cannot really happen today because the current failure cases
   for multifd_new_send_channel_create() are all synchronous,
   resulting from qio_channel_file_new_path() getting a bad
   filename. This would hit all channels equally.

   But I don't want to set a trap for future people, so have all
   channels try to create (even if failing), and only fail after the
   channels_created semaphore has been posted.

While here, remove the error_report_err call. There's one already at
migrate_fd_cleanup later on.

Cc: qemu-stable@nongnu.org
Reported-by: Jim Fehlig <jfehlig@suse.com>
Fixes: b7b03eb614 ("migration/multifd: Add outgoing QIOChannelFile support")
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/multifd.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 0b4cbaddfe..552f9723c8 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -1156,7 +1156,6 @@ static bool multifd_new_send_channel_create(gpointer opaque, Error **errp)
 bool multifd_send_setup(void)
 {
     MigrationState *s = migrate_get_current();
-    Error *local_err = NULL;
     int thread_count, ret = 0;
     uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
     bool use_packets = multifd_use_packets();
@@ -1177,6 +1176,7 @@ bool multifd_send_setup(void)
 
     for (i = 0; i < thread_count; i++) {
         MultiFDSendParams *p = &multifd_send_state->params[i];
+        Error *local_err = NULL;
 
         qemu_sem_init(&p->sem, 0);
         qemu_sem_init(&p->sem_sync, 0);
@@ -1196,7 +1196,8 @@ bool multifd_send_setup(void)
         p->write_flags = 0;
 
         if (!multifd_new_send_channel_create(p, &local_err)) {
-            return false;
+            migrate_set_error(s, local_err);
+            ret = -1;
         }
     }
 
@@ -1209,24 +1210,27 @@ bool multifd_send_setup(void)
         qemu_sem_wait(&multifd_send_state->channels_created);
     }
 
+    if (ret) {
+        goto err;
+    }
+
     for (i = 0; i < thread_count; i++) {
         MultiFDSendParams *p = &multifd_send_state->params[i];
+        Error *local_err = NULL;
 
         ret = multifd_send_state->ops->send_setup(p, &local_err);
         if (ret) {
-            break;
+            migrate_set_error(s, local_err);
+            goto err;
         }
     }
 
-    if (ret) {
-        migrate_set_error(s, local_err);
-        error_report_err(local_err);
-        migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
-                          MIGRATION_STATUS_FAILED);
-        return false;
-    }
-
     return true;
+
+err:
+    migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
+                      MIGRATION_STATUS_FAILED);
+    return false;
 }
 
 bool multifd_recv(void)
-- 
2.35.3

Re: [PULL 0/3] Migration patches for 2024-08-02

[Richard Henderson]

On 8/3/24 00:26, Fabiano Rosas wrote:
> The following changes since commit c4d242501a61093a8b80ee8f6dd071c5110a100c:
> 
>    Merge tag 'net-pull-request' ofhttps://github.com/jasowang/qemu into staging (2024-08-02 15:53:54 +1000)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/farosas/qemu.git tags/migration-20240802-pull-request
> 
> for you to fetch changes up to 0bd5b9284fa94a6242a0d27a46380d93e753488b:
> 
>    migration/multifd: Fix multifd_send_setup cleanup when channel creation fails (2024-08-02 09:47:40 -0300)
> 
> ----------------------------------------------------------------
> Migration pull request
> 
> - Akihiko Odaki's fix for a memory leak on ppc migration
> - Fabiano's fix for asserts during multifd error handling

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/9.1 as appropriate.

r~