thr3ads.net - Btrfs devel - [PATCH] btrfs-progs: ioctl: add may_alias to btrfs_ioctl_search

If this information is useful, please help other people find it:
Share via:

Sage Weil

2010-Nov-01 16:34 UTC

[PATCH] btrfs-progs: ioctl: add may_alias to btrfs_ioctl_search_header

This fixes the errors

btrfs-list.c: In function ''ino_resolve'':
btrfs-list.c:506: error: dereferencing pointer ''sh'' does break
strict-aliasing rules
btrfs-list.c:504: error: dereferencing pointer ''sh'' does break
strict-aliasing rules
btrfs-list.c:502: note: initialized from here

when building with gcc version 4.4.4 (Debian 4.4.4-6).

Signed-off-by: Sage Weil <sage@newdream.net>
---
 ioctl.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/ioctl.h b/ioctl.h
index 5a03317..4a850ff 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -87,7 +87,7 @@ struct btrfs_ioctl_search_header {
 	__u64 offset;
 	__u32 type;
 	__u32 len;
-};
+} __attribute__((__may_alias__));
 
 #define BTRFS_SEARCH_ARGS_BUFSIZE (4096 - sizeof(struct
btrfs_ioctl_search_key))
 /*
-- 
1.7.0

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

Sage Weil

2010-Nov-01 16:34 UTC

head link

[PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands

The ''start-sync'' command initiates a sync, but does not wait
for it to
complete.  A transaction is printed that can be fed to
''wait-sync'', which
will wait for it to commit.

''wait-sync'' can also be used in combination with
''async-snapshot'' to wait
for an async snapshot creation to commit.

Updates the man page too.

Signed-off-by: Sage Weil <sage@newdream.net>
---
 btrfs.c        |    9 +++++++++
 btrfs_cmds.c   |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 btrfs_cmds.h   |    2 ++
 man/btrfs.8.in |   14 ++++++++++++++
 4 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 46314cf..c871f4a 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -77,6 +77,15 @@ static struct Command commands[] = {
 	  "filesystem sync", "<path>\n"
 		"Force a sync on the filesystem <path>."
 	},
+	{ do_start_sync, 1,
+	  "filesystem start-sync", "<path>\n"
+	        "Start a sync on the filesystem <path>, and print the
resulting\n"
+	        "transaction id."
+	},
+	{ do_wait_sync, 2,
+	  "filesystem wait-sync", "<path> <transid>\n"
+	        "Wait for the transaction <transid> on the filesystem at
<path> to commit."
+	},
 	{ do_resize, 2,
 	  "filesystem resize", "[+/-]<newsize>[gkm]|max
<filesystem>\n"
 		"Resize the file system. If ''max'' is passed, the
filesystem\n"
diff --git a/btrfs_cmds.c b/btrfs_cmds.c
index 8031c58..736437d 100644
--- a/btrfs_cmds.c
+++ b/btrfs_cmds.c
@@ -526,6 +526,55 @@ int do_fssync(int argc, char **argv)
 	return 0;
 }
 
+int do_start_sync(int argc, char **argv)
+{
+	int fd, res;
+	char	*path = argv[1];
+	__u64 transid;
+
+	fd = open_file_or_dir(path);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
+		return 12;
+	}
+
+	printf("StartSync ''%s''\n", path);
+	res = ioctl(fd, BTRFS_IOC_START_SYNC, &transid);
+	close(fd);
+	if( res < 0 ){
+		fprintf(stderr, "ERROR: unable to fs-syncing
''%s''\n", path);
+		return 16;
+	} else {
+		printf("transid %llu\n", (unsigned long long)transid);
+	}
+
+	return 0;
+}
+
+int do_wait_sync(int argc, char **argv)
+{
+	int fd, res;
+	char	*path = argv[1];
+	__u64 transid = atoll(argv[2]);
+
+	fd = open_file_or_dir(path);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
+		return 12;
+	}
+
+	printf("WaitSync ''%s'' transid %llu\n", path,
(unsigned long long)transid);
+	res = ioctl(fd, BTRFS_IOC_WAIT_SYNC, &transid);
+	close(fd);
+	if( res < 0 ){
+		fprintf(stderr, "ERROR: unable to wait-sync on ''%s''
transid %llu: %s\n", path,
+			(unsigned long long)transid, strerror(errno));
+		return 16;
+	}
+
+	return 0;
+}
+
 int do_scan(int argc, char **argv)
 {
 	int	i, fd;
diff --git a/btrfs_cmds.h b/btrfs_cmds.h
index 7bde191..84c489f 100644
--- a/btrfs_cmds.h
+++ b/btrfs_cmds.h
@@ -19,6 +19,8 @@ int do_clone(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);
+int do_start_sync(int nargs, char **argv);
+int do_wait_sync(int nargs, char **argv);
 int do_defrag(int argc, char **argv);
 int do_show_filesystem(int nargs, char **argv);
 int do_add_volume(int nargs, char **args);
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 26ef982..e87b5fe 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -19,6 +19,10 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBfilesystem sync\fP\fI <path> \fP
 .PP
+\fBbtrfs\fP \fBfilesystem start-sync\fP\fI <path> \fP
+.PP
+\fBbtrfs\fP \fBfilesystem wait-sync\fP\fI <path> <transid>\fP
+.PP
 \fBbtrfs\fP \fBfilesystem resize\fP\fI [+/\-]<size>[gkm]|max
<filesystem>\fP
 .PP
 \fBbtrfs\fP \fBdevice scan\fP\fI [<device> [<device>..]]\fP
@@ -115,6 +119,16 @@ all the block devices.
 Force a sync for the filesystem identified by \fI<path>\fR.
 .TP
 
+\fBfilesystem start-sync\fR\fI <path> \fR
+Start a sync operation for the filesystem identified by \fI<path>\fR.  A
transaction id
+is printed that can be waited on using the \fBfilesystem wait-sync\fR command.
+.TP
+
+\fBfilesystem wait-sync\fR\fI <path> <transid>\fR
+Wait for a the transaction \fI<transid>\fR to commit to disk.  If
\fI<transid>\fR is zero,
+wait for any currently committing transaction to commit.
+.TP
+
 .\"
 .\" Some wording are extracted by the resize2fs man page
 .\"
-- 
1.7.0

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

Sage Weil

2010-Nov-01 16:34 UTC

head link

[PATCH v2] btrfs-progs: btrfs: implement async option for snapshot command

Add an ''async'' option to the snapshot creating command, that
will let you
avoid waiting for a new snapshot to commit to disk.

Signed-off-by: Sage Weil <sage@newdream.net>
---
 btrfs.c        |    9 ++++++---
 btrfs_cmds.c   |   31 +++++++++++++++++++++++++------
 btrfs_cmds.h   |    2 +-
 man/btrfs.8.in |   15 ++++++++++-----
 4 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index c871f4a..657bb6f 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -44,10 +44,13 @@ static struct Command commands[] = {
 	/*
 		avoid short commands different for the case only
 	*/
-	{ do_clone, 2,
-	  "subvolume snapshot", "<source>
[<dest>/]<name>\n"
+	{ do_create_snap, -2,
+	  "subvolume snapshot", "<source>
[<dest>/]<name> [async]\n"
 		"Create a writable snapshot of the subvolume <source> with\n"
-		"the name <name> in the <dest> directory."
+		"the name <name> in the <dest> directory.  If [async]
is\n"
+	        "specified, we will not wait for the snapshot to be
committed\n"
+	        "to disk, and a transid will be printed that can be fed
to\n"
+	        "''filesystem wait-sync <transid>''."
 	},
 	{ do_delete_subvolume, 1,
 	  "subvolume delete", "<subvolume>\n"
diff --git a/btrfs_cmds.c b/btrfs_cmds.c
index 736437d..276b8fa 100644
--- a/btrfs_cmds.c
+++ b/btrfs_cmds.c
@@ -307,16 +307,22 @@ int do_subvol_list(int argc, char **argv)
 	return 0;
 }
 
-int do_clone(int argc, char **argv)
+int do_create_snap(int argc, char **argv)
 {
 	char	*subvol, *dst;
-	int	res, fd, fddst, len;
+	int	res, fd, fddst, len, async;
 	char	*newname;
 	char	*dstdir;
 
 	subvol = argv[1];
 	dst = argv[2];
-	struct btrfs_ioctl_vol_args	args;
+	if (argc > 3) {
+		if (strstr(argv[3], "async")) {
+			async = 1;
+		} else {
+			fprintf(stderr, "ERROR: ''async'' expected for third
arg\n");
+		}
+	}
 
 	res = test_issubvolume(subvol);
 	if(res<0){
@@ -374,9 +380,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);
diff --git a/btrfs_cmds.h b/btrfs_cmds.h
index 84c489f..bf566ae 100644
--- a/btrfs_cmds.h
+++ b/btrfs_cmds.h
@@ -15,7 +15,7 @@
  */
 
 /* btrfs_cmds.c*/
-int do_clone(int nargs, char **argv);
+int do_create_snap(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);
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index e87b5fe..504fe5f 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 <source>
[<dest>/]<name> [async]\fP
 .PP
 \fBbtrfs\fP \fBsubvolume delete\fP\fI <subvolume>\fP
 .PP
@@ -74,10 +74,15 @@ 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 <source> [<dest>/]<name>
[async]\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.  If \fI[async]\fR is
+specified, we will not wait for the snapshot creation to commit to
+disk before returning, and a transaction id is printed that can be
+waited on1 using the \fBfilesystem wait-sync\fR command.
+.TP
+
 .TP
 
 \fBsubvolume delete\fR\fI <subvolume>\fR
-- 
1.7.0

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

David Nicol

2010-Nov-01 18:51 UTC

head link

Re: [PATCH v2] btrfs-progs: btrfs: implement async option for snapshot command

this would be yet another thing that ioctl#21 could have a flag to wait for.

On Mon, Nov 1, 2010 at 11:34 AM, Sage Weil <sage@newdream.net>
wrote:> Add an ''async'' option to the snapshot creating command,
that will let you
> avoid waiting for a new snapshot to commit to disk.
>
> Signed-off-by: Sage Weil <sage@newdream.net>--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Goffredo Baroncelli

2010-Nov-02 18:58 UTC

head link

Re: [PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands

On Monday, 01 November, 2010, Sage Weil wrote:> The ''start-sync'' command initiates a sync, but does not
wait for it to
> complete.  A transaction is printed that can be fed to
''wait-sync'', which
> will wait for it to commit.
> 
> ''wait-sync'' can also be used in combination with
''async-snapshot'' to wait
> for an async snapshot creation to commit.
> 
> Updates the man page too.
> 
> Signed-off-by: Sage Weil <sage@newdream.net>
> ---
>  btrfs.c        |    9 +++++++++
>  btrfs_cmds.c   |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  btrfs_cmds.h   |    2 ++
>  man/btrfs.8.in |   14 ++++++++++++++
>  4 files changed, 74 insertions(+), 0 deletions(-)
> 
> diff --git a/btrfs.c b/btrfs.c
> index 46314cf..c871f4a 100644
> --- a/btrfs.c
> +++ b/btrfs.c
> @@ -77,6 +77,15 @@ static struct Command commands[] = {
>  	  "filesystem sync", "<path>\n"
>  		"Force a sync on the filesystem <path>."
>  	},
> +	{ do_start_sync, 1,
> +	  "filesystem start-sync", "<path>\n"
> +	        "Start a sync on the filesystem <path>, and print the 
resulting\n"> +	        "transaction id."
> +	},
Like the command "btrfs subvol snapshot", I think that it is better to
add a
modifier instead of a new command.

 btrfs filesystem sync [--async]

Sorry if I noticed this too late. But I don''t see a valid reason to add
another command. From a UI point of view the meaning of the command is the 
same, change only slight the behavior.

Even tough I have to admint that "sync --async" sound strange. May be
flush is
better ?> +	{ do_wait_sync, 2,
> +	  "filesystem wait-sync", "<path>
<transid>\n"
> +	        "Wait for the transaction <transid> on the filesystem
at
<path> to commit."> +	},
If I understood correctly this command may be used to wait the execution of 
several commands: snapshot, subvolume removal, sync...
I can suggest a more general name like:

# btrfs filesystem wait-transaction <path> <transid>.

(which may be shortened as "btrfs filesystem wait ..."). 

Another suggestion: instead of checking if the <transid> is 0, leave the 
<transid> optional. If <transid> is omitted, then the function waits
all the
running transaction.

Finally I suggest to put a little note about the command behvior when a 
transaction is started after the command execution: is there a risk of DOS ?

Anyway great work.

Goffredo

>  	{ do_resize, 2,
>  	  "filesystem resize", "[+/-]<newsize>[gkm]|max
<filesystem>\n"
>  		"Resize the file system. If ''max'' is passed, the
filesystem\n"
> diff --git a/btrfs_cmds.c b/btrfs_cmds.c
> index 8031c58..736437d 100644
> --- a/btrfs_cmds.c
> +++ b/btrfs_cmds.c
> @@ -526,6 +526,55 @@ int do_fssync(int argc, char **argv)
>  	return 0;
>  }
>  
> +int do_start_sync(int argc, char **argv)
> +{
> +	int fd, res;
> +	char	*path = argv[1];
> +	__u64 transid;
> +
> +	fd = open_file_or_dir(path);
> +	if (fd < 0) {
> +		fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
> +		return 12;
> +	}
> +
> +	printf("StartSync ''%s''\n", path);
> +	res = ioctl(fd, BTRFS_IOC_START_SYNC, &transid);
> +	close(fd);
> +	if( res < 0 ){
> +		fprintf(stderr, "ERROR: unable to fs-syncing
''%s''\n", path);
> +		return 16;
> +	} else {
> +		printf("transid %llu\n", (unsigned long long)transid);
> +	}
> +
> +	return 0;
> +}
> +
> +int do_wait_sync(int argc, char **argv)
> +{
> +	int fd, res;
> +	char	*path = argv[1];
> +	__u64 transid = atoll(argv[2]);
> +
> +	fd = open_file_or_dir(path);
> +	if (fd < 0) {
> +		fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
> +		return 12;
> +	}
> +
> +	printf("WaitSync ''%s'' transid %llu\n", path,
(unsigned long
long)transid);> +	res = ioctl(fd, BTRFS_IOC_WAIT_SYNC, &transid);
> +	close(fd);
> +	if( res < 0 ){
> +		fprintf(stderr, "ERROR: unable to wait-sync on
''%s'' transid
%llu: %s\n", path,> +			(unsigned long long)transid, strerror(errno));
> +		return 16;
> +	}
> +
> +	return 0;
> +}
> +
>  int do_scan(int argc, char **argv)
>  {
>  	int	i, fd;
> diff --git a/btrfs_cmds.h b/btrfs_cmds.h
> index 7bde191..84c489f 100644
> --- a/btrfs_cmds.h
> +++ b/btrfs_cmds.h
> @@ -19,6 +19,8 @@ int do_clone(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);
> +int do_start_sync(int nargs, char **argv);
> +int do_wait_sync(int nargs, char **argv);
>  int do_defrag(int argc, char **argv);
>  int do_show_filesystem(int nargs, char **argv);
>  int do_add_volume(int nargs, char **args);
> diff --git a/man/btrfs.8.in b/man/btrfs.8.in
> index 26ef982..e87b5fe 100644
> --- a/man/btrfs.8.in
> +++ b/man/btrfs.8.in
> @@ -19,6 +19,10 @@ btrfs \- control a btrfs filesystem
>  .PP
>  \fBbtrfs\fP \fBfilesystem sync\fP\fI <path> \fP
>  .PP
> +\fBbtrfs\fP \fBfilesystem start-sync\fP\fI <path> \fP
> +.PP
> +\fBbtrfs\fP \fBfilesystem wait-sync\fP\fI <path> <transid>\fP
> +.PP
>  \fBbtrfs\fP \fBfilesystem resize\fP\fI [+/\-]<size>[gkm]|max 
<filesystem>\fP>  .PP
>  \fBbtrfs\fP \fBdevice scan\fP\fI [<device> [<device>..]]\fP
> @@ -115,6 +119,16 @@ all the block devices.
>  Force a sync for the filesystem identified by \fI<path>\fR.
>  .TP
>  
> +\fBfilesystem start-sync\fR\fI <path> \fR
> +Start a sync operation for the filesystem identified by
\fI<path>\fR.  A
transaction id> +is printed that can be waited on using the \fBfilesystem wait-sync\fR 
command.> +.TP
> +
> +\fBfilesystem wait-sync\fR\fI <path> <transid>\fR
> +Wait for a the transaction \fI<transid>\fR to commit to disk.  If 
\fI<transid>\fR is zero,> +wait for any currently committing transaction to commit.
> +.TP
> +
>  .\"
>  .\" Some wording are extracted by the resize2fs man page
>  .\"
> -- 
> 1.7.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo)
<kreijack@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sage Weil

2010-Nov-02 21:56 UTC

head link

Re: [PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands

On Tue, 2 Nov 2010, Goffredo Baroncelli wrote:> Like the command "btrfs subvol snapshot", I think that it is
better to add a
> modifier instead of a new command.
> 
>  btrfs filesystem sync [--async]
> 
> Sorry if I noticed this too late. But I don''t see a valid reason
to add
> another command. From a UI point of view the meaning of the command is the 
> same, change only slight the behavior.
> 
> Even tough I have to admint that "sync --async" sound strange.
May be flush is
> better ?
> > +	{ do_wait_sync, 2,
> > +	  "filesystem wait-sync", "<path>
<transid>\n"
> > +	        "Wait for the transaction <transid> on the
filesystem at
> <path> to commit."
> > +	},
> 
> If I understood correctly this command may be used to wait the execution of
> several commands: snapshot, subvolume removal, sync...
> I can suggest a more general name like:
> 
> # btrfs filesystem wait-transaction <path> <transid>.
> 
> (which may be shortened as "btrfs filesystem wait ..."). 
How about start-transaction and wait-transaction?  Or start-commit and 
wait-commit?  (''transaction'' is a heavily overloaded term.) 
IMO both
would be less weird than sync --async.
> Another suggestion: instead of checking if the <transid> is 0, leave
the
> <transid> optional. If <transid> is omitted, then the function
waits all the
> running transaction.
Definitely.
> Finally I suggest to put a little note about the command behvior when a 
> transaction is started after the command execution: is there a risk of DOS
?
Okay.  There''s no DOS risk (at least no more so than with while true ;
do
sync ; done).  It''ll return immediately if transid has committed.  If 
transid is 0, it''ll find the most recent committing or committed
transid
and wait for that.  If nothing is currently committing, it''ll return 
immediately.

sage


> 
> Anyway great work.
> 
> Goffredo
> 
> 
> >  	{ do_resize, 2,
> >  	  "filesystem resize", "[+/-]<newsize>[gkm]|max
<filesystem>\n"
> >  		"Resize the file system. If ''max'' is passed,
the filesystem\n"
> > diff --git a/btrfs_cmds.c b/btrfs_cmds.c
> > index 8031c58..736437d 100644
> > --- a/btrfs_cmds.c
> > +++ b/btrfs_cmds.c
> > @@ -526,6 +526,55 @@ int do_fssync(int argc, char **argv)
> >  	return 0;
> >  }
> >  
> > +int do_start_sync(int argc, char **argv)
> > +{
> > +	int fd, res;
> > +	char	*path = argv[1];
> > +	__u64 transid;
> > +
> > +	fd = open_file_or_dir(path);
> > +	if (fd < 0) {
> > +		fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
> > +		return 12;
> > +	}
> > +
> > +	printf("StartSync ''%s''\n", path);
> > +	res = ioctl(fd, BTRFS_IOC_START_SYNC, &transid);
> > +	close(fd);
> > +	if( res < 0 ){
> > +		fprintf(stderr, "ERROR: unable to fs-syncing
''%s''\n", path);
> > +		return 16;
> > +	} else {
> > +		printf("transid %llu\n", (unsigned long long)transid);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int do_wait_sync(int argc, char **argv)
> > +{
> > +	int fd, res;
> > +	char	*path = argv[1];
> > +	__u64 transid = atoll(argv[2]);
> > +
> > +	fd = open_file_or_dir(path);
> > +	if (fd < 0) {
> > +		fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
> > +		return 12;
> > +	}
> > +
> > +	printf("WaitSync ''%s'' transid %llu\n",
path, (unsigned long
> long)transid);
> > +	res = ioctl(fd, BTRFS_IOC_WAIT_SYNC, &transid);
> > +	close(fd);
> > +	if( res < 0 ){
> > +		fprintf(stderr, "ERROR: unable to wait-sync on
''%s'' transid
> %llu: %s\n", path,
> > +			(unsigned long long)transid, strerror(errno));
> > +		return 16;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  int do_scan(int argc, char **argv)
> >  {
> >  	int	i, fd;
> > diff --git a/btrfs_cmds.h b/btrfs_cmds.h
> > index 7bde191..84c489f 100644
> > --- a/btrfs_cmds.h
> > +++ b/btrfs_cmds.h
> > @@ -19,6 +19,8 @@ int do_clone(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);
> > +int do_start_sync(int nargs, char **argv);
> > +int do_wait_sync(int nargs, char **argv);
> >  int do_defrag(int argc, char **argv);
> >  int do_show_filesystem(int nargs, char **argv);
> >  int do_add_volume(int nargs, char **args);
> > diff --git a/man/btrfs.8.in b/man/btrfs.8.in
> > index 26ef982..e87b5fe 100644
> > --- a/man/btrfs.8.in
> > +++ b/man/btrfs.8.in
> > @@ -19,6 +19,10 @@ btrfs \- control a btrfs filesystem
> >  .PP
> >  \fBbtrfs\fP \fBfilesystem sync\fP\fI <path> \fP
> >  .PP
> > +\fBbtrfs\fP \fBfilesystem start-sync\fP\fI <path> \fP
> > +.PP
> > +\fBbtrfs\fP \fBfilesystem wait-sync\fP\fI <path>
<transid>\fP
> > +.PP
> >  \fBbtrfs\fP \fBfilesystem resize\fP\fI [+/\-]<size>[gkm]|max 
> <filesystem>\fP
> >  .PP
> >  \fBbtrfs\fP \fBdevice scan\fP\fI [<device>
[<device>..]]\fP
> > @@ -115,6 +119,16 @@ all the block devices.
> >  Force a sync for the filesystem identified by \fI<path>\fR.
> >  .TP
> >  
> > +\fBfilesystem start-sync\fR\fI <path> \fR
> > +Start a sync operation for the filesystem identified by
\fI<path>\fR.  A
> transaction id
> > +is printed that can be waited on using the \fBfilesystem wait-sync\fR
> command.
> > +.TP
> > +
> > +\fBfilesystem wait-sync\fR\fI <path> <transid>\fR
> > +Wait for a the transaction \fI<transid>\fR to commit to disk. 
If
> \fI<transid>\fR is zero,
> > +wait for any currently committing transaction to commit.
> > +.TP
> > +
> >  .\"
> >  .\" Some wording are extracted by the resize2fs man page
> >  .\"
> > -- 
> > 1.7.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe
linux-btrfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 
> -- 
> gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo)
<kreijack@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 majordomo@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Goffredo Baroncelli

2010-Nov-03 06:13 UTC

head link

Re: [PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands

On Tuesday, 02 November, 2010, you (Sage Weil) wrote:> On Tue, 2 Nov 2010, Goffredo Baroncelli wrote:
> > Like the command "btrfs subvol snapshot", I think that it is
better to add
a > > modifier instead of a new command.
> > 
> >  btrfs filesystem sync [--async]
> > 
> > Sorry if I noticed this too late. But I don''t see a valid
reason to add
> > another command. From a UI point of view the meaning of the command is
the
> > same, change only slight the behavior.
> > 
> > Even tough I have to admint that "sync --async" sound
strange. May be
flush is > > better ?
> > > +	{ do_wait_sync, 2,
> > > +	  "filesystem wait-sync", "<path>
<transid>\n"
> > > +	        "Wait for the transaction <transid> on the
filesystem at
> > <path> to commit."
> > > +	},
> > 
> > If I understood correctly this command may be used to wait the
execution
of > > several commands: snapshot, subvolume removal, sync...
> > I can suggest a more general name like:
> > 
> > # btrfs filesystem wait-transaction <path> <transid>.
> > 
> > (which may be shortened as "btrfs filesystem wait ..."). 
> 
> How about start-transaction and wait-transaction?  Or start-commit and 
> wait-commit?  (''transaction'' is a heavily overloaded
term.)  IMO both
> would be less weird than sync --async.
IMHO, commit is when a transaction start; the transaction is what pushes the 
data on the disk. So I prefer wait/start-transaction. 
But I prefer that an english people says the last word.

BTW which guarantees provide the commands start-wait-transaction ? What happen 
if a powerloss happens before wait-* but after a start*.


Goffredo

> > Another suggestion: instead of checking if the <transid> is 0,
leave the
> > <transid> optional. If <transid> is omitted, then the
function waits all
the > > running transaction.
> 
> Definitely.
> 
> > Finally I suggest to put a little note about the command behvior when
a
> > transaction is started after the command execution: is there a risk of
DOS
?> 
> Okay.  There''s no DOS risk (at least no more so than with while
true ; do
> sync ; done).  It''ll return immediately if transid has committed. 
If
> transid is 0, it''ll find the most recent committing or committed
transid
> and wait for that.  If nothing is currently committing, it''ll
return
> immediately.
> 
> sage
> 
> 
> 
> > 
> > Anyway great work.
> > 
> > Goffredo
> > 
> > 
> > >  	{ do_resize, 2,
> > >  	  "filesystem resize",
"[+/-]<newsize>[gkm]|max <filesystem>\n"
> > >  		"Resize the file system. If ''max'' is
passed, the filesystem\n"
> > > diff --git a/btrfs_cmds.c b/btrfs_cmds.c
> > > index 8031c58..736437d 100644
> > > --- a/btrfs_cmds.c
> > > +++ b/btrfs_cmds.c
> > > @@ -526,6 +526,55 @@ int do_fssync(int argc, char **argv)
> > >  	return 0;
> > >  }
> > >  
> > > +int do_start_sync(int argc, char **argv)
> > > +{
> > > +	int fd, res;
> > > +	char	*path = argv[1];
> > > +	__u64 transid;
> > > +
> > > +	fd = open_file_or_dir(path);
> > > +	if (fd < 0) {
> > > +		fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
> > > +		return 12;
> > > +	}
> > > +
> > > +	printf("StartSync ''%s''\n", path);
> > > +	res = ioctl(fd, BTRFS_IOC_START_SYNC, &transid);
> > > +	close(fd);
> > > +	if( res < 0 ){
> > > +		fprintf(stderr, "ERROR: unable to fs-syncing
''%s''\n", path);
> > > +		return 16;
> > > +	} else {
> > > +		printf("transid %llu\n", (unsigned long
long)transid);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int do_wait_sync(int argc, char **argv)
> > > +{
> > > +	int fd, res;
> > > +	char	*path = argv[1];
> > > +	__u64 transid = atoll(argv[2]);
> > > +
> > > +	fd = open_file_or_dir(path);
> > > +	if (fd < 0) {
> > > +		fprintf(stderr, "ERROR: can''t access to
''%s''\n", path);
> > > +		return 12;
> > > +	}
> > > +
> > > +	printf("WaitSync ''%s'' transid
%llu\n", path, (unsigned long
> > long)transid);
> > > +	res = ioctl(fd, BTRFS_IOC_WAIT_SYNC, &transid);
> > > +	close(fd);
> > > +	if( res < 0 ){
> > > +		fprintf(stderr, "ERROR: unable to wait-sync on
''%s'' transid
> > %llu: %s\n", path,
> > > +			(unsigned long long)transid, strerror(errno));
> > > +		return 16;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  int do_scan(int argc, char **argv)
> > >  {
> > >  	int	i, fd;
> > > diff --git a/btrfs_cmds.h b/btrfs_cmds.h
> > > index 7bde191..84c489f 100644
> > > --- a/btrfs_cmds.h
> > > +++ b/btrfs_cmds.h
> > > @@ -19,6 +19,8 @@ int do_clone(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);
> > > +int do_start_sync(int nargs, char **argv);
> > > +int do_wait_sync(int nargs, char **argv);
> > >  int do_defrag(int argc, char **argv);
> > >  int do_show_filesystem(int nargs, char **argv);
> > >  int do_add_volume(int nargs, char **args);
> > > diff --git a/man/btrfs.8.in b/man/btrfs.8.in
> > > index 26ef982..e87b5fe 100644
> > > --- a/man/btrfs.8.in
> > > +++ b/man/btrfs.8.in
> > > @@ -19,6 +19,10 @@ btrfs \- control a btrfs filesystem
> > >  .PP
> > >  \fBbtrfs\fP \fBfilesystem sync\fP\fI <path> \fP
> > >  .PP
> > > +\fBbtrfs\fP \fBfilesystem start-sync\fP\fI <path> \fP
> > > +.PP
> > > +\fBbtrfs\fP \fBfilesystem wait-sync\fP\fI <path>
<transid>\fP
> > > +.PP
> > >  \fBbtrfs\fP \fBfilesystem resize\fP\fI
[+/\-]<size>[gkm]|max
> > <filesystem>\fP
> > >  .PP
> > >  \fBbtrfs\fP \fBdevice scan\fP\fI [<device>
[<device>..]]\fP
> > > @@ -115,6 +119,16 @@ all the block devices.
> > >  Force a sync for the filesystem identified by
\fI<path>\fR.
> > >  .TP
> > >  
> > > +\fBfilesystem start-sync\fR\fI <path> \fR
> > > +Start a sync operation for the filesystem identified by
\fI<path>\fR.
A > > transaction id
> > > +is printed that can be waited on using the \fBfilesystem
wait-sync\fR
> > command.
> > > +.TP
> > > +
> > > +\fBfilesystem wait-sync\fR\fI <path> <transid>\fR
> > > +Wait for a the transaction \fI<transid>\fR to commit to
disk.  If
> > \fI<transid>\fR is zero,
> > > +wait for any currently committing transaction to commit.
> > > +.TP
> > > +
> > >  .\"
> > >  .\" Some wording are extracted by the resize2fs man page
> > >  .\"
> > > -- 
> > > 1.7.0
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
linux-btrfs"
in> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at 
http://vger.kernel.org/majordomo-info.html
> > > 
> > 
> > 
> > -- 
> > gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) 
<kreijack@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 majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> 

-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo)
<kreijack@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hugo Mills

2010-Nov-03 11:49 UTC

head link

Re: [PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands

On Tue, Nov 02, 2010 at 07:58:27PM +0100, Goffredo Baroncelli
wrote:> On Monday, 01 November, 2010, Sage Weil wrote:
> > The ''start-sync'' command initiates a sync, but does
not wait for it to
> > complete.  A transaction is printed that can be fed to
''wait-sync'', which
> > will wait for it to commit.
> > 
> > ''wait-sync'' can also be used in combination with
''async-snapshot'' to wait
> > for an async snapshot creation to commit.
> > 
> > Updates the man page too.
> > 
> > Signed-off-by: Sage Weil <sage@newdream.net>
> > ---
> >  btrfs.c        |    9 +++++++++
> >  btrfs_cmds.c   |   49
+++++++++++++++++++++++++++++++++++++++++++++++++
> >  btrfs_cmds.h   |    2 ++
> >  man/btrfs.8.in |   14 ++++++++++++++
> >  4 files changed, 74 insertions(+), 0 deletions(-)
> > 
> > diff --git a/btrfs.c b/btrfs.c
> > index 46314cf..c871f4a 100644
> > --- a/btrfs.c
> > +++ b/btrfs.c
> > @@ -77,6 +77,15 @@ static struct Command commands[] = {
> >  	  "filesystem sync", "<path>\n"
> >  		"Force a sync on the filesystem <path>."
> >  	},
> > +	{ do_start_sync, 1,
> > +	  "filesystem start-sync", "<path>\n"
> > +	        "Start a sync on the filesystem <path>, and print
the
> resulting\n"
> > +	        "transaction id."
> > +	},
> 
> Like the command "btrfs subvol snapshot", I think that it is
better to add a
> modifier instead of a new command.
> 
>  btrfs filesystem sync [--async]
> 
> Sorry if I noticed this too late. But I don''t see a valid reason
to add
> another command. From a UI point of view the meaning of the command is the 
> same, change only slight the behavior.
> 
> Even tough I have to admint that "sync --async" sound strange.
May be flush is
> better ?
   How about "btrfs filesystem sync --background"?

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ==  PGP
key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
            --- You''re never alone with a rubber duck... ---

Btrfs devel - Nov 2010 - [PATCH] btrfs-progs: ioctl: add may_alias to btrfs_ioctl_search_header

[PATCH] btrfs-progs: ioctl: add may_alias to btrfs_ioctl_search_header

[PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands

[PATCH v2] btrfs-progs: btrfs: implement async option for snapshot command

Re: [PATCH v2] btrfs-progs: btrfs: implement async option for snapshot command

Re: [PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands

Re: [PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands

Re: [PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands

Re: [PATCH v2] btrfs-progs: btrfs: implement ''start-sync'' and ''wait-sync'' commands