From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>,
qemu-devel@nongnu.org, Juan Quintela <quintela@redhat.com>
Subject: Re: [PATCH] migration: NULL transport_data after freeing
Date: Wed, 2 Mar 2022 12:17:12 +0000 [thread overview]
Message-ID: <Yh9gSMGDNtopEpHq@work-vm> (raw)
In-Reply-To: <Yg76z1DYyu8DuJEM@xz-m1.local>
* Peter Xu (peterx@redhat.com) wrote:
> On Thu, Feb 17, 2022 at 06:04:07PM +0100, Hanna Reitz 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>
>
> I had a similar fix here:
>
> https://lore.kernel.org/qemu-devel/20220216062809.57179-15-peterx@redhat.com/
>
> Though there it was because I need migration_incoming_transport_cleanup()
> for other purposes, so the fix came along.
>
> My guess is this small fix will land earlier, if so I'll rebase. :)
Actually it didn't; so since I've pulled a chunk of Peter's series in
anyway I took the one from Peter's series.
Dave
> Thanks,
>
> --
> Peter Xu
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
prev parent reply other threads:[~2022-03-02 13:42 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
2022-02-18 1:47 ` Peter Xu
2022-03-02 12:17 ` Dr. David Alan Gilbert [this message]
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=Yh9gSMGDNtopEpHq@work-vm \
--to=dgilbert@redhat.com \
--cc=hreitz@redhat.com \
--cc=peterx@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.