[PATCH] Prevent btrfsck to run on mounted filesystems

[PATCH] Prevent btrfsck to run on mounted filesystems

[Andi Drebes]

As recently discussed on the list, btrfsck should only be run on unmounted filesystems. This patch adds a short check for the mount status at the beginning of btrfsck. If the FS is mounted, the program aborts showing an error message.

Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
---
diff --git a/btrfsck.c b/btrfsck.c
index 73f1836..87ae776 100644
--- a/btrfsck.c
+++ b/btrfsck.c
@@ -28,6 +28,7 @@
 #include "transaction.h"
 #include "list.h"
 #include "version.h"
+#include "utils.h"
 
 static u64 bytes_used = 0;
 static u64 total_csum_bytes = 0;
@@ -2819,6 +2820,16 @@ int main(int ac, char **av)
 	if (ac < 2)
 		print_usage();
 
+	ret = check_mounted(av[1]);
+	if (ret < 0) {
+		fprintf(stderr, "error checking %s mount status\n", av[1]);
+		return 1;
+	}
+	if (ret == 1) {
+		fprintf(stderr, "%s is mounted. btrfsck can only be run on an unmounted filesystem.\n", av[1]);
+		return 1;
+	}
+
 	radix_tree_init();
 	cache_tree_init(&root_cache);
 	root = open_ctree(av[1], 0, 0);

Re: [PATCH] Prevent btrfsck to run on mounted filesystems

[Christoph Hellwig]

On Thu, Oct 29, 2009 at 09:52:15PM +0100, Andi Drebes wrote:
> As recently discussed on the list, btrfsck should only be run on unmounted filesystems. This patch adds a short check for the mount status at the beginning of btrfsck. If the FS is mounted, the program aborts showing an error message.

Just open the nodes with O_EXCL and you'll get all the checking for
free.  Also make sure that for a pure, read-only checks instead of a
repair to allow running on at least a read-only mounted filesystem.

Re: [PATCH] Prevent btrfsck to run on mounted filesystems

[Chris Mason]

On Fri, Oct 30, 2009 at 01:02:44AM -0400, Christoph Hellwig wrote:
> On Thu, Oct 29, 2009 at 09:52:15PM +0100, Andi Drebes wrote:
> > As recently discussed on the list, btrfsck should only be run on unmounted filesystems. This patch adds a short check for the mount status at the beginning of btrfsck. If the FS is mounted, the program aborts showing an error message.
> 
> Just open the nodes with O_EXCL and you'll get all the checking for
> free.  Also make sure that for a pure, read-only checks instead of a
> repair to allow running on at least a read-only mounted filesystem.
> 

Thanks for working on this patch Andi.

In this case O_EXCL is going to be more accurate just because the
mounted check doesn't cover every disk in the FS.  For now btrfsck
doesn't really give consistent results even readonly on a mounted
filesystem.  We should prevent it with a message just to prevent
confusion.

-chris

Re: [PATCH] Prevent btrfsck to run on mounted filesystems

[Andi Drebes]

> > Just open the nodes with O_EXCL and you'll get all the checking for
> > free.  Also make sure that for a pure, read-only checks instead of a
> > repair to allow running on at least a read-only mounted filesystem.
> 
> Thanks for working on this patch Andi.
> 
> In this case O_EXCL is going to be more accurate just because the
> mounted check doesn't cover every disk in the FS.  For now btrfsck
> doesn't really give consistent results even readonly on a mounted
> filesystem.  We should prevent it with a message just to prevent
> confusion.
Thanks for the reply. I'll do this as soon as I understand the code that is affected by those changes (I'm still in the learning phase and stepping through the code).

	Cheers,
		Andi

Re: [PATCH] Prevent btrfsck to run on mounted filesystems

[Andi Drebes]

Hi!

> > In this case O_EXCL is going to be more accurate just because the
> > mounted check doesn't cover every disk in the FS.  For now btrfsck
> > doesn't really give consistent results even readonly on a mounted
> > filesystem.  We should prevent it with a message just to prevent
> > confusion.
> Thanks for the reply. I'll do this as soon as I understand the code
> that is affected by those changes (I'm still in the learning phase
> and stepping through the code).   

OK, I've stepped through the code so far and I think I now have an idea of what should be changed. However, concerning the solution, I was confusing two things.

Christoph Hellwig earlier wrote:
> Just open the nodes with O_EXCL and you'll get all the checking for
> free.
I thought that open() with O_EXCL would fail if the file is already opened RW somewhere else. But the man page for open() says something else:

"O_EXCL: Ensure that this call creates the file: if this flag  is  specified  in  conjunction with O_CREAT, and pathname already exists, then open() will fail."

And especially:

"The behavior of O_EXCL is  undefined if O_CREAT is not specified."

After having read this, I don't see how this can help to prevent btrfsck to open a device that is mounted. What am I getting wrong? Currently, I would do the same check as in the original patch, but in btrfs_open_devices() instead of main().

	Cheers,
		Andi