From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH for-4.6] libxl: clear O_NONBLOCK|O_NDELAY on migration fd and reinstate afterwards Date: Fri, 11 Sep 2015 15:13:18 +0100 Message-ID: <1441980798.3549.51.camel@citrix.com> References: <1441968171-15844-1-git-send-email-ian.campbell@citrix.com> <55F2DAA1.1050902@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55F2DAA1.1050902@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , ian.jackson@eu.citrix.com, wei.liu2@citrix.com, xen-devel@lists.xen.org Cc: Shriram Rajagopalan , Jim Fehlig , Yang Hongyang List-Id: xen-devel@lists.xenproject.org On Fri, 2015-09-11 at 14:44 +0100, Andrew Cooper wrote: > On 11/09/15 11:42, Ian Campbell wrote: > > The fd passed to us by libvirt for both save and restore has at least > > O_NONBLOCK set, which libxl does not expect and therefore fails to > > handle any EAGAIN which might arise. > > > > This has been observed with migration v2, but if v1 used to work I > > think that would be just be by luck and/or coincidence. > > > > Unix convention (and the principal of least surprise) is usually to > > ensure that an fd has no "strange" properties, such as being > > non-blocking, when handing it to another component. > > > > However for the convenience of the application arrange instead for > > libxl to clear any unexpected flags on the file descriptors it is > > given for save or restore and restore them to their original state at > > the end. O_NDELAY could be similarly problematic so clear that as > > well as O_NONBLOCK. > > > > To do this introduce a pair of new helper functions one to modify+save > > the flags and another to restore them and call them in the appropriate > > places. > > > > The migration v1 code appeared to do some things with O_NONBLOCK in > > the checkpoint case. Migration v2 doesn't seem to do so, and in any > > case I wouldn't expect it to be relying on libvirt's setting of > > O_NONBLOCK when xl doesn't use that flag. > > > > Signed-off-by: Ian Campbell > > Cc: Jim Fehlig > > Cc: Andrew Cooper > > Cc: Shriram Rajagopalan > > Cc: Yang Hongyang > > --- > > For 4.6: This fixes migration with libvirt, which I think is worth > > doing before the release. > > > > For backports: Once "ts-xen-install: Rewrite /etc/hosts to comment out > > 127.0.1.1 entry" passes through osstest's pretest gate and has run on > > some of the older branches we should then know if this is necessary > > for migration v1. Or we could backport it regardless. > > I don't believe any special consideration is needed for the legacy > conversion case, as all other fds used there are created by components > we control. Thanks, I was actually talking about actual migration v1 as in 4.5 migrating to 4.5, but the above is useful info nonetheless. > > + LOG(DEBUG, "fnctl F_GETFL flags for fd %d are %x", fd, fdfl); > > %#x to distinguish decimal and hex numbers in the same message (and > other debug messages) Gah, didn't see this until after I pushed, sorry. Will post a followup Ian.