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? 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)
