* [PATCH] fuse2fs: reopen filesystem read-write for read-only journal recovery
@ 2025-10-10 21:47 Dave Dykstra
2025-10-15 1:15 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Dave Dykstra @ 2025-10-10 21:47 UTC (permalink / raw)
To: linux-ext4; +Cc: Dave Dykstra, Dave Dykstra
This changes the strategy added in c7f2688540d95e7f2cbcd178f8ff62ebe079faf7
for recovery of journals when read-only is requested. That strategy always
opened the filesystem file read-write first, in case there was a journal to
recover. A big problem with that strategy was that the user might not have
write access to the file. The new strategy with read-only mounts is to
open the filesystem read-only first, then if there is a journal that needs
recovery, attempt to reopen it read-write. If that works and the journal
is recovered, reopen it again read-only.
- Fixes https://github.com/tytso/e2fsprogs/issues/244
Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com>
---
misc/fuse2fs.c | 62 ++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 55 insertions(+), 7 deletions(-)
diff --git a/misc/fuse2fs.c b/misc/fuse2fs.c
index cb5620c7..30a46976 100644
--- a/misc/fuse2fs.c
+++ b/misc/fuse2fs.c
@@ -4607,8 +4607,7 @@ int main(int argc, char *argv[])
FILE *orig_stderr = stderr;
char extra_args[BUFSIZ];
int ret;
- int flags = EXT2_FLAG_64BITS | EXT2_FLAG_THREADS | EXT2_FLAG_EXCLUSIVE |
- EXT2_FLAG_RW;
+ int flags = EXT2_FLAG_64BITS | EXT2_FLAG_THREADS | EXT2_FLAG_EXCLUSIVE;
memset(&fctx, 0, sizeof(fctx));
fctx.magic = FUSE2FS_MAGIC;
@@ -4689,6 +4688,8 @@ int main(int argc, char *argv[])
/* Start up the fs (while we still can use stdout) */
ret = 2;
+ if (!fctx.ro)
+ flags |= EXT2_FLAG_RW;
char options[50];
sprintf(options, "offset=%lu", fctx.offset);
if (fctx.directio)
@@ -4751,8 +4752,12 @@ int main(int argc, char *argv[])
* ext4 can't do COW of shared blocks, so if the feature is enabled,
* we must force ro mode.
*/
- if (ext2fs_has_feature_shared_blocks(global_fs->super))
+ if (ext2fs_has_feature_shared_blocks(global_fs->super) && !fctx.ro) {
+ log_printf(&fctx, "%s\n",
+ _("Mounting read-only because shared blocks feature is enabled."));
fctx.ro = 1;
+ /* Note that EXT2_FLAG_RW is left set */
+ }
if (ext2fs_has_feature_journal_needs_recovery(global_fs->super)) {
if (fctx.norecovery) {
@@ -4761,6 +4766,27 @@ int main(int argc, char *argv[])
fctx.ro = 1;
global_fs->flags &= ~EXT2_FLAG_RW;
} else {
+ if (!(flags & EXT2_FLAG_RW)) {
+ /* Attempt to re-open read-write */
+ err = ext2fs_close(global_fs);
+ if (err)
+ com_err(argv[0], err,
+ "while closing filesystem");
+ global_fs = NULL;
+ flags |= EXT2_FLAG_RW;
+ err = ext2fs_open2(fctx.device, options, flags,
+ 0, 0, unix_io_manager,
+ &global_fs);
+ if (err) {
+ err_printf(&fctx, "%s.\n",
+ error_message(err));
+ err_printf(&fctx, "%s\n",
+ _("Journal needs recovery but filesystem cannot be reopened read-write."));
+ err_printf(&fctx, "%s\n",
+ _("Please run e2fsck -fy."));
+ goto out;
+ }
+ }
log_printf(&fctx, "%s\n", _("Recovering journal."));
err = ext2fs_run_ext3_journal(&global_fs);
if (err) {
@@ -4772,12 +4798,32 @@ int main(int argc, char *argv[])
ext2fs_clear_feature_journal_needs_recovery(global_fs->super);
ext2fs_mark_super_dirty(global_fs);
}
+ } else if (fctx.ro && !(flags & EXT2_FLAG_RW)) {
+ log_printf(&fctx, "%s\n", _("Mounting read-only."));
}
- if (global_fs->flags & EXT2_FLAG_RW) {
+ if (fctx.ro && (flags & EXT2_FLAG_RW)) {
+ /* Re-open read-only */
+ err = ext2fs_close(global_fs);
+ if (err)
+ com_err(argv[0], err, "while closing filesystem");
+ global_fs = NULL;
+ flags &= ~EXT2_FLAG_RW;
+ err = ext2fs_open2(fctx.device, options, flags, 0, 0,
+ unix_io_manager, &global_fs);
+ if (err) {
+ err_printf(&fctx, "%s.\n", error_message(err));
+ err_printf(&fctx, "%s\n",
+ _("Failed to remount read-only."));
+ goto out;
+ }
+ log_printf(&fctx, "%s\n", _("Remounted read-only."));
+ }
+
+ if (!fctx.ro) {
if (ext2fs_has_feature_journal(global_fs->super))
log_printf(&fctx, "%s",
- _("Warning: fuse2fs does not support using the journal.\n"
+ _("Warning: fuse2fs does not support writing the journal.\n"
"There may be file system corruption or data loss if\n"
"the file system is not gracefully unmounted.\n"));
err = ext2fs_read_inode_bitmap(global_fs);
@@ -4833,8 +4879,10 @@ int main(int argc, char *argv[])
if (fctx.no_default_opts == 0)
fuse_opt_add_arg(&args, extra_args);
- if (fctx.ro)
+ if (fctx.ro) {
+ /* This is in case ro was implied above and not passed in */
fuse_opt_add_arg(&args, "-oro");
+ }
if (fctx.fakeroot) {
#ifdef HAVE_MOUNT_NODEV
@@ -4892,7 +4940,6 @@ int main(int argc, char *argv[])
ret = 0;
break;
}
-out:
if (ret & 1) {
fprintf(orig_stderr, "%s\n",
_("Mount failed due to unrecognized options. Check dmesg(1) for details."));
@@ -4903,6 +4950,7 @@ out:
_("Mount failed while opening filesystem. Check dmesg(1) for details."));
fflush(orig_stderr);
}
+out:
if (global_fs) {
err = ext2fs_close(global_fs);
if (err)
--
2.43.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] fuse2fs: reopen filesystem read-write for read-only journal recovery
2025-10-10 21:47 [PATCH] fuse2fs: reopen filesystem read-write for read-only journal recovery Dave Dykstra
@ 2025-10-15 1:15 ` Darrick J. Wong
2025-10-15 15:33 ` Dave Dykstra
0 siblings, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2025-10-15 1:15 UTC (permalink / raw)
To: Dave Dykstra; +Cc: linux-ext4, Dave Dykstra
On Fri, Oct 10, 2025 at 04:47:35PM -0500, Dave Dykstra wrote:
> This changes the strategy added in c7f2688540d95e7f2cbcd178f8ff62ebe079faf7
> for recovery of journals when read-only is requested. That strategy always
> opened the filesystem file read-write first, in case there was a journal to
> recover. A big problem with that strategy was that the user might not have
> write access to the file. The new strategy with read-only mounts is to
> open the filesystem read-only first, then if there is a journal that needs
> recovery, attempt to reopen it read-write. If that works and the journal
> is recovered, reopen it again read-only.
>
> - Fixes https://github.com/tytso/e2fsprogs/issues/244
>
> Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com>
> ---
> misc/fuse2fs.c | 62 ++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 55 insertions(+), 7 deletions(-)
>
> diff --git a/misc/fuse2fs.c b/misc/fuse2fs.c
> index cb5620c7..30a46976 100644
> --- a/misc/fuse2fs.c
> +++ b/misc/fuse2fs.c
> @@ -4607,8 +4607,7 @@ int main(int argc, char *argv[])
> FILE *orig_stderr = stderr;
> char extra_args[BUFSIZ];
> int ret;
> - int flags = EXT2_FLAG_64BITS | EXT2_FLAG_THREADS | EXT2_FLAG_EXCLUSIVE |
> - EXT2_FLAG_RW;
> + int flags = EXT2_FLAG_64BITS | EXT2_FLAG_THREADS | EXT2_FLAG_EXCLUSIVE;
>
> memset(&fctx, 0, sizeof(fctx));
> fctx.magic = FUSE2FS_MAGIC;
> @@ -4689,6 +4688,8 @@ int main(int argc, char *argv[])
>
> /* Start up the fs (while we still can use stdout) */
> ret = 2;
> + if (!fctx.ro)
ro and EXT2_FLAG_RW are not the same thing!
The rw/ro arguments are passed through to the kernel; they determine
whether or not user programs can write to the directory tree. That's up
to the kernel.
EXT2_FLAG_RW determines if the filesystem can be written to at all.
It's set by default, but it is cleared if the user passes norecovery
as an option or the block device can't be opened.
You can have a ro mount of a EXT2_FLAG_RW filesystem. That means that
the filesystem can modify/reorganize itself, even if users can't write
to files.
> + flags |= EXT2_FLAG_RW;
> char options[50];
> sprintf(options, "offset=%lu", fctx.offset);
> if (fctx.directio)
> @@ -4751,8 +4752,12 @@ int main(int argc, char *argv[])
> * ext4 can't do COW of shared blocks, so if the feature is enabled,
> * we must force ro mode.
> */
> - if (ext2fs_has_feature_shared_blocks(global_fs->super))
> + if (ext2fs_has_feature_shared_blocks(global_fs->super) && !fctx.ro) {
> + log_printf(&fctx, "%s\n",
> + _("Mounting read-only because shared blocks feature is enabled."));
> fctx.ro = 1;
> + /* Note that EXT2_FLAG_RW is left set */
> + }
Yeah, the logging here could be improved.
>
> if (ext2fs_has_feature_journal_needs_recovery(global_fs->super)) {
> if (fctx.norecovery) {
> @@ -4761,6 +4766,27 @@ int main(int argc, char *argv[])
> fctx.ro = 1;
> global_fs->flags &= ~EXT2_FLAG_RW;
> } else {
> + if (!(flags & EXT2_FLAG_RW)) {
I don't like this, because we should open the filesystem with
EXT2_FLAG_RW set by default and only downgrade to !EXT2_FLAG_RW if we
can't open it...
> + /* Attempt to re-open read-write */
> + err = ext2fs_close(global_fs);
> + if (err)
> + com_err(argv[0], err,
> + "while closing filesystem");
> + global_fs = NULL;
...if the close fails, you just leaked the old global_fs context.
ext2fs_close_free is what you want (and yes that's a bug in fuse2fs).
> + flags |= EXT2_FLAG_RW;
> + err = ext2fs_open2(fctx.device, options, flags,
> + 0, 0, unix_io_manager,
> + &global_fs);
> + if (err) {
> + err_printf(&fctx, "%s.\n",
> + error_message(err));
> + err_printf(&fctx, "%s\n",
> + _("Journal needs recovery but filesystem cannot be reopened read-write."));
> + err_printf(&fctx, "%s\n",
> + _("Please run e2fsck -fy."));
> + goto out;
> + }
...and also, if you re-do ext2fs_open2(), you then have to re-check all
the feature support bits above because we unlocked the filesystem
device, which means its contents could have been replaced completely
in the interim.
Also note that I have a /very large/ pile of fuse2fs improvements and
rewrites and cleanups that are out for review on the list; you might
want to look at those first.
--D
> + }
> log_printf(&fctx, "%s\n", _("Recovering journal."));
> err = ext2fs_run_ext3_journal(&global_fs);
> if (err) {
> @@ -4772,12 +4798,32 @@ int main(int argc, char *argv[])
> ext2fs_clear_feature_journal_needs_recovery(global_fs->super);
> ext2fs_mark_super_dirty(global_fs);
> }
> + } else if (fctx.ro && !(flags & EXT2_FLAG_RW)) {
> + log_printf(&fctx, "%s\n", _("Mounting read-only."));
> }
>
> - if (global_fs->flags & EXT2_FLAG_RW) {
> + if (fctx.ro && (flags & EXT2_FLAG_RW)) {
> + /* Re-open read-only */
> + err = ext2fs_close(global_fs);
> + if (err)
> + com_err(argv[0], err, "while closing filesystem");
> + global_fs = NULL;
> + flags &= ~EXT2_FLAG_RW;
> + err = ext2fs_open2(fctx.device, options, flags, 0, 0,
> + unix_io_manager, &global_fs);
> + if (err) {
> + err_printf(&fctx, "%s.\n", error_message(err));
> + err_printf(&fctx, "%s\n",
> + _("Failed to remount read-only."));
> + goto out;
> + }
> + log_printf(&fctx, "%s\n", _("Remounted read-only."));
> + }
> +
> + if (!fctx.ro) {
> if (ext2fs_has_feature_journal(global_fs->super))
> log_printf(&fctx, "%s",
> - _("Warning: fuse2fs does not support using the journal.\n"
> + _("Warning: fuse2fs does not support writing the journal.\n"
> "There may be file system corruption or data loss if\n"
> "the file system is not gracefully unmounted.\n"));
> err = ext2fs_read_inode_bitmap(global_fs);
> @@ -4833,8 +4879,10 @@ int main(int argc, char *argv[])
> if (fctx.no_default_opts == 0)
> fuse_opt_add_arg(&args, extra_args);
>
> - if (fctx.ro)
> + if (fctx.ro) {
> + /* This is in case ro was implied above and not passed in */
> fuse_opt_add_arg(&args, "-oro");
> + }
>
> if (fctx.fakeroot) {
> #ifdef HAVE_MOUNT_NODEV
> @@ -4892,7 +4940,6 @@ int main(int argc, char *argv[])
> ret = 0;
> break;
> }
> -out:
> if (ret & 1) {
> fprintf(orig_stderr, "%s\n",
> _("Mount failed due to unrecognized options. Check dmesg(1) for details."));
> @@ -4903,6 +4950,7 @@ out:
> _("Mount failed while opening filesystem. Check dmesg(1) for details."));
> fflush(orig_stderr);
> }
> +out:
> if (global_fs) {
> err = ext2fs_close(global_fs);
> if (err)
> --
> 2.43.5
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fuse2fs: reopen filesystem read-write for read-only journal recovery
2025-10-15 1:15 ` Darrick J. Wong
@ 2025-10-15 15:33 ` Dave Dykstra
2025-10-15 21:40 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Dave Dykstra @ 2025-10-15 15:33 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-ext4
On Tue, Oct 14, 2025 at 06:15:05PM -0700, Darrick J. Wong wrote:
> On Fri, Oct 10, 2025 at 04:47:35PM -0500, Dave Dykstra wrote:
> > This changes the strategy added in c7f2688540d95e7f2cbcd178f8ff62ebe079faf7
> > for recovery of journals when read-only is requested.
...
> ro and EXT2_FLAG_RW are not the same thing!
I understand that.
...
> I don't like this, because we should open the filesystem with
> EXT2_FLAG_RW set by default and only downgrade to !EXT2_FLAG_RW if we
> can't open it...
I was following the suggestion of tytso at
https://github.com/tytso/e2fsprogs/issues/244#issuecomment-3390084495
However, I think your suggestion might be better. I will try that.
...
> ...if the close fails, you just leaked the old global_fs context.
> ext2fs_close_free is what you want (and yes that's a bug in fuse2fs).
Ok, thanks, I'll use that.
...
> ...and also, if you re-do ext2fs_open2(), you then have to re-check all
> the feature support bits above because we unlocked the filesystem
> device, which means its contents could have been replaced completely
> in the interim.
I'm not convinced that's something to worry about, but in any case
your suggestion of only opening ro if rw fails should avoid it.
> Also note that I have a /very large/ pile of fuse2fs improvements and
> rewrites and cleanups that are out for review on the list; you might
> want to look at those first.
I do appreciate your efforts. Unfortunately I have too many other
priorities to have enough bandwidth to take on general responsibility
for reviewing fuse2fs patches. I also don't have much experience with
filesystems. I'm only trying to help here because it is impacting a
case that I support. I was very happy when I found that the fuse2fs in
v1.47.3 of e2fsprogs fixed another user-reported problem, but the new
version ended up causing a couple of new problems.
Having said that, if there are particular patches that you think are
important bug fixes that you would like to call my attention to, please
send me a direct message. I could test them and respond.
Dave
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fuse2fs: reopen filesystem read-write for read-only journal recovery
2025-10-15 15:33 ` Dave Dykstra
@ 2025-10-15 21:40 ` Darrick J. Wong
2025-10-16 20:16 ` Dave Dykstra
0 siblings, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2025-10-15 21:40 UTC (permalink / raw)
To: Dave Dykstra; +Cc: linux-ext4
On Wed, Oct 15, 2025 at 10:33:40AM -0500, Dave Dykstra wrote:
> On Tue, Oct 14, 2025 at 06:15:05PM -0700, Darrick J. Wong wrote:
> > On Fri, Oct 10, 2025 at 04:47:35PM -0500, Dave Dykstra wrote:
> > > This changes the strategy added in c7f2688540d95e7f2cbcd178f8ff62ebe079faf7
> > > for recovery of journals when read-only is requested.
> ...
> > ro and EXT2_FLAG_RW are not the same thing!
>
> I understand that.
>
> ...
> > I don't like this, because we should open the filesystem with
> > EXT2_FLAG_RW set by default and only downgrade to !EXT2_FLAG_RW if we
> > can't open it...
>
> I was following the suggestion of tytso at
> https://github.com/tytso/e2fsprogs/issues/244#issuecomment-3390084495
>
> However, I think your suggestion might be better. I will try that.
Urrrrrgh, external conversations that need to be on the mailing list.
Already covered here:
https://lore.kernel.org/linux-ext4/175798064776.350013.6744611652039454651.stgit@frogsfrogsfrogs/
> ...
>
> > ...if the close fails, you just leaked the old global_fs context.
> > ext2fs_close_free is what you want (and yes that's a bug in fuse2fs).
>
> Ok, thanks, I'll use that.
>
> ...
> > ...and also, if you re-do ext2fs_open2(), you then have to re-check all
> > the feature support bits above because we unlocked the filesystem
> > device, which means its contents could have been replaced completely
> > in the interim.
>
> I'm not convinced that's something to worry about, but in any case
> your suggestion of only opening ro if rw fails should avoid it.
...and then in the pile of patches I break up all the stuff in main() so
that there's one fuse2fs_open() routine, after which the fs context
doesn't change and the fd stays open:
https://lore.kernel.org/linux-ext4/175798064597.349841.13113367506205034632.stgit@frogsfrogsfrogs/
> > Also note that I have a /very large/ pile of fuse2fs improvements and
> > rewrites and cleanups that are out for review on the list; you might
> > want to look at those first.
>
> I do appreciate your efforts. Unfortunately I have too many other
> priorities to have enough bandwidth to take on general responsibility
> for reviewing fuse2fs patches. I also don't have much experience with
> filesystems. I'm only trying to help here because it is impacting a
This exact mentality is why filesystem development has become very
frustrating...
> case that I support. I was very happy when I found that the fuse2fs in
> v1.47.3 of e2fsprogs fixed another user-reported problem, but the new
> version ended up causing a couple of new problems.
...though I'm not blaming you (or any other user for that matter),
just venting about the development community. :(
Are there other problems I should know about?
> Having said that, if there are particular patches that you think are
> important bug fixes that you would like to call my attention to, please
> send me a direct message. I could test them and respond.
They're all just waiting for Ted to put out the last 1.47.x release and
open up 1.48 development.
--D
> Dave
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fuse2fs: reopen filesystem read-write for read-only journal recovery
2025-10-15 21:40 ` Darrick J. Wong
@ 2025-10-16 20:16 ` Dave Dykstra
0 siblings, 0 replies; 5+ messages in thread
From: Dave Dykstra @ 2025-10-16 20:16 UTC (permalink / raw)
To: linux-ext4
This patch is replaced by
https://lore.kernel.org/linux-ext4/20251016200206.3035-1-dave.dykstra@cern.ch/
Dave
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-16 20:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-10 21:47 [PATCH] fuse2fs: reopen filesystem read-write for read-only journal recovery Dave Dykstra
2025-10-15 1:15 ` Darrick J. Wong
2025-10-15 15:33 ` Dave Dykstra
2025-10-15 21:40 ` Darrick J. Wong
2025-10-16 20:16 ` Dave Dykstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).