Hello Zenaan Harkness.

Thanks for your bug report.

On Tue, Mar 29, 2016 at 01:03:50PM +1100, Zenaan Harkness wrote:
> Package: mount
> Version: 2.25.2-6
> Severity: normal
> File: /sbin/losetup
> 
> Dear Maintainer,
> 
> Not sure if this is upstream.
> 
> BUG: losetup fails to list loop devices in use
> when /dev/loop directory exists.
> 
> I thought the /dev/loop-control file was all that was needed by
> losetup, but perhaps not - it's been a few years since I perused the
> kernel's loop module source code, which also reminds me, at some point
> it would be nice for kernel loop device support to be fully dynamic -
> for someone inclined, that could be an opportunity to learn to code in
> C and get some kernel cred.
> 
> Background:
> I created a number of scripts a few years back to facilitate working
> with chroots in loop mounted files, run deboostrap in these etc.
> 
> To facilitate my automation I found it convenient to create my own
> loop device nodes in a directory I created, namely:
> /dev/loop/
> 
> Today I finally solved the Subject problem that has arisen (I don't
> know when) with the /sbin/losetup command for me - it fails to list
> info/ list all loop devices (as in produces no output whatsoever)
> when there is a directory named /dev/loop - at least, when I deleted
> this directory, losetup started working properly again.
> 
> >From memory, an 'ancient' version of Debian actually made use of and/ or
> supported loop devices being in the sub directory /dev/loop/ - perhaps
> 5 or more years ago, but don't quote me, my memory may be faulty.
> 
> FWIW, I think it is much tidier to have all loop devices in a sub
> directory of /dev/ , but my personal scripts need their own directory
> anyway, and I think that /dev/loop/ should contain all "default" /
> "auto generated"/ system loop devices - but that's just appearance
> and would probably require changes to many packages, just guessing.
> 
> WORKAROUND: rename loop device directory to /dev/lloop (or
> pick any name other than something beginning with "/dev/loop").

The losetup utility supports both /dev/loopN and /dev/loop/N layout
(the latter likely for compatibility with older kernels/usecases).

Atleast in my testing the attached patch should fix your particular
example, but I'm at the same time not sure how much else it breaks.

I'm inclided to "wontfix" this because as far as I can tell the
problem only exists when you're /mixing/ both new and old way.
That's something I'd just call not supported. I'd refer to using
either or. Either put your loop nodes in a the subdirectory or
don't. (I'd suggest don't.... and don't mess around with setting
things up more manually than you absolutely need. The loop nodes
are usually dynamically created these days so you don't need to
mess with them yourself. The automatic way will DTRT.)

Any comments to convince me to reconsider closing this as wontfix?


Regards,
Andreas Henriksson
diff --git a/lib/loopdev.c b/lib/loopdev.c
index a57e7a7..3cf59c9 100644
--- a/lib/loopdev.c
+++ b/lib/loopdev.c
@@ -107,13 +107,24 @@ int loopcxt_set_device(struct loopdev_cxt *lc, const char *device)
 
 			/* compose device name for /dev/loop<n> or /dev/loop/<n> */
 			if (lc->flags & LOOPDEV_FL_DEVSUBDIR) {
+				int deviceoffset = 4;
+
 				if (strlen(device) < 5)
-					return -1;
-				device += 4;
+					deviceoffset = 0;
 				dir = _PATH_DEV_LOOP "/";	/* _PATH_DEV uses tailing slash */
+				snprintf(lc->device, sizeof(lc->device),
+					"%s%s", dir, device+deviceoffset);
+
+				if (!is_loopdev(lc->device)) {
+					DBG(CXT, ul_debugobj(lc, "%s does not exist, trying without subdir", lc->device));
+					snprintf(lc->device,
+						sizeof(lc->device),
+						"%s%s", _PATH_DEV, device);
+				}
+			} else {
+				snprintf(lc->device, sizeof(lc->device),
+					"%s%s", dir, device);
 			}
-			snprintf(lc->device, sizeof(lc->device), "%s%s",
-				dir, device);
 		} else {
 			strncpy(lc->device, device, sizeof(lc->device));
 			lc->device[sizeof(lc->device) - 1] = '\0';

Reply via email to