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

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(a)redhat.com&gt; > --- > 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.