[Xen-devel] Re: [PATCH] libxl: handle the return value of wait_for_dev_destroy select and pass it to caller function

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2011-10-19 at 12:43 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> # 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@xxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel