All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Lukas Straub <lukasstraub2@web.de>,
	Juan Quintela <quintela@redhat.com>,
	qemu-devel@nongnu.org, Peter Xu <peterx@redhat.com>,
	Leonardo Bras Soares Passos <lsoaresp@redhat.com>
Subject: Re: [PATCH 2/5] migration: Shutdown src in await_return_path_close_on_source()
Date: Wed, 21 Jul 2021 16:57:47 +0100	[thread overview]
Message-ID: <YPhD+7YFY7IbHn+n@redhat.com> (raw)
In-Reply-To: <YPfu9N98PyTp7AKX@work-vm>

On Wed, Jul 21, 2021 at 10:55:00AM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > We have a logic in await_return_path_close_on_source() that we will explicitly
> > shutdown the socket when migration encounters errors.  However it could be racy
> > because from_dst_file could have been reset right after checking it but before
> > passing it to qemu_file_shutdown() by the rp_thread.
> > 
> > Fix it by shutdown() on the src file instead.  Since they must be a pair of
> > qemu files, shutdown on either of them will work the same.
> > 
> > Since at it, drop the check for from_dst_file directly, which makes the
> > behavior even more predictable.
> 
> So while the existing code maybe racy, I'm not sure that this change
> keeps the semantics; the channel may well have dup()'d the fd's for the
> two directions, and I'm not convinced that a shutdown() on one will
> necessarily impact the other; and if the shutdown doesn't happen the
> rp_thread might not exit, and we might block on the koin.

My understanding is that 'shutdown' operation affects the state of
the socket connection. An FD is merely the way a socket is exposed
to userspace. Thus if you have multiple FDs all pointing to the same
underlying socket (thanks to dup()), then I expect that the effects
of 'shutdown' will apply equally to all of the FD copies.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  parent reply	other threads:[~2021-07-21 16:03 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21  1:21 [PATCH 0/5] migrations: Fix potential rare race of migration-test after yank Peter Xu
2021-07-21  1:21 ` [PATCH 1/5] migration: Fix missing join() of rp_thread Peter Xu
2021-07-21  9:49   ` Dr. David Alan Gilbert
2021-07-21  1:21 ` [PATCH 2/5] migration: Shutdown src in await_return_path_close_on_source() Peter Xu
2021-07-21  9:55   ` Dr. David Alan Gilbert
2021-07-21 15:40     ` Peter Xu
2021-07-21 18:00       ` Dr. David Alan Gilbert
2021-07-21 18:12         ` Peter Xu
2021-07-21 15:57     ` Daniel P. Berrangé [this message]
2021-07-21  1:21 ` [PATCH 3/5] migration: Introduce migration_ioc_[un]register_yank() Peter Xu
2021-07-21  9:58   ` Dr. David Alan Gilbert
2021-07-21  1:21 ` [PATCH 4/5] migration: Teach QEMUFile to be QIOChannel-aware Peter Xu
2021-07-21 10:27   ` Dr. David Alan Gilbert
2021-07-21 10:57     ` Daniel P. Berrangé
2021-07-21  1:21 ` [PATCH 5/5] migration: Move the yank unregister of channel_close out Peter Xu
2021-07-21 10:39   ` Dr. David Alan Gilbert
2021-07-21 15:45     ` 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=YPhD+7YFY7IbHn+n@redhat.com \
    --to=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=lsoaresp@redhat.com \
    --cc=lukasstraub2@web.de \
    --cc=peter.maydell@linaro.org \
    --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.