From: Ryusuke Konishi <konishi.ryusuke@gmail.com>
To: Eric Sandeen <sandeen@redhat.com>
Cc: linux-nilfs@vger.kernel.org
Subject: Re: [PATCH V3] nilfs2: convert to use the new mount API
Date: Sat, 20 Apr 2024 22:57:36 +0900 [thread overview]
Message-ID: <CAKFNMomMKfqBMFeJa7Ct4zR7MWiwN72D1ccTMQEhUEygXyhgew@mail.gmail.com> (raw)
In-Reply-To: <98da70e0-8a2d-43d3-863d-edc37b56ece4@redhat.com>
On Sat, Apr 20, 2024 at 7:06 AM Eric Sandeen wrote:
>
> On 4/19/24 3:12 PM, Ryusuke Konishi wrote:
> > On Fri, Apr 19, 2024 at 1:45 AM Eric Sandeen wrote:
> >>
> >> Convert nilfs2 to use the new mount API.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>
>
> >> + if (result.uint_64 == 0) {
> >> + nilfs_err(NULL,
> >> + "invalid option \"cp=0\": invalid checkpoint number 0");
> >> + return -EINVAL;
> >
> > At first glance, I wondered why the nilfs_err() super_block instance
> > argument was NULL here, but I see, it can only be used by remount..
>
> Yup that's right, no sb yet on initial parsing. Hopefully the message
> is ok this way. There is also an option to emit mount option errors through
> the API, but nothing is listening for that yet.
The above implementation is fine (the original
nilfs_parse_snapshot_option() also does not output the device name, so
nothing is lost). I just found it interesting.
>
> > ...
> >> @@ -1172,7 +1157,7 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
> >> "couldn't remount RDWR because of unsupported optional features (%llx)",
> >> (unsigned long long)features);
> >> err = -EROFS;
> >> - goto restore_opts;
> >> + goto ignore_opts;
> >> }
> >>
> >> sb->s_flags &= ~SB_RDONLY;
> >> @@ -1180,130 +1165,56 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
> >> root = NILFS_I(d_inode(sb->s_root))->i_root;
> >> err = nilfs_attach_log_writer(sb, root);
> >> if (err)
> >> - goto restore_opts;
> >> + goto ignore_opts;
> >>
> >> down_write(&nilfs->ns_sem);
> >> nilfs_setup_super(sb, true);
> >> up_write(&nilfs->ns_sem);
> >> }
> >
> > There is still an issue where the SB_RDONLY flag on sb->s_flags is not
> > repaired in the error path of nilfs_attach_log_writer().
> >
> > This seems to be the only essential issue remaining, so I can add the
> > following fix (safer one - drop the SB_RDONLY flag for
> > nilfs_attach_log_writer, call it, and repair the flag if it fails),
> > and send it upstream.
> > Is this okay? Please let me know if you have any opinions.
> >
> > diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> > index a54fa43331f5..a8f03c860e87 100644
> > --- a/fs/nilfs2/super.c
> > +++ b/fs/nilfs2/super.c
> > @@ -1164,8 +1164,10 @@ static int nilfs_reconfigure(struct fs_context *fc)
> >
> > root = NILFS_I(d_inode(sb->s_root))->i_root;
> > err = nilfs_attach_log_writer(sb, root);
> > - if (err)
> > + if (err) {
> > + sb->s_flags |= SB_RDONLY;
> > goto ignore_opts;
> > + }
> >
> > down_write(&nilfs->ns_sem);
> > nilfs_setup_super(sb, true);
>
> Oh, I'm sorry I missed that :( Yes, I think that looks fine. Thank you.
>
> -Eric
Good.
Well, I'll continue testing, so I'll keep this patch for now and let
you know if anything happens.
I would like to send it to the -mm tree as soon as possible.
Thank you!
Ryusuke Konishi
>
> >
> > Thanks,
> > Ryusuke Konishi
> >
>
prev parent reply other threads:[~2024-04-20 13:57 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-18 16:45 [PATCH V3] nilfs2: convert to use the new mount API Eric Sandeen
2024-04-19 20:12 ` Ryusuke Konishi
2024-04-19 22:06 ` Eric Sandeen
2024-04-20 13:57 ` Ryusuke Konishi [this message]
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=CAKFNMomMKfqBMFeJa7Ct4zR7MWiwN72D1ccTMQEhUEygXyhgew@mail.gmail.com \
--to=konishi.ryusuke@gmail.com \
--cc=linux-nilfs@vger.kernel.org \
--cc=sandeen@redhat.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.