Re: [PATCH for-4.6] libxl: clear O_NONBLOCK|O_NDELAY on migration fd and reinstate afterwards

[Ian Campbell <ian.campbell@citrix.com>]

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 <ian.campbell@citrix.com>
> > Cc: Jim Fehlig <jfehlig@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> > Cc: Yang Hongyang <yanghy@cn.fujitsu.com>
> > ---
> > 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.