On 6/2/17 2:52 PM, Darrick J. Wong wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Add freespace mapping tool modelled on the xfs_db freesp command.
> The advantage of this command over xfs_db is that it can be done
> online and is coherent with concurrent modifications to the
> filesystem.
>
> This requires the kernel to support the XFS_IOC_GETFSMAP ioctl to map
> free space indexes.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> [darrick: port from FIEMAPFS to GETFSMAP]
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> +static int
> +init(
> + int argc,
> + char **argv)
> +{
> + int c;
> + int speced = 0;
> +
> + agcount = dumpflag = equalsize = multsize = optind = 0;
> + histcount = seen1 = summaryflag = 0;
ew, setting histcount to 0 (already done, right? it's global?)
and checking it here but never setting it is a little confusing.
Maybe the comment below would help.
> + totblocks = totexts = 0;
> + aglist = NULL;
> + hist = NULL;
> + rtflag = false;
> + while ((c = getopt(argc, argv, "a:bde:h:m:rs")) != EOF) {
> + switch (c) {
> + case 'a':
> + aglistadd(optarg);
> + break;
> + case 'b':
> + if (speced)
> + return 0;
> + multsize = 2;
> + speced = 1;
> + break;
> + case 'd':
> + dumpflag = 1;
> + break;
> + case 'e':
> + if (speced)
> + return 0;
> + equalsize = atoi(optarg);
> + speced = 1;
> + break;
> + case 'h':
> + if (speced && !histcount)
> + return 0;
maybe: /* increments histcount */
> + addhistent(atoi(optarg));
> + speced = 1;
> + break;
> + case 'm':
> + if (speced)
> + return 0;
> + multsize = atoi(optarg);
> + speced = 1;
> + break;
> + case 'r':
> + rtflag = true;
> + break;
> + case 's':
> + summaryflag = 1;
> + break;
> + case '?':
> + return 0;
> + }
> + }
> + if (optind != argc)
> + return 0;
> + if (!speced)
> + multsize = 2;
Manpage indicates that -b, -e, and -h are mutually exclusive, but that's
not enforced here (?)
Specifying more than one seems to break the command:
xfs_spaceman> freesp -b
from to extents blocks pct
2 3 1 3 0.00
32768 65536 4 259537 100.00
xfs_spaceman> freesp -e 4096
from to extents blocks pct
1 4096 1 3 0.00
61441 65536 4 259537 100.00
xfs_spaceman> freesp -e 4096 -b
xfs_spaceman>
Oh I see, if more than one is "speced" init returns 0 (silently).
I'd document what speced is for, and I guess print a warning about
the exclusive nature of the 3 arguments in each case if re-spec'd
before returning.
And note the exclusive nature somehow in the help.
(hm, looks like -m is in this exclusive group too?)
in gneneral need to sanitize behavior w/ built in help as well
as manpage...
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
