[libvirt] [PATCH] Use loop-control to allocate loopback device.
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@redhat.com> --- configure.ac | 12 +++++++++++ src/util/virfile.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 72 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..f70f9fe 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -528,7 +528,47 @@ 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 +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; + } + + 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."); + } 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; +} int virFileLoopDeviceAssociate(const char *file, char **dev) -- 1.8.1.4
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@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 :|
On Tue, Aug 27, 2013 at 03:28:07PM +0100, Daniel P. Berrange wrote:
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@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;
You bet. To be clear does this mean you don't want to fall back to the search method in the case of an error while using loop-control allocation? My thinking was we'd just give it a go regardless of the issue.. Ian
+ } + + 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 :|
On Tue, Aug 27, 2013 at 10:34:54AM -0700, Ian Main wrote:
On Tue, Aug 27, 2013 at 03:28:07PM +0100, Daniel P. Berrange wrote:
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@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;
You bet. To be clear does this mean you don't want to fall back to the search method in the case of an error while using loop-control allocation? My thinking was we'd just give it a go regardless of the issue..
nm, I see. heh Ian
participants (2)
-
Daniel P. Berrange -
Ian Main