Re: [Qemu-devel] [PATCH RFC 4/6] migration: shut src return path unconditionally

[Juan Quintela]

Peter Xu <address@hidden> wrote:
> On Tue, May 30, 2017 at 05:50:27PM +0200, Juan Quintela wrote:
>> Peter Xu <address@hidden> wrote:
>> > We were do the shutting off only for postcopy. Now we do this as long as
>> > the source return path is there.
>> >
>> > Moving the cleanup of from_src_file there too.
>> >
>> > Signed-off-by: Peter Xu <address@hidden>
>> > ---
>> >  migration/migration.c    | 8 +++++++-
>> >  migration/postcopy-ram.c | 1 -
>> >  2 files changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/migration/migration.c b/migration/migration.c
>> > index 92617fc..a4006b4 100644
>> > --- a/migration/migration.c
>> > +++ b/migration/migration.c
>> > @@ -131,10 +131,17 @@ void migration_incoming_state_destroy(void)
>> >      struct MigrationIncomingState *mis = migration_incoming_get_current();
>> >  
>> >      if (mis->to_src_file) {
>> > +        /* Tell source that we are done */
>> > +        migrate_send_rp_shut(mis, qemu_file_get_error(mis->from_src_file) 
>> > != 0);
>> 
>> Reviewed-by: Juan Quintela <address@hidden>
>> 
>> 
>> I think this one belongs to previous patch (with accompaining line from 
>> below).
>> But just if you want to change it.
>
> I separated it since these two patches were actually doing different
> things:
>
> - previous patch fixed one possible leak, while
>
> - this patch postponed MIG_RP_MSG_SHUT a bit to the end, and let it
>   not depending on postcopy, but the return path itself (so that we
>   can enable the return path even without postcopy then)
>
> Meanwhile, there might be problem if we just put this single line into
> previous patch, since this line depends on below change [1]
> (from_src_file should better be closed after this
> qemu_file_get_error() call). So... I would still prefer to separate
> them using current way. Even if we really want to merge them, I would
> prefer directly squashing current patch into previous one.

ok, it is up to you.