From: Eric Sandeen <sandeen@redhat.com>
To: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Cc: linux-nilfs@vger.kernel.org
Subject: Re: [PATCH V3] nilfs2: convert to use the new mount API
Date: Fri, 19 Apr 2024 17:06:50 -0500 [thread overview]
Message-ID: <98da70e0-8a2d-43d3-863d-edc37b56ece4@redhat.com> (raw)
In-Reply-To: <CAKFNMokRBcoP8z8Y6m60bYb=Se9SSn3N459hE0KWAnN7tEop=g@mail.gmail.com>
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.
> ...
>> @@ -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
>
> Thanks,
> Ryusuke Konishi
>
next prev parent reply other threads:[~2024-04-19 22:06 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 [this message]
2024-04-20 13:57 ` Ryusuke Konishi
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=98da70e0-8a2d-43d3-863d-edc37b56ece4@redhat.com \
--to=sandeen@redhat.com \
--cc=konishi.ryusuke@gmail.com \
--cc=linux-nilfs@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 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.