[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] libxl: stat the path for all non-qdisk backends (including unknown)



On 26/04/13 16:12, Ian Campbell wrote:
> The commit a8a1f236a296 "libxl: Only call stat() when adding a disk if we
> expect a device to exist." changed things to only stat the file when the phy
> backend was explicitly requested. This broke the case where we are probing and
> would normally be able to decide on the phy option.
> 
> Since the intention of that commit was to allow for qdisk backends with no
> explicit file in dom0 (i.e. network remote backend such as ceph) the lowest
> impact fix appears to be to make that explicit. This should probably be
> revisited to rationalize the probing.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

But I think we should consider this as a band-aid. As said by Sylvain,
tap could also support backends that cannot be stated, and the backend
checks we have in libxl are rather fragile and hard to understand IMHO.

> Cc: dave.scott@xxxxxxxxxx
> Cc: george.dunlap@xxxxxxxxxx
> Cc: roger.pau@xxxxxxxxxx
> Cc: ian.jackson@xxxxxxxxxx
> ---
>  tools/libxl/libxl_device.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index eb60fd5..b01d109 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -236,7 +236,7 @@ int libxl__device_disk_set_backend(libxl__gc *gc, 
> libxl_device_disk *disk) {
>              return ERROR_INVAL;
>          }
>          memset(&a.stab, 0, sizeof(a.stab));
> -    } else if (disk->backend == LIBXL_DISK_BACKEND_PHY &&
> +    } else if (disk->backend != LIBXL_DISK_BACKEND_QDISK &&
>                 disk->backend_domid == LIBXL_TOOLSTACK_DOMID &&
>                 !disk->script) {
>          if (stat(disk->pdev_path, &a.stab)) {
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.