Re: [PATCH] Btrfs-progs: allow compression property gets for read-only subvolumes

2014-04-25 Thread Liu Bo
On Tue, Apr 15, 2014 at 08:47:27PM +0100, Filipe David Borba Manana wrote:
> Because the function open_file_or_dir() always opened the input file in
> read/write mode (O_RDWR), we were not able to due a compression property
> get against a file living in a read-only subvolume/snapshot.
> Fix this by opening the file with O_RDONLY mode if we're doing a property
> get.

Reviewed-by: Liu Bo 

thanks,
-liubo

> 
> Signed-off-by: Filipe David Borba Manana 
> ---
>  props.c | 3 ++-
>  utils.c | 9 +++--
>  utils.h | 1 +
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/props.c b/props.c
> index 4d0aeea..bd74975 100644
> --- a/props.c
> +++ b/props.c
> @@ -119,8 +119,9 @@ static int prop_compression(enum prop_object_type type,
>   DIR *dirstream = NULL;
>   char *buf = NULL;
>   char *xattr_name = NULL;
> + int open_flags = value ? O_RDWR : O_RDONLY;
>  
> - fd = open_file_or_dir(object, &dirstream);
> + fd = open_file_or_dir3(object, &dirstream, open_flags);
>   if (fd == -1) {
>   ret = -errno;
>   fprintf(stderr, "ERROR: open %s failed. %s\n",
> diff --git a/utils.c b/utils.c
> index 0bfb9d9..458ba54 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1625,7 +1625,7 @@ u64 parse_size(char *s)
>   return strtoull(s, NULL, 10) * mult;
>  }
>  
> -int open_file_or_dir(const char *fname, DIR **dirstream)
> +int open_file_or_dir3(const char *fname, DIR **dirstream, int open_flags)
>  {
>   int ret;
>   struct stat st;
> @@ -1641,7 +1641,7 @@ int open_file_or_dir(const char *fname, DIR **dirstream)
>   return -1;
>   fd = dirfd(*dirstream);
>   } else if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
> - fd = open(fname, O_RDWR);
> + fd = open(fname, open_flags);
>   } else {
>   /*
>* we set this on purpose, in case the caller output
> @@ -1658,6 +1658,11 @@ int open_file_or_dir(const char *fname, DIR 
> **dirstream)
>   return fd;
>  }
>  
> +int open_file_or_dir(const char *fname, DIR **dirstream)
> +{
> + return open_file_or_dir3(fname, dirstream, O_RDWR);
> +}
> +
>  void close_file_or_dir(int fd, DIR *dirstream)
>  {
>   if (dirstream)
> diff --git a/utils.h b/utils.h
> index 06ec938..fc581ca 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -73,6 +73,7 @@ int btrfs_scan_block_devices(int run_ioctl);
>  u64 parse_size(char *s);
>  u64 arg_strtou64(const char *str);
>  int open_file_or_dir(const char *fname, DIR **dirstream);
> +int open_file_or_dir3(const char *fname, DIR **dirstream, int open_flags);
>  void close_file_or_dir(int fd, DIR *dirstream);
>  int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>   struct btrfs_ioctl_dev_info_args **di_ret);
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Btrfs-progs: allow compression property gets for read-only subvolumes

2014-04-15 Thread Filipe David Borba Manana
Because the function open_file_or_dir() always opened the input file in
read/write mode (O_RDWR), we were not able to due a compression property
get against a file living in a read-only subvolume/snapshot.
Fix this by opening the file with O_RDONLY mode if we're doing a property
get.

Signed-off-by: Filipe David Borba Manana 
---
 props.c | 3 ++-
 utils.c | 9 +++--
 utils.h | 1 +
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/props.c b/props.c
index 4d0aeea..bd74975 100644
--- a/props.c
+++ b/props.c
@@ -119,8 +119,9 @@ static int prop_compression(enum prop_object_type type,
DIR *dirstream = NULL;
char *buf = NULL;
char *xattr_name = NULL;
+   int open_flags = value ? O_RDWR : O_RDONLY;
 
-   fd = open_file_or_dir(object, &dirstream);
+   fd = open_file_or_dir3(object, &dirstream, open_flags);
if (fd == -1) {
ret = -errno;
fprintf(stderr, "ERROR: open %s failed. %s\n",
diff --git a/utils.c b/utils.c
index 0bfb9d9..458ba54 100644
--- a/utils.c
+++ b/utils.c
@@ -1625,7 +1625,7 @@ u64 parse_size(char *s)
return strtoull(s, NULL, 10) * mult;
 }
 
-int open_file_or_dir(const char *fname, DIR **dirstream)
+int open_file_or_dir3(const char *fname, DIR **dirstream, int open_flags)
 {
int ret;
struct stat st;
@@ -1641,7 +1641,7 @@ int open_file_or_dir(const char *fname, DIR **dirstream)
return -1;
fd = dirfd(*dirstream);
} else if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
-   fd = open(fname, O_RDWR);
+   fd = open(fname, open_flags);
} else {
/*
 * we set this on purpose, in case the caller output
@@ -1658,6 +1658,11 @@ int open_file_or_dir(const char *fname, DIR **dirstream)
return fd;
 }
 
+int open_file_or_dir(const char *fname, DIR **dirstream)
+{
+   return open_file_or_dir3(fname, dirstream, O_RDWR);
+}
+
 void close_file_or_dir(int fd, DIR *dirstream)
 {
if (dirstream)
diff --git a/utils.h b/utils.h
index 06ec938..fc581ca 100644
--- a/utils.h
+++ b/utils.h
@@ -73,6 +73,7 @@ int btrfs_scan_block_devices(int run_ioctl);
 u64 parse_size(char *s);
 u64 arg_strtou64(const char *str);
 int open_file_or_dir(const char *fname, DIR **dirstream);
+int open_file_or_dir3(const char *fname, DIR **dirstream, int open_flags);
 void close_file_or_dir(int fd, DIR *dirstream);
 int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
struct btrfs_ioctl_dev_info_args **di_ret);
-- 
1.9.1

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


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

2011-04-26 Thread 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 
---
 btrfs_cmds.c |   48 
 1 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/btrfs_cmds.c b/btrfs_cmds.c
index 8031c58..9367bac 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,38 @@ 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;
+   struct btrfs_ioctl_vol_args_v2  args;
 
-   subvol = argv[1];
-   dst = argv[2];
-   struct btrfs_ioctl_vol_args args;
+   memset(&args, 0, sizeof(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 subvolume snapshot\n");
+   free(argv);
+   return 1;
+   }
+
+   subvol = argv[optind+1];
+   dst = argv[optind+2];
 
res = test_issubvolume(subvol);
if(res<0){
@@ -372,11 +397,18 @@ int do_clone(int argc, char **argv)
return 12;
}
 
-   printf("Create a snapshot of '%s' in '%s/%s'\n",
-  subvol, dstdir, newname);
+   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);
+   }
+
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

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


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

2011-04-25 Thread 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-04-25 Thread 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-04-25 Thread 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 
> ---
>  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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-04-25 Thread 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 
> > ---
> >  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;
> > -intres, fd, fddst, len;
> > +intres, fd, fddst, len, optind = 0, readonly = 0;
> >  char*newname;
> >  char*dstdir;
> >  
> > -subvol = argv[1];
> > -dst = argv[2];
> > -struct btrfs_ioctl_vol_argsargs;
> > +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_v2args;
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-04-25 Thread 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 
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-04-25 Thread 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 
---
 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

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


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

2011-04-13 Thread 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 
---
 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

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


Re: read-only subvolumes?

2011-03-24 Thread Li Zefan
> IMHO, this is related to how the debug options of the kernel are
> configured. Attached you find two config files, both for kernel
> version 2.6.38, with the one named 2.6.38-debug everything works and
> with the other one newly created subvolumes are read only.
> 

I've figured out what's wrong.

The root cause is the flags field of the root item for a new subvol
is never _initialized_!! so the on disk root_item->flags can be of
arbitrary value..

(so is root_item->byte_limit btw.)

I don't have a perfect solution at the moment, but I think a workaround
is to use a flag in root_item->inode_item->flags to indicate if
root->flags is initialized.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: read-only subvolumes?

2011-03-23 Thread Li Zefan
> When I am creating subvolumes I get this strange behavior. If
> I create a subvolume with a name longer than 4 characters it
> is read-only, if the name is shorter than 5 characters the
> subvolume is writeable as expected. I think it is since I
> upgraded to kernel version 2.6.38 (I do not create
> subvolumes on a regular basis.). I will compile one of the
> latest 2.6.37 kernels to see whether there the problem
> exists, too. Another interesting point is that previously
> created subvolumes are not affected.
>
> Thanks, Andreas Philipp
>
> thor btrfs # btrfs subvolume create 123456789 Create
> subvolume './123456789' thor btrfs # touch 123456789/lsdkfj
> touch: cannot touch `123456789/lsdkfj': Read-only file
> system
>>
 This is really odd, but I can't reproduce it.
>>
 I created a btrfs filesystem on 2.6.37 kernel, and rebooted to
 latest 2.6.38+, and tried the procedures as you did, but
 nothing bad happend.
>>> While playing around I found the following three new points: -
>>> Now the length of the subvolume name does not matter. So even the
>>> ones with short names are read-only. - It also happens to a fresh
>>> newly created btrfs filesystem. - If I take a snapshot of an
>>> "old" (= writeable) subvolume this is writeable.
>>
>>> I will now reboot into 2.6.37.4, check there, and then report
>>> back.
>>
>> Well, this was fast. Everything works as expected on 2.6.37.4. See
>> the output of uname -a for the exact kernel version below. I will
>> now reboot into a differently configured kernel version 2.6.38 and
>> look whether the problem is gone there.
>>
>> Thanks, Andreas Philipp
>>
>> thor ~ # uname -a Linux thor 2.6.37.4 #2 SMP Wed Mar 23 10:25:54
>> CET 2011 x86_64 Intel(R) Core(TM)2 CPU 6600 @ 2.40GHz GenuineIntel
>> GNU/Linux
> 
> IMHO, this is related to how the debug options of the kernel are
> configured. Attached you find two config files, both for kernel
> version 2.6.38, with the one named 2.6.38-debug everything works and
> with the other one newly created subvolumes are read only.
> 

I'll see if I can reproduce the problem using your config. Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: read-only subvolumes?

2011-03-23 Thread Andreas Philipp

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
 
On 23.03.2011 11:07, Andreas Philipp wrote:
>
> On 23.03.2011 10:25, Li Zefan wrote:
>>> Hi all,
>>>
>>> When I am creating subvolumes I get this strange behavior. If I
>>> create a subvolume with a name longer than 4 characters it is
>>> read-only, if the name is shorter than 5 characters the
>>> subvolume is writeable as expected. I think it is since I
>>> upgraded to kernel version 2.6.38 (I do not create subvolumes
>>> on a regular basis.). I will compile one of the latest 2.6.37
>>> kernels to see whether there the problem exists, too. Another
>>> interesting point is that previously created subvolumes are
>>> not affected.
>>>
>>> Thanks, Andreas Philipp
>>>
>>> thor btrfs # btrfs subvolume create 123456789 Create subvolume
>>> './123456789' thor btrfs # touch 123456789/lsdkfj touch:
>>> cannot touch `123456789/lsdkfj': Read-only file system
>
>> This is really odd, but I can't reproduce it.
>
>> I created a btrfs filesystem on 2.6.37 kernel, and rebooted to
>> latest 2.6.38+, and tried the procedures as you did, but nothing
>> bad happend.
> While playing around I found the following three new points: - Now
> the length of the subvolume name does not matter. So even the ones
> with short names are read-only. - It also happens to a fresh newly
> created btrfs filesystem. - If I take a snapshot of an "old" (=
> writeable) subvolume this is writeable.
>
> I will now reboot into 2.6.37.4, check there, and then report
> back.

Well, this was fast. Everything works as expected on 2.6.37.4. See the
output of uname -a for the exact kernel version below.
I will now reboot into a differently configured kernel version 2.6.38
and look whether the problem is gone there.

Thanks,
Andreas Philipp

thor ~ # uname -a
Linux thor 2.6.37.4 #2 SMP Wed Mar 23 10:25:54 CET 2011 x86_64
Intel(R) Core(TM)2 CPU 6600 @ 2.40GHz GenuineIntel GNU/Linux
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
 
iQIcBAEBAgAGBQJNicgcAAoJEJIcBJ3+XkgiDRwP/jVRcrc+Qwq8D3rE50MjBox0
vy8ZKna2MXO4dl6Et8td1ikL0T91eueIjvOeaU5cS8vxknG7xqGh9k89Nd74j98a
2paWOiR49vYYhcKF1EZm6oKgHri/N/1RfLWvhXJef3POprwz3/n3YZcSDsiXcAnJ
M8RnGgYFoXNGamPorp32rR5XMln9A6Uma+cUZuaL4eitvsZ+YDsYk4XKZ/8O+cql
u5xKihRNDRqQL7LCfqfL0iJxDl3AReOdXUo8sBmo2ioLNv+syJJhhJ2XRbx7r8rM
LDWOnsBE1oCq2QuM49MDxuD4JFhCmTJ6oJotaBShcU0J0S8Dlu1URucDO7P33BOK
qBFnavR3HaUR+MRor7U+LmeYvasmhj/hUa1nx5jvMEQqeTIioQmYLdllyvHGApfy
R4n1+/L91mRr56s96DC31mF7xnSC13LVLJLG+r3ktlj9/u6B+8LAISgo1uDJX681
YQ5KkI8O+5AcAT8Hu1pwdQVC+LXDPp8HIqL59pUWD2v4zyynVqSKgCSKLJ10npLF
+NZRhSb6czNSvM0UrUBXPLq1th+ErfMNn4b6RCrAPbA4T5bejvCUUlkx7FiAMmVx
rnfiyolblNMfQ+9rY9k8zzZeJfR88wx7yS2VoZlV7n68K01GMy+NDRK203TjcX36
+Y8kUmwptiXc48H6teUN
=aSSq
-END PGP SIGNATURE-

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


Re: read-only subvolumes?

2011-03-23 Thread Andreas Philipp

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
 
On 23.03.2011 10:25, Li Zefan wrote:
>> Hi all,
>>
>> When I am creating subvolumes I get this strange behavior. If I
>> create a subvolume with a name longer than 4 characters it is
>> read-only, if the name is shorter than 5 characters the subvolume
>> is writeable as expected. I think it is since I upgraded to
>> kernel version 2.6.38 (I do not create subvolumes on a regular
>> basis.). I will compile one of the latest 2.6.37 kernels to see
>> whether there the problem exists, too. Another interesting point
>> is that previously created subvolumes are not affected.
>>
>> Thanks, Andreas Philipp
>>
>> thor btrfs # btrfs subvolume create 123456789 Create subvolume
>> './123456789' thor btrfs # touch 123456789/lsdkfj touch: cannot
>> touch `123456789/lsdkfj': Read-only file system
>
> This is really odd, but I can't reproduce it.
>
> I created a btrfs filesystem on 2.6.37 kernel, and rebooted to
> latest 2.6.38+, and tried the procedures as you did, but nothing
> bad happend.
While playing around I found the following three new points:
- - Now the length of the subvolume name does not matter. So even the
ones with short names are read-only.
- - It also happens to a fresh newly created btrfs filesystem.
- - If I take a snapshot of an "old" (= writeable) subvolume this is
writeable.

I will now reboot into 2.6.37.4, check there, and then report back.

Thanks,
Andreas Philipp
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
 
iQIcBAEBAgAGBQJNicZIAAoJEJIcBJ3+XkgiDysP/1oo770VqaEhf3F9gXq5/V3W
AkGGuRb0Upkwie5Y7L3YjCAjAJplYCemncsjqLDQVIQP6iYfmC3bLIM1GjDjMLfT
uwt89/pDte2JStW6kFx0u5i7IwYD6NO7vh3/i7+l1RB4qpZ7DAomroeHS5FFgD2M
y6hZcQ/bhiRKDv82c7YscBVE3ZgKIDPUHoNeduCGsCj8hSd4+/8PR7auGjv42a/l
C92G01cx4mMS0pmnwLUL4U54n1rbJNrKkaoQwINNW/E3fj6gQRwtI1QyDhDWnmfO
Y6c3JRtyYeWGadCaMq4SYGWvSFhG8jlR/a17ozubrLf/An14ywohx1pUZq0fPp9z
oxSlZCINhGBDSeahGQBw7szmU45lXf8N99TgaUTLiHyStnlQfcqpD5RyJUTSBOa2
VAVpMeuvjqw1ng+Tsd1r35e/WBtPQOd9aUj6r5Hcjt4oGlV0mL7oBAR/J0DjNYfl
kii8Ah+NWHFVw/pUVfWC3lzcwfqFIikvn3KVsR2X4LrOTmi6thrh0EG+eSOhfWuf
dI/agqONGzNGH73V7jFtWaEjetrhqRrr5Q22syqWfqX/AYbzTAlISHm574RPtf0G
P2r1fn/s/3FXGKo4zfTsscuvEE4LJaKFrjFxz5mW4wOz9hhFmTBox71ex538ZiMv
NfZzNRKpmXyZCm8USF/i
=b3lE
-END PGP SIGNATURE-

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


Re: read-only subvolumes?

2011-03-23 Thread Li Zefan
> Hi all,
> 
> When I am creating subvolumes I get this strange behavior. If I create
> a subvolume with a name longer than 4 characters it is read-only, if
> the name is shorter than 5 characters the subvolume is writeable as
> expected. I think it is since I upgraded to kernel version 2.6.38 (I
> do not create subvolumes on a regular basis.). I will compile one of
> the latest 2.6.37 kernels to see whether there the problem exists,
> too. Another interesting point is that previously created subvolumes
> are not affected.
> 
> Thanks,
> Andreas Philipp
> 
> thor btrfs # btrfs subvolume create 123456789
> Create subvolume './123456789'
> thor btrfs # touch 123456789/lsdkfj
> touch: cannot touch `123456789/lsdkfj': Read-only file system

This is really odd, but I can't reproduce it.

I created a btrfs filesystem on 2.6.37 kernel, and rebooted to latest
2.6.38+, and tried the procedures as you did, but nothing bad happend.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: read-only subvolumes?

2011-03-23 Thread Fajar A. Nugraha
On Wed, Mar 23, 2011 at 3:21 PM, Andreas Philipp
 wrote:
> I think it is since I upgraded to kernel version 2.6.38 (I
> do not create subvolumes on a regular basis.).

> thor btrfs # btrfs subvolume create 123456789
> Create subvolume './123456789'
> thor btrfs # touch 123456789/lsdkfj
> touch: cannot touch `123456789/lsdkfj': Read-only file system

It works on my system

# touch test1
# btrfs su cr 123456789
Create subvolume './123456789'
# touch 123456789/lsdkfj
# uname -a
Linux HP 2.6.38-020638-generic #201103151303 SMP Tue Mar 15 14:33:40
UTC 2011 i686 GNU/Linux

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


read-only subvolumes?

2011-03-23 Thread Andreas Philipp

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
 
Hi all,

When I am creating subvolumes I get this strange behavior. If I create
a subvolume with a name longer than 4 characters it is read-only, if
the name is shorter than 5 characters the subvolume is writeable as
expected. I think it is since I upgraded to kernel version 2.6.38 (I
do not create subvolumes on a regular basis.). I will compile one of
the latest 2.6.37 kernels to see whether there the problem exists,
too. Another interesting point is that previously created subvolumes
are not affected.

Thanks,
Andreas Philipp

thor btrfs # btrfs subvolume create 123456789
Create subvolume './123456789'
thor btrfs # touch 123456789/lsdkfj
touch: cannot touch `123456789/lsdkfj': Read-only file system

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
 
iQIcBAEBAgAGBQJNia2nAAoJEJIcBJ3+XkgiTQQQAJqvn+zYbDmqmSo8ZRG86ssR
Tj0hsaAYjSWiIUxs7Y9XulmC0sZoSpvX5BLIjW1pYwECzhzrA//pQrwVbXgwbW5H
VjZ+YxcOcw4jxoUbW3lG+KYtSFMJFtbdMejmCY3GgYYIq1mtn0hBrCqZJ0syl4LO
IjyTHR/v0r7FMIgL26F1jOfC478RfhIxAgZOtd65kl7/pHOv5At+99tgM4teUoy0
I76CWu6Ls9+1XevxMWp39XNceYCtQ/WoEThuQCvPERq6Th3NWczPBTP3POSBetVA
Kcomq0TmgXQx1ZalFAFpMi9iRriDXbSm3ITSZW6Jp2BSEPurzpydchfhg0AWVNcC
Icp5b+dy2RVM/K5UNDO6lNf8p+K1wk8GGpD/Pr+K0lO0FlKX+6rApzgx54GYL3cx
0RYL+NAAwSpy1i2uBIw72gyGX/yBliX7CB+YZZ/iULk0eUd36FvpJJAJ1Isk+QNn
6WFBoRwsMrL3WfiqR5/ODO+i+z+CUzYU0mUnD9IuQkdCANyXOeQhs5AyMOPkB1NC
SS9ChpL60khwmLs9c99AyIzcvZU/q12JMvOZ2YUnfEHNIC/XmaThq11RbCIWIsl2
vjPr1QvKK+aykaOfjiTgLTwvB3mq147uEAylzIkduiQSFizMudbsI9vcO/X2pcy3
SVO9m6tlBUsCq3dU1dcA
=NEEb
-END PGP SIGNATURE-

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