Re: [PATCH v7] libxl: allow 'phy' backend to use empty files

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, 3 Mar 2016, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v7] libxl: allow 'phy' backend to use empty files"):
> > This was introduced by 97ee1f (~5 years ago), but was probably never
> > surfaced because most people used regular files as CDROM images, so the PHY
> > backend was actually never selected. A year ago this was changed, and now
> > regular RAW files are also handled by the PHY backend, which has made this
> > bug suface.
> > 
> > Fix it by allowing empty disks to use the PHY backend, skipping the stat
> > tests.
> 
> > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> > index 8bb5e93..e0a81e3 100644
> > --- a/tools/libxl/libxl_device.c
> > +++ b/tools/libxl/libxl_device.c
> > @@ -196,6 +196,12 @@ static int disk_try_backend(disk_try_backend_args *a,
> >              goto bad_format;
> >          }
> >  
> > +        if (a->disk->format == LIBXL_DISK_FORMAT_EMPTY) {
> > +            LOG(DEBUG, "Disk vdev=%s is empty, skipping physical device check",
> > +                a->disk->vdev);
> > +            return backend;
> 
> This implicitly assumes that every backend can cope with absent
> devices.  I don't think that is necessarily true.  I think this check
> should be in what is now libxl__try_phy_backend.

This check is done inside of a switch branch that's only used by 
LIBXL_DISK_BACKEND_PHY, so it doesn't assume that every backend can handle 
empty files.
 
> And skipping the other tests is probably not right either.  For
> example, checking the backend_domid is probably still necessary (and
> maybe the script too).  For LIBXL_DISK_BACKEND_TAP, we still need to
> check libxl__blktap_enabled.  etc.

I can move the check to a lower place (after the other checks), but those 
are not error checking, they are just checks to make sure the right 
backend is used, just like this one. I don't see how altering their order 
is going to make any difference.

Roger.

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