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

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

[Anand jain]

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

Re: [PATCH] Btrfs-progs: btrfs subvolume delete could delete subvolumes

[Ilya Dryomov]

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

[PATCH v2] Btrfs-progs: btrfs subvolume delete could delete subvolumes

[Anand jain]

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

Re: [PATCH] Btrfs-progs: btrfs subvolume delete could delete subvolumes

[David Sterba]

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

Re: [PATCH] Btrfs-progs: btrfs subvolume delete could delete subvolumes

[Anand Jain]

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

Re: [PATCH] Btrfs-progs: btrfs subvolume delete could delete subvolumes

[David Sterba]

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

Re: [PATCH] Btrfs-progs: btrfs subvolume delete could delete subvolumes

[cwillu]

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.

Re: [PATCH] Btrfs-progs: btrfs subvolume delete could delete subvolumes

[David Sterba]

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