Re: [PATCH v3] btrfs-progs: cmds: subvolume: add -p option on creating subvol

[David Sterba <dsterba@suse.cz>]

On Sun, Feb 20, 2022 at 03:12:54AM +0000, Sidong Yang wrote:
> This patch resolves github issue #429. This patch adds an option that
> create parent directories when it creates subvolumes on nonexisting
> parents. This option tokenizes dstdir and checks each paths whether
> it's existing directory and make directory for creating subvolume.
> 
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
> v2: fixed the case using realpath() that path nonexists, added
> description on docs.
> v3: added longopt, used strncpy_null than strcpy, use perm 0777

I've noticed the issue and this patch, sorry for the delay, now added to
devel.

> ---
>  Documentation/btrfs-subvolume.rst |  5 +++-
>  cmds/subvolume.c                  | 41 ++++++++++++++++++++++++++++++-
>  2 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/btrfs-subvolume.rst b/Documentation/btrfs-subvolume.rst
> index 4591d4bb..2c138154 100644
> --- a/Documentation/btrfs-subvolume.rst
> +++ b/Documentation/btrfs-subvolume.rst
> @@ -49,7 +49,7 @@ do not affect the files in the original subvolume.
>  SUBCOMMAND
>  -----------
>  
> -create [-i <qgroupid>] [<dest>/]<name>
> +create [options] [<dest>/]<name>
>          Create a subvolume *name* in *dest*.
>  
>          If *dest* is not given, subvolume *name* will be created in the current
> @@ -61,6 +61,9 @@ create [-i <qgroupid>] [<dest>/]<name>
>                  Add the newly created subvolume to a qgroup. This option can be given multiple
>                  times.
>  
> +        -p|--parents
> +                Make any missing parent directories for each argument.
> +
>  delete [options] [<subvolume> [<subvolume>...]], delete -i|--subvolid <subvolid> <path>
>          Delete the subvolume(s) from the filesystem.
>  
> diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> index fbf56566..9c13839e 100644
> --- a/cmds/subvolume.c
> +++ b/cmds/subvolume.c
> @@ -88,6 +88,7 @@ static const char * const cmd_subvol_create_usage[] = {
>  	"",
>  	"-i <qgroupid>  add the newly created subvolume to a qgroup. This",
>  	"               option can be given multiple times.",
> +	"-p|--parents   make any missing parent directories for each argument.",
>  	HELPINFO_INSERT_GLOBALS,
>  	HELPINFO_INSERT_QUIET,
>  	NULL
> @@ -105,10 +106,18 @@ static int cmd_subvol_create(const struct cmd_struct *cmd,
>  	char	*dst;
>  	struct btrfs_qgroup_inherit *inherit = NULL;
>  	DIR	*dirstream = NULL;
> +	bool create_parents = false;
>  
>  	optind = 0;
>  	while (1) {
> -		int c = getopt(argc, argv, "c:i:");
> +		int c;
> +		static const struct option long_options[] = {
> +			{NULL, required_argument, NULL, 'i'},

Plain short options don't need the long option definition.

> +			{"parents", no_argument, NULL, 'p'},
> +			{NULL, 0, NULL, 0}
> +		};
> +
> +		c = getopt_long(argc, argv, "i:p", long_options, NULL);
>  		if (c < 0)
>  			break;
>  
> @@ -127,6 +136,9 @@ static int cmd_subvol_create(const struct cmd_struct *cmd,
>  				goto out;
>  			}
>  			break;
> +		case 'p':
> +			create_parents = true;
> +			break;
>  		default:
>  			usage_unknown_option(cmd, argv);
>  		}
> @@ -167,6 +179,33 @@ static int cmd_subvol_create(const struct cmd_struct *cmd,
>  		goto out;
>  	}
>  
> +	if (create_parents) {
> +		char p[PATH_MAX] = {0};
> +		char dstdir_dup[PATH_MAX];
> +		char *token;
> +
> +		strncpy_null(dstdir_dup, dstdir);
> +		if (dstdir_dup[0] == '/')
> +			strcat(p, "/");
> +
> +		token = strtok(dstdir_dup, "/");
> +		while(token) {
> +			strcat(p, token);
> +			res = path_is_dir(p);
> +			if (res == -ENOENT) {
> +				if (mkdir(p, 0777)) {

The exact error values from mkfs should be reported, so I used the 'res'
value for that.

> +					error("failed to make dir: %s", p);
> +					goto out;
> +				}
> +			} else if (res <= 0) {
> +				error("failed to check dir: %s", p);				

Here also it's good to print the exact error, not just the path.

> +				goto out;
> +			}
> +			strcat(p, "/");
> +			token = strtok(NULL, "/");
> +		}
> +	}
> +
>  	fddst = btrfs_open_dir(dstdir, &dirstream, 1);
>  	if (fddst < 0)
>  		goto out;
> -- 
> 2.25.1