* [PATCH 0/2] btrfs device remove alias @ 2015-06-24 16:09 Omar Sandoval 2015-06-24 16:09 ` [PATCH 1/2] btrfs-progs: replace struct cmd_group->hidden with flags Omar Sandoval ` (4 more replies) 0 siblings, 5 replies; 12+ messages in thread From: Omar Sandoval @ 2015-06-24 16:09 UTC (permalink / raw) To: linux-btrfs; +Cc: 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 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(-) -- 2.4.4 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] btrfs-progs: replace struct cmd_group->hidden with flags 2015-06-24 16:09 [PATCH 0/2] btrfs device remove alias Omar Sandoval @ 2015-06-24 16:09 ` Omar Sandoval 2015-06-26 14:34 ` David Sterba 2015-06-24 16:09 ` [PATCH 2/2] btrfs-progs: alias btrfs device delete to btrfs device remove Omar Sandoval ` (3 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Omar Sandoval @ 2015-06-24 16:09 UTC (permalink / raw) To: linux-btrfs; +Cc: 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 <osandov@fb.com> --- cmds-filesystem.c | 2 +- commands.h | 8 ++++++-- help.c | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cmds-filesystem.c b/cmds-filesystem.c index b93bb33028f2..284dc87bc464 100644 --- a/cmds-filesystem.c +++ b/cmds-filesystem.c @@ -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, diff --git a/commands.h b/commands.h index bd23340d45ae..42d31781f1a3 100644 --- a/commands.h +++ b/commands.h @@ -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} diff --git a/help.c b/help.c index 56aaf9c3c64c..34754c16e6fe 100644 --- a/help.c +++ b/help.c @@ -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) -- 2.4.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] btrfs-progs: replace struct cmd_group->hidden with flags 2015-06-24 16:09 ` [PATCH 1/2] btrfs-progs: replace struct cmd_group->hidden with flags Omar Sandoval @ 2015-06-26 14:34 ` David Sterba 0 siblings, 0 replies; 12+ messages in thread From: David Sterba @ 2015-06-26 14:34 UTC (permalink / raw) To: Omar Sandoval; +Cc: linux-btrfs On Wed, Jun 24, 2015 at 09:09:16AM -0700, Omar Sandoval wrote: > 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 <osandov@fb.com> Fixed compilation breakage and applied, thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] btrfs-progs: alias btrfs device delete to btrfs device remove 2015-06-24 16:09 [PATCH 0/2] btrfs device remove alias Omar Sandoval 2015-06-24 16:09 ` [PATCH 1/2] btrfs-progs: replace struct cmd_group->hidden with flags Omar Sandoval @ 2015-06-24 16:09 ` Omar Sandoval 2015-06-26 14:46 ` David Sterba 2015-06-25 4:06 ` [PATCH 0/2] btrfs device remove alias Duncan ` (2 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Omar Sandoval @ 2015-06-24 16:09 UTC (permalink / raw) To: linux-btrfs; +Cc: 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 <osandov@fb.com> --- Documentation/btrfs-device.asciidoc | 5 ++++- cmds-device.c | 35 ++++++++++++++++++++++++++--------- commands.h | 1 + help.c | 10 ++++++---- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/Documentation/btrfs-device.asciidoc b/Documentation/btrfs-device.asciidoc index c56cf5ef48fb..2827598a37f5 100644 --- a/Documentation/btrfs-device.asciidoc +++ b/Documentation/btrfs-device.asciidoc @@ -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. diff --git a/cmds-device.c b/cmds-device.c index 1022656988c2..6972156fdf70 100644 --- a/cmds-device.c +++ b/cmds-device.c @@ -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 }, diff --git a/commands.h b/commands.h index 42d31781f1a3..90bbd05a3542 100644 --- a/commands.h +++ b/commands.h @@ -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 { diff --git a/help.c b/help.c index 34754c16e6fe..6ecf01d57b83 100644 --- a/help.c +++ b/help.c @@ -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; } -- 2.4.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs-progs: alias btrfs device delete to btrfs device remove 2015-06-24 16:09 ` [PATCH 2/2] btrfs-progs: alias btrfs device delete to btrfs device remove Omar Sandoval @ 2015-06-26 14:46 ` David Sterba 0 siblings, 0 replies; 12+ messages in thread From: David Sterba @ 2015-06-26 14:46 UTC (permalink / raw) To: Omar Sandoval; +Cc: linux-btrfs On Wed, Jun 24, 2015 at 09:09:17AM -0700, Omar Sandoval wrote: > 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 <osandov@fb.com> Applied, thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] btrfs device remove alias 2015-06-24 16:09 [PATCH 0/2] btrfs device remove alias Omar Sandoval 2015-06-24 16:09 ` [PATCH 1/2] btrfs-progs: replace struct cmd_group->hidden with flags Omar Sandoval 2015-06-24 16:09 ` [PATCH 2/2] btrfs-progs: alias btrfs device delete to btrfs device remove Omar Sandoval @ 2015-06-25 4:06 ` Duncan 2015-06-25 13:41 ` David Sterba 2015-06-26 1:10 ` Anand Jain 4 siblings, 0 replies; 12+ messages in thread From: Duncan @ 2015-06-25 4:06 UTC (permalink / raw) To: linux-btrfs Omar Sandoval posted on Wed, 24 Jun 2015 09:09:15 -0700 as excerpted: > 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] -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] btrfs device remove alias 2015-06-24 16:09 [PATCH 0/2] btrfs device remove alias Omar Sandoval ` (2 preceding siblings ...) 2015-06-25 4:06 ` [PATCH 0/2] btrfs device remove alias Duncan @ 2015-06-25 13:41 ` David Sterba 2015-06-25 13:45 ` Chris Mason 2015-06-26 1:10 ` Anand Jain 4 siblings, 1 reply; 12+ messages in thread From: David Sterba @ 2015-06-25 13:41 UTC (permalink / raw) To: Omar Sandoval; +Cc: linux-btrfs On Wed, Jun 24, 2015 at 09:09:15AM -0700, Omar Sandoval wrote: > 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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] btrfs device remove alias 2015-06-25 13:41 ` David Sterba @ 2015-06-25 13:45 ` Chris Mason 0 siblings, 0 replies; 12+ messages in thread From: Chris Mason @ 2015-06-25 13:45 UTC (permalink / raw) To: dsterba, Omar Sandoval, linux-btrfs On Thu, Jun 25, 2015 at 03:41:51PM +0200, David Sterba wrote: > On Wed, Jun 24, 2015 at 09:09:15AM -0700, Omar Sandoval wrote: > > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] btrfs device remove alias 2015-06-24 16:09 [PATCH 0/2] btrfs device remove alias Omar Sandoval ` (3 preceding siblings ...) 2015-06-25 13:41 ` David Sterba @ 2015-06-26 1:10 ` Anand Jain 2015-06-26 4:21 ` Duncan 2015-06-26 13:33 ` David Sterba 4 siblings, 2 replies; 12+ messages in thread From: Anand Jain @ 2015-06-26 1:10 UTC (permalink / raw) To: Omar Sandoval, linux-btrfs; +Cc: David Sterba, Chris Mason while on this. its also good idea to create alias for btrfs replace start -> btrfs device replace. any comments ? On 06/25/2015 12:09 AM, Omar Sandoval wrote: > 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(-) > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] btrfs device remove alias 2015-06-26 1:10 ` Anand Jain @ 2015-06-26 4:21 ` Duncan 2015-06-26 13:33 ` David Sterba 1 sibling, 0 replies; 12+ messages in thread From: Duncan @ 2015-06-26 4:21 UTC (permalink / raw) To: linux-btrfs Anand Jain posted on Fri, 26 Jun 2015 09:10:56 +0800 as excerpted: > 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. -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] btrfs device remove alias 2015-06-26 1:10 ` Anand Jain 2015-06-26 4:21 ` Duncan @ 2015-06-26 13:33 ` David Sterba 2015-06-27 11:38 ` Goffredo Baroncelli 1 sibling, 1 reply; 12+ messages in thread From: David Sterba @ 2015-06-26 13:33 UTC (permalink / raw) To: Anand Jain; +Cc: Omar Sandoval, linux-btrfs, David Sterba, Chris Mason On Fri, Jun 26, 2015 at 09:10:56AM +0800, Anand Jain wrote: > 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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] btrfs device remove alias 2015-06-26 13:33 ` David Sterba @ 2015-06-27 11:38 ` Goffredo Baroncelli 0 siblings, 0 replies; 12+ messages in thread From: Goffredo Baroncelli @ 2015-06-27 11:38 UTC (permalink / raw) To: dsterba, Anand Jain, Omar Sandoval, linux-btrfs, Chris Mason On 2015-06-26 15:33, David Sterba wrote: > On Fri, Jun 26, 2015 at 09:10:56AM +0800, Anand Jain wrote: >> 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. 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 -- gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-06-27 11:38 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-24 16:09 [PATCH 0/2] btrfs device remove alias Omar Sandoval 2015-06-24 16:09 ` [PATCH 1/2] btrfs-progs: replace struct cmd_group->hidden with flags Omar Sandoval 2015-06-26 14:34 ` David Sterba 2015-06-24 16:09 ` [PATCH 2/2] btrfs-progs: alias btrfs device delete to btrfs device remove Omar Sandoval 2015-06-26 14:46 ` David Sterba 2015-06-25 4:06 ` [PATCH 0/2] btrfs device remove alias Duncan 2015-06-25 13:41 ` David Sterba 2015-06-25 13:45 ` Chris Mason 2015-06-26 1:10 ` Anand Jain 2015-06-26 4:21 ` Duncan 2015-06-26 13:33 ` David Sterba 2015-06-27 11:38 ` Goffredo Baroncelli
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.