diff options
author | Lennart Poettering <lennart@poettering.net> | 2020-09-25 15:22:48 +0200 |
---|---|---|
committer | Lennart Poettering <lennart@poettering.net> | 2020-10-22 14:58:27 +0200 |
commit | 95c5009248086f8a6fd86808654072adb1395afa (patch) | |
tree | 6fdbaf0e9d526d5baa71cf4c3722ece80d4505ad /src/shared/loop-util.c | |
parent | 738f29cb53b457ca0c17885f119de5bc1f10dead (diff) | |
download | systemd-95c5009248086f8a6fd86808654072adb1395afa.tar.gz |
loop-util: LOOP_CLR_FD is async, don't retry to reuse a device right after issuing it
When we fall back to classic LOOP_SET_FD logic in case LOOP_CONFIGURE
didn't work we issue LOOP_CLR_FD first. But that call turns out to be
potentially async in the kernel: if something else (let's say
udev/blkid) is accessing the device the ioctl just sets the autoclear
flag and exits. Hence quite often the LOOP_SET_FD will subsequently
fail. Let's avoid the trouble, and immediately exit with EBUSY if
LOOP_CONFIGURE fails, and but remember that LOOP_CONFIGURE is not
available so that on the next iteration we go directly for LOOP_SET_FD
instead.
Diffstat (limited to 'src/shared/loop-util.c')
-rw-r--r-- | src/shared/loop-util.c | 97 |
1 files changed, 58 insertions, 39 deletions
diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 156a41dd3a..e4b44ab89b 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -33,60 +33,78 @@ static void cleanup_clear_loop_close(int *fd) { (void) safe_close(*fd); } -static int loop_configure(int fd, const struct loop_config *c) { +static int loop_configure( + int fd, + const struct loop_config *c, + bool *try_loop_configure) { + _cleanup_close_ int lock_fd = -1; int r; assert(fd >= 0); assert(c); + assert(try_loop_configure); + + if (*try_loop_configure) { + if (ioctl(fd, LOOP_CONFIGURE, c) < 0) { + /* Do fallback only if LOOP_CONFIGURE is not supported, propagate all other + * errors. Note that the kernel is weird: non-existing ioctls currently return EINVAL + * rather than ENOTTY on loopback block devices. They should fix that in the kernel, + * but in the meantime we accept both here. */ + if (!ERRNO_IS_NOT_SUPPORTED(errno) && errno != EINVAL) + return -errno; - if (ioctl(fd, LOOP_CONFIGURE, c) < 0) { - /* Do fallback only if LOOP_CONFIGURE is not supported, propagate all other errors. Note that - * the kernel is weird: non-existing ioctls currently return EINVAL rather than ENOTTY on - * loopback block devices. They should fix that in the kernel, but in the meantime we accept - * both here. */ - if (!ERRNO_IS_NOT_SUPPORTED(errno) && errno != EINVAL) - return -errno; - } else { - bool good = true; + *try_loop_configure = false; + } else { + bool good = true; - if (c->info.lo_sizelimit != 0) { - /* Kernel 5.8 vanilla doesn't properly propagate the size limit into the block - * device. If it's used, let's immediately check if it had the desired effect - * hence. And if not use classic LOOP_SET_STATUS64. */ - uint64_t z; + if (c->info.lo_sizelimit != 0) { + /* Kernel 5.8 vanilla doesn't properly propagate the size limit into the + * block device. If it's used, let's immediately check if it had the desired + * effect hence. And if not use classic LOOP_SET_STATUS64. */ + uint64_t z; - if (ioctl(fd, BLKGETSIZE64, &z) < 0) { - r = -errno; - goto fail; - } + if (ioctl(fd, BLKGETSIZE64, &z) < 0) { + r = -errno; + goto fail; + } - if (z != c->info.lo_sizelimit) { - log_debug("LOOP_CONFIGURE is broken, doesn't honour .lo_sizelimit. Falling back to LOOP_SET_STATUS64."); - good = false; + if (z != c->info.lo_sizelimit) { + log_debug("LOOP_CONFIGURE is broken, doesn't honour .lo_sizelimit. Falling back to LOOP_SET_STATUS64."); + good = false; + } } - } - if (FLAGS_SET(c->info.lo_flags, LO_FLAGS_PARTSCAN)) { - /* Kernel 5.8 vanilla doesn't properly propagate the partition scanning flag into the - * block device. Let's hence verify if things work correctly here before - * returning. */ + if (FLAGS_SET(c->info.lo_flags, LO_FLAGS_PARTSCAN)) { + /* Kernel 5.8 vanilla doesn't properly propagate the partition scanning flag + * into the block device. Let's hence verify if things work correctly here + * before returning. */ + + r = blockdev_partscan_enabled(fd); + if (r < 0) + goto fail; + if (r == 0) { + log_debug("LOOP_CONFIGURE is broken, doesn't honour LO_FLAGS_PARTSCAN. Falling back to LOOP_SET_STATUS64."); + good = false; + } + } - r = blockdev_partscan_enabled(fd); - if (r < 0) + if (!good) { + /* LOOP_CONFIGURE doesn't work. Remember that. */ + *try_loop_configure = false; + + /* We return EBUSY here instead of retrying immediately with LOOP_SET_FD, + * because LOOP_CLR_FD is async: if the operation cannot be executed right + * away it just sets the autoclear flag on the device. This means there's a + * good chance we cannot actually reuse the loopback device right-away. Hence + * let's assume it's busy, avoid the trouble and let the calling loop call us + * again with a new, likely unused device. */ + r = -EBUSY; goto fail; - if (r == 0) { - log_debug("LOOP_CONFIGURE is broken, doesn't honour LO_FLAGS_PARTSCAN. Falling back to LOOP_SET_STATUS64."); - good = false; } - } - if (good) return 0; - - /* Otherwise, undo the attachment and use the old APIs */ - if (ioctl(fd, LOOP_CLR_FD) < 0) - return -errno; + } } /* Since kernel commit 5db470e229e22b7eda6e23b5566e532c96fb5bc3 (kernel v5.0) the LOOP_SET_STATUS64 @@ -143,6 +161,7 @@ int loop_device_make( LoopDevice **ret) { _cleanup_free_ char *loopdev = NULL; + bool try_loop_configure = true; struct loop_config config; LoopDevice *d = NULL; struct stat st; @@ -233,7 +252,7 @@ int loop_device_make( if (!IN_SET(errno, ENOENT, ENXIO)) return -errno; } else { - r = loop_configure(loop, &config); + r = loop_configure(loop, &config, &try_loop_configure); if (r >= 0) { loop_with_fd = TAKE_FD(loop); break; |