[libvirt] [PATCH] Use loop-control to allocate loopback device.

[Daniel P. Berrange]

On Mon, Aug 26, 2013 at 12:30:39PM -0700, Ian Main wrote:
> This patch changes virFileLoopDeviceOpen() to use the new loop-control
> device to allocate a new loop device.  If this behavior is unsupported
> or an error occurs while trying to do this it falls back to the previous
> method of searching /dev for a free device.
> 
> With this patch you can start as many image based LXC domains as you
> like (well almost).
> 
> Fixes bug https://bugzilla.redhat.com/show_bug.cgi?id=995543
> 
> Signed-off-by: Ian Main <imain at redhat.com>
> ---
>  configure.ac       | 12 +++++++++++
>  src/util/virfile.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 72 insertions(+), 1 deletion(-)
> 


>  #if defined(__linux__) && HAVE_DECL_LO_FLAGS_AUTOCLEAR
> -static int virFileLoopDeviceOpen(char **dev_name)
> +
> +#if HAVE_DECL_LOOP_CTL_GET_FREE
> +static int virFileLoopDeviceOpenLoopCtl(char **dev_name)
> +{
> +    int fd = -1;
> +    int devnr;
> +    char *looppath = NULL;
> +
> +    VIR_DEBUG("Opening loop-control device");
> +    if ((fd = open("/dev/loop-control", O_RDWR)) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to open /dev/loop-control"));
> +        return -1;

We need to distinguish ENOENT (which should be non-fatal) from
other (fatal) errors here.

I'd suggest that we add an 'int *fd' parameter to this method,
and use the return value to indicate success / failure only.

eg, 

  if (virFileLoopDeviceOpenLoopCtl(devname, &fd) < 0)
    return -1;
  if (fd == -1)
    fd = virFileLoopDeviceOpenSearch(devname);
  return fd;


> +    }
> +
> +    if ((devnr = ioctl(fd, LOOP_CTL_GET_FREE)) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to get free loop device via ioctl"));
> +        close(fd);
> +        return -1;
> +    }
> +    close(fd);
> +
> +    VIR_DEBUG("Found free loop device number %i", devnr);
> +
> +    if (virAsprintf(&looppath, "/dev/loop%i", devnr) < 0)
> +        return -1;
> +
> +    if ((fd = open(looppath, O_RDWR)) < 0) {
> +        virReportSystemError(errno,
> +                _("Unable to open %s"), looppath);
> +        VIR_FREE(looppath);
> +        return -1;
> +    }
> +
> +    *dev_name = looppath;
> +    return fd;
> +}
> +#endif /* HAVE_DECL_LOOP_CTL_GET_FREE */
> +
> +static int virFileLoopDeviceOpenSearch(char **dev_name)
>  {
>      int fd = -1;
>      DIR *dh = NULL;
> @@ -601,6 +641,25 @@ cleanup:
>      return fd;
>  }
>  
> +static int virFileLoopDeviceOpen(char **dev_name)
> +{
> +    int loop_fd;
> +
> +#ifdef HAVE_DECL_LOOP_CTL_GET_FREE
> +    loop_fd = virFileLoopDeviceOpenLoopCtl(dev_name);
> +    VIR_DEBUG("Return from loop-control got fd %d\n", loop_fd);
> +    if (loop_fd < 0) {
> +        VIR_WARN("loop-control allocation failed, trying search technique.");

See note earlier, about distinguishing fatal from non-fatal errors.

> +    } else {
> +        return loop_fd;
> +    }
> +#endif /* HAVE_DECL_LOOP_CTL_GET_FREE */
> +
> +    /* Without the loop control device we just use the old technique. */
> +    loop_fd = virFileLoopDeviceOpenSearch(dev_name);
> +
> +    return loop_fd;
> +}
>  


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|