On Mon, Oct 02, 2017 at 11:39:05AM +0300, Andrei Borzenkov wrote: > On Mon, Oct 2, 2017 at 11:19 AM, Misono, Tomohiro > <misono.tomohiro@xxxxxxxxxxxxxx> wrote: > > This patch changes "subvol set-default" to also accept the subvolume path > > for convenience. > > > > This is the one of the issue on github: > > https://github.com/kdave/btrfs-progs/issues/35 > > > > If there are two args, they are assumed as subvol id and path to the fs > > (the same as current behavior), and if there is only one arg, it is assumed > > as the path to the subvolume. Therefore there is no ambiguity between subvol > > id and subvol name, which is mentioned in the above issue page. > > > > Only the absolute path to the subvolume is allowed, for the safety when > > multiple filesystems are used. > > > > subvol id is resolved by get_subvol_info() which is used by "subvol show". > > > > change to v2: > > restrict the path to only allow absolute path. > > This is absolutely arbitrary restriction. Why we can do "btrfs > subvolume create ./relative/path" but cannot do "btrfs subvolume > set-default ./relative/path"? Indeed. In fact, it's precisely the _opposite_ of the way that every other command works -- you provide the path to the subvolume in the *current namespace*. This approach would be just a major misfeature at this point. Hugo. > > documents are updated accordingly. > > > > Signed-off-by: Tomohiro Misono <misono.tomohiro@xxxxxxxxxxxxxx> > > --- > > Documentation/btrfs-subvolume.asciidoc | 11 +++---- > > cmds-subvolume.c | 53 +++++++++++++++++++++++++++------- > > 2 files changed, 48 insertions(+), 16 deletions(-) > > > > diff --git a/Documentation/btrfs-subvolume.asciidoc b/Documentation/btrfs-subvolume.asciidoc > > index 5cfe885..7973567 100644 > > --- a/Documentation/btrfs-subvolume.asciidoc > > +++ b/Documentation/btrfs-subvolume.asciidoc > > @@ -142,12 +142,13 @@ you can add \'\+' or \'-' in front of each items, \'+' means ascending, > > for --sort you can combine some items together by \',', just like > > --sort=+ogen,-gen,path,rootid. > > > > -*set-default* <id> <path>:: > > -Set the subvolume of the filesystem <path> which is mounted as > > -default. > > +*set-default* [<path>|<id> <mountpoint>]:: > > +Set the subvolume of the filesystem which is mounted as default. > > + > > -The subvolume is identified by <id>, which is returned by the *subvolume list* > > -command. > > +Only absolute path is allowed to specify the subvolume. > > +Alterantively, the pair of <id> and <mountpoint> can be used. In that > > +case, the subvolume is identified by <id>, which is returned by the > > +*subvolume list* command. The filesystem is specified by <mountpoint>. > > > > *show* <path>:: > > Show information of a given subvolume in the <path>. > > diff --git a/cmds-subvolume.c b/cmds-subvolume.c > > index 666f6e0..dda9e73 100644 > > --- a/cmds-subvolume.c > > +++ b/cmds-subvolume.c > > @@ -803,28 +803,59 @@ out: > > } > > > > static const char * const cmd_subvol_set_default_usage[] = { > > - "btrfs subvolume set-default <subvolid> <path>", > > - "Set the default subvolume of a filesystem", > > + "btrfs subvolume set-default [<path>|<id> <mountpoint>]", > > + "Set the default subvolume of the filesystem mounted as default.", > > + "The subvolume can be specified by its absolute path,", > > + "or the pair of subvolume id and mount point to the filesystem.", > > NULL > > }; > > > > static int cmd_subvol_set_default(int argc, char **argv) > > { > > - int ret=0, fd, e; > > - u64 objectid; > > - char *path; > > - char *subvolid; > > - DIR *dirstream = NULL; > > + int ret = 0; > > + int fd, e; > > + u64 objectid; > > + char *path; > > + char *subvolid; > > + DIR *dirstream = NULL; > > > > clean_args_no_options(argc, argv, cmd_subvol_set_default_usage); > > > > - if (check_argc_exact(argc - optind, 2)) > > + if (check_argc_min(argc - optind, 1) || > > + check_argc_max(argc - optind, 2)) > > usage(cmd_subvol_set_default_usage); > > > > - subvolid = argv[optind]; > > - path = argv[optind + 1]; > > + if (argc - optind == 1) { > > + /* path to the subvolume is specified */ > > + struct root_info ri; > > + char *fullpath; > > > > - objectid = arg_strtou64(subvolid); > > + path = argv[optind]; > > + if (path[0] != '/') { > > + error("only absolute path is allowed"); > > + return 1; > > + } > > + > > + fullpath = realpath(path, NULL); > > + if (!fullpath) { > > + error("cannot find real path for '%s': %s", > > + path, strerror(errno)); > > + return 1; > > + } > > + > > + ret = get_subvol_info(fullpath, &ri); > > + free(fullpath); > > + > > + if (ret) > > + return 1; > > + > > + objectid = ri.root_id; > > + } else { > > + /* subvol id and path to the filesystem are specified */ > > + subvolid = argv[optind]; > > + path = argv[optind + 1]; > > + objectid = arg_strtou64(subvolid); > > + } > > > > fd = btrfs_open_dir(path, &dirstream, 1); > > if (fd < 0) -- Hugo Mills | Great oxymorons of the world, no. 4: hugo@... carfax.org.uk | Future Perfect http://carfax.org.uk/ | PGP: E2AB1DE4 |
Attachment:
signature.asc
Description: Digital signature
