[PATCH] btrfs: indicate iversion option in show_options

[PATCH] btrfs: indicate iversion option in show_options

[Josef Bacik]

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(+)

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))
-- 
2.24.1

Re: [PATCH] btrfs: indicate iversion option in show_options

[Eric Sandeen]

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))
> 

Re: [PATCH] btrfs: indicate iversion option in show_options

[Josef Bacik]

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

Re: [PATCH] btrfs: indicate iversion option in show_options

[Eric Sandeen]

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