util-linux/umount.c umount error: Invalid argument

[Christian Melki]

Rob Landley wrote:
> On Friday 18 August 2006 8:33 am, Christian Melki wrote:
>>>> Shouldn't the code use m->dir in the places of "path" in the umount? 
>>> If /etc/mtab exists, yes.  But if /etc/mtab doesn't exist, it still
>>> needs to 
>>> umount something.
>> Ok. Hmm.. I guess it depends on how nice you want to be against the user..
> 
> "If you umount /proc, you can't umount anything else afterwards" is a heck of 
> a side effect, and not one I consider acceptable.  When you maintain 
> an /etc/mtab you don't have that problem, but I want to encourage people 
> _not_ to use that old legacy crap.  And introducing large unobvious side 
> effects is _not_ a good way to wean people off the legacy stuff.

Of course you shouldnt encourage legacy stuff. That wasn't my point. My 
point was: If we dont find mtab or proc/mounts. We shouldnt assume that 
the user is stupid for not having a "db" of mountpaths and doesnt know 
mount paths himself. The umount should still succeed without mtab. A 
really small embedded system might not want mtab or /proc... I dont 
considering umount not failing without a mtab a side effect...

>> My idea of "umount flow" would be something like this.. 
>> 1. Parse path from commandline, expand it. quit if fail.
> 
> Define "expand" and "fail".

expand in the sense of realpath or abspath if you please. Failing 
meaning that we should terminate, since the path described doesnt exist 
in our system?

>> 2. Read mtab if it exists (we might be nice and check for proc/mounts
>> (ugly) if the user forgot to symlink /etc/mtab). If we dont find mtab (or
>> proc/mounts), continue with a warning assuming the user is keeping state
>> himself, knowing the exact mount path, double mounts etc (not devices).
> 
> A warning which would make the binary larger and convey little information 
> ("This might or might not work"), and the "it didn't work" case is pretty 
> easy to spot afterwards.

You're right. I just thought issuing "hey, you're doing this without 
mtab" with
verbose or something would be nice..

>> 3. Assuming -a, and assuming the list is  >= 1, loop through all mount
>> paths, umounting them with umount or umount2 if used with flags,
> 
> Off the top of my head you're forgetting the fallbacks for force and read 
> only.

Most probably. :) Im completely new to busybox code and digging in umount.

>> else just 
>> umount or umount2 the lookup of "path" in the parsed list of "devices"
> 
> By "the lookup of path" do you mean the absolute path, or the one we looked up 
> in mtab?  (Which are not necessarily the same.  Absolute path to a block 
> device is converted to an 
...
(missing a scentence?)
the one looked up in mtab if we did find one. otherwise assume that the 
commandline path is correct.

> Another fun little corner case is the guy with the jffs2 partition.  My vague 
> recollection is that was mounted by MTD label because the driver understood 
> that.  The device entry there was NOT A PATH, it was something 
> like "mtd:fred".  So we took the abspath of it, did NOT find that in mtab 
> (because it wasn't an abspath in mtab), and thus couldn't umount his device 
> by device name but only by path, despite it having been _mounted_ by device 
> name because that's what was in fstab...
> 
> Reality is, unfortunately, complicated and full of strange corner cases.

hmm.. maybe one should ignore abspath? or just use it as a safetycheck?

>> | 
>> "paths" as it does now and using the true "dir" in mtab as umount argument.
>>
>> Of course this has to be done while paying attention to loop devices etc.. 
>> Btw. Isn't there a way to check for a loopdev instead of just issuing the
>> ioctl?
> 
> Nope.  The ioctl is specific to loop devices and nothing else should be using 
> that number range (according to Documentation/ioctl-number.txt in the 
> kernel).  So it shouldn't hurt to call that on things that aren't loop 
> devices, it should just fail harmlessly.
> 
> I checked. :)

If you say so. ;) I just reacted agains issuing ioctls against bds that 
that aren't loopdevs. :)

>>>> Umount should look up mtab, either maintained or symlinked to kernel 
>>>> parse it, match "path" for both dir and device and if it finds a 
>>>> appropriate entry, use the "dir" part of that entry to umount it.
>>>> This is not the case.
>>> let's see, if you're doing a "umount /mnt" what happens:
>>>
>>> parse arguments
>>> line 60: open /etc/mtab and read contents into mtl.  This leaves "m" 
>>> initialized to the last element in the list, but the list has been
>>> reversed 
>>> so this is the start of the list for traversal purposes.  If there was 
>>> no /etc/mtab, then m and mtl will be null.
>>> line 73: If ! -a, null out m and make sure we've got at least one
>>> argument.
>>>
>>> line 83 (in the loop starting on line 79): if m is set then we're in
>>> the -a 
>>> case and there was an /etc/mtab.  Not relevant here.
>>> line 85: check for -a, and if we get here we hit EOF on /etc/mtab. 
>>> Again not 
>>> relevant.
>>> Line 87: Stop if we're out of command line arguments.  Again, not
>>> relevant.
>>> line 89: play with path.  Where did path come from?  Set directly from
>>> argv...  
>>> And in this case, that's what winds up in the umount() calls.
>>>
>>> Right, we need a new variable...
>> Or just checking that it isn't empty when reaching the umount calls?
>> Hmm.. Maybe i just misunderstood you..
> 
> I believe we don't want the abspath if we didn't find it in mtab.  (I'm not 
> quite sure about this, I need to set up a test environment ot find out what 
> the various failures are and make sure we handle them, and I haven't got an 
> MTD in my laptop.  I believe qemu 0.8.2 can fake 'em, though...)

Sounds like the correct way based on the MTD section..

>>> Huh, I should add a comment that "umount /dev/hdc" will unmount _every_
>>> instance of hdc, not just the most recent.  Or possibly I should make
>>> it so 
>>> you have to go "umount -a /dev/hdc" in order to get that behavior? 
>>> (I'm open 
>>> to suggestions.)
>> I would prefer -a /dev/hdc for multiple umounts. -a would then naturally
>> become "more that one umount, whatever the case". Instead of the corner
>> case where you have multiple mounts on a device and just assumes that you
>> want to umount them all.. since preferably umount would be called with the
>> path.. not the device.. assuming umount all without -a would be strange.
> 
> Before the 2.6 kernel this wasn't an issue.  In 2.4 a mount point could only 
> be mounted in one place.  The semantics of the mainstream mount pretty much 
> punted.
> 
>>> Yes, I tested this code fairly extensively at one point.  Not sure
>>> when/why it 
>>> stopped working.
>> Maybe the magic bitmunching aliens in the svn repository broke it. Not
>> that uncommon. ;)
> 
> Or the kernel changed out from under me.  It's been doing that.  (I really 
> thought that the kernel handled umount "/dev/hdc", but the 2.6.15 in ubuntu 
> doesn't.  Yes, I've seen kernel changes in the 2.6 series affect mount/umount 
> semantics before. :)

Found this in the umount 2 manual. :)
In Linux 2.3.99-pre7 the  call  umount(device)  was
removed,  leaving only umount(dir) (since now devices can be mounted in
more than one place, so specifying the device does not suffice).

> But in this case, I could easily just be wrong.  Dealing with mount/umount 
> makes my brain hurt after a while because it's constructed entirely of corner 
> cases.  (The new shared subtree stuff _FAILS_ if you feed it the MS_SILENT 
> flag.  That's a clear kernel bug, but we've got to deal with it...)
> 
> Rob