|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 6/6] tools/libxl: Fix datacopier POLLHUP handling to not always be fatal
POLLIN|POLLHUP is a valid revent to recieve from poll() when the far end has
hung up, but data is still available to be read.
Currently all POLLHUPs are fatal. This is a problem when using the legacy
stream conversion script which will exit 0 when it has completed its task and
has left valid data in the pipe. In this case, libxl wishes to read until
eof, rather than dying because the conversion script exited.
Adjustments are as follows:
1. The datacopier pollhup callback changes type to include a return value.
2. datacopier_handle_pollhup() takes revents by pointer, and calls the
datacopier pollhup callback ahead of killing the datacopier.
3. The callback may now indicate that the POLLHUP is not fatal, in which case
datacopier_handle_pollhup() mask POLLHUP out of revents, and
datacopier_{read,write}able() allowed to proceed as before.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
---
tools/libxl/libxl_aoutils.c | 34 ++++++++++++++++++++++------------
tools/libxl/libxl_bootloader.c | 22 ++++++++++++++++++++--
tools/libxl/libxl_internal.h | 14 ++++++++++----
3 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index a402e5c..f3e5f98 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -182,21 +182,31 @@ void libxl__datacopier_prefixdata(libxl__egc *egc,
libxl__datacopier_state *dc,
}
}
-static int datacopier_pollhup_handled(libxl__egc *egc,
+/*
+ * Handle a POLLHUP signal on a datacopier fd. Return boolean indicating
+ * whether further processing should cease.
+ */
+static bool datacopier_handle_pollhup(libxl__egc *egc,
libxl__datacopier_state *dc,
- short revents, int onwrite)
+ short *revents, int onwrite)
{
STATE_AO_GC(dc->ao);
- if (dc->callback_pollhup && (revents & POLLHUP)) {
- LOG(DEBUG, "received POLLHUP on %s during copy of %s",
- onwrite ? dc->writewhat : dc->readwhat,
- dc->copywhat);
- libxl__datacopier_kill(dc);
- dc->callback_pollhup(egc, dc, onwrite, -1);
- return 1;
+ if (dc->callback_pollhup && (*revents & POLLHUP)) {
+ if (dc->callback_pollhup(egc, dc, onwrite)) {
+ LOG(DEBUG, "received POLLHUP on %s during copy of %s",
+ onwrite ? dc->writewhat : dc->readwhat, dc->copywhat);
+ libxl__datacopier_kill(dc);
+ return true;
+ } else {
+ LOG(DEBUG, "Ignoring POLLHUP on %s during copy of %s",
+ onwrite ? dc->writewhat : dc->readwhat, dc->copywhat);
+ *revents &= ~POLLHUP;
+ return false;
+ }
}
- return 0;
+
+ return false;
}
static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
@@ -204,7 +214,7 @@ static void datacopier_readable(libxl__egc *egc,
libxl__ev_fd *ev,
libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
STATE_AO_GC(dc->ao);
- if (datacopier_pollhup_handled(egc, dc, revents, 0))
+ if (datacopier_handle_pollhup(egc, dc, &revents, 0))
return;
if (revents & ~POLLIN) {
@@ -279,7 +289,7 @@ static void datacopier_writable(libxl__egc *egc,
libxl__ev_fd *ev,
libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, towrite);
STATE_AO_GC(dc->ao);
- if (datacopier_pollhup_handled(egc, dc, revents, 1))
+ if (datacopier_handle_pollhup(egc, dc, &revents, 1))
return;
if (revents & ~POLLOUT) {
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 1503101..db5ee02 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -31,8 +31,12 @@
static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op);
static void bootloader_keystrokes_copyfail(libxl__egc *egc,
libxl__datacopier_state *dc, int onwrite, int errnoval);
+static bool bootloader_keystrokes_pollhup(libxl__egc *egc,
+ libxl__datacopier_state *dc, int onwrite);
static void bootloader_display_copyfail(libxl__egc *egc,
libxl__datacopier_state *dc, int onwrite, int errnoval);
+static bool bootloader_display_pollhup(libxl__egc *egc,
+ libxl__datacopier_state *dc, int onwrite);
static void bootloader_domaindeath(libxl__egc*, libxl__domaindeathcheck *dc);
static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
pid_t pid, int status);
@@ -520,7 +524,7 @@ static void bootloader_gotptys(libxl__egc *egc,
libxl__openpty_state *op)
bl->keystrokes.copywhat =
GCSPRINTF("bootloader input for domain %"PRIu32, bl->domid);
bl->keystrokes.callback = bootloader_keystrokes_copyfail;
- bl->keystrokes.callback_pollhup = bootloader_keystrokes_copyfail;
+ bl->keystrokes.callback_pollhup = bootloader_keystrokes_pollhup;
/* pollhup gets called with errnoval==-1 which is not otherwise
* possible since errnos are nonnegative, so it's unambiguous */
rc = libxl__datacopier_start(&bl->keystrokes);
@@ -532,7 +536,7 @@ static void bootloader_gotptys(libxl__egc *egc,
libxl__openpty_state *op)
bl->display.copywhat =
GCSPRINTF("bootloader output for domain %"PRIu32, bl->domid);
bl->display.callback = bootloader_display_copyfail;
- bl->display.callback_pollhup = bootloader_display_copyfail;
+ bl->display.callback_pollhup = bootloader_display_pollhup;
rc = libxl__datacopier_start(&bl->display);
if (rc) goto out;
@@ -603,12 +607,26 @@ static void bootloader_keystrokes_copyfail(libxl__egc
*egc,
libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, keystrokes);
bootloader_copyfail(egc, "bootloader input", bl, 0, onwrite, errnoval);
}
+static bool bootloader_keystrokes_pollhup(libxl__egc *egc,
+ libxl__datacopier_state *dc, int onwrite)
+{
+ libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, keystrokes);
+ bootloader_copyfail(egc, "bootloader input", bl, 0, onwrite, -1);
+ return true;
+}
static void bootloader_display_copyfail(libxl__egc *egc,
libxl__datacopier_state *dc, int onwrite, int errnoval)
{
libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, display);
bootloader_copyfail(egc, "bootloader output", bl, 1, onwrite, errnoval);
}
+static bool bootloader_display_pollhup(libxl__egc *egc,
+ libxl__datacopier_state *dc, int onwrite)
+{
+ libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, display);
+ bootloader_copyfail(egc, "bootloader output", bl, 1, onwrite, -1);
+ return true;
+}
static void bootloader_domaindeath(libxl__egc *egc, libxl__domaindeathcheck
*dc)
{
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c2e7aa4..0c7c821 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2528,11 +2528,17 @@ _hidden void
libxl__device_disk_local_initiate_detach(libxl__egc *egc,
* written when in read mode
* errnoval!=0 means we had a read error, logged
* onwrite==-1 means some other internal failure, errnoval not valid, logged
- * If we get POLLHUP, we call callback_pollhup(..., onwrite, -1);
- * or if callback_pollhup==0 this is an internal failure, as above.
- * In all cases copier is killed before calling this callback */
+ *
+ * If we get POLLHUP, we call callback_pollhup(..., onwrite) which may choose
+ * whether the POLLHUP is fatal with its return value. If
+ * callback_pollhup==NULL this is an internal failure, as above. The copier
+ * has never been killed before a pollhup callback, but will subequently be
+ * killed if the callback return true.
+ */
typedef void libxl__datacopier_callback(libxl__egc *egc,
libxl__datacopier_state *dc, int onwrite, int errnoval);
+typedef bool libxl__datacopier_pollhup_callback(libxl__egc *egc,
+ libxl__datacopier_state *dc, int onwrite);
struct libxl__datacopier_buf {
/* private to datacopier */
@@ -2550,7 +2556,7 @@ struct libxl__datacopier_state {
const char *copywhat, *readwhat, *writewhat; /* for error msgs */
FILE *log; /* gets a copy of everything */
libxl__datacopier_callback *callback;
- libxl__datacopier_callback *callback_pollhup;
+ libxl__datacopier_pollhup_callback *callback_pollhup;
/* remaining fields are private to datacopier */
libxl__ev_fd toread, towrite;
ssize_t used;
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |