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

Re: [libvirt] [PATCH V3] Use loop-control to allocate loop device.



On Thu, Sep 05, 2013 at 12:33:44PM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 03, 2013 at 11:45:51AM -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
> > we fall 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
> > 
> > V2:
> > - Modified to use a dedicated error return for loop-control allocation
> >   function.
> > - Only do fallback if /dev/loop-control does not exist, otherwise return
> >   error.
> > 
> > V3:
> > - Remove warning about falling back to search technique.
> > 
> > Signed-off-by: Ian Main <imain redhat com>
> > ---
> >  configure.ac       | 12 ++++++++++
> >  src/util/virfile.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 81 insertions(+), 1 deletion(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index ac8cfa1..10cd872 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -913,6 +913,18 @@ if test "$with_lxc" = "yes" || test "$with_lxc" = "check"; then
> >              AC_MSG_ERROR([Required kernel features for LXC were not found])
> >          fi
> >      ])
> > +    AC_LINK_IFELSE([AC_LANG_PROGRAM(
> > +    [[
> > +        #include <sched.h>
> > +        #include <linux/loop.h>
> > +        #include <sys/epoll.h>
> > +    ]], [[
> > +        unshare(!(LOOP_CTL_GET_FREE));
> > +    ]])], [
> > +       AC_DEFINE([HAVE_DECL_LOOP_CTL_GET_FREE], [1],
> > +         [Define to 1 if you have the declaration of `LOOP_CTL_GET_FREE',
> > +         and to 0 if you don't.])
> > +    ])
> >  fi
> >  if test "$with_lxc" = "yes" ; then
> >      AC_DEFINE_UNQUOTED([WITH_LXC], 1, [whether LXC driver is enabled])
> > diff --git a/src/util/virfile.c b/src/util/virfile.c
> > index 2b07ac9..9d42380 100644
> > --- a/src/util/virfile.c
> > +++ b/src/util/virfile.c
> > @@ -528,7 +528,56 @@ int virFileUpdatePerm(const char *path,
> >  
> >  
> >  #if defined(__linux__) && HAVE_DECL_LO_FLAGS_AUTOCLEAR
> > -static int virFileLoopDeviceOpen(char **dev_name)
> > +
> > +#if HAVE_DECL_LOOP_CTL_GET_FREE
> > +
> > +/* virFileLoopDeviceOpenLoopCtl() returns -1 when a real failure has occured
> > + * while in the process of allocating or opening the loop device.  On success
> > + * we return 0 and modify the fd to the appropriate file descriptor.
> > + * If /dev/loop-control does not exist, we return 0 and do not set fd. */
> > +
> > +static int virFileLoopDeviceOpenLoopCtl(char **dev_name, int *fd)
> > +{
> > +    int devnr;
> > +    int ctl_fd;
> > +    char *looppath = NULL;
> > +
> > +    VIR_DEBUG("Opening loop-control device");
> > +    if ((ctl_fd = open("/dev/loop-control", O_RDWR)) < 0) {
> > +        virReportSystemError(errno, "%s",
> > +                             _("Unable to open /dev/loop-control"));
> > +        if (errno == ENOENT) {
> > +            return 0;
> > +        }
> 
> The virReportSystemError needs to come after the errno check
> so we only report the error in the fatal code path.
> 
> > +        return -1;
> > +    }
> > +
> > +    if ((devnr = ioctl(ctl_fd, LOOP_CTL_GET_FREE)) < 0) {
> > +        virReportSystemError(errno, "%s",
> > +                             _("Unable to get free loop device via ioctl"));
> > +        close(ctl_fd);
> > +        return -1;
> > +    }
> > +    close(ctl_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 0;
> > +}
> > +#endif /* HAVE_DECL_LOOP_CTL_GET_FREE */
> > +
> > +static int virFileLoopDeviceOpenSearch(char **dev_name)
> >  {
> >      int fd = -1;
> >      DIR *dh = NULL;
> > @@ -601,6 +650,25 @@ cleanup:
> >      return fd;
> >  }
> >  
> > +static int virFileLoopDeviceOpen(char **dev_name)
> > +{
> > +    int loop_fd = -1;
> > +
> > +#ifdef HAVE_DECL_LOOP_CTL_GET_FREE
> 
> s/ifdef/if/ since HAVE_* is either 0 or 1, never undefined
> 
> > +    if (virFileLoopDeviceOpenLoopCtl(dev_name, &loop_fd) < 0)
> > +        return -1;
> > +
> > +    VIR_DEBUG("Return from loop-control got fd %d\n", loop_fd);
> > +
> > +    if (loop_fd >= 0)
> > +        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;
> > +}
> >  
> >  int virFileLoopDeviceAssociate(const char *file,
> >                                 char **dev)
> 
> I made the two changes and pushed this.
> 
> 
> BTW, run 'make syntax-check' for patches on libvirt - it would have
> told you about the second error.

Thanks Daniel,

	Ian


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