|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen master] libxl: clear O_NONBLOCK|O_NDELAY on migration fd and reinstate afterwards
commit 93f5194e72708776783f2d34894c9b42031e116e
Author: Ian Campbell <ian.campbell@xxxxxxxxxx>
AuthorDate: Fri Sep 11 11:42:51 2015 +0100
Commit: Ian Campbell <ian.campbell@xxxxxxxxxx>
CommitDate: Fri Sep 11 14:42:40 2015 +0100
libxl: clear O_NONBLOCK|O_NDELAY on migration fd and reinstate afterwards
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@xxxxxxxxxx>
Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Cc: Jim Fehlig <jfehlig@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: Shriram Rajagopalan <rshriram@xxxxxxxxx>
Cc: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>
---
tools/libxl/libxl.c | 65 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_create.c | 23 ++++++++++++++-
tools/libxl/libxl_internal.h | 13 ++++++++
3 files changed, 100 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 10d1909..6ebb96e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -952,6 +952,12 @@ static void domain_suspend_cb(libxl__egc *egc,
libxl__domain_suspend_state *dss, int rc)
{
STATE_AO_GC(dss->ao);
+ int flrc;
+
+ flrc = libxl__fd_flags_restore(gc, dss->fd, dss->fdfl);
+ /* If suspend has failed already then report that error not this one. */
+ if (flrc && !rc) rc = flrc;
+
libxl__ao_complete(egc,ao,rc);
}
@@ -980,6 +986,11 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid,
int fd, int flags,
dss->live = flags & LIBXL_SUSPEND_LIVE;
dss->debug = flags & LIBXL_SUSPEND_DEBUG;
+ rc = libxl__fd_flags_modify_save(gc, dss->fd,
+ ~(O_NONBLOCK|O_NDELAY), 0,
+ &dss->fdfl);
+ if (rc < 0) goto out_err;
+
libxl__domain_save(egc, dss);
return AO_INPROGRESS;
@@ -6507,6 +6518,60 @@ int libxl_fd_set_cloexec(libxl_ctx *ctx, int fd, int
cloexec)
int libxl_fd_set_nonblock(libxl_ctx *ctx, int fd, int nonblock)
{ return fd_set_flags(ctx,fd, F_GETFL,F_SETFL,"FL", O_NONBLOCK, nonblock); }
+int libxl__fd_flags_modify_save(libxl__gc *gc, int fd,
+ int mask, int val, int *r_oldflags)
+{
+ int rc, ret, fdfl;
+
+ fdfl = fcntl(fd, F_GETFL);
+ if (fdfl < 0) {
+ LOGE(ERROR, "failed to fcntl.F_GETFL for fd %d", fd);
+ rc = ERROR_FAIL;
+ goto out_err;
+ }
+
+ LOG(DEBUG, "fnctl F_GETFL flags for fd %d are %x", fd, fdfl);
+
+ if (r_oldflags)
+ *r_oldflags = fdfl;
+
+ fdfl &= mask;
+ fdfl |= val;
+
+ LOG(DEBUG, "fnctl F_SETFL of fd %d to %x", fd, fdfl);
+
+ ret = fcntl(fd, F_SETFL, fdfl);
+ if (ret < 0) {
+ LOGE(ERROR, "failed to fcntl.F_SETFL for fd %d", fd);
+ rc = ERROR_FAIL;
+ goto out_err;
+ }
+
+ rc = 0;
+
+out_err:
+ return rc;
+}
+
+int libxl__fd_flags_restore(libxl__gc *gc, int fd, int fdfl)
+{
+ int ret, rc;
+
+ LOG(DEBUG, "fnctl F_SETFL of fd %d to %x", fd, fdfl);
+
+ ret = fcntl(fd, F_SETFL, fdfl);
+ if (ret < 0) {
+ LOGE(ERROR, "failed to fcntl.F_SETFL for fd %x", fd);
+ rc = ERROR_FAIL;
+ goto out_err;
+ }
+
+ rc = 0;
+
+out_err:
+ return rc;
+
+}
void libxl_hwcap_copy(libxl_ctx *ctx,libxl_hwcap *dst, libxl_hwcap *src)
{
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 29c2641..f5771da 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1556,6 +1556,7 @@ static int do_domain_create(libxl_ctx *ctx,
libxl_domain_config *d_config,
{
AO_CREATE(ctx, 0, ao_how);
libxl__app_domain_create_state *cdcs;
+ int rc;
GCNEW(cdcs);
cdcs->dcs.ao = ao;
@@ -1563,8 +1564,13 @@ static int do_domain_create(libxl_ctx *ctx,
libxl_domain_config *d_config,
libxl_domain_config_init(&cdcs->dcs.guest_config_saved);
libxl_domain_config_copy(ctx, &cdcs->dcs.guest_config_saved, d_config);
cdcs->dcs.restore_fd = cdcs->dcs.libxc_fd = restore_fd;
- if (restore_fd > -1)
+ if (restore_fd > -1) {
cdcs->dcs.restore_params = *params;
+ rc = libxl__fd_flags_modify_save(gc, cdcs->dcs.restore_fd,
+ ~(O_NONBLOCK|O_NDELAY), 0,
+ &cdcs->dcs.restore_fdfl);
+ if (rc < 0) goto out_err;
+ }
cdcs->dcs.callback = domain_create_cb;
libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how);
cdcs->domid_out = domid;
@@ -1572,6 +1578,10 @@ static int do_domain_create(libxl_ctx *ctx,
libxl_domain_config *d_config,
initiate_domain_create(egc, &cdcs->dcs);
return AO_INPROGRESS;
+
+ out_err:
+ return AO_CREATE_FAIL(rc);
+
}
static void domain_create_cb(libxl__egc *egc,
@@ -1579,10 +1589,21 @@ static void domain_create_cb(libxl__egc *egc,
int rc, uint32_t domid)
{
libxl__app_domain_create_state *cdcs = CONTAINER_OF(dcs, *cdcs, dcs);
+ int flrc;
STATE_AO_GC(cdcs->dcs.ao);
*cdcs->domid_out = domid;
+ if (dcs->restore_fd > -1) {
+ flrc = libxl__fd_flags_restore(gc,
+ dcs->restore_fd, dcs->restore_fdfl);
+ /*
+ * If restore has failed already then report that error not
+ * this one.
+ */
+ if (flrc && !rc) rc = flrc;
+ }
+
libxl__ao_complete(egc, ao, rc);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c3c3b7b..5fa55a7 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -631,6 +631,17 @@ _hidden int libxl__pipe_nonblock(libxl_ctx *ctx, int
fds[2]);
* `not open'. Ignores any errors. Sets fds[] to -1. */
_hidden void libxl__pipe_close(int fds[2]);
+/* Change the flags for the file description associated with fd to
+ * (flags & mask) | val.
+ * If r_oldflags != NULL then sets *r_oldflags to the original set of
+ * flags.
+ */
+_hidden int libxl__fd_flags_modify_save(libxl__gc *gc, int fd,
+ int mask, int val, int *r_oldflags);
+/* Restores the flags for the file description associated with fd to
+ * to the previous value (returned by libxl__fd_flags_modify_save)
+ */
+_hidden int libxl__fd_flags_restore(libxl__gc *gc, int fd, int old_flags);
/* Each of these logs errors and returns a libxl error code.
* They do not mind if path is already removed.
@@ -3050,6 +3061,7 @@ struct libxl__domain_suspend_state {
uint32_t domid;
int fd;
+ int fdfl; /* original flags on fd */
libxl_domain_type type;
int live;
int debug;
@@ -3391,6 +3403,7 @@ struct libxl__domain_create_state {
libxl_domain_config *guest_config;
libxl_domain_config guest_config_saved; /* vanilla config */
int restore_fd, libxc_fd;
+ int restore_fdfl; /* original flags of restore_fd */
libxl_domain_restore_params restore_params;
libxl__domain_create_cb *callback;
libxl_asyncprogress_how aop_console_how;
--
generated by git-patchbot for /home/xen/git/xen.git#master
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |