From: "Darrick J. Wong" <djwong@kernel.org>
To: Dave Dykstra <dwd@cern.ch>
Cc: linux-ext4@vger.kernel.org,
Dave Dykstra <2129743+DrDaveD@users.noreply.github.com>
Subject: Re: [PATCH] fuse2fs: updates for message reporting journal is not supported
Date: Fri, 17 Oct 2025 16:25:21 -0700 [thread overview]
Message-ID: <20251017232521.GI6170@frogsfrogsfrogs> (raw)
In-Reply-To: <aPKilSNCQRW9c6rl@cern.ch>
On Fri, Oct 17, 2025 at 03:09:57PM -0500, Dave Dykstra wrote:
> On Fri, Oct 17, 2025 at 12:18:00PM -0700, Darrick J. Wong wrote:
> > On Thu, Oct 16, 2025 at 03:09:03PM -0500, Dave Dykstra wrote:
> > > This makes two changes to the message that is shown saying that fuse2fs
> > > does not support the journal. First is that it reverts the check to
> > > what it was before 3875380 to look at the ro option not being set
> > > instead of checking the RW flag. That's because I don't think this
> > > message needs to be shown when the ro option is set even when it was
> > > opened RW; there should be nothing to corrupt when it is ro.
> > >
> > > Second, it changes the message to say that writing is not supported
> > > rather than using the journal is not supported. The current message is
> > > confusing because in fact the journal is used for recovery when needed
> > > and possible.
> > >
> > > Also submitted as PR https://github.com/tytso/e2fsprogs/pull/251
> > >
> > > Signed-off-by: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com>
> > > ---
> > > misc/fuse2fs.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/misc/fuse2fs.c b/misc/fuse2fs.c
> > > index cb5620c7..c46cc03b 100644
> > > --- a/misc/fuse2fs.c
> > > +++ b/misc/fuse2fs.c
> > > @@ -4774,10 +4774,10 @@ int main(int argc, char *argv[])
> > > }
> > > }
> > >
> > > - if (global_fs->flags & EXT2_FLAG_RW) {
> > > + if (!fctx.ro) {
> >
> > Again, rw != EXT2_FLAG_RW.
> >
> > The ro and rw mount options specify if the filesystem mount is writable.
> > You can mount a filesystem in multiple places, and some of the mounts
> > can be ro and some can be rw.
> >
> > EXT2_FLAG_RW specifies that the filesystem driver can write to the block
> > device. fuse2fs should warn about incomplete journal support any time
> > the **filesystem** is writable, independent of the write state of the
> > mount.
>
> Are you saying that is indeed possible for a read-only mount to cause
> file corruption or data loss if there's not a graceful unmount? If so,
No, I'm saying that filesystem drivers can *themselves* write metadata
to a filesystem mounted ro. ro means that user programs can't write to
the files under a particular mountpoint. This has long been the case
for the kernel implementations of ext*, XFS, btrfs, etc.
I've said this three times now, and this is the last time I'm going to
say it.
> it sure seems like that should be avoided if possible! Since fuse2fs
> does not support writing the journal, perhaps its behavior should be
> different than the kernel's behavior for this too. Perhaps once the
> journal is recovered it should be remounted without EXT2_FLAG_RW.
No. Ted and I are trying to minimize the differences between the kernel
and fuse2fs.
> In any case, during further testing I did find a serious problem with
> this change in that it changes more than just the message; it also skips
> reading in the inode bitmap, which causes a problem later. So at
> minimum this patch should only affect the message, not the rest of the
> stuff in that if statement.
Are you running fstests QA on these patches before you send them out?
--D
> > Filesystems are allowed to write to the block device even if the mount
> > itself is readonly, e.g. kernel ext4 recovering the journal on an ro
> > mount.
> >
> > NAK.
> >
> > --D
> >
> > > 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"
>
> What do you think about this message change?
>
> Dave
>
> > > "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);
> > > --
> > > 2.43.5
> > >
> > >
> >
>
next prev parent reply other threads:[~2025-10-17 23:25 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-16 20:09 [PATCH] fuse2fs: updates for message reporting journal is not supported Dave Dykstra
2025-10-17 19:18 ` Darrick J. Wong
2025-10-17 20:09 ` Dave Dykstra
2025-10-17 23:25 ` Darrick J. Wong [this message]
2025-10-21 22:33 ` Dave Dykstra
2025-10-22 1:25 ` Darrick J. Wong
2025-10-22 1:36 ` Theodore Tso
2025-10-23 0:21 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251017232521.GI6170@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=2129743+DrDaveD@users.noreply.github.com \
--cc=dwd@cern.ch \
--cc=linux-ext4@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox