[Ocfs2-devel] [PATCH 1/1] ocfs2: fix ocfs2_find_slot repeats alloc same slot issue

[Joseph Qi]

On 6/14/22 10:59 AM, heming.zhao at suse.com wrote:
> On 6/13/22 23:43, Junxiao Bi wrote:
>>
>>> 在 2022年6月13日，上午1:48，heming.zhao at suse.com 写道：
>>>
>>> On 6/13/22 16:21, Joseph Qi wrote:
>>>>> On 6/13/22 3:59 PM, heming.zhao at suse.com wrote:
>>>>> On 6/12/22 22:16, Joseph Qi wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Why can't use local mount? I don't remember if we discuss about this.
>>>>>>
>>>>> Sorry, I can't follow your question.
>>>>> Do you mean why revert commit 912f655d78c5?
>>>>>
>>>>> or you are interest with the feature local mount?
>>>>> the local mount is created by mkfs.ocfs2, which can't be converted to clustered.
>>>>> see mkfs.ocfs2(8) '-M' option.
>>>>>
>>>> What Junxiao's main concern is data corruption, so I'm afraid we have to
>>>> introduce an ondisk feature bit to prevent mixed nocluster and cluster
>>>> mount, similar to local mount.
>>>
>>> this patch defined two new variants/flags:
>>> #define OCFS2_SLOTMAP_CLUSTER   1
>>> #define OCFS2_SLOTMAP_NOCLUSTER 2
>>>
>>> (I expect) Under OCFS2_SLOTMAP_CLUSTER, for clustered mount mode and for compatibility,
>>> anything doesn't need to be changed.
>>>
>>> OCFS2_SLOTMAP_NOCLUSTER will introduce a new value for slotmap area.
>>> this new value only take effect after a successfully nocluster mount.
>>> (pls fix me), existed kernel/user space code don't do any special handle for
>>> noclustered mount mode in slotmap area. So the new value is also compatibility.
>>>
>>> And the patch can also prevent mixed mount, the related code is in ocfs2_find_slot().
>>> code logic:
>>> - noclustered mount condition: slotmap is empty or already mounted with noclustered
>>> - clustered mount condition: slotmap is empty or already mounted with clustered.
>>> - all other conditions will be denied.
>> Finding slot required reading slot map and then  update slot map. It is not atomic , you can’t prevent mixed mount until you have a cluster lock.
> 
> Could I say your mentioned use case is invalid.
> I believe all (yes, *all*) the ocfs2 users use this fs under cluster mode in
> their product env.
> The nocluster mount is only used on maintained env (eg. backup, fsck).
> 
> We only concern two ways:
> 1. user forgets to unmount (eg crash) before using another mount mode.
> 2. when ocfs2 volume is working, which should deny volume is mounted by another mode.
>    this may happened user mistakenly runs command or script.
> 
> Base on above 1 & 2, Junxiao above mentioned use case only happens on one scenario:
> the volume doesn't be mounted by any node, then user mistakenly mounts volume
> with [no]cluster mode at same time. I believe this use case is invalid. And I guess
> gfs2 may also has the same issue.
> 
> With this patch, ocfs2 has more sanity check ability than other fs, eg: xfs, ext4.
> SUSE HA stack with corosync+pacemaker also supports running xfs/ext4 with A/P mode.
> The xfs/ext4 never have detecting mount status code, Junxiao mentioned mixed mount
> can also happens on these fs. How do xfs/ext4/HA maintainers handle it? Under these
> fs mounting behavior, these fields maintainers also treat the mixed mount as
> invalid use case and ignore handle it.

Ext4 has a feature mmp (multiple mount protection) to do this.
More details please refer to:
https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout

Thanks,
Joseph