On Tue, Sep 25, 2018 at 10:37:38AM +0100, Dr. David Alan Gilbert wrote:
> * Marc-André Lureau (marcandre.lur...@redhat.com) wrote:
> > Spotted by ASAN while running:
> >
> > $ tests/migration-test -p /x86_64/migration/postcopy/recovery
> >
> > =================================================================
> > ==18034==ERROR: LeakSanitizer: detected memory leaks
> >
> > Direct leak of 33864 byte(s) in 1 object(s) allocated from:
> > #0 0x7f3da7f31e50 in calloc (/lib64/libasan.so.5+0xeee50)
> > #1 0x7f3da644441d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
> > #2 0x55af9db15440 in qemu_fopen_channel_input
> > /home/elmarco/src/qemu/migration/qemu-file-channel.c:183
> > #3 0x55af9db15413 in channel_get_output_return_path
> > /home/elmarco/src/qemu/migration/qemu-file-channel.c:159
> > #4 0x55af9db0d4ac in qemu_file_get_return_path
> > /home/elmarco/src/qemu/migration/qemu-file.c:78
> > #5 0x55af9dad5e4f in open_return_path_on_source
> > /home/elmarco/src/qemu/migration/migration.c:2295
> > #6 0x55af9dadb3bf in migrate_fd_connect
> > /home/elmarco/src/qemu/migration/migration.c:3111
> > #7 0x55af9dae1bf3 in migration_channel_connect
> > /home/elmarco/src/qemu/migration/channel.c:91
> > #8 0x55af9daddeca in socket_outgoing_migration
> > /home/elmarco/src/qemu/migration/socket.c:108
> > #9 0x55af9e13d3db in qio_task_complete
> > /home/elmarco/src/qemu/io/task.c:158
> > #10 0x55af9e13ca03 in qio_task_thread_result
> > /home/elmarco/src/qemu/io/task.c:89
> > #11 0x7f3da643b1ca in g_idle_dispatch gmain.c:5535
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > ---
> > migration/migration.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 05d0a7296a..d214ea12c8 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2268,7 +2268,10 @@ out:
> > */
> > if (postcopy_pause_return_path_thread(ms)) {
> > /* Reload rp, reset the rest */
> > - rp = ms->rp_state.from_dst_file;
> > + if (rp != ms->rp_state.from_dst_file) {
> > + qemu_fclose(rp);
> > + rp = ms->rp_state.from_dst_file;
> > + }
> > ms->rp_state.error = false;
> > goto retry;
> > }
>
> Yep, I think that's right (cc'ing in Peter);
>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
Thanks for the report and the fix. I think the "if" will always be
true, so imho we can even drop that check. Anyway:
Reviewed-by: Peter Xu <pet...@redhat.com>
Regards,
--
Peter Xu
