btrfs device remove alias
Subsystems (2)
cover btrfs device remove alias
The opposite of btrfs device add is btrfs device delete. This really should be btrfs device remove.
Changes from v1:
- Add support for flags to cmd_struct and a CMD_ALIAS flag which only prints the one-line usage string
- Rearrange the command wrappers in a way that could be made generic if needed
Thanks!
Omar Sandoval (2):
btrfs-progs: replace struct cmd_group->hidden with flags
btrfs-progs: alias btrfs device delete to btrfs device remove
Documentation/btrfs-device.asciidoc | 5 ++++- cmds-device.c | 35 ++++++++++++++++++++++++++--------- cmds-filesystem.c | 2 +- commands.h | 9 +++++++-- help.c | 12 +++++++----- 5 files changed, 45 insertions(+), 18 deletions(-)
Signature · 1 line
2.4.4
PATCH 1/2 btrfs-progs: replace struct cmd_group->hidden with flags
We're also going to want to support aliases, so rather than adding another member, replace "hidden" with a "flags" member.
--- cmds-filesystem.c | 2 +- commands.h | 8 ++++++-- help.c | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-)
cmds-filesystem.c +1 -1
@@ -1327,7 +1327,7 @@ const struct cmd_group filesystem_cmd_group = { { "show", cmd_show, cmd_show_usage, NULL, 0 }, { "sync", cmd_sync, cmd_sync_usage, NULL, 0 }, { "defragment", cmd_defrag, cmd_defrag_usage, NULL, 0 }, - { "balance", cmd_balance, NULL, &balance_cmd_group, 1 }, + { "balance", cmd_balance, NULL, &balance_cmd_group, CMD_HIDDEN }, { "resize", cmd_resize, cmd_resize_usage, NULL, 0 }, { "label", cmd_label, cmd_label_usage, NULL, 0 }, { "usage", cmd_filesystem_usage,
commands.h +6 -2
@@ -17,6 +17,10 @@ #ifndef __BTRFS_COMMANDS_H__ #define __BTRFS_COMMANDS_H__ +enum { + CMD_HIDDEN = (1 << 0), /* should not be in help listings */ +}; + struct cmd_struct { const char *token; int (*fn)(int, char **);
@@ -47,8 +51,8 @@ struct cmd_struct {/* should be NULL if token is not a subgroup */ const struct cmd_group *next; -/* if true don't list this token in help listings */ -int hidden; +/* CMD_* flags above */ +int flags; };#define NULL_CMD_STRUCT {NULL, NULL, NULL, NULL, 0}
help.c +1 -1
@@ -133,7 +133,7 @@ static void usage_command_group_internal(const struct cmd_group *grp, int full,int do_sep =0 ;for (; cmd->token; cmd++) { -if (cmd->hidden) +if (cmd->flags &CMD_HIDDEN )continue ;if (full && cmd != grp->commands)
Signature · 1 line
2.4.4
PATCH 2/2 btrfs-progs: alias btrfs device delete to btrfs device remove
In reply to Omar SandovalThere's an awkward asymmetry between btrfs device add and btrfs device delete. Resolve this by aliasing delete to remove.
--- Documentation/btrfs-device.asciidoc | 5 ++++- cmds-device.c | 35 ++++++++++++++++++++++++++--------- commands.h | 1 + help.c | 10 ++++++---- 4 files changed, 37 insertions(+), 14 deletions(-)
Documentation/btrfs-device.asciidoc +4 -1
@@ -74,9 +74,12 @@ do not perform discard by default -f|--force:::: force overwrite of existing filesystem on the given disk(s) -*delete* <dev> [<dev>...] <path>:: +*remove* <dev> [<dev>...] <path>:: Remove device(s) from a filesystem identified by <path>. +*delete* <dev> [<dev>...] <path>:: +Alias of remove kept for backwards compatability + *ready* <device>:: Check device to see if it has all of it's devices in cache for mounting.
cmds-device.c +26 -9
@@ -144,20 +144,14 @@ error_out:return !!ret; } -static const char *const cmd_rm_dev_usage[] = { -"btrfs device delete <device> [<device>...] <path>" , -"Remove a device from a filesystem" , -NULL -}; - -static int cmd_rm_dev (int argc,char **argv) +static int _cmd_rm_dev (int argc,char **argv,const char *const *usagestr) {char *mntpnt;int i, fdmnt, ret=0 , e;DIR *dirstream =NULL ;if (check_argc_min (argc,3 )) -usage (cmd_rm_dev_usage); +usage (usagestr); mntpnt = argv[argc -1 ];
@@ -198,6 +192,28 @@ static int cmd_rm_dev(int argc, char **argv)return !!ret; } +static const char *const cmd_rm_dev_usage[] = { +"btrfs device remove <device> [<device>...] <path>" , +"Remove a device from a filesystem" , +NULL +}; + +static int cmd_rm_dev (int argc,char **argv) +{ +return _cmd_rm_dev (argc, argv, cmd_rm_dev_usage); +} + +static const char *const cmd_del_dev_usage[] = { +"btrfs device delete <device> [<device>...] <path>" , +"Remove a device from a filesystem" , +NULL +}; + +static int cmd_del_dev (int argc,char **argv) +{ +return _cmd_rm_dev (argc, argv, cmd_del_dev_usage); +} +static const char *const cmd_scan_dev_usage[] = {"btrfs device scan [(-d|--all-devices)|<device> [<device>...]]" ,"Scan devices for a btrfs filesystem" ,
@@ -586,7 +602,8 @@ 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 }, + {"delete" , cmd_del_dev, cmd_del_dev_usage,NULL ,CMD_ALIAS }, + {"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 },
commands.h +1 -0
@@ -19,6 +19,7 @@enum {CMD_HIDDEN = (1 <<0 ),/* should not be in help listings */ +CMD_ALIAS = (1 <<1 ),/* alias of next command in cmd_group */ };struct cmd_struct {
help.c +6 -4
@@ -81,11 +81,13 @@ static int do_usage_one_command(const char * const *usagestr,static int usage_command_internal (const char *const *usagestr,const char *token,int full,int lst, -FILE *outf) +int alias,FILE *outf) { -unsigned int flags =USAGE_SHORT ; +unsigned int flags =0 ;int ret; +if (!alias) + flags |=USAGE_SHORT ;if (full) flags |=USAGE_LONG |USAGE_OPTIONS ;if (lst)
@@ -110,7 +112,7 @@ static void usage_command_usagestr(const char * const *usagestr,FILE *outf = err ? stderr : stdout;int ret; - ret =usage_command_internal (usagestr, token, full,0 , outf); + ret =usage_command_internal (usagestr, token, full,0 ,0 , outf);if (!ret)fputc ('\n' , outf); }
@@ -146,7 +148,7 @@ static void usage_command_group_internal(const struct cmd_group *grp, int full, }usage_command_internal (cmd->usagestr, cmd->token, full, -1 , outf); +1 , cmd->flags &CMD_ALIAS , outf);continue ; }
Signature · 1 line
2.4.4
Omar Sandoval posted on Wed, 24 Jun 2015 09:09:15 -0700 as excerpted:
Quoted 2 lines
The opposite of btrfs device add is btrfs device delete. This really should be btrfs device remove.
What about btrfs device subtract? That's what _I_'d call the opposite of add. Otherwise, add/remove instead of add/delete seems fine to me. Six of one, half dozen of the other. [shrug]
Signature · 3 lines
Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman
Quoted 2 lines · Omar Sandoval
The opposite of btrfs device add is btrfs device delete. This really should be btrfs device remove.
I think people got used to the 'delete' command over time, but for convenience I don't mind to add the alias. Also you delete files by 'rm' which is short for 'remove' and probably don't mind either.
Quoted 7 lines · David Sterba
Quoted 2 lines · Omar Sandoval
The opposite of btrfs device add is btrfs device delete. This really should be btrfs device remove.
I think people got used to the 'delete' command over time, but for convenience I don't mind to add the alias. Also you delete files by 'rm' which is short for 'remove' and probably don't mind either.
I do agree that people got used to delete, but its one of those things that new users are likely to trip over. And since we're highly unlikely to ever use 'rm' for something other than deletion, it makes sense to just alias them.
-chris
while on this. its also good idea to create alias for
btrfs replace start -> btrfs device replace.
any comments ?
Quoted 22 lines · Omar Sandoval
The opposite of btrfs device add is btrfs device delete. This really should be btrfs device remove.
Changes from v1:
- Add support for flags tocmd_structand aCMD_ALIASflag which only prints the one-line usage string
- Rearrange the command wrappers in a way that could be made generic if neededThanks!
Omar Sandoval (2):
btrfs-progs: replace structcmd_group->hiddenwith flags
btrfs-progs: alias btrfs device delete to btrfs device removeDocumentation/btrfs-device.asciidoc | 5 ++++- cmds-device.c | 35 ++++++++++++++++++++++++++--------- cmds-filesystem.c | 2 +- commands.h | 9 +++++++-- help.c | 12 +++++++----- 5 files changed, 45 insertions(+), 18 deletions(-)
Anand Jain posted on Fri, 26 Jun 2015 09:10:56 +0800 as excerpted:
Quoted 5 lines
while on this. its also good idea to create alias for
btrfs replace start -> btrfs device replace.
any comments ?
That's actually the one that makes more sense to me. Delete/remove/ subtract, all about the same to me, so while I'm not opposed to alias for that, I don't really see the need. But with btrfs device add/remove, having btrfs replace instead of btrfs device replace, makes absolutely no sense to me, so I'm all for that alias.
Signature · 3 lines
Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman
Quoted 3 lines · Anand Jain
while on this. its also good idea to create alias for
btrfs replace start -> btrfs device replace.
This was asked for back then, and briefly discussed on irc (11/2012). The preference was not to do too much typing, although the command hierarchy would become less structured and can cause some trouble when looking for docs.
I'm slightly worried about adding more aliases as it can cause confusion when writing documentaiton and recommending how to do things. But I understand the motivations to make the interface more consistent or convenient to use.
In this case it's moving replace to the expected place. I'm not against it but more feedback would help.
Re: [PATCH 1/2] btrfs-progs: replace struct cmd_group->hidden with flags
In reply to Omar SandovalQuoted 4 lines · Omar Sandoval
We're also going to want to support aliases, so rather than adding another member, replace "hidden" with a "flags" member.
Signed-off-by Omar Sandoval <[email protected]>
Fixed compilation breakage and applied, thanks.
David Sterba
AppliedRe: [PATCH 2/2] btrfs-progs: alias btrfs device delete to btrfs device remove
In reply to Omar SandovalQuoted 4 lines · Omar Sandoval
There's an awkward asymmetry between btrfs device add and btrfs device delete. Resolve this by aliasing delete to remove.
Signed-off-by Omar Sandoval <[email protected]>
Applied, thanks.
Quoted 10 lines · David Sterba
Quoted 3 lines · Anand Jain
while on this. its also good idea to create alias for
btrfs replace start -> btrfs device replace.
This was asked for back then, and briefly discussed on irc (11/2012). The preference was not to do too much typing, although the command hierarchy would become less structured and can cause some trouble when looking for docs.
Quoted 7 lines
I'm slightly worried about adding more aliases as it can cause confusion when writing documentaiton and recommending how to do things. But I understand the motivations to make the interface more consistent or convenient to use.
In this case it's moving replace to the expected place. I'm not against it but more feedback would help.
I agree that btrfs device replace makes sense. The point is what about btrfs
replace status and cancel.
We could add these subcommand to device too, also because it makes sense to extend these to support the remove/delete command. I am not suggesting to do that now, but our decision has to take in account also these (possible ?) further alias.
Goffredo
Signature · 2 lines
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5