Re: [PATCH] Btrfs-progs: allow compression property gets for read-only subvolumes
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
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.
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.
>> +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.
> + 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.
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.
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.
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.
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.
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?
> 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?
> 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?
-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?
-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?
> 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?
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?
-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