Xen devel - Sep 2010 - [PATCH] libxc: restore: reset I/O fd to flags to back to state caller passed us

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1283530025 -3600
# Node ID f23b8e767b18bbeb867ec18e3db9bf8639da40b0
# Parent  35cc5da0dd970c885fafaaf05bb3c1594dc68020
libxc: restore: reset I/O fd to flags to back to state caller passed us

In particular this causes us to turn O_NONBLOCK back off if we set it.

The caller may continue to use the fd for it''s own protocol needs and
may not be prepared to have it become non-blocking.

This probably only effects Remus now after my previous patch to signal
the last checkpoint, since a regular migration will no longer set the
fd non-blocking.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 35cc5da0dd97 -r f23b8e767b18 tools/libxc/xc_domain_restore.c
--- a/tools/libxc/xc_domain_restore.c	Fri Sep 03 16:53:12 2010 +0100
+++ b/tools/libxc/xc_domain_restore.c	Fri Sep 03 17:07:05 2010 +0100
@@ -1094,6 +1094,8 @@ int xc_domain_restore(xc_interface *xch,
     void* vcpup;
     uint64_t console_pfn = 0;
 
+    int orig_io_fd_flags;
+
     static struct restore_ctx _ctx = {
         .live_p2m = NULL,
         .p2m = NULL,
@@ -1110,6 +1112,11 @@ int xc_domain_restore(xc_interface *xch,
 
     if ( superpages )
         return 1;
+
+    if ( (orig_io_fd_flags = fcntl(io_fd, F_GETFL, 0)) < 0 ) {
+        PERROR("unable to read IO FD flags");
+        goto out;
+    }
 
     if ( read_exact(io_fd, &dinfo->p2m_size, sizeof(unsigned long)) )
     {
@@ -1294,7 +1301,6 @@ int xc_domain_restore(xc_interface *xch,
     // DPRINTF("Received all pages (%d races)\n", nraces);
 
     if ( !ctx->completed ) {
-        int flags = 0;
 
         if ( buffer_tail(xch, ctx, &tailbuf, io_fd, max_vcpu_id, vcpumap,
                          ext_vcpucontext) < 0 ) {
@@ -1308,11 +1314,7 @@ int xc_domain_restore(xc_interface *xch,
          * nonblocking mode for the remainder.
          */
         if ( !ctx->last_checkpoint )
-        {
-            if ( (flags = fcntl(io_fd, F_GETFL,0)) < 0 )
-                flags = 0;
-            fcntl(io_fd, F_SETFL, flags | O_NONBLOCK);
-        }
+            fcntl(io_fd, F_SETFL, orig_io_fd_flags | O_NONBLOCK);
     }
 
     if ( ctx->last_checkpoint )
@@ -1805,8 +1807,10 @@ int xc_domain_restore(xc_interface *xch,
     /* discard cache for save file  */
     discard_file_cache(xch, io_fd, 1 /*flush*/);
 
+    fcntl(io_fd, F_SETFL, orig_io_fd_flags);
+
     DPRINTF("Restore exit with rc=%d\n", rc);
-    
+
     return rc;
 }
 /*

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH] libxc: restore: reset I/O fd to
flags to back to state caller passed us"):
> libxc: restore: reset I/O fd to flags to back to state caller passed us
> 
> In particular this causes us to turn O_NONBLOCK back off if we set it.
> 
> The caller may continue to use the fd for it''s own protocol needs
and
> may not be prepared to have it become non-blocking.
> 
> This probably only effects Remus now after my previous patch to signal
> the last checkpoint, since a regular migration will no longer set the
> fd non-blocking.
This looks very sensible but I''ll wait for Brendan.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Friday, 03 September 2010 at 17:07, Ian Campbell
wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1283530025 -3600
> # Node ID f23b8e767b18bbeb867ec18e3db9bf8639da40b0
> # Parent  35cc5da0dd970c885fafaaf05bb3c1594dc68020
> libxc: restore: reset I/O fd to flags to back to state caller passed us
> 
> In particular this causes us to turn O_NONBLOCK back off if we set it.
> 
> The caller may continue to use the fd for it''s own protocol needs
and
> may not be prepared to have it become non-blocking.
> 
> This probably only effects Remus now after my previous patch to signal
> the last checkpoint, since a regular migration will no longer set the
> fd non-blocking.
Although I don''t think this currently does anything useful, it seems
harmless. I''m applying and testing this and patch 3 of your previous
series now.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Friday, 03 September 2010 at 17:07, Ian Campbell
wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1283530025 -3600
> # Node ID f23b8e767b18bbeb867ec18e3db9bf8639da40b0
> # Parent  35cc5da0dd970c885fafaaf05bb3c1594dc68020
> libxc: restore: reset I/O fd to flags to back to state caller passed us
> 
> In particular this causes us to turn O_NONBLOCK back off if we set it.
> 
> The caller may continue to use the fd for it''s own protocol needs
and
> may not be prepared to have it become non-blocking.
> 
> This probably only effects Remus now after my previous patch to signal
> the last checkpoint, since a regular migration will no longer set the
> fd non-blocking.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
It doesn''t appear to break Remus.

Acked-by: Brendan Cully <brendan@cs.ubc.ca>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH] libxc: restore: reset I/O fd to
flags to back to state caller passed us"):
> libxc: restore: reset I/O fd to flags to back to state caller passed us
> 
> In particular this causes us to turn O_NONBLOCK back off if we set it.
This one doesn''t apply any more.  This hunk:
> -        {
> -            if ( (flags = fcntl(io_fd, F_GETFL,0)) < 0 )
> -                flags = 0;
> -            fcntl(io_fd, F_SETFL, flags | O_NONBLOCK);
> -        }
> +            fcntl(io_fd, F_SETFL, orig_io_fd_flags | O_NONBLOCK);
seems to have become stale.  Have I applied these in the wrong order
or something ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

I wrote:
> This one doesn''t apply any more.  This hunk:
...
> seems to have become stale.  Have I applied these in the wrong order
> or something ?
Oh, I see, it needs to be on top of your other save/restore patch.
Never mind, I''ve applied both now.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, 2010-09-07 at 19:12 +0100, Ian Jackson wrote: 
> I wrote:
> > This one doesn''t apply any more.  This hunk:
> ...
> > seems to have become stale.  Have I applied these in the wrong order
> > or something ?
> 
> Oh, I see, it needs to be on top of your other save/restore patch.
> Never mind, I''ve applied both now.
Thanks, I''ll do a sweep through of my patch queue tomorrow to see if
anything got missed, some of these patches were a bit scattershot as I
noticed things.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel