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

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.
> Another scenario is journal replay after crash.
> 
this patch set a rule:
If last mount didn't do umount, (eg: crash happened), the next mount MUST be
same mount type.
(please also check above lines of 'code logic'.)

In my view, this rule is enough to handle crash scenario.
So my patch should be polished in somewhere, but it is workable.

Thanks,
Heming

> ? 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.
> 
>> Another scenario is journal replay after crash.
> 
> this patch set a rule:
> If last mount didn't do umount, (eg: crash happened), the next mount
MUST be same mount type.
> (please also check above lines of 'code logic'.)
> 
> In my view, this rule is enough to handle crash scenario.
> So my patch should be polished in somewhere, but it is workable.
> 
> Thanks,
> Heming