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 05:12:11 +0900 [thread overview]
Message-ID: <CAKFNMokRBcoP8z8Y6m60bYb=Se9SSn3N459hE0KWAnN7tEop=g@mail.gmail.com> (raw)
In-Reply-To: <33d078a7-9072-4d8e-a3a9-dec23d4191da@redhat.com>
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>
> ---
>
> Sorry for the delay on resend, other work got in the way...
> This all paged out a bit but I think I addressed all of the review
> items. Thanks!
>
> V3:
> - Add identifiers to nilfs_store_magic() prototype
> - switch Opt_snapshot to use fsparam_u64 vs. fsparam_u32
> - add error message for invalid cp=0 case
> - remove early sb->s_flags change in nilfs2_reconfigure()
> - un-split error message string in nilfs_get_tree()
>
Thank you for the revised patch!
...
> + case Opt_snapshot:
> + if (is_remount) {
> + struct super_block *sb = fc->root->d_sb;
> +
> + nilfs_err(sb,
> + "\"%s\" option is invalid for remount",
> + param->key);
> + return -EINVAL;
> + }
> + 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..
...
> @@ -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);
Thanks,
Ryusuke Konishi
next prev parent reply other threads:[~2024-04-19 20:12 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 [this message]
2024-04-19 22:06 ` Eric Sandeen
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='CAKFNMokRBcoP8z8Y6m60bYb=Se9SSn3N459hE0KWAnN7tEop=g@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.