On Tue, Dec 02, 2025 at 10:55:04AM -0300, Fabiano Rosas wrote: > Peter Xu <[email protected]> writes: > > > Based-on: <[email protected]> > > > > This series is based on Markus's recent fix: > > > > [PATCH] migration: Fix double-free on error path > > https://lore.kernel.org/r/[email protected] > > > > 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/[email protected] > > Thank you Markus for this. It's very helpful to have someone keeping us > in check regarding the usage of generic QEMU interfaces. Migration code > tends to drift incredibly.. > > > > > 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. > > > > I think with this series we could now work to reduce the complexity of > migration_connect(): > > The outgoing code in socket.c and tls.c could call > migration_connect_error_propagate directly so migration_channel_connect > only needs to check migrate_has_error() and then exit as early as > possible. From migration_connect onwards we can assume connection > success. > > What do you think?
As long as you read commit 688a3dcba980bf and will manage all those, it sounds like a good thing to try. > > tangent: > (is it too much bikeshedding if I send a patch doing s/migrat*_/mig_/ > all over the place? it's so annoying having to check the code to get the > prefix correct when writing emails) >From downstream POV, it'll be a slight burden whenever we need to backport later patches to "the world before the rename". It's not a huge deal but we should consider that. I'd confess it's likely the best time to do this if you want it for upstream POV - we don't have a lot concurrent projects ongoing, so if this lands it can be in the 1st pull for 11.0. If you have some "vibe coding" tools, maybe you can spend 2 mins to see how it looks like and decide whether to send a patch. I would say don't spend too much time on this (while you're still keep rebasing the options series! :) -- Peter Xu
