[PATCH 1/5] Add support for read-only subvolumes.

[PATCH 1/5] Add support for read-only subvolumes.

[Andreas Philipp]

Use BTRFS_IOC_CREATE_SNAP_V2 instead of BTRFS_IOC_CREATE_SNAP and add
an option for the creation of a readonly snapshot.

Signed-off-by: Andreas Philipp <philipp.andreas@gmail.com>
---
 btrfs_cmds.c |   44 ++++++++++++++++++++++++++++++++++++--------
 1 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/btrfs_cmds.c b/btrfs_cmds.c
index 8031c58..baec675 100644
--- a/btrfs_cmds.c
+++ b/btrfs_cmds.c
@@ -43,7 +43,7 @@
 
 #ifdef __CHECKER__
 #define BLKGETSIZE64 0
-#define BTRFS_IOC_SNAP_CREATE 0
+#define BTRFS_IOC_SNAP_CREATE_V2 0
 #define BTRFS_VOL_NAME_MAX 255
 struct btrfs_ioctl_vol_args { char name[BTRFS_VOL_NAME_MAX]; };
 static inline int ioctl(int fd, int define, void *arg) { return 0; }
@@ -310,13 +310,34 @@ int do_subvol_list(int argc, char **argv)
 int do_clone(int argc, char **argv)
 {
 	char	*subvol, *dst;
-	int	res, fd, fddst, len;
+	int	res, fd, fddst, len, optind = 0, readonly = 0;
 	char	*newname;
 	char	*dstdir;
 
-	subvol = argv[1];
-	dst = argv[2];
-	struct btrfs_ioctl_vol_args	args;
+	while(1) {
+		int c = getopt(argc, argv, "r");
+		if (c < 0)
+			break;
+		switch(c) {
+			case 'r':
+				optind++;
+				readonly = 1;
+				break;
+	                default:
+				fprintf(stderr, "Invalid arguments for subvolume snapshot\n");
+				free(argv);
+				return 1;
+		}
+	}
+	if (argc - optind < 2) {
+		fprintf(stderr, "Invalid arguments for defragment\n");
+		free(argv);
+		return 1;
+	}
+
+	subvol = argv[optind+1];
+	dst = argv[optind+2];
+	struct btrfs_ioctl_vol_args_v2	args;
 
 	res = test_issubvolume(subvol);
 	if(res<0){
@@ -371,12 +392,19 @@ int do_clone(int argc, char **argv)
 		fprintf(stderr, "ERROR: can't access to '%s'\n", dstdir);
 		return 12;
 	}
+	
+	if (readonly) {
+		args.flags |= BTRFS_SUBVOL_RDONLY;
+		printf("Create a readonly snapshot of '%s' in '%s/%s'\n",
+		       subvol, dstdir, newname);
+	}
+	else
+		printf("Create a snapshot of '%s' in '%s/%s'\n",
+		       subvol, dstdir, newname);
 
-	printf("Create a snapshot of '%s' in '%s/%s'\n",
-	       subvol, dstdir, newname);
 	args.fd = fd;
 	strcpy(args.name, newname);
-	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE, &args);
+	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_V2, &args);
 
 	close(fd);
 	close(fddst);
-- 
1.7.4.1

Read Only snapshots

[David Morgado]

Hi, btrfs-progs patches for read only snapshots aren't in Chris
repository yet but btrfs already has support for that in place so,
someone forgot about this?

It appears that some new patches are coming to Chris btrfs-progs (new
tmp branch) so this was a nice opportunity to get those patches in.


Thanks

David Morgado

Re: Read Only snapshots

[Chris Mason]

Excerpts from David Morgado's message of 2011-04-24 21:24:16 -0400:
> Hi, btrfs-progs patches for read only snapshots aren't in Chris
> repository yet but btrfs already has support for that in place so,
> someone forgot about this?
> 
> It appears that some new patches are coming to Chris btrfs-progs (new
> tmp branch) so this was a nice opportunity to get those patches in.

Right, the tmp branch is the one I'm pushing around and testing for
release.  I'll make sure the readonly snapshots code gets in there too.

-chris

[PATCH 0/5] Add user-space support for read-only snapshot creation.

[Andreas Philipp]

There is kernel side support for the creation of read-only snapshots
for some time now, but I did not find any patches for the userspace
btrfs utilites. Thus I created this patchset which is tested and
working with kernel version 2.6.39-rc2.

Andreas Philipp (5):
  Add support for read-only subvolumes.
  Support the new parameters in do_clone(int argc, char** argv).
  Added support for an additional ioctl.
  Test the additional ioctl.
  Updated documentation for btrfs subvolume snapshot.

 btrfs.c        |    6 +++---
 btrfs_cmds.c   |   44 ++++++++++++++++++++++++++++++++++++--------
 ioctl-test.c   |    1 +
 ioctl.h        |   14 ++++++++++++++
 man/btrfs.8.in |   11 ++++++-----
 5 files changed, 60 insertions(+), 16 deletions(-)

-- 
1.7.4.1

[PATCH 1/5] Add support for read-only subvolumes.

[Andreas Philipp]

Use BTRFS_IOC_CREATE_SNAP_V2 instead of BTRFS_IOC_CREATE_SNAP and add
an option for the creation of a readonly snapshot.

Signed-off-by: Andreas Philipp <philipp.andreas@gmail.com>
---
 btrfs_cmds.c |   44 ++++++++++++++++++++++++++++++++++++--------
 1 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/btrfs_cmds.c b/btrfs_cmds.c
index 8031c58..baec675 100644
--- a/btrfs_cmds.c
+++ b/btrfs_cmds.c
@@ -43,7 +43,7 @@
 
 #ifdef __CHECKER__
 #define BLKGETSIZE64 0
-#define BTRFS_IOC_SNAP_CREATE 0
+#define BTRFS_IOC_SNAP_CREATE_V2 0
 #define BTRFS_VOL_NAME_MAX 255
 struct btrfs_ioctl_vol_args { char name[BTRFS_VOL_NAME_MAX]; };
 static inline int ioctl(int fd, int define, void *arg) { return 0; }
@@ -310,13 +310,34 @@ int do_subvol_list(int argc, char **argv)
 int do_clone(int argc, char **argv)
 {
 	char	*subvol, *dst;
-	int	res, fd, fddst, len;
+	int	res, fd, fddst, len, optind = 0, readonly = 0;
 	char	*newname;
 	char	*dstdir;
 
-	subvol = argv[1];
-	dst = argv[2];
-	struct btrfs_ioctl_vol_args	args;
+	while(1) {
+		int c = getopt(argc, argv, "r");
+		if (c < 0)
+			break;
+		switch(c) {
+			case 'r':
+				optind++;
+				readonly = 1;
+				break;
+	                default:
+				fprintf(stderr, "Invalid arguments for subvolume snapshot\n");
+				free(argv);
+				return 1;
+		}
+	}
+	if (argc - optind < 2) {
+		fprintf(stderr, "Invalid arguments for defragment\n");
+		free(argv);
+		return 1;
+	}
+
+	subvol = argv[optind+1];
+	dst = argv[optind+2];
+	struct btrfs_ioctl_vol_args_v2	args;
 
 	res = test_issubvolume(subvol);
 	if(res<0){
@@ -371,12 +392,19 @@ int do_clone(int argc, char **argv)
 		fprintf(stderr, "ERROR: can't access to '%s'\n", dstdir);
 		return 12;
 	}
+	
+	if (readonly) {
+		args.flags |= BTRFS_SUBVOL_RDONLY;
+		printf("Create a readonly snapshot of '%s' in '%s/%s'\n",
+		       subvol, dstdir, newname);
+	}
+	else
+		printf("Create a snapshot of '%s' in '%s/%s'\n",
+		       subvol, dstdir, newname);
 
-	printf("Create a snapshot of '%s' in '%s/%s'\n",
-	       subvol, dstdir, newname);
 	args.fd = fd;
 	strcpy(args.name, newname);
-	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE, &args);
+	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_V2, &args);
 
 	close(fd);
 	close(fddst);
-- 
1.7.4.1

[PATCH 2/5] Support the new parameters in do_clone(int argc, char** argv).

[Andreas Philipp]

Now 'btrfs subvolume snapshot' takes not two but only at least two
parameters. Additionally, the help message is updated accordingly.

Signed-off-by: Andreas Philipp <philipp.andreas@gmail.com>
---
 btrfs.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 46314cf..f70d64b 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -44,9 +44,9 @@ static struct Command commands[] = {
 	/*
 		avoid short commands different for the case only
 	*/
-	{ do_clone, 2,
-	  "subvolume snapshot", "<source> [<dest>/]<name>\n"
-		"Create a writable snapshot of the subvolume <source> with\n"
+	{ do_clone, -2,
+	  "subvolume snapshot", "[-r] <source> [<dest>/]<name>\n"
+		"Create a writable/readonly snapshot of the subvolume <source> with\n"
 		"the name <name> in the <dest> directory."
 	},
 	{ do_delete_subvolume, 1,
-- 
1.7.4.1

[PATCH 3/5] Added support for an additional ioctl.

[Andreas Philipp]

Added BTRFS_IOC_SNAP_CREATE_V2 and struct btrfs_ioctl_vol_args_v2 as
defined in fs/btrfs/ioctl.h in the kernel sources.

Signed-off-by: Andreas Philipp <philipp.andreas@gmail.com>
---
 ioctl.h |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/ioctl.h b/ioctl.h
index 776d7a9..358f814 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -30,6 +30,17 @@ struct btrfs_ioctl_vol_args {
 	char name[BTRFS_PATH_NAME_MAX + 1];
 };
 
+#define BTRFS_SUBVOL_RDONLY		(1ULL << 1)
+#define BTRFS_SUBVOL_NAME_MAX 4039
+
+struct btrfs_ioctl_vol_args_v2 {
+	__s64 fd;
+	__u64 transid;
+	__u64 flags;
+	__u64 unused[4];
+	char name[BTRFS_SUBVOL_NAME_MAX + 1];
+};
+
 struct btrfs_ioctl_search_key {
 	/* which root are we searching.  0 is the tree of tree roots */
 	__u64 tree_id;
@@ -132,6 +143,7 @@ struct btrfs_ioctl_space_args {
 	struct btrfs_ioctl_space_info spaces[0];
 };
 
+/* BTRFS_IOC_SNAP_CREATE is no longer used by the btrfs command */
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -169,4 +181,6 @@ struct btrfs_ioctl_space_args {
 #define BTRFS_IOC_DEFAULT_SUBVOL _IOW(BTRFS_IOCTL_MAGIC, 19, u64)
 #define BTRFS_IOC_SPACE_INFO _IOWR(BTRFS_IOCTL_MAGIC, 20, \
 				    struct btrfs_ioctl_space_args)
+#define BTRFS_IOC_SNAP_CREATE_V2 _IOW(BTRFS_IOCTL_MAGIC, 23, \
+				   struct btrfs_ioctl_vol_args_v2)
 #endif
-- 
1.7.4.1

[PATCH 4/5] Test the additional ioctl.

[Andreas Philipp]

Signed-off-by: Andreas Philipp <philipp.andreas@gmail.com>
---
 ioctl-test.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/ioctl-test.c b/ioctl-test.c
index 7cf3bc2..1c27d61 100644
--- a/ioctl-test.c
+++ b/ioctl-test.c
@@ -22,6 +22,7 @@ unsigned long ioctls[] = {
 	BTRFS_IOC_INO_LOOKUP,
 	BTRFS_IOC_DEFAULT_SUBVOL,
 	BTRFS_IOC_SPACE_INFO,
+	BTRFS_IOC_SNAP_CREATE_V2,
 	0 };
 
 int main(int ac, char **av)
-- 
1.7.4.1

[PATCH 5/5] Updated documentation for btrfs subvolume snapshot.

[Andreas Philipp]

Signed-off-by: Andreas Philipp <philipp.andreas@gmail.com>
---
 man/btrfs.8.in |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 26ef982..b59bc6f 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -5,7 +5,7 @@
 .SH NAME
 btrfs \- control a btrfs filesystem
 .SH SYNOPSIS
-\fBbtrfs\fP \fBsubvolume snapshot\fP\fI <source> [<dest>/]<name>\fP
+\fBbtrfs\fP \fBsubvolume snapshot\fP\fI [-r] <source> [<dest>/]<name>\fP
 .PP
 \fBbtrfs\fP \fBsubvolume delete\fP\fI <subvolume>\fP
 .PP
@@ -70,10 +70,11 @@ command.
 .SH COMMANDS
 .TP
 
-\fBsubvolume snapshot\fR\fI <source> [<dest>/]<name>\fR
-Create a writable snapshot of the subvolume \fI<source>\fR with the name
-\fI<name>\fR in the \fI<dest>\fR directory. If \fI<source>\fR is not a
-subvolume, \fBbtrfs\fR returns an error.
+\fBsubvolume snapshot\fR\fI [-r] <source> [<dest>/]<name>\fR
+Create a writable/readonly snapshot of the subvolume \fI<source>\fR with the
+name \fI<name>\fR in the \fI<dest>\fR directory. If \fI<source>\fR is not a
+subvolume, \fBbtrfs\fR returns an error. If \fI-r\fR is given, the snapshot
+will be readonly.
 .TP
 
 \fBsubvolume delete\fR\fI <subvolume>\fR
-- 
1.7.4.1

Re: [PATCH 1/5] Add support for read-only subvolumes.

[Goffredo Baroncelli]

Hi Andreas,

On 04/25/2011 03:47 PM, Andreas Philipp wrote:
> Use BTRFS_IOC_CREATE_SNAP_V2 instead of BTRFS_IOC_CREATE_SNAP and add
> an option for the creation of a readonly snapshot.
> 
> Signed-off-by: Andreas Philipp <philipp.andreas@gmail.com>
> ---
>  btrfs_cmds.c |   44 ++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/btrfs_cmds.c b/btrfs_cmds.c
> index 8031c58..baec675 100644
> --- a/btrfs_cmds.c
> +++ b/btrfs_cmds.c
> @@ -43,7 +43,7 @@
>  
>  #ifdef __CHECKER__

It is not related to your parch.. but does anyone know why we re-define
some constants if __CHECKER__ is defined ?

>  #define BLKGETSIZE64 0
> -#define BTRFS_IOC_SNAP_CREATE 0
> +#define BTRFS_IOC_SNAP_CREATE_V2 0
>  #define BTRFS_VOL_NAME_MAX 255
>  struct btrfs_ioctl_vol_args { char name[BTRFS_VOL_NAME_MAX]; };
>  static inline int ioctl(int fd, int define, void *arg) { return 0; }
> @@ -310,13 +310,34 @@ int do_subvol_list(int argc, char **argv)
>  int do_clone(int argc, char **argv)
>  {
>  	char	*subvol, *dst;
> -	int	res, fd, fddst, len;
> +	int	res, fd, fddst, len, optind = 0, readonly = 0;
>  	char	*newname;
>  	char	*dstdir;
>  
> -	subvol = argv[1];
> -	dst = argv[2];
> -	struct btrfs_ioctl_vol_args	args;
> +	while(1) {
> +		int c = getopt(argc, argv, "r");
> +		if (c < 0)
> +			break;
> +		switch(c) {
> +			case 'r':
> +				optind++;
> +				readonly = 1;
> +				break;
> +	                default:
> +				fprintf(stderr, "Invalid arguments for subvolume snapshot\n");
> +				free(argv);
> +				return 1;
> +		}
> +	}
> +	if (argc - optind < 2) {
> +		fprintf(stderr, "Invalid arguments for defragment\n");
                                                       ^^^^^^^^^^^^

May be that you need to replace "defragment" with "subvolume snapshot" ?

> +		free(argv);
> +		return 1;
> +	}
> +
> +	subvol = argv[optind+1];
> +	dst = argv[optind+2];
> +	struct btrfs_ioctl_vol_args_v2	args;

Does the "standard C" allow to define a variable in the middle in a
function instead of in the begin ?
Anyway, even not required, I suggest to fill "args" by zero.

+	memset(&args, 0, sizeog(args));

>  
>  	res = test_issubvolume(subvol);
>  	if(res<0){
> @@ -371,12 +392,19 @@ int do_clone(int argc, char **argv)
>  		fprintf(stderr, "ERROR: can't access to '%s'\n", dstdir);
>  		return 12;
>  	}
> +	
> +	if (readonly) {
> +		args.flags |= BTRFS_SUBVOL_RDONLY;
> +		printf("Create a readonly snapshot of '%s' in '%s/%s'\n",
> +		       subvol, dstdir, newname);
> +	}
> +	else
> +		printf("Create a snapshot of '%s' in '%s/%s'\n",
> +		       subvol, dstdir, newname);
>  
It is not required, but I suggest to use also in the "else" the brackets
( {} ).

> -	printf("Create a snapshot of '%s' in '%s/%s'\n",
> -	       subvol, dstdir, newname);
>  	args.fd = fd;
>  	strcpy(args.name, newname);
> -	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE, &args);
> +	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_V2, &args);
>  
>  	close(fd);
>  	close(fddst);

Regards
G.Baroncelli

Re: [PATCH 1/5] Add support for read-only subvolumes.

[Chris Mason]

Excerpts from Goffredo Baroncelli's message of 2011-04-25 17:34:46 -0400:
> Hi Andreas,
> 
> On 04/25/2011 03:47 PM, Andreas Philipp wrote:
> > Use BTRFS_IOC_CREATE_SNAP_V2 instead of BTRFS_IOC_CREATE_SNAP and add
> > an option for the creation of a readonly snapshot.
> > 
> > Signed-off-by: Andreas Philipp <philipp.andreas@gmail.com>
> > ---
> >  btrfs_cmds.c |   44 ++++++++++++++++++++++++++++++++++++--------
> >  1 files changed, 36 insertions(+), 8 deletions(-)
> > 
> > diff --git a/btrfs_cmds.c b/btrfs_cmds.c
> > index 8031c58..baec675 100644
> > --- a/btrfs_cmds.c
> > +++ b/btrfs_cmds.c
> > @@ -43,7 +43,7 @@
> >  
> >  #ifdef __CHECKER__
> 
> It is not related to your parch.. but does anyone know why we re-define
> some constants if __CHECKER__ is defined ?

This is old support for the sparse program, which finds a good number of
bugs.

> 
> >  #define BLKGETSIZE64 0
> > -#define BTRFS_IOC_SNAP_CREATE 0
> > +#define BTRFS_IOC_SNAP_CREATE_V2 0
> >  #define BTRFS_VOL_NAME_MAX 255
> >  struct btrfs_ioctl_vol_args { char name[BTRFS_VOL_NAME_MAX]; };
> >  static inline int ioctl(int fd, int define, void *arg) { return 0; }
> > @@ -310,13 +310,34 @@ int do_subvol_list(int argc, char **argv)
> >  int do_clone(int argc, char **argv)
> >  {
> >      char    *subvol, *dst;
> > -    int    res, fd, fddst, len;
> > +    int    res, fd, fddst, len, optind = 0, readonly = 0;
> >      char    *newname;
> >      char    *dstdir;
> >  
> > -    subvol = argv[1];
> > -    dst = argv[2];
> > -    struct btrfs_ioctl_vol_args    args;
> > +    while(1) {
> > +        int c = getopt(argc, argv, "r");
> > +        if (c < 0)
> > +            break;
> > +        switch(c) {
> > +            case 'r':
> > +                optind++;
> > +                readonly = 1;
> > +                break;
> > +                    default:
> > +                fprintf(stderr, "Invalid arguments for subvolume snapshot\n");
> > +                free(argv);
> > +                return 1;
> > +        }
> > +    }
> > +    if (argc - optind < 2) {
> > +        fprintf(stderr, "Invalid arguments for defragment\n");
>                                                        ^^^^^^^^^^^^
> 
> May be that you need to replace "defragment" with "subvolume snapshot" ?
> 
> > +        free(argv);
> > +        return 1;
> > +    }
> > +
> > +    subvol = argv[optind+1];
> > +    dst = argv[optind+2];
> > +    struct btrfs_ioctl_vol_args_v2    args;
> 
> Does the "standard C" allow to define a variable in the middle in a
> function instead of in the begin ?
> Anyway, even not required, I suggest to fill "args" by zero.

I'd prefer all the variables are at the stop of a scope.

> 
> +    memset(&args, 0, sizeog(args));
> 
> >  
> >      res = test_issubvolume(subvol);
> >      if(res<0){
> > @@ -371,12 +392,19 @@ int do_clone(int argc, char **argv)
> >          fprintf(stderr, "ERROR: can't access to '%s'\n", dstdir);
> >          return 12;
> >      }
> > +    
> > +    if (readonly) {
> > +        args.flags |= BTRFS_SUBVOL_RDONLY;
> > +        printf("Create a readonly snapshot of '%s' in '%s/%s'\n",
> > +               subvol, dstdir, newname);
> > +    }
> > +    else
> > +        printf("Create a snapshot of '%s' in '%s/%s'\n",
> > +               subvol, dstdir, newname);
> >  
> It is not required, but I suggest to use also in the "else" the brackets
> ( {} ).

Yes please.

> 
> > -    printf("Create a snapshot of '%s' in '%s/%s'\n",
> > -           subvol, dstdir, newname);
> >      args.fd = fd;
> >      strcpy(args.name, newname);
> > -    res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE, &args);
> > +    res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_V2, &args);
> >  
> >      close(fd);
> >      close(fddst);
> 

-chris

Re: [PATCH 0/5] Add user-space support for read-only snapshot creation.

[Li Zefan]

Andreas Philipp wrote:
> There is kernel side support for the creation of read-only snapshots
> for some time now, but I did not find any patches for the userspace
> btrfs utilites. Thus I created this patchset which is tested and
> working with kernel version 2.6.39-rc2.
> 
> Andreas Philipp (5):
>   Add support for read-only subvolumes.
>   Support the new parameters in do_clone(int argc, char** argv).
>   Added support for an additional ioctl.
>   Test the additional ioctl.
>   Updated documentation for btrfs subvolume snapshot.
> 

Thanks for doing this!

Re: [PATCH 1/5] Add support for read-only subvolumes.

[Li Zefan]

Andreas Philipp wrote:
> Use BTRFS_IOC_CREATE_SNAP_V2 instead of BTRFS_IOC_CREATE_SNAP and add
> an option for the creation of a readonly snapshot.
> 
> Signed-off-by: Andreas Philipp <philipp.andreas@gmail.com>
> ---
>  btrfs_cmds.c |   44 ++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/btrfs_cmds.c b/btrfs_cmds.c
> index 8031c58..baec675 100644
> --- a/btrfs_cmds.c
> +++ b/btrfs_cmds.c
> @@ -43,7 +43,7 @@
>  
>  #ifdef __CHECKER__
>  #define BLKGETSIZE64 0
> -#define BTRFS_IOC_SNAP_CREATE 0
> +#define BTRFS_IOC_SNAP_CREATE_V2 0
>  #define BTRFS_VOL_NAME_MAX 255
>  struct btrfs_ioctl_vol_args { char name[BTRFS_VOL_NAME_MAX]; };
>  static inline int ioctl(int fd, int define, void *arg) { return 0; }
> @@ -310,13 +310,34 @@ int do_subvol_list(int argc, char **argv)
>  int do_clone(int argc, char **argv)
>  {
>  	char	*subvol, *dst;
> -	int	res, fd, fddst, len;
> +	int	res, fd, fddst, len, optind = 0, readonly = 0;
>  	char	*newname;
>  	char	*dstdir;
>  
> -	subvol = argv[1];
> -	dst = argv[2];
> -	struct btrfs_ioctl_vol_args	args;
> +	while(1) {
> +		int c = getopt(argc, argv, "r");

need a blank line after variable declarations.

> +		if (c < 0)
> +			break;
> +		switch(c) {

switch (c)

> +			case 'r':

switch (c) {
case:
	...
...

> +				optind++;
> +				readonly = 1;
> +				break;
> +	                default:
> +				fprintf(stderr, "Invalid arguments for subvolume snapshot\n");
> +				free(argv);
> +				return 1;
> +		}
> +	}
> +	if (argc - optind < 2) {
> +		fprintf(stderr, "Invalid arguments for defragment\n");
> +		free(argv);
> +		return 1;
> +	}
> +
> +	subvol = argv[optind+1];
> +	dst = argv[optind+2];
> +	struct btrfs_ioctl_vol_args_v2	args;

memset(&args, 0, sizeof(args));

>  
>  	res = test_issubvolume(subvol);
>  	if(res<0){
> @@ -371,12 +392,19 @@ int do_clone(int argc, char **argv)
>  		fprintf(stderr, "ERROR: can't access to '%s'\n", dstdir);
>  		return 12;
>  	}
> +	
> +	if (readonly) {
> +		args.flags |= BTRFS_SUBVOL_RDONLY;
> +		printf("Create a readonly snapshot of '%s' in '%s/%s'\n",
> +		       subvol, dstdir, newname);
> +	}
> +	else

} else 

> +		printf("Create a snapshot of '%s' in '%s/%s'\n",
> +		       subvol, dstdir, newname);
>  
> -	printf("Create a snapshot of '%s' in '%s/%s'\n",
> -	       subvol, dstdir, newname);
>  	args.fd = fd;
>  	strcpy(args.name, newname);
> -	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE, &args);
> +	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_V2, &args);
>  
>  	close(fd);
>  	close(fddst);

I recomend running checkpatch.pl for those patches.

Re: [PATCH 1/5] Add support for read-only subvolumes.

[Li Zefan]

> +	while(1) {
> +		int c = getopt(argc, argv, "r");
> +		if (c < 0)
> +			break;
> +		switch(c) {
> +			case 'r':
> +				optind++;
> +				readonly = 1;
> +				break;
> +	                default:
> +				fprintf(stderr, "Invalid arguments for subvolume snapshot\n");
> +				free(argv);
> +				return 1;
> +		}
> +	}
> +	if (argc - optind < 2) {
> +		fprintf(stderr, "Invalid arguments for defragment\n");
> +		free(argv);
> +		return 1;
> +	}
> +
> +	subvol = argv[optind+1];
> +	dst = argv[optind+2];
> +	struct btrfs_ioctl_vol_args_v2	args;
>  

I don't think this can compile.. This structure is added in the 3rd patch.

Re: [PATCH 1/5] Add support for read-only subvolumes.

[Li Zefan]

>> +	subvol = argv[optind+1];
>> +	dst = argv[optind+2];
>> +	struct btrfs_ioctl_vol_args_v2	args;
> 
> Does the "standard C" allow to define a variable in the middle in a
> function instead of in the begin ?
> Anyway, even not required, I suggest to fill "args" by zero.
> 
> +	memset(&args, 0, sizeog(args));
> 

It's necessary, otherwise args.unused[4] and args.transid will have
arbitrary value.