What was bug 1604?

[Rob Landley]

On Tuesday 02 November 2010 15:37:29 Denys Vlasenko wrote:
> On Tuesday 02 November 2010 20:49, Rob Landley wrote:
> > > > Busybox umount was damaged by commit b2e578a1f2c3cf317 which made
> > > > umount leak /dev/loopX entries by default, but didn't stop umount
> > > > from auto-allocating the suckers.  So busybox mount/umount are now
> > > > incoherent. Wheee...
> > >
> > > Yes.
> > >
> > > > I have no idea what the rationale for this change was.
> > >
> > > Users got bitten by the unnecessary discrepancy between bbox umount
> > > and util-linux one.
> >
> > A) I believe util-linux added its -d option _after_ I rewrote the busybox
> > mount and umount commands.  They weren't there when I was looking at it
> > to figure out what to implement.
> >
> > B) There is no mount/umount standard, so I don't see how this could be
> > called "standards compliance".
> >
> > C) I made mount auto-allocate loop devices as appropriate without being
> > told, and similarly clean up after itself by default.  The existence of
> > loop devices is really a leaked implementation detail, something that
> > should be internal to the mount command anyway.  I made it fully
> > automated so you didn't have to care, and was working on a way to specify
> > offsets and encryption keys (don't remember if I actually implemented
> > that) to eliminate the cases where losetup needed to be called directly.
> >
> > I.E. It was a design decision.
> >
> > > They would rather prefer that both utilities, where
> > > possible, have the same options to specify the same thing.
> >
> > They being more than one person?
>
> I don't know. When I put myself in the their shoes, I agree with them:
> migrating from $BIG_DISTRO to busybox and finding that "umount -d"
> suddently starts complaining about unknown option is wrong.
>
> > > In this case,
> > > util-linux umount has -d meaning "free loop device if there is one"
> > > and we had -D "do not free loop device even if there is one".
> >
> > Except that umount -a, or something like my zapchroot script, has no way
> > of knowing which entries are loop devices and which aren't, so you pretty
> > much always have to specify -d, which is harmless when it's _not_ a loop
> > device.
>
> Yes. However, is there a way to still free loop device, but not
> bomb out on -d?
>
> Yes. It's to accept but ignore -d. You are happy (you are not using -d
> anyway). Former users of $BIG_DISTRO are mostly happy: "umount -d" doesn't
> bomb out. (They are a bit unhappy about "umount" without -d not doing what
> it is supposed to do, but are there many users who need that?)
>
> This idea didn't occur to me at the time because I did not know that
> keeping "always free loop device" behavior is important
> for other group of people (namely, you).
>
> We can also return -D option.

The reason I added -D is that since losetup isn't part of mount, it's 
conceivable somebody would want to unmount a loop device without 
disassociating it, if it's got an offset or encryption key or something they 
don't want to have to set back up.  It's by no means the common case, but 
there's a conceivable need for it, so I implemented a way to disable the 
default behavior.

You can't disassociate a loop device while it's still in use, so if you mount 
the same loop device in two places the ioctl should still fail while a mount 
is pinning it.  Yes, I tested this, once upon a time.  Note that you can 
associate the same file with two loop devices, the -o offset thing to handle 
partitions wouldn't work otherwise.  (Somewhere I have a shell script that 
reads a partition table and calculates the correct offset to mount a 
partition...)

I vaguely recall at one point writing code to interate through the loop 
devices and query if any of them were already associated with the current file, 
so I could re-use that one.  But there was a race against disassociating 
between the query and the attempt to use, and the offset might be different, and 
it was only really a good idea for read-only mounts and for read/write ones 
what it should do is _fail_ with a good error message (note this would only 
trigger if you were mounting a file, if you were mounting /dev/loopX directly 
it would detect the type "block device" and not associate a new one, so doing 
losetup by hand and then mounting the resulting loop device would still work 
fine)...

 I think I just put it back on the todo list, actually.  Too many unresolved 
design bits to think through at the time...

By the way, I added the umounting stuff after running _out_ of loop devices 
repeatedly while playing with this stuff.  Since there are generally 9 or so 
you can leak several and not notice, and loopback mounting isn't common enough 
that most people run out before they reboot, so dealing with this at _all_ is 
more out of architectural pride and wanting to get the design to do the right 
thing rather than actually inconveniencing most users.  (I admit it leads to 
some head-scratching the first time loop device exhaustion happens, but my 
extensive abuse of loop devices is a bit unusual and I vaguely recall that the 
util-linux error message at the time was completely unhelpful.  Probably 
improved now...)

> > Adding a config option shouldn't break existing behavior.  You didn't add
> > a config option to umount, you broke compatability with what it was
> > already doing with no way to recover the old behavior.
>
> See above. There is a way to do it. Do you see any holes in my proposal?

At this point, the existing behavior of busybox is -d.  The other behavior 
made more _sense_, but I can't in good conscience recommend breaking it again.

Rob

(Ok, reading the Temeraire novels and then answering email has a noticeable 
impact on my prose.  If Lawrence had been a sysadmin...)
-- 
GPLv3: as worthy a successor as The Phantom Menace, as timely as Duke Nukem 
Forever, and as welcome as New Coke.