On 04/15/2012 08:19 PM, Denys Vlasenko wrote:
> On Sunday 15 April 2012 17:18, Florian Fainelli wrote:
>> Le 07/04/2012 01:18, Rob Landley a écrit :
>>> On 04/06/2012 01:20 AM, [email protected] wrote:
>>>> Hi Florian !
>>>>
>>>>> 1) loopback mount "foo" to mount /bar
>>>>> 2) umount /bar
>>>>> 3) append new files and re-generate the "foo" cramfs image
>>>>> 4) loopback mount "foo" to mount /bar
>>>>> 5) the contents of /bar are the same as in 1) and not 3)
>>>>
>>>>> Obviously using umount -d in 2) fixes the issue, but I was wondering
>>>>> whether it would not be preferable to unconditionnaly delete the
>>>>> loopback device upon umount? util-linux does this actually, so other
>>>>> users might also be puzzled by such a case.
>>>>
>>>> I hit that too, some time ago, not cramfs but squashfs and ISO images.
>>>> That was the reason I added an "alias umount='umount -d'" to
>>>> my /etc/profile and added the "-d" to all umounts in scripts.
>>>>
>>>> IMHO it would be better to reverse definition of the "-d" option to
>>>> umount and do NOT delete the loop device if option gets specified and
>>>> drop/delete it in the default case.
>>>
>>> You mean the way I originally wrote it before this commit broke it?
>>>
>>> b2e578a1f2c3cf317b391a7d2c059d6a5f5368b8 is the first bad commit
>>> commit b2e578a1f2c3cf317b391a7d2c059d6a5f5368b8
>>> Author: Denis Vlasenko<[email protected]>
>>> Date:   Thu Feb 14 12:00:21 2008 +0000
>>>
>>>      umount: instead of non-standard -D, use -d with opposite meaning
>>>        (closes bug 1604)
>>>
>>> I have no idea what bug 1604 was, but leaking loopback devices was
>>> wrong.  I had code to automatically clean them up, it ran by default,
>>> and now it doesn't.
>>
>> Denys, what do you think about this? I agree with Rob here, not having 
>> the loopback deleted by default is definitively confusing.
> 
> I agree, and I will implement that if util-linux's umount does the same.
> Does it?

Why on earth would that matter?

The position here seems to be "oh no, we can't be better than other
implementations". I do not understand this position.

The busybox mount command I wrote 6 years ago autodetected the need for
--loop when you did "mount file dir".  The util-linux one didn't, and
still doesn't. The busybox behavior was an improvement, especially since
the util-linux mount behavior was STUPID:

  $ sudo mount tccboot.iso reference
  mount: /home/landley/qemu/images/tccboot.iso is not a block device
  (maybe try `-o loop'?)

I.E. it knows what you want to do, and _tells_ you it knows what you
want to do, but refuses to do it unless you say please. I wrote my
implementation to just _do_ it.

Similarly, the busybox umount command also autodetected the need to
deallocate an unmounted loop device. At the time, this was also an
improvement.  Since then, util-linux grew yet more complicated behavior,
from the man page:

> the Loop Device
> The umount command will free the loop device (if any) associated with
> the mount, in case it finds the option 'loop=...' in /etc/mtab, or
> when the -d option was given. Any pending loop devices can be freed
> using 'losetup -d', see losetup(8).

I.E. it looks for loop= in mtab, which you won't get if /etc/mtab is a
symlink to /proc/mounts. It's a complicated, roundabout bit of
unnecessary code, which breaks.

The ioctl you call on loop devices to deallocate them is harmless: if
the device is in use it won't break other uses of it, and it isn't used
by anyone else. And yes, I looked this up at the time: according to
linux/documentation/ioctl/ioctl-decoding.txt bits 15-8 of the ioctl
number are intended to be unique to each driver.  In practice loop.c,
the scsi driver, and the encrypted disk driver have range overlap, but
according to ioctl-number.txt entries 0x00 through 0x0F are only used by
the loop driver, and the ioctl to disassociate a loop device (0x4c01) is
unique to the loopback driver, thus calling it on the block device we're
unmounting will be ignored (return an error) on any other type of block
device.  If the block device is in use elsewhere this will also return
an error and refuse to deallocate the loop device (I tested it). Thus
it's safe to call unconditionally from umount, which is the simple
behavior I implemented in 2006.

That's why I put this comment, which was still there in the commit that
removed -D:

+ // De-allocate the loop device.  This ioctl should be ignored on
+ // any non-loop block devices.

I really don't understand this deference to the limitations of
util-linux. If we have a spec we should document and justify deviations
from the spec. But when all we have is some example implementation, they
can be _wrong_. It doesn't _matter_ what they do, what matters is that
we get the behavior right.

/me wanders back to toybox and stops bothering you...

Rob
-- 
GNU/Linux isn't: Linux=GPLv2, GNU=GPLv3+, they can't share code.
Either it's "mere aggregation", or a license violation.  Pick one.
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to