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

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

[Sidong Yang]

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
---
 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'},
+			{"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)) {
+					error("failed to make dir: %s", p);
+					goto out;
+				}
+			} else if (res <= 0) {
+				error("failed to check dir: %s", p);				
+				goto out;
+			}
+			strcat(p, "/");
+			token = strtok(NULL, "/");
+		}
+	}
+
 	fddst = btrfs_open_dir(dstdir, &dirstream, 1);
 	if (fddst < 0)
 		goto out;
-- 
2.25.1

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

[David Sterba]

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