| Message ID | 20120703155212.GG5326@twin.jikos.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/03/2012 08:52 AM, David Sterba wrote: > On Tue, Jul 03, 2012 at 04:22:08PM +0100, Hugo Mills wrote: >> Correct, by default it just checks the filesystem. Just to be sure: >> the filesystems in question weren't mounted, were they? > > fsck will refuse to run on a mounted filesystem, though in case of a > read-only mount it might be useful during debugging, I'm using this > patch > > --- a/btrfsck.c > +++ b/btrfsck.c > @@ -3474,6 +3474,7 @@ static struct option long_options[] = { > { "repair", 0, NULL, 0 }, > { "init-csum-tree", 0, NULL, 0 }, > { "init-extent-tree", 0, NULL, 0 }, > + { "force", 0, NULL, 0 }, If we were to run with this, I think it should be called something other than force. fsck.ext* has trained people to think that 'forcing' a fsck means doing a full repair pass even if the fs thinks that it was shut down cleanly. --read-only would be good if fsck was taught to not even try to write in this mode. - z -- 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
On Tue, Jul 03, 2012 at 09:26:41AM -0700, Zach Brown wrote: > On 07/03/2012 08:52 AM, David Sterba wrote: > >--- a/btrfsck.c > >+++ b/btrfsck.c > >@@ -3474,6 +3474,7 @@ static struct option long_options[] = { > > { "repair", 0, NULL, 0 }, > > { "init-csum-tree", 0, NULL, 0 }, > > { "init-extent-tree", 0, NULL, 0 }, > >+ { "force", 0, NULL, 0 }, > > If we were to run with this, I think it should be called something other > than force. fsck.ext* has trained people to think that 'forcing' a fsck > means doing a full repair pass even if the fs thinks that it was shut > down cleanly. Agreed, it's not a good name and was rather a quick aid to myself, I didn't put much thinking into the user interface as I usually do :) > --read-only would be good if fsck was taught to not even try to write in > this mode. read-only mode is default and (hopefully) does no writes to the device, this would require the --repair option so what you propose is sort of a sanity check, right? david -- 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
> read-only mode is default and (hopefully) does no writes to the device, > this would require the --repair option so what you propose is sort of a > sanity check, right? Ah, I didn't realize that it didn't write without --repair. Yeah, making sure that people don't try to combine the repair and read-from-mounted-devices options seems reasonable. - z -- 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
On Tue, Jul 03, 2012 at 07:37:42PM +0200, David Sterba wrote: > On Tue, Jul 03, 2012 at 09:26:41AM -0700, Zach Brown wrote: > > On 07/03/2012 08:52 AM, David Sterba wrote: > > >--- a/btrfsck.c > > >+++ b/btrfsck.c > > >@@ -3474,6 +3474,7 @@ static struct option long_options[] = { > > > { "repair", 0, NULL, 0 }, > > > { "init-csum-tree", 0, NULL, 0 }, > > > { "init-extent-tree", 0, NULL, 0 }, > > >+ { "force", 0, NULL, 0 }, > > > > If we were to run with this, I think it should be called something other > > than force. fsck.ext* has trained people to think that 'forcing' a fsck > > means doing a full repair pass even if the fs thinks that it was shut > > down cleanly. > > Agreed, it's not a good name and was rather a quick aid to myself, I > didn't put much thinking into the user interface as I usually do :) xfs_repair uses: -d Repair dangerously. Allow xfs_repair to repair an XFS filesystem mounted read only. This is typically done on a root fileystem from single user mode, immediately followed by a reboot. > > --read-only would be good if fsck was taught to not even try to write in > > this mode. > > read-only mode is default and (hopefully) does no writes to the device, > this would require the --repair option so what you propose is sort of a > sanity check, right? If you run fsck/reapir on a mounted filesystem, and it changes the block device (i.e. fixes something) the mounted filesystem does not know about it and so may use stale metadata and bad things will happen. That's why it's called "dangerous". ;) Cheers, Dave.
--- a/btrfsck.c +++ b/btrfsck.c @@ -3474,6 +3474,7 @@ static struct option long_options[] = { { "repair", 0, NULL, 0 }, { "init-csum-tree", 0, NULL, 0 }, { "init-extent-tree", 0, NULL, 0 }, + { "force", 0, NULL, 0 }, { 0, 0, 0, 0} }; @@ -3484,12 +3485,13 @@ int main(int ac, char **av) struct btrfs_fs_info *info; struct btrfs_trans_handle *trans = NULL; u64 bytenr = 0; - int ret; + int ret = 0; int num; int repair = 0; int option_index = 0; int init_csum_tree = 0; int rw = 0; + int force = 0; while(1) { int c; @@ -3516,6 +3518,9 @@ int main(int ac, char **av) printf("Creating a new CRC tree\n"); init_csum_tree = 1; rw = 1; + } else if (option_index == 4) { + printf("Skip mount checks\n"); + force = 1; } } @@ -3527,7 +3532,7 @@ int main(int ac, char **av) radix_tree_init(); cache_tree_init(&root_cache); - if((ret = check_mounted(av[optind])) < 0) { + if(!force && (ret = check_mounted(av[optind])) < 0) { fprintf(stderr, "Could not check mount status: %s\n", strerror(-ret)); return ret; } else if(ret) {