[PATCH] size reduction and clean up swaponoff.c

Rob Landley rob at landley.net
Sat Sep 17 10:39:38 UTC 2005


On Friday 16 September 2005 15:10, Mike Frysinger wrote:

> > > i did play with it, `swapon -a` and `swapoff -a` from util-linux will
> > > error if fstab lists a path which doesnt exist ... the -a option doesnt
> > > suppress all errors, just the ones that make sense to supress
> >
> > Yup.  Unless I'm misreading it, the patch you proposed suppressed all
> > errors for the -a case.
>
> you did read it slightly wrong ;)

Yeah, I suspected that.  I was looking just at the patch, not the context it 
applies to...

> it doesnt suppress the error from stat or the swapfile-has-holes check, but
> it does ignore all results from the swapon() call ... in order to fully
> match the behavior of util-linux, we'd have to check errno after the
> swapon() call, parse /proc/swaps, and compare with /etc/fstab ... but we
> could get pretty damn close if we just check the value of errno and warn
> when (verbose == 1 || errno == EBUSY) ...

I don't see a real need for -v.  In the -a case it's simply not an error for 
the swap file to already be on.

> the only corner case we'd have left is if you have say /dev/hda3 mounted as
> ext3 and have /dev/hda3 listed as swap space in /etc/fstab ... a
> swapon(/dev/hda3) call would too set errno to EBUSY ...

Now that is a problem, but I'd call this a kernel bug -- returning the same 
error for two very different cases.  (If /dev/hda3 is formatted ext3 and I 
attempt to swapon, I get -EINVAL.  The fact that it's mounted changes that is 
_stupid_.  Looking at the implementation I can see why this happens, but it's 
still annoying, and actually just a one-line patch to fix...)

I just submitted a patch to linux-kernel for this.

> > By the way, having looked at the multiple arguments fix, the ret=0 right
> > before that isn't currently needed since the ret!=0 case from
> > getopt_ulflags returns, so we can only get there right now if it is zero,
> > correct?
>
> indeed ... as long as we dont add anymore options like -v :)

I don't see the need for -v.

Rob



More information about the busybox mailing list