* mdadm Consistency Policy initialization
@ 2017-04-18 16:50 Jes Sorensen
2017-04-19 10:29 ` Artur Paszkiewicz
0 siblings, 1 reply; 6+ messages in thread
From: Jes Sorensen @ 2017-04-18 16:50 UTC (permalink / raw)
To: Artur Paszkiewicz; +Cc: linux-raid
Hi Artur,
In 5308f11727b889965efe5ac0e854d197c2b51f6d you introduced struct
mdinfo: enum consistency_policy, but in mdadm.c you initialize it to
UnSet which isn't part of the enum.
Is there any actual difference between CONSISTENCY_POLICY_UNKNOWN and
UnSet? It seems suboptimal to mix and match within the enum like this,
and if CONSISTENCY_POLICY_UNKNOWN does the job, couldn't we just
initialize with that?
Cheers,
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mdadm Consistency Policy initialization
2017-04-18 16:50 mdadm Consistency Policy initialization Jes Sorensen
@ 2017-04-19 10:29 ` Artur Paszkiewicz
2017-04-19 16:19 ` Jes Sorensen
2017-04-20 16:08 ` Jes Sorensen
0 siblings, 2 replies; 6+ messages in thread
From: Artur Paszkiewicz @ 2017-04-19 10:29 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
On 04/18/2017 06:50 PM, Jes Sorensen wrote:
> Hi Artur,
>
> In 5308f11727b889965efe5ac0e854d197c2b51f6d you introduced struct mdinfo: enum consistency_policy, but in mdadm.c you initialize it to UnSet which isn't part of the enum.
>
> Is there any actual difference between CONSISTENCY_POLICY_UNKNOWN and UnSet? It seems suboptimal to mix and match within the enum like this, and if CONSISTENCY_POLICY_UNKNOWN does the job, couldn't we just initialize with that?
Hi Jes,
The "enum consistency_policy" and "mapping_t consistency_policies[]"
represent values that can appear in sysfs. md/consistency_policy can be
"unknown" when the array is inactive. On the other hand, UnSet just
means that the --consistency-policy= parameter was not provided by the
user. I wanted to differentiate between these two cases. If you think
this is redundant I can change it and use CONSISTENCY_POLICY_UNKNOWN
instead, this should be straightforward.
Thanks,
Artur
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mdadm Consistency Policy initialization
2017-04-19 10:29 ` Artur Paszkiewicz
@ 2017-04-19 16:19 ` Jes Sorensen
2017-04-20 16:08 ` Jes Sorensen
1 sibling, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2017-04-19 16:19 UTC (permalink / raw)
To: Artur Paszkiewicz; +Cc: linux-raid
On 04/19/2017 06:29 AM, Artur Paszkiewicz wrote:
> On 04/18/2017 06:50 PM, Jes Sorensen wrote:
>> Hi Artur,
>>
>> In 5308f11727b889965efe5ac0e854d197c2b51f6d you introduced struct mdinfo: enum consistency_policy, but in mdadm.c you initialize it to UnSet which isn't part of the enum.
>>
>> Is there any actual difference between CONSISTENCY_POLICY_UNKNOWN and UnSet? It seems suboptimal to mix and match within the enum like this, and if CONSISTENCY_POLICY_UNKNOWN does the job, couldn't we just initialize with that?
>
> Hi Jes,
>
> The "enum consistency_policy" and "mapping_t consistency_policies[]"
> represent values that can appear in sysfs. md/consistency_policy can be
> "unknown" when the array is inactive. On the other hand, UnSet just
> means that the --consistency-policy= parameter was not provided by the
> user. I wanted to differentiate between these two cases. If you think
> this is redundant I can change it and use CONSISTENCY_POLICY_UNKNOWN
> instead, this should be straightforward.
Hi Artur,
I would prefer to either use CONSISTENCY_POLICY_UNKNOWN or introduce a
new state within the enum so we don't cross pollute the namespace.
Cheers,
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mdadm Consistency Policy initialization
2017-04-19 10:29 ` Artur Paszkiewicz
2017-04-19 16:19 ` Jes Sorensen
@ 2017-04-20 16:08 ` Jes Sorensen
2017-04-24 10:07 ` Artur Paszkiewicz
1 sibling, 1 reply; 6+ messages in thread
From: Jes Sorensen @ 2017-04-20 16:08 UTC (permalink / raw)
To: Artur Paszkiewicz; +Cc: linux-raid
On 04/19/2017 06:29 AM, Artur Paszkiewicz wrote:
> On 04/18/2017 06:50 PM, Jes Sorensen wrote:
>> Hi Artur,
>>
>> In 5308f11727b889965efe5ac0e854d197c2b51f6d you introduced struct mdinfo: enum consistency_policy, but in mdadm.c you initialize it to UnSet which isn't part of the enum.
>>
>> Is there any actual difference between CONSISTENCY_POLICY_UNKNOWN and UnSet? It seems suboptimal to mix and match within the enum like this, and if CONSISTENCY_POLICY_UNKNOWN does the job, couldn't we just initialize with that?
>
> Hi Jes,
>
> The "enum consistency_policy" and "mapping_t consistency_policies[]"
> represent values that can appear in sysfs. md/consistency_policy can be
> "unknown" when the array is inactive. On the other hand, UnSet just
> means that the --consistency-policy= parameter was not provided by the
> user. I wanted to differentiate between these two cases. If you think
> this is redundant I can change it and use CONSISTENCY_POLICY_UNKNOWN
> instead, this should be straightforward.
Hi Artur,
I made some changes to map_name() and noticed that you already there
default to CONSISTENCY_POLICY_UNKNOWN if it returns UnSet in
sysfs_read(). However given that you do a lot of checks manually outside
of sysfs.c, I dind't change the code there for now. I think we the
change I made to map_name() it should be possible to simply a bunch of
that checking code.
Cheers,
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mdadm Consistency Policy initialization
2017-04-20 16:08 ` Jes Sorensen
@ 2017-04-24 10:07 ` Artur Paszkiewicz
2017-04-24 13:45 ` Jes Sorensen
0 siblings, 1 reply; 6+ messages in thread
From: Artur Paszkiewicz @ 2017-04-24 10:07 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
On 04/20/2017 06:08 PM, Jes Sorensen wrote:
> On 04/19/2017 06:29 AM, Artur Paszkiewicz wrote:
>> On 04/18/2017 06:50 PM, Jes Sorensen wrote:
>>> Hi Artur,
>>>
>>> In 5308f11727b889965efe5ac0e854d197c2b51f6d you introduced struct mdinfo: enum consistency_policy, but in mdadm.c you initialize it to UnSet which isn't part of the enum.
>>>
>>> Is there any actual difference between CONSISTENCY_POLICY_UNKNOWN and UnSet? It seems suboptimal to mix and match within the enum like this, and if CONSISTENCY_POLICY_UNKNOWN does the job, couldn't we just initialize with that?
>>
>> Hi Jes,
>>
>> The "enum consistency_policy" and "mapping_t consistency_policies[]"
>> represent values that can appear in sysfs. md/consistency_policy can be
>> "unknown" when the array is inactive. On the other hand, UnSet just
>> means that the --consistency-policy= parameter was not provided by the
>> user. I wanted to differentiate between these two cases. If you think
>> this is redundant I can change it and use CONSISTENCY_POLICY_UNKNOWN
>> instead, this should be straightforward.
>
> Hi Artur,
>
> I made some changes to map_name() and noticed that you already there default to CONSISTENCY_POLICY_UNKNOWN if it returns UnSet in sysfs_read(). However given that you do a lot of checks manually outside of sysfs.c, I dind't change the code there for now. I think we the change I made to map_name() it should be possible to simply a bunch of that checking code.
Hi Jes,
It does seem to be better this way. I'll send a patch that removes using
UnSet from all consistency policy related places soon.
Thanks,
Artur
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mdadm Consistency Policy initialization
2017-04-24 10:07 ` Artur Paszkiewicz
@ 2017-04-24 13:45 ` Jes Sorensen
0 siblings, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2017-04-24 13:45 UTC (permalink / raw)
To: Artur Paszkiewicz; +Cc: linux-raid
On 04/24/2017 06:07 AM, Artur Paszkiewicz wrote:
> On 04/20/2017 06:08 PM, Jes Sorensen wrote:
>> On 04/19/2017 06:29 AM, Artur Paszkiewicz wrote:
>>> Hi Jes,
>>>
>>> The "enum consistency_policy" and "mapping_t consistency_policies[]"
>>> represent values that can appear in sysfs. md/consistency_policy can be
>>> "unknown" when the array is inactive. On the other hand, UnSet just
>>> means that the --consistency-policy= parameter was not provided by the
>>> user. I wanted to differentiate between these two cases. If you think
>>> this is redundant I can change it and use CONSISTENCY_POLICY_UNKNOWN
>>> instead, this should be straightforward.
>>
>> Hi Artur,
>>
>> I made some changes to map_name() and noticed that you already there default to CONSISTENCY_POLICY_UNKNOWN if it returns UnSet in sysfs_read(). However given that you do a lot of checks manually outside of sysfs.c, I dind't change the code there for now. I think we the change I made to map_name() it should be possible to simply a bunch of that checking code.
>
> Hi Jes,
>
> It does seem to be better this way. I'll send a patch that removes using
> UnSet from all consistency policy related places soon.
Awesome!
I am glad you like it.
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-24 13:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 16:50 mdadm Consistency Policy initialization Jes Sorensen
2017-04-19 10:29 ` Artur Paszkiewicz
2017-04-19 16:19 ` Jes Sorensen
2017-04-20 16:08 ` Jes Sorensen
2017-04-24 10:07 ` Artur Paszkiewicz
2017-04-24 13:45 ` Jes Sorensen
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.