Btrfs devel - Jun 2012 - btrfs filesystem defragment exits with non-zero return code (20) upon success

Hi,

running "btrfs filesystem defrag" somehow always returns a non-zero
exit code,
even when it succeeds:

[lenz@metis btrfs-progs]% sudo ./btrfs filesystem defrag -v /mnt
/mnt
Btrfs v0.19-102-g2482539-dirty
[lenz@metis btrfs-progs]% echo $?
20
[lenz@metis btrfs-progs]% sudo ./btrfs filesystem defrag -v /dev/loop0
/dev/loop0
ERROR: defrag failed on /dev/loop0 - Invalid argument
Btrfs v0.19-102-g2482539-dirty
total 1 failures
[lenz@metis btrfs-progs]% echo $?
1
[lenz@metis btrfs-progs]% sudo ./btrfs filesystem defrag -v /dev/loop0
/dev/loop1
/dev/loop0
ERROR: defrag failed on /dev/loop0 - Invalid argument
/dev/loop1
ERROR: defrag failed on /dev/loop1 - Invalid argument
Btrfs v0.19-102-g2482539-dirty
total 2 failures
[lenz@metis btrfs-progs]% echo $?
1
[lenz@metis btrfs-progs]% sudo ./btrfs filesystem defrag -v /tmp
/tmp
ERROR: defrag failed on /tmp - Inappropriate ioctl for device
Btrfs v0.19-102-g2482539-dirty
total 1 failures
[lenz@metis btrfs-progs]% echo $?
1

I''m no C programmer, but looking at the end of the do_defrag function
in
btrfs_cmds.c, I wonder if the last "return errors + 20" is correct? In
case
that errors is greater than zero, the function would be left via the exit(1)
anyway, wouldn''t it? In that case, wouldn''t "return
0" at the end be more
appropriate?

[SNIP]
  if (errors) {
    fprintf(stderr, "total %d failures\n", errors);
    exit(1);
  }

  free(av);
  return errors + 20;
[SNIP]

Thanks!

-- 
  Lenz Grimmer <lenz@grimmer.com> - http://www.lenzg.net/

On Wed, Jun 27, 2012 at 02:05:55PM +0200, Lenz Grimmer
wrote:
> Hi,
> 
> running "btrfs filesystem defrag" somehow always returns a
non-zero exit code,
> even when it succeeds:
   Yes, this is a known problem, and one that''s on my list of things
to deal with. Thanks for the reminder, though.
 
> I''m no C programmer, but looking at the end of the do_defrag
function in
> btrfs_cmds.c, I wonder if the last "return errors + 20" is
correct? In case
> that errors is greater than zero, the function would be left via the
exit(1)
> anyway, wouldn''t it? In that case, wouldn''t "return
0" at the end be more
> appropriate?
   Yeah, basically, it''s doing something silly and unexpected with
return codes.

   Hugo.
> [SNIP]
>   if (errors) {
>     fprintf(stderr, "total %d failures\n", errors);
>     exit(1);
>   }
> 
>   free(av);
>   return errors + 20;
> [SNIP]
> 
> Thanks!
> 
-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ==  PGP
key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
       --- Mixing mathematics and alcohol is dangerous.  Don''t ---
                            drink and derive.