I rethink this and conclude that we should only allow the absolute path to
the subvolume in order to prevent setting wrong filesystem by mistake when 
multiple filesystems are used.

I will submit the patch again and please ignore this.

Regards,
Tomohiro

On 2017/10/02 15:25, Misono, Tomohiro 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.
> 
> subvol id is resolved by get_subvol_info() which is used by "subvol show".
> 
> Signed-off-by: Tomohiro Misono <[email protected]>
> ---
>  Documentation/btrfs-subvolume.asciidoc | 10 +++----
>  cmds-subvolume.c                       | 48 
> ++++++++++++++++++++++++++--------
>  2 files changed, 42 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/btrfs-subvolume.asciidoc 
> b/Documentation/btrfs-subvolume.asciidoc
> index 5cfe885..df27460 100644
> --- a/Documentation/btrfs-subvolume.asciidoc
> +++ b/Documentation/btrfs-subvolume.asciidoc
> @@ -142,12 +142,12 @@ 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* [<subvolume>|<id> <path>]::
> +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.
> +If <id> and <path> is given, the subvolume is identified by <id>,
> +which is returned by the *subvolume list* command. The filesystem
> +is specified by <path>.
>  
>  *show* <path>::
>  Show information of a given subvolume in the <path>.
> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
> index 666f6e0..5bf8295 100644
> --- a/cmds-subvolume.c
> +++ b/cmds-subvolume.c
> @@ -803,28 +803,54 @@ 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 [<subvolume>|<subvolid> <path>]",
> +     "Set the default subvolume of the filesystem mounted as default.",
> +     "The subvolume can be specified by its path,",
> +     "or the pair of subvolume id and path 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];
> +             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)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to