Btrfs devel - Sep 2012 - [PATCH] Btrfs-progs: btrfs subvolume delete could delete subvolumes

From: Anand Jain <anand.jain@oracle.com>

With this user will be able to provide more than one subvolume
to delete.
eg: btrfs subvolume delete <subvol1> <subvol2>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-subvolume.c |   36 ++++++++++++++++++++++++------------
 man/btrfs.8.in   |    4 ++--
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index f4aa80f..cfeaa8d 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -188,31 +188,34 @@ int test_issubvolume(char *path)
 }
 
 static const char * const cmd_subvol_delete_usage[] = {
-	"btrfs subvolume delete <name>",
-	"Delete a subvolume",
+	"btrfs subvolume delete <subvolume> [<subvolume>...]",
+	"Delete subvolume(s)",
 	NULL
 };
 
 static int cmd_subvol_delete(int argc, char **argv)
 {
-	int	res, fd, len, e;
+	int	res, fd, len, e, cnt = 1, ret = 0;
 	struct btrfs_ioctl_vol_args	args;
 	char	*dname, *vname, *cpath;
 	char	*path;
 
-	if (check_argc_exact(argc, 2))
+	if (argc < 2)
 		usage(cmd_subvol_delete_usage);
 
-	path = argv[1];
+again:
+	path = argv[cnt];
 
 	res = test_issubvolume(path);
 	if(res<0){
 		fprintf(stderr, "ERROR: error accessing ''%s''\n",
path);
-		return 12;
+		ret = 12;
+		goto out;
 	}
 	if(!res){
 		fprintf(stderr, "ERROR: ''%s'' is not a
subvolume\n", path);
-		return 13;
+		ret = 13;
+		goto out;
 	}
 
 	cpath = realpath(path, 0);
@@ -226,21 +229,24 @@ static int cmd_subvol_delete(int argc, char **argv)
 	     strchr(vname, ''/'') ){
 		fprintf(stderr, "ERROR: incorrect subvolume name
(''%s'')\n",
 			vname);
-		return 14;
+		ret = 14;
+		goto out;
 	}
 
 	len = strlen(vname);
 	if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
 		fprintf(stderr, "ERROR: snapshot name too long (''%s)\n",
 			vname);
-		return 14;
+		ret = 14;
+		goto out;
 	}
 
 	fd = open_file_or_dir(dname);
 	if (fd < 0) {
 		close(fd);
 		fprintf(stderr, "ERROR: can''t access to
''%s''\n", dname);
-		return 12;
+		ret = 12;
+		goto out;
 	}
 
 	printf("Delete subvolume ''%s/%s''\n", dname,
vname);
@@ -254,10 +260,16 @@ static int cmd_subvol_delete(int argc, char **argv)
 	if(res < 0 ){
 		fprintf( stderr, "ERROR: cannot delete ''%s/%s'' -
%s\n",
 			dname, vname, strerror(e));
-		return 11;
+		ret = 11;
+		goto out;
 	}
 
-	return 0;
+out:
+	cnt++;
+	if (cnt < argc)
+		goto again;
+
+	return ret;
 }
 
 static const char * const cmd_subvol_list_usage[] = {
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 4b0a9f9..a1f172e 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -7,7 +7,7 @@ btrfs \- control a btrfs filesystem
 .SH SYNOPSIS
 \fBbtrfs\fP \fBsubvolume snapshot\fP\fI [-r] <source>
[<dest>/]<name>\fP
 .PP
-\fBbtrfs\fP \fBsubvolume delete\fP\fI <subvolume>\fP
+\fBbtrfs\fP \fBsubvolume delete\fP\fI <subvolume>
[<subvolume>...]\fP
 .PP
 \fBbtrfs\fP \fBsubvolume create\fP\fI [<dest>/]<name>\fP
 .PP
@@ -98,7 +98,7 @@ subvolume, \fBbtrfs\fR returns an error. If \fI-r\fR is given,
the snapshot
 will be readonly.
 .TP
 
-\fBsubvolume delete\fR\fI <subvolume>\fR
+\fBsubvolume delete\fR\fI <subvolume> [<subvolume>...]\fR
 Delete the subvolume \fI<subvolume>\fR. If \fI<subvolume>\fR is not
a
 subvolume, \fBbtrfs\fR returns an error.
 .TP
-- 
1.7.1

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

On Fri, Sep 21, 2012 at 02:54:08PM +0800, Anand jain
wrote:
> From: Anand Jain <anand.jain@oracle.com>
> 
> With this user will be able to provide more than one subvolume
> to delete.
> eg: btrfs subvolume delete <subvol1> <subvol2>
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  cmds-subvolume.c |   36 ++++++++++++++++++++++++------------
>  man/btrfs.8.in   |    4 ++--
>  2 files changed, 26 insertions(+), 14 deletions(-)
> 
> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
> index f4aa80f..cfeaa8d 100644
> --- a/cmds-subvolume.c
> +++ b/cmds-subvolume.c
> @@ -188,31 +188,34 @@ int test_issubvolume(char *path)
>  }
>  
>  static const char * const cmd_subvol_delete_usage[] = {
> -	"btrfs subvolume delete <name>",
> -	"Delete a subvolume",
> +	"btrfs subvolume delete <subvolume>
[<subvolume>...]",
> +	"Delete subvolume(s)",
>  	NULL
>  };
>  
>  static int cmd_subvol_delete(int argc, char **argv)
>  {
> -	int	res, fd, len, e;
> +	int	res, fd, len, e, cnt = 1, ret = 0;
>  	struct btrfs_ioctl_vol_args	args;
>  	char	*dname, *vname, *cpath;
>  	char	*path;
>  
> -	if (check_argc_exact(argc, 2))
> +	if (argc < 2)
>  		usage(cmd_subvol_delete_usage);
check_argc_min(argc, 2)

Haven''t looked at the rest.

Thanks,

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

On Fri, Sep 21, 2012 at 02:54:08PM +0800, Anand jain
wrote:
> From: Anand Jain <anand.jain@oracle.com>
> 
> With this user will be able to provide more than one subvolume
> to delete.
> eg: btrfs subvolume delete <subvol1> <subvol2>
Yes, this is useful, thanks. I''m thinking if it''s ok to stop
on
first error, ie. when the subvolume does not exist or is a directory.

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

From: Anand Jain <anand.jain@oracle.com>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-subvolume.c |   36 ++++++++++++++++++++++++------------
 1 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index f4aa80f..f6c488e 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -188,31 +188,34 @@ int test_issubvolume(char *path)
 }
 
 static const char * const cmd_subvol_delete_usage[] = {
-	"btrfs subvolume delete <name>",
-	"Delete a subvolume",
+	"btrfs subvolume delete <subvolume> [<subvolume>...]",
+	"Delete subvolume(s)",
 	NULL
 };
 
 static int cmd_subvol_delete(int argc, char **argv)
 {
-	int	res, fd, len, e;
+	int	res, fd, len, e, cnt = 1, ret = 0;
 	struct btrfs_ioctl_vol_args	args;
 	char	*dname, *vname, *cpath;
 	char	*path;
 
-	if (check_argc_exact(argc, 2))
+	if (check_argc_min(argc, 2))
 		usage(cmd_subvol_delete_usage);
 
-	path = argv[1];
+again:
+	path = argv[cnt];
 
 	res = test_issubvolume(path);
 	if(res<0){
 		fprintf(stderr, "ERROR: error accessing ''%s''\n",
path);
-		return 12;
+		ret = 12;
+		goto out;
 	}
 	if(!res){
 		fprintf(stderr, "ERROR: ''%s'' is not a
subvolume\n", path);
-		return 13;
+		ret = 13;
+		goto out;
 	}
 
 	cpath = realpath(path, 0);
@@ -226,21 +229,24 @@ static int cmd_subvol_delete(int argc, char **argv)
 	     strchr(vname, ''/'') ){
 		fprintf(stderr, "ERROR: incorrect subvolume name
(''%s'')\n",
 			vname);
-		return 14;
+		ret = 14;
+		goto out;
 	}
 
 	len = strlen(vname);
 	if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
 		fprintf(stderr, "ERROR: snapshot name too long (''%s)\n",
 			vname);
-		return 14;
+		ret = 14;
+		goto out;
 	}
 
 	fd = open_file_or_dir(dname);
 	if (fd < 0) {
 		close(fd);
 		fprintf(stderr, "ERROR: can''t access to
''%s''\n", dname);
-		return 12;
+		ret = 12;
+		goto out;
 	}
 
 	printf("Delete subvolume ''%s/%s''\n", dname,
vname);
@@ -254,10 +260,16 @@ static int cmd_subvol_delete(int argc, char **argv)
 	if(res < 0 ){
 		fprintf( stderr, "ERROR: cannot delete ''%s/%s'' -
%s\n",
 			dname, vname, strerror(e));
-		return 11;
+		ret = 11;
+		goto out;
 	}
 
-	return 0;
+out:
+	cnt++;
+	if (cnt < argc)
+		goto again;
+
+	return ret;
 }
 
 static const char * const cmd_subvol_list_usage[] = {
-- 
1.7.1

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

David,

  Thanks for the comments.

 > Yes, this is useful, thanks. I''m thinking if it''s ok to
stop on
 > first error, ie. when the subvolume does not exist or is a directory.

  I am fine with either ways. I shall just keep it as it is as of now.

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

On Mon, Sep 24, 2012 at 10:59:34AM +0800, Anand Jain
wrote:
> > Yes, this is useful, thanks. I''m thinking if it''s ok
to stop on
> > first error, ie. when the subvolume does not exist or is a directory.
> 
>  I am fine with either ways. I shall just keep it as it is as of now.
Yes, I''m ok with that. The usecase I had in mind was something like

  btrfs subvol del *

but I should know the dir I''m in and what I''m doing there,
errors are
to be expected.


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

On Mon, Sep 24, 2012 at 6:02 AM, David Sterba <dave@jikos.cz>
wrote:
> On Mon, Sep 24, 2012 at 10:59:34AM +0800, Anand Jain wrote:
>> > Yes, this is useful, thanks. I''m thinking if
it''s ok to stop on
>> > first error, ie. when the subvolume does not exist or is a
directory.
>>
>>  I am fine with either ways. I shall just keep it as it is as of now.
>
> Yes, I''m ok with that. The usecase I had in mind was something
like
>
>   btrfs subvol del *
>
> but I should know the dir I''m in and what I''m doing
there, errors are
> to be expected.
For what it''s worth, rmdir''s behaviour is to continue after
errors
(i.e., "mkdir 1; mkdir 3; rmdir 1 2 3" deletes 1 and 3, and exits with
a non-zero exit code); unless there''s a good reason to do otherwise,
matching that behaviour is probably best.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, Sep 24, 2012 at 08:36:47AM -0600, cwillu wrote:
> For what it''s worth, rmdir''s behaviour is to continue
after errors
> (i.e., "mkdir 1; mkdir 3; rmdir 1 2 3" deletes 1 and 3, and exits
with
> a non-zero exit code); unless there''s a good reason to do
otherwise,
> matching that behaviour is probably best.
Thanks for your input. I have tried it and agree with the proposed
implementation (ie. to process all arguments and skip non-subvols).

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