Re: [PATCH for-11.0 v2 0/7] migration: Error reporting cleanups

[Peter Xu <peterx@redhat.com>]

On Mon, Dec 01, 2025 at 02:45:03PM -0500, Peter Xu wrote:
> Based-on: <20251125070554.2256181-1-armbru@redhat.com>
> 
> This series is based on Markus's recent fix:
> 
> [PATCH] migration: Fix double-free on error path
> https://lore.kernel.org/r/20251125070554.2256181-1-armbru@redhat.com
> 
> v2:
> - Added R-bs
> - Patch 1:
>   - update commit message on s/accidentally merged/merged without proper
>     review/ [Markus]
> - Patch 2:
>   - Added a new follow up patch here from Markus to poison Error's autoptr
> - Patch 3:
>   - Rename migration_connect_set_error to migration_connect_error_propagate
>     [Markus]
>   - Add comments in commit log for both migrate_connect() and the rename
>     [Markus]
> - Patch 4:
>   - Rename multifd_send_set_error to multifd_send_error_propagate [Markus]
> - Patch 6:
>   - Make migrate_error_propagate() take MigrationState* as before [Markus]
>   - Remove the one use case of g_clear_pointer() [Markus]
>   - Touch up commit message for the change
> 
> This series should address the issues discussed in this thread here:
> 
> https://lore.kernel.org/r/871plmk1bc.fsf@pond.sub.org
> 
> The problem is Error is not a good candidate of g_autoptr, however the
> cleanup function was merged without enough review.  Luckily, we only have
> two users so far (after Markus's patch above lands).  This series removes
> the last two in migration code and reverts the auto cleanup function for
> Error.  Instead, poison the auto cleanup function.
> 
> When at it, it'll also change migrate_set_error() to start taking ownership
> of errors, just like what most error APIs do.  When at it, it is renamed to
> migrate_error_propagate() to imply migration version of error_propagate().
> 
> Comments welcomed, thanks.
> 
> Markus Armbruster (1):
>   error: Poison g_autoptr(Error) to prevent its use
> 
> Peter Xu (6):
>   migration: Use explicit error_free() instead of g_autoptr
>   Revert "error: define g_autoptr() cleanup function for the Error type"
>   migration: Make migration_connect_set_error() own the error
>   migration: Make multifd_send_set_error() own the error
>   migration: Make multifd_recv_terminate_threads() own the error
>   migration: Replace migrate_set_error() with migrate_error_propagate()
> 
>  include/qapi/error.h             | 20 ++++++++++++-
>  migration/migration.h            |  2 +-
>  migration/channel.c              |  1 -
>  migration/cpr-exec.c             |  5 ++--
>  migration/migration.c            | 51 +++++++++++++++-----------------
>  migration/multifd-device-state.c |  6 ++--
>  migration/multifd.c              | 30 +++++++++----------
>  migration/postcopy-ram.c         |  5 ++--
>  migration/ram.c                  |  4 +--
>  migration/savevm.c               | 17 +++++------
>  10 files changed, 73 insertions(+), 68 deletions(-)

Thanks for the reviews, I queued all 8 patches for 11.0 (with small
tweaks per discussion).

-- 
Peter Xu