From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 1 Sep 2022 13:17:01 +0000 (+0200)
Subject: loop-util: drop code to attach empty file
X-Git-Tag: v252-rc1~275^2~3
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=247738b4f52665d54858355816e9ec911eee17f5;p=thirdparty%2Fsystemd.git

loop-util: drop code to attach empty file

Back when I wrote this code I wasn't aware of BLKPG and what it can do.
Hence I came up with this hack to attach an empty file to delete all
partitions. But today we can do better with BLKPG: let's just explicitly
remove all partitions, and then try again.
---

diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c
index 50177aed27c..81682994de6 100644
--- a/src/shared/loop-util.c
+++ b/src/shared/loop-util.c
@@ -173,10 +173,9 @@ static int loop_configure(
 
         /* Let's see if the device is really detached, i.e. currently has no associated partition block
          * devices. On various kernels (such as 5.8) it is possible to have a loopback block device that
-         * superficially is detached but still has partition block devices associated for it. They only go
-         * away when the device is reattached. (Yes, LOOP_CLR_FD doesn't work then, because officially
-         * nothing is attached and LOOP_CTL_REMOVE doesn't either, since it doesn't care about partition
-         * block devices. */
+         * superficially is detached but still has partition block devices associated for it. Let's then
+         * manually remove the partitions via BLKPG, and tell the caller we did that via EUCLEAN, so they try
+         * again. */
         r = device_has_block_children(d);
         if (r < 0)
                 return r;
@@ -187,8 +186,14 @@ static int loop_configure(
                 if (r > 0)
                         return -EBUSY;
 
-                return -EUCLEAN; /* Bound but children? Tell caller to reattach something so that the
-                                  * partition block devices are gone too. */
+                /* Unbound but has children? Remove all partitions, and report this to the caller, to try
+                 * again, and count this as an attempt. */
+
+                r = block_device_remove_all_partitions(fd);
+                if (r < 0)
+                        return r;
+
+                return -EUCLEAN;
         }
 
         if (*try_loop_configure) {
@@ -340,44 +345,6 @@ fail:
         return r;
 }
 
-static int attach_empty_file(int loop, int nr) {
-        _cleanup_close_ int fd = -1;
-
-        /* So here's the thing: on various kernels (5.8 at least) loop block devices might enter a state
-         * where they are detached but nonetheless have partitions, when used heavily. Accessing these
-         * partitions results in immediatey IO errors. There's no pretty way to get rid of them
-         * again. Neither LOOP_CLR_FD nor LOOP_CTL_REMOVE suffice (see above). What does work is to
-         * reassociate them with a new fd however. This is what we do here hence: we associate the devices
-         * with an empty file (i.e. an image that definitely has no partitions). We then immediately clear it
-         * again. This suffices to make the partitions go away. Ugly but appears to work. */
-
-        log_debug("Found unattached loopback block device /dev/loop%i with partitions. Attaching empty file to remove them.", nr);
-
-        fd = open_tmpfile_unlinkable(NULL, O_RDONLY);
-        if (fd < 0)
-                return fd;
-
-        if (flock(loop, LOCK_EX) < 0)
-                return -errno;
-
-        if (ioctl(loop, LOOP_SET_FD, fd) < 0)
-                return -errno;
-
-        if (ioctl(loop, LOOP_SET_STATUS64, &(struct loop_info64) {
-                                .lo_flags = LO_FLAGS_READ_ONLY|
-                                            LO_FLAGS_AUTOCLEAR|
-                                            LO_FLAGS_PARTSCAN, /* enable partscan, so that the partitions really go away */
-                        }) < 0)
-                return -errno;
-
-        if (ioctl(loop, LOOP_CLR_FD) < 0)
-                return -errno;
-
-        /* The caller is expected to immediately close the loopback device after this, so that the BSD lock
-         * is released, and udev sees the changes. */
-        return 0;
-}
-
 static int loop_device_make_internal(
                 int fd,
                 int open_flags,
@@ -543,12 +510,8 @@ static int loop_device_make_internal(
                                 loop_with_fd = TAKE_FD(loop);
                                 break;
                         }
-                        if (r == -EUCLEAN) {
-                                /* Make left-over partition disappear hack (see above) */
-                                r = attach_empty_file(loop, nr);
-                                if (r < 0 && r != -EBUSY)
-                                        return r;
-                        } else if (r != -EBUSY)
+                        if (!IN_SET(r, -EBUSY, -EUCLEAN)) /* Busy, or some left-over partition devices that
+                                                           * were cleaned up. */
                                 return r;
                 }