Roger Pau Monne
2011-Oct-19 11:43 UTC
[Xen-devel] [PATCH] libxl: handle the return value of wait_for_dev_destroy select and pass it to caller function
# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1319024152 -7200
# Node ID 5941638b04693f7d8bfa2d6b5563132f54942a28
# Parent a2a3c4d7333ec15b818b3403f148ad61c254ea82
libxl: handle the return value of wait_for_dev_destroy select and pass it to
caller function.
Handle the return value of the select call inside wait_for_dev_destroy properly,
and return 0 if the device is removed, or 1 if a timeout or error happened. Use
the return value of wait_for_dev_destroy inside libxl__device_remove to properly
return from that function.
This patch should be applied after Ian Campbell''s v3 "libxl:
rationalise libxl_device_* APIs".
Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
diff -r a2a3c4d7333e -r 5941638b0469 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c Tue Oct 18 13:36:43 2011 +0100
+++ b/tools/libxl/libxl_device.c Wed Oct 19 13:35:52 2011 +0200
@@ -367,6 +367,7 @@ int libxl__device_disk_dev_number(const
return -1;
}
+/* Returns 0 if a device is removed, 1 if an error or timeout occurred */
static int wait_for_dev_destroy(libxl__gc *gc, struct timeval *tv)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -375,22 +376,34 @@ static int wait_for_dev_destroy(libxl__g
fd_set rfds;
char **l1 = NULL;
+start:
rc = 1;
nfds = xs_fileno(ctx->xsh) + 1;
FD_ZERO(&rfds);
FD_SET(xs_fileno(ctx->xsh), &rfds);
- if (select(nfds, &rfds, NULL, NULL, tv) > 0) {
- l1 = xs_read_watch(ctx->xsh, &n);
- if (l1 != NULL) {
- char *state = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
- if (!state || atoi(state) == 6) {
- xs_unwatch(ctx->xsh, l1[0], l1[1]);
- xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Destroyed device
backend at %s", l1[XS_WATCH_TOKEN]);
- rc = 0;
+ switch (select(nfds, &rfds, NULL, NULL, tv)) {
+ case -1:
+ if (errno == EINTR)
+ goto start;
+ break;
+ case 0:
+ break;
+ default:
+ l1 = xs_read_watch(ctx->xsh, &n);
+ if (l1 != NULL) {
+ char *state = libxl__xs_read(gc, XBT_NULL,
+ l1[XS_WATCH_PATH]);
+ if (!state || atoi(state) == 6) {
+ xs_unwatch(ctx->xsh, l1[0], l1[1]);
+ xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+ "Destroyed device backend at %s",
+ l1[XS_WATCH_TOKEN]);
+ rc = 0;
+ }
+ free(l1);
}
- free(l1);
- }
+ break;
}
return rc;
}
@@ -436,7 +449,7 @@ retry_transaction:
struct timeval tv;
tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
tv.tv_usec = 0;
- (void)wait_for_dev_destroy(gc, &tv);
+ rc = wait_for_dev_destroy(gc, &tv);
xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
} else {
rc = 1; /* Caller must wait_for_dev_destroy */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2011-Oct-19 11:54 UTC
[Xen-devel] Re: [PATCH] libxl: handle the return value of wait_for_dev_destroy select and pass it to caller function
On Wed, 2011-10-19 at 12:43 +0100, Roger Pau Monne wrote:> # HG changeset patch > # User Roger Pau Monne <roger.pau@entel.upc.edu> > # Date 1319024152 -7200 > # Node ID 5941638b04693f7d8bfa2d6b5563132f54942a28 > # Parent a2a3c4d7333ec15b818b3403f148ad61c254ea82 > libxl: handle the return value of wait_for_dev_destroy select and pass it to caller function. > > Handle the return value of the select call inside wait_for_dev_destroy properly, and return 0 if the device is removed, or 1 if a timeout or error happened. Use the return value of wait_for_dev_destroy inside libxl__device_remove to properly return from that function. > > This patch should be applied after Ian Campbell''s v3 "libxl: rationalise libxl_device_* APIs". > > Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu> > > diff -r a2a3c4d7333e -r 5941638b0469 tools/libxl/libxl_device.c > --- a/tools/libxl/libxl_device.c Tue Oct 18 13:36:43 2011 +0100 > +++ b/tools/libxl/libxl_device.c Wed Oct 19 13:35:52 2011 +0200 > @@ -367,6 +367,7 @@ int libxl__device_disk_dev_number(const > return -1; > } > > +/* Returns 0 if a device is removed, 1 if an error or timeout occurred */ > static int wait_for_dev_destroy(libxl__gc *gc, struct timeval *tv) > { > libxl_ctx *ctx = libxl__gc_owner(gc); > @@ -436,7 +449,7 @@ retry_transaction: > struct timeval tv; > tv.tv_sec = LIBXL_DESTROY_TIMEOUT; > tv.tv_usec = 0; > - (void)wait_for_dev_destroy(gc, &tv); > + rc = wait_for_dev_destroy(gc, &tv); > xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev)); > } else { > rc = 1; /* Caller must wait_for_dev_destroy */If wait_for_dev_destroy returns 1 (error or timeout) then this will get returned to the caller, where it means "you must wait". But they will not be expecting this (because they passed wait==1) and even if they were having them call wait_for_dev_destroy when it has already timed out doesn''t seem likely. I think the right thing is for wait_for_dev_destroy to return 0 or an ERROR_* where one of the ERROR_* can be ERROR_TIMEDOUT and for this to be propagated to the caller. The caller in libxl__devices_destroy needs to do something with this return value as well. It would be good to consume all the watches but return the first error, or something like that. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel