Re: [PATCH 2/3] btrfs: implement 'async-snapshot' command

2010-11-02 Thread Sage Weil
On Tue, 2 Nov 2010, Goffredo Baroncelli wrote:
 On Monday, 01 November, 2010, Sage Weil wrote:
  On Sat, 30 Oct 2010, Goffredo Baroncelli wrote:
   On Saturday, 30 October, 2010, Sage Weil wrote:
This is identical to 'snapshot', but uses the new async snapshot 
 creation
ioctl, and prints out the transid the new snapshot will be committed
with.

Signed-off-by: Sage Weil s...@newdream.net
---
 btrfs.c  |8 +++-
 btrfs_cmds.c |   32 +++-
 btrfs_cmds.h |3 ++-
 3 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 46314cf..c4b9a31 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -44,11 +44,17 @@ static struct Command commands[] = {
/*
avoid short commands different for the case only
*/
-   { do_clone, 2,
+   { do_create_snap, 2,
  subvolume snapshot, source [dest/]name\n
Create a writable snapshot of the subvolume source 
with\n
the name name in the dest directory.
},
+   { do_create_snap_async, 2,
+ subvolume async-snapshot, source [dest/]name\n
+   Create a writable snapshot of the subvolume source 
with\n
+   the name name in the dest directory.  Do not wait 
for\n
+   the snapshot creation to commit to disk before 
returning.
+   },
   
   Why create another command ? I think that it is better add a switch like
   
   btrfs subvolume snapshot [--async] source [dest/]name
  
  How about
  
   btrfs subvolume snapshot source [dest/]name [async]
  
  That avoids ambiguity when source is named '--async' and simplifies 
  parsing.  Updated patches follow (including man page updates).
  
  sage
  
 
 No please ! Sorry but this is too strange as syntax.
 
 When the source is named --async it is possible to execute the command 
 passing ./--async. This workaround is used in any unix command (have you 
 ever thought what happens if you execute rm * when exists a file named 
 -rf  
 ?).

Oh right, that's better.  I'm used to using -- (as in 'grep -- -foo'; rm 
supports the same syntax).  Will resend.

sage


 I think that --async is a modification of the standard behaviour of btrfs 
 s 
 s command, so it is natural to use the switch syntax (30+ years of unix say 
 so :-) )
 
 Goffredo
 
 -- 
 gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) kreij...@inwind.it
 Key fingerprint = 4769 7E51 5293 D36C 814E  C054 BF04 F161 3DC5 0512
 
 
--
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 2/3] btrfs: implement 'async-snapshot' command

2010-11-01 Thread Sage Weil
On Sat, 30 Oct 2010, Goffredo Baroncelli wrote:
 On Saturday, 30 October, 2010, Sage Weil wrote:
  This is identical to 'snapshot', but uses the new async snapshot creation
  ioctl, and prints out the transid the new snapshot will be committed
  with.
  
  Signed-off-by: Sage Weil s...@newdream.net
  ---
   btrfs.c  |8 +++-
   btrfs_cmds.c |   32 +++-
   btrfs_cmds.h |3 ++-
   3 files changed, 36 insertions(+), 7 deletions(-)
  
  diff --git a/btrfs.c b/btrfs.c
  index 46314cf..c4b9a31 100644
  --- a/btrfs.c
  +++ b/btrfs.c
  @@ -44,11 +44,17 @@ static struct Command commands[] = {
  /*
  avoid short commands different for the case only
  */
  -   { do_clone, 2,
  +   { do_create_snap, 2,
subvolume snapshot, source [dest/]name\n
  Create a writable snapshot of the subvolume source with\n
  the name name in the dest directory.
  },
  +   { do_create_snap_async, 2,
  + subvolume async-snapshot, source [dest/]name\n
  +   Create a writable snapshot of the subvolume source with\n
  +   the name name in the dest directory.  Do not wait for\n
  +   the snapshot creation to commit to disk before returning.
  +   },
 
 Why create another command ? I think that it is better add a switch like
 
 btrfs subvolume snapshot [--async] source [dest/]name

How about

 btrfs subvolume snapshot source [dest/]name [async]

That avoids ambiguity when source is named '--async' and simplifies 
parsing.  Updated patches follow (including man page updates).

sage




 
   Create a writable snapshot of the subvolume source with
   the name name in the dest directory.  If --async is
   passed, do not wait for the snapshot creation to commit to
   disk before returning.
 
 In any case, please update the man page too.
 
 
 
  { do_delete_subvolume, 1,
subvolume delete, subvolume\n
  Delete the subvolume subvolume.
  diff --git a/btrfs_cmds.c b/btrfs_cmds.c
  index 8031c58..6da5862 100644
  --- a/btrfs_cmds.c
  +++ b/btrfs_cmds.c
  @@ -307,7 +307,7 @@ int do_subvol_list(int argc, char **argv)
  return 0;
   }
   
  -int do_clone(int argc, char **argv)
  +static int create_snap(int argc, char **argv, int async)
   {
  char*subvol, *dst;
  int res, fd, fddst, len;
  @@ -316,7 +316,6 @@ int do_clone(int argc, char **argv)
   
  subvol = argv[1];
  dst = argv[2];
  -   struct btrfs_ioctl_vol_args args;
   
  res = test_issubvolume(subvol);
  if(res0){
  @@ -374,9 +373,22 @@ int do_clone(int argc, char **argv)
   
  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);
  +   if (async) {
  +   struct btrfs_ioctl_async_vol_args   async_args;
  +   async_args.fd = fd;
  +   async_args.transid = 0;
  +   strcpy(async_args.name, newname);
  +   res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_ASYNC, async_args);
  +   if (res == 0)
  +   printf(transid %llu\n,
  +  (unsigned long long)async_args.transid);
  +   } else {
  +   struct btrfs_ioctl_vol_args args;
  +
  +   args.fd = fd;
  +   strcpy(args.name, newname);
  +   res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE, args);
  +   }
   
  close(fd);
  close(fddst);
  @@ -390,6 +402,16 @@ int do_clone(int argc, char **argv)
   
   }
   
  +int do_create_snap_async(int argc, char **argv)
  +{
  +   return create_snap(argc, argv, 1);
  +}
  +
  +int do_create_snap(int argc, char **argv)
  +{
  +   return create_snap(argc, argv, 0);
  +}
  +
   int do_delete_subvolume(int argc, char **argv)
   {
  int res, fd, len;
  diff --git a/btrfs_cmds.h b/btrfs_cmds.h
  index 7bde191..c44dc79 100644
  --- a/btrfs_cmds.h
  +++ b/btrfs_cmds.h
  @@ -15,7 +15,8 @@
*/
   
   /* btrfs_cmds.c*/
  -int do_clone(int nargs, char **argv);
  +int do_create_snap(int nargs, char **argv);
  +int do_create_snap_async(int nargs, char **argv);
   int do_delete_subvolume(int nargs, char **argv);
   int do_create_subvol(int nargs, char **argv);
   int do_fssync(int nargs, char **argv);
  -- 
  1.7.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
  
 
 
 -- 
 gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) kreij...@inwind.it
 Key fingerprint = 4769 7E51 5293 D36C 814E  C054 BF04 F161 3DC5 0512
 --
 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 

[PATCH 2/3] btrfs: implement 'async-snapshot' command

2010-10-30 Thread Sage Weil
This is identical to 'snapshot', but uses the new async snapshot creation
ioctl, and prints out the transid the new snapshot will be committed
with.

Signed-off-by: Sage Weil s...@newdream.net
---
 btrfs.c  |8 +++-
 btrfs_cmds.c |   32 +++-
 btrfs_cmds.h |3 ++-
 3 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 46314cf..c4b9a31 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -44,11 +44,17 @@ static struct Command commands[] = {
/*
avoid short commands different for the case only
*/
-   { do_clone, 2,
+   { do_create_snap, 2,
  subvolume snapshot, source [dest/]name\n
Create a writable snapshot of the subvolume source with\n
the name name in the dest directory.
},
+   { do_create_snap_async, 2,
+ subvolume async-snapshot, source [dest/]name\n
+   Create a writable snapshot of the subvolume source with\n
+   the name name in the dest directory.  Do not wait for\n
+   the snapshot creation to commit to disk before returning.
+   },
{ do_delete_subvolume, 1,
  subvolume delete, subvolume\n
Delete the subvolume subvolume.
diff --git a/btrfs_cmds.c b/btrfs_cmds.c
index 8031c58..6da5862 100644
--- a/btrfs_cmds.c
+++ b/btrfs_cmds.c
@@ -307,7 +307,7 @@ int do_subvol_list(int argc, char **argv)
return 0;
 }
 
-int do_clone(int argc, char **argv)
+static int create_snap(int argc, char **argv, int async)
 {
char*subvol, *dst;
int res, fd, fddst, len;
@@ -316,7 +316,6 @@ int do_clone(int argc, char **argv)
 
subvol = argv[1];
dst = argv[2];
-   struct btrfs_ioctl_vol_args args;
 
res = test_issubvolume(subvol);
if(res0){
@@ -374,9 +373,22 @@ int do_clone(int argc, char **argv)
 
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);
+   if (async) {
+   struct btrfs_ioctl_async_vol_args   async_args;
+   async_args.fd = fd;
+   async_args.transid = 0;
+   strcpy(async_args.name, newname);
+   res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_ASYNC, async_args);
+   if (res == 0)
+   printf(transid %llu\n,
+  (unsigned long long)async_args.transid);
+   } else {
+   struct btrfs_ioctl_vol_args args;
+
+   args.fd = fd;
+   strcpy(args.name, newname);
+   res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE, args);
+   }
 
close(fd);
close(fddst);
@@ -390,6 +402,16 @@ int do_clone(int argc, char **argv)
 
 }
 
+int do_create_snap_async(int argc, char **argv)
+{
+   return create_snap(argc, argv, 1);
+}
+
+int do_create_snap(int argc, char **argv)
+{
+   return create_snap(argc, argv, 0);
+}
+
 int do_delete_subvolume(int argc, char **argv)
 {
int res, fd, len;
diff --git a/btrfs_cmds.h b/btrfs_cmds.h
index 7bde191..c44dc79 100644
--- a/btrfs_cmds.h
+++ b/btrfs_cmds.h
@@ -15,7 +15,8 @@
  */
 
 /* btrfs_cmds.c*/
-int do_clone(int nargs, char **argv);
+int do_create_snap(int nargs, char **argv);
+int do_create_snap_async(int nargs, char **argv);
 int do_delete_subvolume(int nargs, char **argv);
 int do_create_subvol(int nargs, char **argv);
 int do_fssync(int nargs, char **argv);
-- 
1.7.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 2/3] btrfs: implement 'async-snapshot' command

2010-10-30 Thread Goffredo Baroncelli
On Saturday, 30 October, 2010, Sage Weil wrote:
 This is identical to 'snapshot', but uses the new async snapshot creation
 ioctl, and prints out the transid the new snapshot will be committed
 with.
 
 Signed-off-by: Sage Weil s...@newdream.net
 ---
  btrfs.c  |8 +++-
  btrfs_cmds.c |   32 +++-
  btrfs_cmds.h |3 ++-
  3 files changed, 36 insertions(+), 7 deletions(-)
 
 diff --git a/btrfs.c b/btrfs.c
 index 46314cf..c4b9a31 100644
 --- a/btrfs.c
 +++ b/btrfs.c
 @@ -44,11 +44,17 @@ static struct Command commands[] = {
   /*
   avoid short commands different for the case only
   */
 - { do_clone, 2,
 + { do_create_snap, 2,
 subvolume snapshot, source [dest/]name\n
   Create a writable snapshot of the subvolume source with\n
   the name name in the dest directory.
   },
 + { do_create_snap_async, 2,
 +   subvolume async-snapshot, source [dest/]name\n
 + Create a writable snapshot of the subvolume source with\n
 + the name name in the dest directory.  Do not wait for\n
 + the snapshot creation to commit to disk before returning.
 + },

Why create another command ? I think that it is better add a switch like

btrfs subvolume snapshot [--async] source [dest/]name

Create a writable snapshot of the subvolume source with
the name name in the dest directory.  If --async is
passed, do not wait for the snapshot creation to commit to
disk before returning.

In any case, please update the man page too.



   { do_delete_subvolume, 1,
 subvolume delete, subvolume\n
   Delete the subvolume subvolume.
 diff --git a/btrfs_cmds.c b/btrfs_cmds.c
 index 8031c58..6da5862 100644
 --- a/btrfs_cmds.c
 +++ b/btrfs_cmds.c
 @@ -307,7 +307,7 @@ int do_subvol_list(int argc, char **argv)
   return 0;
  }
  
 -int do_clone(int argc, char **argv)
 +static int create_snap(int argc, char **argv, int async)
  {
   char*subvol, *dst;
   int res, fd, fddst, len;
 @@ -316,7 +316,6 @@ int do_clone(int argc, char **argv)
  
   subvol = argv[1];
   dst = argv[2];
 - struct btrfs_ioctl_vol_args args;
  
   res = test_issubvolume(subvol);
   if(res0){
 @@ -374,9 +373,22 @@ int do_clone(int argc, char **argv)
  
   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);
 + if (async) {
 + struct btrfs_ioctl_async_vol_args   async_args;
 + async_args.fd = fd;
 + async_args.transid = 0;
 + strcpy(async_args.name, newname);
 + res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_ASYNC, async_args);
 + if (res == 0)
 + printf(transid %llu\n,
 +(unsigned long long)async_args.transid);
 + } else {
 + struct btrfs_ioctl_vol_args args;
 +
 + args.fd = fd;
 + strcpy(args.name, newname);
 + res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE, args);
 + }
  
   close(fd);
   close(fddst);
 @@ -390,6 +402,16 @@ int do_clone(int argc, char **argv)
  
  }
  
 +int do_create_snap_async(int argc, char **argv)
 +{
 + return create_snap(argc, argv, 1);
 +}
 +
 +int do_create_snap(int argc, char **argv)
 +{
 + return create_snap(argc, argv, 0);
 +}
 +
  int do_delete_subvolume(int argc, char **argv)
  {
   int res, fd, len;
 diff --git a/btrfs_cmds.h b/btrfs_cmds.h
 index 7bde191..c44dc79 100644
 --- a/btrfs_cmds.h
 +++ b/btrfs_cmds.h
 @@ -15,7 +15,8 @@
   */
  
  /* btrfs_cmds.c*/
 -int do_clone(int nargs, char **argv);
 +int do_create_snap(int nargs, char **argv);
 +int do_create_snap_async(int nargs, char **argv);
  int do_delete_subvolume(int nargs, char **argv);
  int do_create_subvol(int nargs, char **argv);
  int do_fssync(int nargs, char **argv);
 -- 
 1.7.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
 


-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) kreij...@inwind.it
Key fingerprint = 4769 7E51 5293 D36C 814E  C054 BF04 F161 3DC5 0512
--
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 2/3] btrfs: implement 'async-snapshot' command

2010-10-30 Thread Goffredo Baroncelli
On Saturday, 30 October, 2010, Sage Weil wrote:
 This is identical to 'snapshot', but uses the new async snapshot creation
 ioctl, and prints out the transid the new snapshot will be committed
 with.

Only for curiosity, how long may take snapshot a tree ? It should be only a 
copy and update of few pages on the disk (the head of the trees), so the time 
should be O(1)...

Goffredo

[...]
-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) kreij...@inwind.it
Key fingerprint = 4769 7E51 5293 D36C 814E  C054 BF04 F161 3DC5 0512
--
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