qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Lukas Straub <lukasstraub2@web.de>,
	"Daniel P . Berrange" <berrange@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	qemu-devel@nongnu.org,
	Leonardo Bras Soares Passos <lsoaresp@redhat.com>
Subject: Re: [PATCH v2 5/5] migration: Move the yank unregister of channel_close out
Date: Thu, 22 Jul 2021 12:19:14 -0400	[thread overview]
Message-ID: <YPmagh393LlpVSm9@t490s> (raw)
In-Reply-To: <YPmOZ3LGm0u7GoBA@work-vm>

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 :)

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?

Thanks,

-- 
Peter Xu



  reply	other threads:[~2021-07-22 16:20 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21 19:34 [PATCH v2 0/5] migrations: Fix potential rare race of migration-test after yank Peter Xu
2021-07-21 19:34 ` [PATCH v2 1/5] migration: Fix missing join() of rp_thread Peter Xu
2021-07-21 19:34 ` [PATCH v2 2/5] migration: Make from_dst_file accesses thread-safe Peter Xu
2021-07-21 21:15   ` Eric Blake
2021-07-22 14:44     ` Peter Xu
2021-07-22 15:19   ` Dr. David Alan Gilbert
2021-07-21 19:34 ` [PATCH v2 3/5] migration: Introduce migration_ioc_[un]register_yank() Peter Xu
2021-07-21 19:34 ` [PATCH v2 4/5] migration: Teach QEMUFile to be QIOChannel-aware Peter Xu
2021-07-22 15:21   ` Dr. David Alan Gilbert
2021-07-21 19:34 ` [PATCH v2 5/5] migration: Move the yank unregister of channel_close out Peter Xu
2021-07-22 15:27   ` Dr. David Alan Gilbert
2021-07-22 16:19     ` Peter Xu [this message]
2021-07-22 17:09       ` Dr. David Alan Gilbert
2021-07-22 17:35         ` Peter Xu

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=YPmagh393LlpVSm9@t490s \
    --to=peterx@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=lsoaresp@redhat.com \
    --cc=lukasstraub2@web.de \
    --cc=peter.maydell@linaro.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).