On Fri, 9 Jan 2026 at 18:19, Fabiano Rosas <farosas@suse.de> wrote:
> When setting a callback on a Glib source and giving it a data pointer,
> it's natural to also provide the destructor for the data in question.
>
> Since migrate_hup_add() already needs to clone the MigrationAddress
> when setting the qmp_migrate_finish_cb callback, also pass the
> qapi_free_MigrationAddress as the GDestroyNotify callback.
>
> With this the address doesn't need to be freed at the callback body,
> making the management of that memory slight simpler.
* slight -> slightly OR just skip it. -> ... memory simpler.
> (also fix the indentation of migrate_hup_add)
* This note could be purged.
> Cc: Mark Kanda <mark.kanda@oracle.com>
> Cc: Ben Chaney <bchaney@akamai.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> migration/migration.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 52c1bb5da2..5167233f76 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2007,9 +2007,11 @@ static void qmp_migrate_finish(MigrationAddress *addr, Error **errp);
> static void migrate_hup_add(MigrationState *s, QIOChannel *ioc, GSourceFunc cb,
> void *opaque)
> {
> - s->hup_source = qio_channel_create_watch(ioc, G_IO_HUP);
> - g_source_set_callback(s->hup_source, cb, opaque, NULL);
> - g_source_attach(s->hup_source, NULL);
> + s->hup_source = qio_channel_create_watch(ioc, G_IO_HUP);
> + g_source_set_callback(s->hup_source, cb,
> + QAPI_CLONE(MigrationAddress, opaque),
> + (GDestroyNotify)qapi_free_MigrationAddress);
> + g_source_attach(s->hup_source, NULL);
> }
>
> static void migrate_hup_delete(MigrationState *s)
> @@ -2028,7 +2030,6 @@ static gboolean qmp_migrate_finish_cb(QIOChannel *channel,
> MigrationAddress *addr = opaque;
>
> qmp_migrate_finish(addr, NULL);
> - qapi_free_MigrationAddress(addr);
> return G_SOURCE_REMOVE;
> }
>
> @@ -2083,7 +2084,7 @@ void qmp_migrate(const char *uri, bool has_channels,
> */
> if (migrate_mode() == MIG_MODE_CPR_TRANSFER) {
> migrate_hup_add(s, cpr_state_ioc(), (GSourceFunc)qmp_migrate_finish_cb,
> - QAPI_CLONE(MigrationAddress, main_ch->addr));
> + main_ch->addr);
>
> } else {
> qmp_migrate_finish(main_ch->addr, errp);
> --
* Looks okay.
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Thank you.
---
- Prasad