From: Ian Jackson Date: Tue, 10 Feb 2015 19:37:33 +0000 (+0000) Subject: libxl: ao: datacopier callback gets an rc X-Git-Tag: archive/raspbian/4.8.0-1+rpi1~1^2~2989 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=c3c8da9f5e731c518fcfb798beeaea9e8ccfad40;p=xen.git libxl: ao: datacopier callback gets an rc libxl__datacopier_* now provides its caller's callback function with an rc value. This relieves the caller of the need to figure out an appropriate rc value. Arrange that the `other internal failure' cases now get a valid positive errno value (EIO). In a few places, assert that errno is nonzero before passing it to our caller. Extend the datacopier callback API to permit the dc to signal ABORTED. (It doesn't actually do this yet, though.) Signed-off-by: Ian Jackson Acked-by: Ian Campbell CC: Andrew Cooper CC: David Vrabel --- v2: New in this version of the series. --- diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c index 593a5754e5..124582886d 100644 --- a/tools/libxl/libxl_aoutils.c +++ b/tools/libxl/libxl_aoutils.c @@ -121,10 +121,10 @@ void libxl__datacopier_kill(libxl__datacopier_state *dc) } static void datacopier_callback(libxl__egc *egc, libxl__datacopier_state *dc, - int onwrite, int errnoval) + int rc, int onwrite, int errnoval) { libxl__datacopier_kill(dc); - dc->callback(egc, dc, onwrite, errnoval); + dc->callback(egc, dc, rc, onwrite, errnoval); } static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev, @@ -142,14 +142,14 @@ static void datacopier_check_state(libxl__egc *egc, libxl__datacopier_state *dc) if (rc) { LOG(ERROR, "unable to establish write event on %s" " during copy of %s", dc->writewhat, dc->copywhat); - datacopier_callback(egc, dc, -1, 0); + datacopier_callback(egc, dc, ERROR_FAIL, -1, EIO); return; } } } else if (!libxl__ev_fd_isregistered(&dc->toread) || dc->bytes_to_read == 0) { /* we have had eof */ - datacopier_callback(egc, dc, 0, 0); + datacopier_callback(egc, dc, 0, 0, 0); return; } else { /* nothing buffered, but still reading */ @@ -195,7 +195,7 @@ static int datacopier_pollhup_handled(libxl__egc *egc, onwrite ? dc->writewhat : dc->readwhat, dc->copywhat); libxl__datacopier_kill(dc); - dc->callback_pollhup(egc, dc, onwrite, -1); + dc->callback_pollhup(egc, dc, ERROR_FAIL, onwrite, -1); return 1; } return 0; @@ -213,7 +213,7 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev, LOG(ERROR, "unexpected poll event 0x%x (expected POLLIN and/or POLLHUP)" " on %s during copy of %s", revents, dc->readwhat, dc->copywhat); - datacopier_callback(egc, dc, -1, 0); + datacopier_callback(egc, dc, ERROR_FAIL, -1, EIO); return; } assert(revents & (POLLIN|POLLHUP)); @@ -245,20 +245,21 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev, } if (r < 0) { if (errno == EINTR) continue; + assert(errno); if (errno == EWOULDBLOCK) { if (revents & POLLHUP) { LOG(ERROR, "poll reported HUP but fd read gave EWOULDBLOCK" " on %s during copy of %s", dc->readwhat, dc->copywhat); - datacopier_callback(egc, dc, -1, 0); + datacopier_callback(egc, dc, ERROR_FAIL, -1, 0); return; } break; } LOGE(ERROR, "error reading %s during copy of %s", dc->readwhat, dc->copywhat); - datacopier_callback(egc, dc, 0, errno); + datacopier_callback(egc, dc, ERROR_FAIL, 0, errno); return; } if (r == 0) { @@ -287,7 +288,7 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev, assert(ferror(dc->log)); assert(errno); LOGE(ERROR, "error logging %s", dc->copywhat); - datacopier_callback(egc, dc, 0, errno); + datacopier_callback(egc, dc, ERROR_FAIL, 0, errno); return; } } @@ -315,7 +316,7 @@ static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev, if (revents & ~POLLOUT) { LOG(ERROR, "unexpected poll event 0x%x (should be POLLOUT)" " on %s during copy of %s", revents, dc->writewhat, dc->copywhat); - datacopier_callback(egc, dc, -1, 0); + datacopier_callback(egc, dc, ERROR_FAIL, -1, EIO); return; } assert(revents & POLLOUT); @@ -332,9 +333,10 @@ static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev, if (r < 0) { if (errno == EINTR) continue; if (errno == EWOULDBLOCK) break; + assert(errno); LOGE(ERROR, "error writing to %s during copy of %s", dc->writewhat, dc->copywhat); - datacopier_callback(egc, dc, 1, errno); + datacopier_callback(egc, dc, ERROR_FAIL, 1, errno); return; } assert(r > 0); diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c index 6db958e098..95dde986f2 100644 --- a/tools/libxl/libxl_bootloader.c +++ b/tools/libxl/libxl_bootloader.c @@ -30,9 +30,9 @@ 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); + libxl__datacopier_state *dc, int rc, int onwrite, int errnoval); static void bootloader_display_copyfail(libxl__egc *egc, - libxl__datacopier_state *dc, int onwrite, int errnoval); + libxl__datacopier_state *dc, int rc, int onwrite, int errnoval); static void bootloader_domaindeath(libxl__egc*, libxl__domaindeathcheck *dc, int rc); static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child, @@ -579,10 +579,10 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op) /* perhaps one of these will be called, but perhaps not */ static void bootloader_copyfail(libxl__egc *egc, const char *which, - libxl__bootloader_state *bl, int ondisplay, int onwrite, int errnoval) + libxl__bootloader_state *bl, int ondisplay, + int rc, int onwrite, int errnoval) { STATE_AO_GC(bl->ao); - int rc = ERROR_FAIL; if (errnoval==-1) { /* POLLHUP */ @@ -593,22 +593,24 @@ static void bootloader_copyfail(libxl__egc *egc, const char *which, LOG(ERROR, "unexpected POLLHUP on %s", which); } } - if (!onwrite && !errnoval) + if (!rc) { LOG(ERROR, "unexpected eof copying %s", which); + rc = ERROR_FAIL; + } bootloader_stop(egc, bl, rc); } static void bootloader_keystrokes_copyfail(libxl__egc *egc, - libxl__datacopier_state *dc, int onwrite, int errnoval) + libxl__datacopier_state *dc, int rc, int onwrite, int errnoval) { libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, keystrokes); - bootloader_copyfail(egc, "bootloader input", bl, 0, onwrite, errnoval); + bootloader_copyfail(egc, "bootloader input", bl, 0, rc,onwrite,errnoval); } static void bootloader_display_copyfail(libxl__egc *egc, - libxl__datacopier_state *dc, int onwrite, int errnoval) + libxl__datacopier_state *dc, int rc, int onwrite, int errnoval) { libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, display); - bootloader_copyfail(egc, "bootloader output", bl, 1, onwrite, errnoval); + bootloader_copyfail(egc, "bootloader output", bl, 1, rc,onwrite,errnoval); } static void bootloader_domaindeath(libxl__egc *egc, diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index b081ef7af5..bdc04657fa 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -2158,7 +2158,7 @@ out: } static void save_device_model_datacopier_done(libxl__egc *egc, - libxl__datacopier_state *dc, int onwrite, int errnoval); + libxl__datacopier_state *dc, int rc, int onwrite, int errnoval); void libxl__domain_save_device_model(libxl__egc *egc, libxl__domain_suspend_state *dss, @@ -2218,11 +2218,11 @@ void libxl__domain_save_device_model(libxl__egc *egc, return; out: - save_device_model_datacopier_done(egc, dc, -1, 0); + save_device_model_datacopier_done(egc, dc, rc, -1, EIO); } static void save_device_model_datacopier_done(libxl__egc *egc, - libxl__datacopier_state *dc, int onwrite, int errnoval) + libxl__datacopier_state *dc, int our_rc, int onwrite, int errnoval) { libxl__domain_suspend_state *dss = CONTAINER_OF(dc, *dss, save_dm_datacopier); @@ -2230,14 +2230,10 @@ static void save_device_model_datacopier_done(libxl__egc *egc, /* Convenience aliases */ const char *const filename = dss->dm_savefile; - int our_rc = 0; int rc; libxl__datacopier_kill(dc); - if (onwrite || errnoval) - our_rc = ERROR_FAIL; - if (dc->readfd >= 0) { close(dc->readfd); dc->readfd = -1; diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index ad20168746..140725199b 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -2620,17 +2620,21 @@ _hidden void libxl__device_disk_local_initiate_detach(libxl__egc *egc, typedef struct libxl__datacopier_state libxl__datacopier_state; typedef struct libxl__datacopier_buf libxl__datacopier_buf; -/* onwrite==1 means failure happened when writing, logged, errnoval is valid - * onwrite==0 means failure happened when reading - * errnoval==0 means we got eof and all data was written - * 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); +/* onwrite==1 means problem happened when writing + * rc==FAIL errnoval >0 we had a write error, logged + * onwrite==0 means problem happened when reading + * rc==0 errnoval==0 we got eof and all data was written + * rc==FAIL errnoval >0 we had a read error, logged + * onwrite==-1 means some other internal problem + * rc==FAIL errnoval==EIO some other internal failure, logged + * rc==ABORTED errnoval==0 abort requested, not logged + * If we get POLLHUP, we call callback_pollhup with + * rc==FAIL errnoval==-1 POLLHUP signalled * or if callback_pollhup==0 this is treated as eof (if POLLIN|POLLHUP * on the reading fd) or an internal failure (otherwise), as above. * In all cases copier is killed before calling this callback */ typedef void libxl__datacopier_callback(libxl__egc *egc, - libxl__datacopier_state *dc, int onwrite, int errnoval); + libxl__datacopier_state *dc, int rc, int onwrite, int errnoval); struct libxl__datacopier_buf { /* private to datacopier */