| Message ID | 20200729164656.7153-1-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: indicate iversion option in show_options | expand |
On 7/29/20 9:46 AM, Josef Bacik wrote: > Eric reported a problem where if you did > > mount -o remount /some/btrfs/fs > > you would lose SB_I_VERSION on the mountpoint. After a very convoluted > search I discovered this is because the remount infrastructure doesn't > just say "change these things specifically", but it actually depends on > userspace to tell it fucking everything that needs to be set on the > mountpoint. This led to the fucking horrifying discovery that > util-linux actually has to parse /proc/mounts to figure out what the > fuck is set on the mount point in order to preserve any of the options > it's not actually fucking with, so in this case iversion. If we don't > indicate iversion is set, then we get iversion cleared on the mount, > because util-linux doesn't pass in MS_I_VERSION as it's mount flags. > > So work around this fucking insanity by spitting out iversion in > /proc/mounts so we get the correct flags passed to us in remount. Hmmm: # mount -o loop,noiversion btrfsfile mnt # grep btrfs /proc/mounts /dev/loop0 /tmp/mnt btrfs rw,seclabel,relatime,iversion,space_cache,subvolid=5,subvol=/ 0 0 # > Reported-by: Eric Sandeen <sandeen@redhat.com> > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/super.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > index aa73422b0678..fe64aa2f5c7a 100644 > --- a/fs/btrfs/super.c > +++ b/fs/btrfs/super.c > @@ -1427,6 +1427,10 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry) > seq_puts(seq, ",discard=async"); > if (!(info->sb->s_flags & SB_POSIXACL)) > seq_puts(seq, ",noacl"); > + if (info->sb->s_flags & SB_I_VERSION) > + seq_puts(seq, ",iversion"); > + else > + seq_puts(seq, ",noiversion"); > if (btrfs_test_opt(info, SPACE_CACHE)) > seq_puts(seq, ",space_cache"); > else if (btrfs_test_opt(info, FREE_SPACE_TREE)) >
On 7/29/20 1:42 PM, Eric Sandeen wrote: > On 7/29/20 9:46 AM, Josef Bacik wrote: >> Eric reported a problem where if you did >> >> mount -o remount /some/btrfs/fs >> >> you would lose SB_I_VERSION on the mountpoint. After a very convoluted >> search I discovered this is because the remount infrastructure doesn't >> just say "change these things specifically", but it actually depends on >> userspace to tell it fucking everything that needs to be set on the >> mountpoint. This led to the fucking horrifying discovery that >> util-linux actually has to parse /proc/mounts to figure out what the >> fuck is set on the mount point in order to preserve any of the options >> it's not actually fucking with, so in this case iversion. If we don't >> indicate iversion is set, then we get iversion cleared on the mount, >> because util-linux doesn't pass in MS_I_VERSION as it's mount flags. >> >> So work around this fucking insanity by spitting out iversion in >> /proc/mounts so we get the correct flags passed to us in remount. > > Hmmm: > > # mount -o loop,noiversion btrfsfile mnt > # grep btrfs /proc/mounts > /dev/loop0 /tmp/mnt btrfs rw,seclabel,relatime,iversion,space_cache,subvolid=5,subvol=/ 0 0 > # > Ugh that's because we just set it unconditionally like XFS does, but don't actually pay attention to noiversion otherwise. I'll fix that separately. Thanks, Josef
On 7/29/20 10:47 AM, Josef Bacik wrote: > On 7/29/20 1:42 PM, Eric Sandeen wrote: >> On 7/29/20 9:46 AM, Josef Bacik wrote: >>> Eric reported a problem where if you did >>> >>> mount -o remount /some/btrfs/fs >>> >>> you would lose SB_I_VERSION on the mountpoint. After a very convoluted >>> search I discovered this is because the remount infrastructure doesn't >>> just say "change these things specifically", but it actually depends on >>> userspace to tell it fucking everything that needs to be set on the >>> mountpoint. This led to the fucking horrifying discovery that >>> util-linux actually has to parse /proc/mounts to figure out what the >>> fuck is set on the mount point in order to preserve any of the options >>> it's not actually fucking with, so in this case iversion. If we don't >>> indicate iversion is set, then we get iversion cleared on the mount, >>> because util-linux doesn't pass in MS_I_VERSION as it's mount flags. >>> >>> So work around this fucking insanity by spitting out iversion in >>> /proc/mounts so we get the correct flags passed to us in remount. >> >> Hmmm: >> >> # mount -o loop,noiversion btrfsfile mnt >> # grep btrfs /proc/mounts >> /dev/loop0 /tmp/mnt btrfs rw,seclabel,relatime,iversion,space_cache,subvolid=5,subvol=/ 0 0 >> # >> > > Ugh that's because we just set it unconditionally like XFS does, but don't actually pay attention to noiversion otherwise. I'll fix that separately. Thanks, FWIW, # mount -o remount,noiversion mnt # grep btrfs /proc/mounts /dev/loop0 /tmp/mnt btrfs rw,seclabel,relatime,noiversion,space_cache,subvolid=5,subvol=/ 0 0 -Eric
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index aa73422b0678..fe64aa2f5c7a 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -1427,6 +1427,10 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry) seq_puts(seq, ",discard=async"); if (!(info->sb->s_flags & SB_POSIXACL)) seq_puts(seq, ",noacl"); + if (info->sb->s_flags & SB_I_VERSION) + seq_puts(seq, ",iversion"); + else + seq_puts(seq, ",noiversion"); if (btrfs_test_opt(info, SPACE_CACHE)) seq_puts(seq, ",space_cache"); else if (btrfs_test_opt(info, FREE_SPACE_TREE))
Eric reported a problem where if you did mount -o remount /some/btrfs/fs you would lose SB_I_VERSION on the mountpoint. After a very convoluted search I discovered this is because the remount infrastructure doesn't just say "change these things specifically", but it actually depends on userspace to tell it fucking everything that needs to be set on the mountpoint. This led to the fucking horrifying discovery that util-linux actually has to parse /proc/mounts to figure out what the fuck is set on the mount point in order to preserve any of the options it's not actually fucking with, so in this case iversion. If we don't indicate iversion is set, then we get iversion cleared on the mount, because util-linux doesn't pass in MS_I_VERSION as it's mount flags. So work around this fucking insanity by spitting out iversion in /proc/mounts so we get the correct flags passed to us in remount. Reported-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/super.c | 4 ++++ 1 file changed, 4 insertions(+)