[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH] losetup/mount: Revise to use /dev/loop-control and thereby fix 'losetup -f'.
From: Denys Vlasenko <vda.linux () googlemail ! com>
Date: 2019-06-09 21:22:18
Message-ID: CAK1hOcMvXV2EBFM6mENuuM94tpbLrB3i3ewcLawVaX8cxiAeWw () mail ! gmail ! com
[Download RAW message or body]
Looks too big. Please try current git, it has a smaller version.
Please describe unwanted behavior (which you are fixing) more clearly
On Tue, Mar 5, 2019 at 12:06 PM Nicolas Hüppelshäuser
<nicolas.hueppelshaeuser@emlix.com> wrote:
>
> Fixed 'losetup -f' and 'losetup -f <file>' issue
> =================================================
>
> With FEATURE_MOUNT_LOOP_CREATE:
>
> On a system with mounted devtmpfs /dev there might be less loop nodes in
> existence than required.
>
> (1) If all existing nodes are in use, 'losetup -f' would correctly return
> the next free node, but it would not create that node.
>
> (2) If all existing nodes are in use, 'losetup -f <file>' would also not
> create the required node file.
>
> Why revise to use /dev/loop-control
> ====================================
>
> (1) Because using /dev/loop-control puts the Linux kernel in charge to
> ensure the returned free node is an existing file and busybox does not
> need to create new files below /dev.
>
> (2) Because libbb/loop.c:set_loop() already contained a "to be done"
> note to switch from iterating the existing nodes to using
> /dev/loop-control.
>
> This fix/feature is activated via config option
> FEATURE_MOUNT_LOOP_LOOP_CONTROL (enabled by default).
>
> Signed-off-by: Nicolas Hüppelshäuser <nicolas.hueppelshaeuser@emlix.com>
> ---
> include/libbb.h | 3 +
> libbb/loop.c | 165 +++++++++++++++++++++++++++++++++++-------
> util-linux/Config.src | 14 +++-
> util-linux/losetup.c | 16 +++-
> 4 files changed, 169 insertions(+), 29 deletions(-)
>
> diff --git a/include/libbb.h b/include/libbb.h
> index daa96728b..12b73b7d3 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -1447,6 +1447,9 @@ extern void bb_warn_ignoring_args(char *arg) FAST_FUNC;
> extern int get_linux_version_code(void) FAST_FUNC;
>
> extern char *query_loop(const char *device) FAST_FUNC;
> +#if ENABLE_FEATURE_MOUNT_LOOP_LOOP_CONTROL
> +extern char *get_loop(void) FAST_FUNC;
> +#endif
> extern int del_loop(const char *device) FAST_FUNC;
> /*
> * If *devname is not NULL, use that name, otherwise try to find free one,
> diff --git a/libbb/loop.c b/libbb/loop.c
> index c78535a20..3f7315ca1 100644
> --- a/libbb/loop.c
> +++ b/libbb/loop.c
> @@ -65,6 +65,28 @@ char* FAST_FUNC query_loop(const char *device)
> return dev;
> }
>
> +#if ENABLE_FEATURE_MOUNT_LOOP_LOOP_CONTROL
> +/* Obtain an unused loop device node */
> +char* FAST_FUNC get_loop(void)
> +{
> + int fd;
> + int loopdevno;
> +
> + fd = open("/dev/loop-control", O_RDWR | O_CLOEXEC);
> + if (fd == -1) {
> + return NULL;
> + }
> +
> + loopdevno = ioctl(fd, LOOP_CTL_GET_FREE);
> + close(fd);
> + if (loopdevno == -1) {
> + return NULL;
> + }
> +
> + return xasprintf(LOOP_FORMAT, loopdevno);
> +}
> +#endif
> +
> int FAST_FUNC del_loop(const char *device)
> {
> int fd, rc;
> @@ -78,6 +100,120 @@ int FAST_FUNC del_loop(const char *device)
> return rc;
> }
>
> +/*
> + * Local helper function used by both implementations of set_loop()
> + * to associate a file with a loop device.
> + * loopfd file descriptor of loop device
> + * filefd file descriptor of file to bind to loop device
> + * file file name of filefd
> + * flags flags used to open() file with
> + * offset to be fed into loop_info struct
> + * Returns 0 on success, -1 on error.
> + */
> +static int FAST_FUNC associate_with_loop(int loopfd, int filefd, const char *file, \
> unsigned flags, unsigned long long offset) +{
> + bb_loop_info loopinfo;
> + int rc = -1;
> +
> + /* Associate free loop device with file. */
> + if (ioctl(loopfd, LOOP_SET_FD, filefd) == 0) {
> + memset(&loopinfo, 0, sizeof(loopinfo));
> + safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
> + loopinfo.lo_offset = offset;
> + /*
> + * Used by mount to set LO_FLAGS_AUTOCLEAR.
> + * LO_FLAGS_READ_ONLY is not set because RO is controlled by open \
> type of the file. + * Note that closing LO_FLAGS_AUTOCLEARed loopfd \
> before mount + * is wrong (would free the loop device!)
> + */
> + loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
> + rc = ioctl(loopfd, BB_LOOP_SET_STATUS, &loopinfo);
> + if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
> + /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
> + /* (this code path is not tested) */
> + loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> + rc = ioctl(loopfd, BB_LOOP_SET_STATUS, &loopinfo);
> + }
> + if (rc != 0) {
> + ioctl(loopfd, LOOP_CLR_FD, 0);
> + }
> + }
> + return rc;
> +}
> +
> +/* two implementations of set_loop() depending on \
> ENABLE_FEATURE_MOUNT_LOOP_LOOP_CONTROL: */ +#if \
> ENABLE_FEATURE_MOUNT_LOOP_LOOP_CONTROL +/* Returns opened fd to the loop device, <0 \
> on error. + * *device is loop device to use, or if *device==NULL finds a loop \
> device to + * mount it on and sets *device to a strdup of that loop device name.
> + */
> +int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offset, \
> unsigned flags) +{
> + char *try;
> + bb_loop_info loopinfo;
> + int dfd, ffd, mode, rc;
> +
> + rc = dfd = -1;
> +
> + /* Open the file. Barf if this doesn't work. */
> + mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
> + open_ffd:
> + ffd = open(file, mode);
> + if (ffd < 0) {
> + if (mode != O_RDONLY) {
> + mode = O_RDONLY;
> + goto open_ffd;
> + }
> + return -errno;
> + }
> +
> + /*
> + * If caller did not provide an explicit loop node file name
> + * we obtain a free loop node; otherwise there is no need
> + * to check for existence of the loop node since the Linux
> + * kernel should have provided one.
> + */
> + try = *device;
> + if (try == NULL) {
> + try = get_loop();
> + if (try == NULL) {
> + close(ffd);
> + return -1;
> + }
> + }
> +
> + /* Open the sucker and check its loopiness. */
> + dfd = open(try, mode);
> + if (dfd < 0 && errno == EROFS) {
> + mode = O_RDONLY;
> + dfd = open(try, mode);
> + }
> + if (dfd < 0) {
> + close(ffd);
> + return -1;
> + }
> +
> + /* If device is free, claim it (LOOP_GET_STATUS returns error number ENXIO \
> in this case). */ + rc = ioctl(dfd, BB_LOOP_GET_STATUS, &loopinfo);
> + if (rc && errno == ENXIO) {
> + rc = associate_with_loop(dfd, ffd, file, flags, offset);
> + } else {
> + rc = -1;
> + }
> +
> + close(ffd);
> + if (rc == 0) {
> + if (!*device)
> + *device = try;
> + return dfd;
> + } else {
> + close(dfd);
> + }
> + return rc;
> +}
> +
> +#else /* if not ENABLE_FEATURE_MOUNT_LOOP_LOOP_CONTROL */
> +
> /* Returns opened fd to the loop device, <0 on error.
> * *device is loop device to use, or if *device==NULL finds a loop device to
> * mount it on and sets *device to a strdup of that loop device name. This
> @@ -106,10 +242,6 @@ int FAST_FUNC set_loop(char **device, const char *file, \
> unsigned long long offse return -errno;
> }
>
> -//TODO: use LOOP_CTL_GET_FREE instead of trying every loopN in sequence? a-la:
> -// fd = open("/dev/loop-control", O_RDWR);
> -// loopN = ioctl(fd, LOOP_CTL_GET_FREE);
> -//
> /* Find a loop device. */
> try = *device ? *device : dev;
> /* 1048575 (0xfffff) is a max possible minor number in Linux circa 2010 */
> @@ -150,29 +282,7 @@ int FAST_FUNC set_loop(char **device, const char *file, \
> unsigned long long offse
> /* If device is free, claim it. */
> if (rc && errno == ENXIO) {
> - /* Associate free loop device with file. */
> - if (ioctl(dfd, LOOP_SET_FD, ffd) == 0) {
> - memset(&loopinfo, 0, sizeof(loopinfo));
> - safe_strncpy((char *)loopinfo.lo_file_name, file, \
> LO_NAME_SIZE);
> - loopinfo.lo_offset = offset;
> - /*
> - * Used by mount to set LO_FLAGS_AUTOCLEAR.
> - * LO_FLAGS_READ_ONLY is not set because RO is \
> controlled by open type of the file.
> - * Note that closing LO_FLAGS_AUTOCLEARed dfd \
> before mount
> - * is wrong (would free the loop device!)
> - */
> - loopinfo.lo_flags = (flags & \
> ~BB_LO_FLAGS_READ_ONLY);
> - rc = ioctl(dfd, BB_LOOP_SET_STATUS, &loopinfo);
> - if (rc != 0 && (loopinfo.lo_flags & \
> BB_LO_FLAGS_AUTOCLEAR)) {
> - /* Old kernel, does not support \
> LO_FLAGS_AUTOCLEAR? */
> - /* (this code path is not tested) */
> - loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> - rc = ioctl(dfd, BB_LOOP_SET_STATUS, \
> &loopinfo);
> - }
> - if (rc != 0) {
> - ioctl(dfd, LOOP_CLR_FD, 0);
> - }
> - }
> + rc = associate_with_loop(dfd, ffd, file, flags, offset);
> } else {
> rc = -1;
> }
> @@ -190,3 +300,4 @@ int FAST_FUNC set_loop(char **device, const char *file, \
> unsigned long long offse }
> return rc;
> }
> +#endif /* if ENABLE_FEATURE_MOUNT_LOOP_LOOP_CONTROL */
> diff --git a/util-linux/Config.src b/util-linux/Config.src
> index 0fad3e5c0..5f5121ccf 100644
> --- a/util-linux/Config.src
> +++ b/util-linux/Config.src
> @@ -27,9 +27,21 @@ config FEATURE_MOUNT_LOOP
> specify an offset or cryptographic options to the loopback device.
> (If you don't want umount to free the loop device, use "umount -D".)
>
> +config FEATURE_MOUNT_LOOP_LOOP_CONTROL
> + bool "Use Linux kernel /dev/loop-control for handling loopback devices"
> + default y
> + depends on FEATURE_MOUNT_LOOP && !FEATURE_MOUNT_LOOP_CREATE
> + help
> + Linux kernels >= 3.1 provide the /dev/loop-control device,
> + which permits an application to dynamically find a free device, and
> + to add and remove loop devices from the system.
> +
> + This feature lets mount (and losetup) use /dev/loop-control to find
> + and add free loop devices.
> +
> config FEATURE_MOUNT_LOOP_CREATE
> bool "Create new loopback devices if needed"
> - default y
> + default n
> depends on FEATURE_MOUNT_LOOP
> help
> Linux kernels >= 2.6.24 support unlimited loopback devices. They are
> diff --git a/util-linux/losetup.c b/util-linux/losetup.c
> index bf480e9bf..8b7124d29 100644
> --- a/util-linux/losetup.c
> +++ b/util-linux/losetup.c
> @@ -99,8 +99,20 @@ int losetup_main(int argc UNUSED_PARAM, char **argv)
> /* contains -f */
> if (opt & OPT_f) {
> char *s;
> - int n = 0;
>
> +#if ENABLE_FEATURE_MOUNT_LOOP_LOOP_CONTROL
> + /* obtain free loop device */
> + s = get_loop();
> + if (s == NULL) {
> + bb_error_msg_and_die("no free loop devices");
> + }
> + if (strlen(s) >= sizeof(dev)) {
> + bb_error_msg_and_die("internal error: LOOP_NAMESIZE too \
> small"); + }
> + strcpy(dev, s);
> + free(s);
> +#else /* if not ENABLE_FEATURE_MOUNT_LOOP_LOOP_CONTROL */
> + int n = 0;
> do {
> if (n > MAX_LOOP_NUM)
> bb_error_msg_and_die("no free loop devices");
> @@ -108,6 +120,8 @@ int losetup_main(int argc UNUSED_PARAM, char **argv)
> s = query_loop(dev);
> free(s);
> } while (s);
> +#endif /* ENABLE_FEATURE_MOUNT_LOOP_LOOP_CONTROL */
> +
> /* now: dev is next free "/dev/loopN" */
> if ((opt == OPT_f) && !argv[0]) {
> puts(dev);
> --
> 2.17.1
>
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic