* [PATCH] btrfs-progs: cmds: subvolume: add -p option on creating subvol
@ 2022-02-19 9:01 Sidong Yang
2022-02-19 14:11 ` Goffredo Baroncelli
0 siblings, 1 reply; 3+ messages in thread
From: Sidong Yang @ 2022-02-19 9:01 UTC (permalink / raw)
To: linux-btrfs; +Cc: 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>
---
cmds/subvolume.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index fbf56566..1070c74a 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 make any missing parent directories for each argument",
HELPINFO_INSERT_GLOBALS,
HELPINFO_INSERT_QUIET,
NULL
@@ -105,10 +106,11 @@ 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 = getopt(argc, argv, "c:i:p");
if (c < 0)
break;
@@ -127,6 +129,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 +172,30 @@ static int cmd_subvol_create(const struct cmd_struct *cmd,
goto out;
}
+ if (create_parents) {
+ char p[PATH_MAX];
+ char dstdir_real[PATH_MAX];
+ char *token;
+
+ realpath(dstdir, dstdir_real);
+ token = strtok(dstdir_real, "/");
+ while(token) {
+ strcat(p, "/");
+ strcat(p, token);
+ res = path_is_dir(p);
+ if (res == -ENOENT) {
+ if (mkdir(p, 0755)) {
+ error("failed to make dir: %s", p);
+ goto out;
+ }
+ } else if (res <= 0) {
+ error("failed to check dir: %s", p);
+ goto out;
+ }
+ token = strtok(NULL, "/");
+ }
+ }
+
fddst = btrfs_open_dir(dstdir, &dirstream, 1);
if (fddst < 0)
goto out;
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] btrfs-progs: cmds: subvolume: add -p option on creating subvol
2022-02-19 9:01 [PATCH] btrfs-progs: cmds: subvolume: add -p option on creating subvol Sidong Yang
@ 2022-02-19 14:11 ` Goffredo Baroncelli
2022-02-19 14:40 ` Sidong Yang
0 siblings, 1 reply; 3+ messages in thread
From: Goffredo Baroncelli @ 2022-02-19 14:11 UTC (permalink / raw)
To: Sidong Yang, linux-btrfs
On 19/02/2022 10.01, 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>
> ---
> cmds/subvolume.c | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> index fbf56566..1070c74a 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 make any missing parent directories for each argument",
you should update the man page too
> HELPINFO_INSERT_GLOBALS,
> HELPINFO_INSERT_QUIET,
> NULL
[...]
> + if (create_parents) {
> + char p[PATH_MAX];
> + char dstdir_real[PATH_MAX];
> + char *token;
> +
> + realpath(dstdir, dstdir_real);
realpath(3) behaves strangely when the path doesn't exist: if exists only /tmp, realpath("/tmp/1/2/3/4/5") returns "/tmp/1" discarding 2/3/4....
In fact my gcc warns:
$ make
[CC] cmds/subvolume.o
cmds/subvolume.c: In function ‘cmd_subvol_create’:
cmds/subvolume.c:180:17: warning: ignoring return value of ‘realpath’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
180 | realpath(dstdir, dstdir_real);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
because realpath(3) returns an error if the path doesn't exist.
This causes:
# only /tmp/ exists
btrfs sub cre -p /tmp/1/2/3/4/5/6
ERROR: cannot access '/tmp/1/2/3/4/5': No such file or directory
> + token = strtok(dstdir_real, "/");
> + while(token) {
> + strcat(p, "/");
where is initialized p ?
Unfortunately it works and the gcc doesn't warn, but I think only because in the stack there are zeros where p is allocated; but this is not guarantee.
> + strcat(p, token);
> + res = path_is_dir(p);
> + if (res == -ENOENT) {
> + if (mkdir(p, 0755)) {
> + error("failed to make dir: %s", p);
> + goto out;
> + }
> + } else if (res <= 0) {
> + error("failed to check dir: %s", p);
> + goto out;
> + }
> + token = strtok(NULL, "/");
> + }
> + }
> +
> fddst = btrfs_open_dir(dstdir, &dirstream, 1);
> if (fddst < 0)
> goto out;
--
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] 3+ messages in thread* Re: [PATCH] btrfs-progs: cmds: subvolume: add -p option on creating subvol
2022-02-19 14:11 ` Goffredo Baroncelli
@ 2022-02-19 14:40 ` Sidong Yang
0 siblings, 0 replies; 3+ messages in thread
From: Sidong Yang @ 2022-02-19 14:40 UTC (permalink / raw)
To: kreijack; +Cc: linux-btrfs
On Sat, Feb 19, 2022 at 03:11:36PM +0100, Goffredo Baroncelli wrote:
Hi, Thanks for review.
> On 19/02/2022 10.01, 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>
> > ---
> > cmds/subvolume.c | 31 ++++++++++++++++++++++++++++++-
> > 1 file changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> > index fbf56566..1070c74a 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 make any missing parent directories for each argument",
>
> you should update the man page too
Thanks I'll do it for next version.
>
> > HELPINFO_INSERT_GLOBALS,
> > HELPINFO_INSERT_QUIET,
> > NULL
>
> [...]
>
> > + if (create_parents) {
> > + char p[PATH_MAX];
> > + char dstdir_real[PATH_MAX];
> > + char *token;
> > +
> > + realpath(dstdir, dstdir_real);
>
> realpath(3) behaves strangely when the path doesn't exist: if exists only /tmp, realpath("/tmp/1/2/3/4/5") returns "/tmp/1" discarding 2/3/4....
> In fact my gcc warns:
>
> $ make
> [CC] cmds/subvolume.o
> cmds/subvolume.c: In function ‘cmd_subvol_create’:
> cmds/subvolume.c:180:17: warning: ignoring return value of ‘realpath’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> 180 | realpath(dstdir, dstdir_real);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> because realpath(3) returns an error if the path doesn't exist.
>
> This causes:
>
> # only /tmp/ exists
> btrfs sub cre -p /tmp/1/2/3/4/5/6
> ERROR: cannot access '/tmp/1/2/3/4/5': No such file or directory
Yes, It's completely wrong code. I'll correct it. For now, I think I
need to check whether the path is absolute or relative for checking
dstdir[0] == '/'.
>
>
> > + token = strtok(dstdir_real, "/");
> > + while(token) {
> > + strcat(p, "/");
>
> where is initialized p ?
I missed it. Thanks.
>
> Unfortunately it works and the gcc doesn't warn, but I think only because in the stack there are zeros where p is allocated; but this is not guarantee.
>
> > + strcat(p, token);
> > + res = path_is_dir(p);
> > + if (res == -ENOENT) {
> > + if (mkdir(p, 0755)) {
> > + error("failed to make dir: %s", p);
> > + goto out;
> > + }
> > + } else if (res <= 0) {
> > + error("failed to check dir: %s", p);
> > + goto out;
> > + }
> > + token = strtok(NULL, "/");
> > + }
> > + }
> > +
> > fddst = btrfs_open_dir(dstdir, &dirstream, 1);
> > if (fddst < 0)
> > goto out;
>
>
> --
> 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] 3+ messages in thread
end of thread, other threads:[~2022-02-19 14:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-19 9:01 [PATCH] btrfs-progs: cmds: subvolume: add -p option on creating subvol Sidong Yang
2022-02-19 14:11 ` Goffredo Baroncelli
2022-02-19 14:40 ` Sidong Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox