From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Hanna Reitz <hreitz@redhat.com>
Cc: qemu-devel@nongnu.org, Juan Quintela <quintela@redhat.com>
Subject: Re: [PATCH] migration: NULL transport_data after freeing
Date: Thu, 17 Feb 2022 17:29:34 +0000 [thread overview]
Message-ID: <Yg6F/kMaUSzX95Gc@work-vm> (raw)
In-Reply-To: <20220217170407.24906-1-hreitz@redhat.com>
* Hanna Reitz (hreitz@redhat.com) wrote:
> migration_incoming_state_destroy() NULLs all objects it frees after they
> are freed, presumably so that a subsequent call to the same function
> will not free them again, unless new objects have been created in the
> meantime.
>
> transport_data is the exception, and it shows exactly this problem: When
> an incoming migration uses transport_cleanup() and transport_data, and a
> subsequent incoming migration (e.g. loadvm) occurs that does not, then
> when this second one is done, it will call transport_cleanup() on the
> old transport_data again -- which has already been freed. This is
> sometimes visible in the iotest 201, though for some reason I can only
> reproduce it with -m32.
>
> To fix this, call transport_cleanup() only when transport_data is not
> NULL (otherwise there is nothing to clean up), and set transport_data to
> NULL when it has been cleaned up (i.e. freed).
>
> (transport_cleanup() is used only by migration/socket.c, where
> socket_start_incoming_migration_internal() sets both it and
> transport_data to non-NULL values.)
>
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
That probably deserves a fixes: a59136f
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/migration.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index bcc385b94b..cdb2e76d02 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -287,8 +287,9 @@ void migration_incoming_state_destroy(void)
> g_array_free(mis->postcopy_remote_fds, TRUE);
> mis->postcopy_remote_fds = NULL;
> }
> - if (mis->transport_cleanup) {
> + if (mis->transport_cleanup && mis->transport_data) {
> mis->transport_cleanup(mis->transport_data);
> + mis->transport_data = NULL;
> }
>
> qemu_event_reset(&mis->main_thread_load_event);
> --
> 2.34.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
next prev parent reply other threads:[~2022-02-17 17:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-17 17:04 [PATCH] migration: NULL transport_data after freeing Hanna Reitz
2022-02-17 17:29 ` Dr. David Alan Gilbert [this message]
2022-02-18 1:47 ` Peter Xu
2022-03-02 12:17 ` Dr. David Alan Gilbert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yg6F/kMaUSzX95Gc@work-vm \
--to=dgilbert@redhat.com \
--cc=hreitz@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.