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