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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-04-26 at 15:29 +0100, Roger Pau Monne wrote:
> 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.

Right, this was just the smallest fix to get us back to a working state
and it should certainly be revisited. Dave had a convincing looking
proposal in the "Only call stat() when adding a disk if we expect a
device to exist." thread, once that becomes more concrete we need to
make a call on 4.3 vs 4.4 material.

>  As said by Sylvain,
> tap could also support backends that cannot be stated,

Yes, I sent out V2 to support that.

> and the backend checks we have in libxl are rather fragile and hard to 
> understand IMHO.

Ack.

> 
> > 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