public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Dave Dykstra <dwd@cern.ch>
Cc: tytso@mit.edu, linux-ext4@vger.kernel.org
Subject: Re: [PATCH 1/2] fuse2fs: mount norecovery if main block device is readonly
Date: Fri, 17 Oct 2025 12:38:41 -0700	[thread overview]
Message-ID: <20251017193841.GH6170@frogsfrogsfrogs> (raw)
In-Reply-To: <aPFIultQzQd6fk-o@cern.ch>

On Thu, Oct 16, 2025 at 02:34:18PM -0500, Dave Dykstra wrote:
> I have a few problems with this patch, details below.
> 
> I have proposed an alternative at
>     https://github.com/tytso/e2fsprogs/pull/250
> and I'll email that here next.
> 
> On Mon, Sep 15, 2025 at 05:03:14PM -0700, Darrick J. Wong wrote:
> ...
> > diff --git a/misc/fuse2fs.c b/misc/fuse2fs.c
> > index 48473321f469dc..fb44b0a79b53e6 100644
> > --- a/misc/fuse2fs.c
> > +++ b/misc/fuse2fs.c
> > @@ -946,6 +946,15 @@ static errcode_t fuse2fs_open(struct fuse2fs *ff, int libext2_flags)
> >  
> >  	err = ext2fs_open2(ff->device, options, flags, 0, 0, unix_io_manager,
> >  			   &ff->fs);
> > +	if (err == EPERM) {
> 
> In my case the error here is EACCES (Permission denied) rather than EPERM
> so I in my patch I included both.

Ok, I'll go update my own patch.

> > +		err_printf(ff, "%s.\n",
> > +			   _("read-only device, trying to mount norecovery"));
> > +		flags &= ~EXT2_FLAG_RW;
> > +		ff->ro = 1;
> > +		ff->norecovery = 1;
> 
> I don't think it's good to switch to read-only+norecovery even when a
> read-write mode was requested.  That goes too far.

The block device cannot be opened for write, so the mount cannot allow
user programs to write to files, and the fs driver cannot recover the
journal and it cannot write to the disk.  The only other choice would
be to fail the mount.

norecovery is wrong though.  The kernel fails the mount if the journal
needs recovery, the block device is ro, and the user didn't specify
norecovery.

> It also doesn't catch when recovery is needed.

What specifically do you mean "catch when recovery is needed"?  68 lines
down from the ext2fs_open2 call is a check for the needsrecovery state,
followed by recovering the journal.

> My proposed patch only reopens read-only
> when ro was requested and then later checks to see if recovery is needed
> and if so, errors out.

Your patch also didn't re-check the feature support after reopening the
block device, which you dismissed even though that can lead to
catastrophic behavior.

--D

> 
> Dave
> 

  reply	other threads:[~2025-10-17 19:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-15 23:59 [PATCHSET 4/6] fuse2fs: use fuseblk mode Darrick J. Wong
2025-09-16  0:03 ` [PATCH 1/2] fuse2fs: mount norecovery if main block device is readonly Darrick J. Wong
2025-10-16 19:34   ` Dave Dykstra
2025-10-17 19:38     ` Darrick J. Wong [this message]
2025-10-17 20:20       ` Dave Dykstra
2025-10-17 23:30         ` Darrick J. Wong
2025-09-16  0:03 ` [PATCH 2/2] fuse2fs: use fuseblk mode for mounting filesystems 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=20251017193841.GH6170@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=dwd@cern.ch \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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