On Tue, Jun 23, 2015 at 05:40:39PM +0200, David Sterba wrote:
> On Thu, Jun 18, 2015 at 03:07:09PM -0700, Omar Sandoval wrote:
> > @@ -586,12 +606,13 @@ out:
> > const struct cmd_group device_cmd_group = {
> > device_cmd_group_usage, NULL, {
> > { "add", cmd_add_dev, cmd_add_dev_usage, NULL, 0 },
> > - { "delete", cmd_rm_dev, cmd_rm_dev_usage, NULL, 0 },
> > + { "remove", cmd_rm_dev, cmd_rm_dev_usage, NULL, 0 },
> > { "scan", cmd_scan_dev, cmd_scan_dev_usage, NULL, 0 },
> > { "ready", cmd_ready_dev, cmd_ready_dev_usage, NULL, 0 },
> > { "stats", cmd_dev_stats, cmd_dev_stats_usage, NULL, 0 },
> > { "usage", cmd_device_usage,
> > cmd_device_usage_usage, NULL, 0 },
> > + { "delete", cmd_del_dev, cmd_del_dev_usage, NULL, 0 },
>
> No need to introduce the wrappers, it's enough to add an alternative
> usage string and the callback function will be the same. Also please
> keep the aliased entries next to each other.
So the reason I did that way is that this:
static int cmd_rm_dev(int argc, char **argv)
{
char *mntpnt;
int i, fdmnt, ret=0, e;
DIR *dirstream = NULL;
if (check_argc_min(argc, 3))
usage(cmd_rm_dev_usage);
would use the same usage string for both commands. E.g., if you do
"btrfs device delete", the usage string would say
"btrfs device remove...". That's a small cosmetic issue, but what do you
think?
> Suggestion for separate change: redefine the last argument (currently to
> denote hidden commands) to be a flag set and add a new one for aliases.
> That way we can automatically generate a compact help. Currently, the
> device section looks like this:
>
> btrfs device add [options] <device> [<device>...] <path>
> Add a device to a filesystem
> btrfs device remove <device> [<device>...] <path>
> Remove a device from a filesystem
> btrfs device scan [(-d|--all-devices)|<device> [<device>...]]
> Scan devices for a btrfs filesystem
> btrfs device ready <device>
> Check device to see if it has all of its devices in cache for mounting
> btrfs device stats [-z] <path>|<device>
> Show current device IO stats. -z to reset stats afterwards.
> btrfs device usage [options] <path> [<path>..]
> Show detailed information about internal allocations in devices.
> btrfs device delete <device> [<device>...] <path>
> Remove a device from a filesystem (alias of remove)
>
> For the first version it could be simply the first line of the usage string:
>
> btrfs device add [options] <device> [<device>...] <path>
> Add a device to a filesystem
> btrfs device delete <device> [<device>...] <path>
> btrfs device remove <device> [<device>...] <path>
> Remove a device from a filesystem
> btrfs device scan [(-d|--all-devices)|<device> [<device>...]]
> Scan devices for a btrfs filesystem
> btrfs device ready <device>
> Check device to see if it has all of its devices in cache for mounting
> btrfs device stats [-z] <path>|<device>
> Show current device IO stats. -z to reset stats afterwards.
> btrfs device usage [options] <path> [<path>..]
> Show detailed information about internal allocations in devices.
> Remove a device from a filesystem (alias of remove)
Cool, that sounds like a good idea.
Thanks!
--
Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
