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

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