Re: [PATCH v2 5/5] migration: Move the yank unregister of channel_close out

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> On Thu, Jul 22, 2021 at 04:27:35PM +0100, Dr. David Alan Gilbert wrote:
> > > @@ -3352,6 +3355,8 @@ static MigThrError postcopy_pause(MigrationState *s)
> > >  
> > >          /* Current channel is possibly broken. Release it. */
> > >          assert(s->to_dst_file);
> > > +        /* Unregister yank for current channel */
> > > +        migration_ioc_unregister_yank_from_file(s->to_dst_file);
> > 
> > Should this go inside the lock?
> 
> Shouldn't need to; as we've got the assert() right above so otherwise we'll
> abrt otherwise :)

Hmm OK; although with out always having to think about it then you worry
about something getting in after the assert.

> The mutex lock/unlock right below this one is not protecting us from someone
> changing it but really for being able to wait until someone finished using it
> then we won't crash someone else.
> 
> I think the rational is to_dst_file is managed by migration thread while
> from_dst_file is managed by rp_thread.
> 
> Maybe I add a comment above?

OK, I almost pushed this further, but then I started to get worried that
we'd trace off a race with ordering on two locks with yank_lock; so yeh
lets just add a comment.

Dave

> Thanks,
> 
> -- 
> Peter Xu
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK