(fwd) busybox mount -a fix.

[Rob Landley]

On Friday 16 June 2006 3:42 pm, Paul Fox wrote:
> a co-worker noticed that "mount -a" wasn't doing the right
> thing with error codes, and sent me this message to forward
> to the list.  patch attached below.

Does your co-worker have a name?

>  > There are essentially three issues:
>  >
>  > (1) mount -a is exiting with status=1 when all is fine.

How do I reproduce this?  (Could I have a test case please?)  I believe you, I 
just have no way of checking that this actually fixes it.

>  > (2) Because of bug (1) and the placement of a call to
>  > bb_perror_msg in a command path, a stray diagnostic is being
>  > produced indicating that the dummy device given in
>  > "mount -t sysfs sysfs /sys" does not exist.

I believe that was fixed by a one-line patch from Andre I just applied 5 
minutes ago, which admittedly didn't exist when you submitted this but it's 
hard to make much progress on the backlog when there's this constant stream 
of new emails bringing up new issues.  (Tackling them in reverse 
chronological order at the moment just to try to get a few of the _recent_ 
ones done today...)

>  > (3) individual mount errors when performing mount -a are not
>  > recorded, but this was masked by bug 1.

This was a todo item mentioned in the big comment block this patch deletes.  
The mount(1) man page actually lists specific error return codes for this, 
and returning _those_ error codes is a bit of a pain, actually.  Ordinarily I 
wouldn't care, but the 32 vs 64 thing is the only way a script has of knowing 
that some of the mounts worked (if not how many)...

Possibly I should just return the number of failed mounts in the mount -a 
case, and document that as what we do. :)

> i've turned his work into a patch against current svn.  i can't
> remember if there are other mount.c patches floating around or
> not, but i figure rob will have them all in any case.

It's a pretty decent patch.  Looks reasonable and it only adds 2 bytes.  Can 
be cleaned up a bit though.  Let's see.  mount_it_now() returns the rc from 
mount(2), so that's either -1 or 0 according to the man page.  So the only 
positive rc is errno, and if I consolidate singlemount back together (just 
add a report_error: label at the end of do_singlemount() that does what the 
singlemount() wrapper did and have all the "return rc" clauses jump to that, 
and just let return errno return as it was doing...  Hmmm, when does the 
initial value of rc get returned?  When filesystem type is unspecified and 
fslist is empty, meaning both /etc/filesystems and /proc/filesystems have no 
block backed filesystems listed in them.)

Right.

Hmmm, possibly some of this error reporting logic belongs in mount_main?  No, 
that got moved because mount_main() frees some of the memory for the 
arguments before falling through to a common recovery point...  But I cleaned 
that up earlier, and I believe now the only thing it's freeing first is the 
options which we don't print in the error message anyway...  But the mount -a 
case doesn't go to the common recovery point each time through the loop, only 
after exiting the loop.  But the mount -a case is the only one that needs the 
"EBUSY isn't an error" check, and it would be _nice_ if we could avoid 
passing in that extra argument...

But I can't think of a clean way to do that right now.  Ok.

Applied (svn 15455).

Rob
-- 
Never bet against the cheap plastic solution.