[Ocfs2-devel] [PATCH] ocfs2: o2hb: revert hb threshold to keep compatible

[Ocfs2-devel] [PATCH] ocfs2: o2hb: revert hb threshold to keep compatible

[Junxiao Bi]

Configfs is the interface for ocfs2-tools to set configure to
kernel. Change heartbeat dead threshold name in configfs will
cause compatible issue, so revert it.

Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods")
Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com>
---
 fs/ocfs2/cluster/heartbeat.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index f6e871760f8d..0da0332725aa 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -2242,13 +2242,13 @@ static void o2hb_heartbeat_group_drop_item(struct config_group *group,
 	spin_unlock(&o2hb_live_lock);
 }
 
-static ssize_t o2hb_heartbeat_group_threshold_show(struct config_item *item,
+static ssize_t o2hb_heartbeat_group_dead_threshold_show(struct config_item *item,
 		char *page)
 {
 	return sprintf(page, "%u\n", o2hb_dead_threshold);
 }
 
-static ssize_t o2hb_heartbeat_group_threshold_store(struct config_item *item,
+static ssize_t o2hb_heartbeat_group_dead_threshold_store(struct config_item *item,
 		const char *page, size_t count)
 {
 	unsigned long tmp;
@@ -2297,11 +2297,11 @@ static ssize_t o2hb_heartbeat_group_mode_store(struct config_item *item,
 
 }
 
-CONFIGFS_ATTR(o2hb_heartbeat_group_, threshold);
+CONFIGFS_ATTR(o2hb_heartbeat_group_, dead_threshold);
 CONFIGFS_ATTR(o2hb_heartbeat_group_, mode);
 
 static struct configfs_attribute *o2hb_heartbeat_group_attrs[] = {
-	&o2hb_heartbeat_group_attr_threshold,
+	&o2hb_heartbeat_group_attr_dead_threshold,
 	&o2hb_heartbeat_group_attr_mode,
 	NULL,
 };
-- 
1.7.9.5

[Ocfs2-devel] [PATCH] ocfs2: o2hb: revert hb threshold to keep compatible

[Joseph Qi]

Acked-by: Joseph Qi <jiangqi903@gmail.com>

On 17/3/28 09:40, Junxiao Bi wrote:
> Configfs is the interface for ocfs2-tools to set configure to
> kernel. Change heartbeat dead threshold name in configfs will
> cause compatible issue, so revert it.
>
> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods")
> Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com>
> ---
>   fs/ocfs2/cluster/heartbeat.c |    8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
> index f6e871760f8d..0da0332725aa 100644
> --- a/fs/ocfs2/cluster/heartbeat.c
> +++ b/fs/ocfs2/cluster/heartbeat.c
> @@ -2242,13 +2242,13 @@ static void o2hb_heartbeat_group_drop_item(struct config_group *group,
>   	spin_unlock(&o2hb_live_lock);
>   }
>   
> -static ssize_t o2hb_heartbeat_group_threshold_show(struct config_item *item,
> +static ssize_t o2hb_heartbeat_group_dead_threshold_show(struct config_item *item,
>   		char *page)
>   {
>   	return sprintf(page, "%u\n", o2hb_dead_threshold);
>   }
>   
> -static ssize_t o2hb_heartbeat_group_threshold_store(struct config_item *item,
> +static ssize_t o2hb_heartbeat_group_dead_threshold_store(struct config_item *item,
>   		const char *page, size_t count)
>   {
>   	unsigned long tmp;
> @@ -2297,11 +2297,11 @@ static ssize_t o2hb_heartbeat_group_mode_store(struct config_item *item,
>   
>   }
>   
> -CONFIGFS_ATTR(o2hb_heartbeat_group_, threshold);
> +CONFIGFS_ATTR(o2hb_heartbeat_group_, dead_threshold);
>   CONFIGFS_ATTR(o2hb_heartbeat_group_, mode);
>   
>   static struct configfs_attribute *o2hb_heartbeat_group_attrs[] = {
> -	&o2hb_heartbeat_group_attr_threshold,
> +	&o2hb_heartbeat_group_attr_dead_threshold,
>   	&o2hb_heartbeat_group_attr_mode,
>   	NULL,
>   };

[Ocfs2-devel] [PATCH] ocfs2: o2hb: revert hb threshold to keep compatible

[Andrew Morton]

On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote:

> Configfs is the interface for ocfs2-tools to set configure to
> kernel. Change heartbeat dead threshold name in configfs will
> cause compatible issue, so revert it.
> 
> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods")

I don't get it.  45b997737a80 was merged nearly two years ago, so isn't
it a bit late to fix compatibility issues?

[Ocfs2-devel] [PATCH] ocfs2: o2hb: revert hb threshold to keep compatible

[Junxiao Bi]

On 03/29/2017 06:31 AM, Andrew Morton wrote:
> On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote:
> 
>> Configfs is the interface for ocfs2-tools to set configure to
>> kernel. Change heartbeat dead threshold name in configfs will
>> cause compatible issue, so revert it.
>>
>> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods")
> 
> I don't get it.  45b997737a80 was merged nearly two years ago, so isn't
> it a bit late to fix compatibility issues?
> 
This compatibility will not cause ocfs2 down, just some configure (hb
dead threshold) lose effect. If someone want to use the new kernel, they
should apply this fix.

Thanks,
Junxiao.

[Ocfs2-devel] [PATCH] ocfs2: o2hb: revert hb threshold to keep compatible

[Andrew Morton]

On Wed, 29 Mar 2017 09:07:08 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote:

> On 03/29/2017 06:31 AM, Andrew Morton wrote:
> > On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote:
> > 
> >> Configfs is the interface for ocfs2-tools to set configure to
> >> kernel. Change heartbeat dead threshold name in configfs will
> >> cause compatible issue, so revert it.
> >>
> >> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods")
> > 
> > I don't get it.  45b997737a80 was merged nearly two years ago, so isn't
> > it a bit late to fix compatibility issues?
> > 
> This compatibility will not cause ocfs2 down, just some configure (hb
> dead threshold) lose effect. If someone want to use the new kernel, they
> should apply this fix.

Well could someone please send a better changelog?  One which carefully
describes the present behaviour, what is wrong with it and how the
patch fixes it?

One reason for doing this is to permit effecitive patch review.

Another reason is to permit others to decide whether the patch should
be backported into -stable kernels.

Yet another reason is so that maintainers of other kernels can
determine whether this patch will fix behaviour which their users are
reporting.

Thanks.

[Ocfs2-devel] [PATCH] ocfs2: o2hb: revert hb threshold to keep compatible

[Joseph Qi]

On 17/3/29 09:07, Junxiao Bi wrote:
> On 03/29/2017 06:31 AM, Andrew Morton wrote:
>> On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote:
>>
>>> Configfs is the interface for ocfs2-tools to set configure to
>>> kernel. Change heartbeat dead threshold name in configfs will
>>> cause compatible issue, so revert it.
>>>
>>> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods")
>> I don't get it.  45b997737a80 was merged nearly two years ago, so isn't
>> it a bit late to fix compatibility issues?
>>
> This compatibility will not cause ocfs2 down, just some configure (hb
> dead threshold) lose effect. If someone want to use the new kernel, they
> should apply this fix.
The threshold configuration file has default value in kernel, so it will
only affect changing this value in user space.

Thanks,
Joseph
>
> Thanks,
> Junxiao.
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel

[Ocfs2-devel] [PATCH] ocfs2: o2hb: revert hb threshold to keep compatible

[Junxiao Bi]

Hi Andrew,

On 03/29/2017 11:31 AM, Andrew Morton wrote:
> On Wed, 29 Mar 2017 09:07:08 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote:
> 
>> On 03/29/2017 06:31 AM, Andrew Morton wrote:
>>> On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com> wrote:
>>>
>>>> Configfs is the interface for ocfs2-tools to set configure to
>>>> kernel. Change heartbeat dead threshold name in configfs will
>>>> cause compatible issue, so revert it.
>>>>
>>>> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and store methods")
>>>
>>> I don't get it.  45b997737a80 was merged nearly two years ago, so isn't
>>> it a bit late to fix compatibility issues?
>>>
>> This compatibility will not cause ocfs2 down, just some configure (hb
>> dead threshold) lose effect. If someone want to use the new kernel, they
>> should apply this fix.
> 
> Well could someone please send a better changelog?  One which carefully
> describes the present behaviour, what is wrong with it and how the
> patch fixes it?

A new one, please help review.

Configfs is the interface for ocfs2-tools to set configure to
kernel and there
$configfs_dir/cluster/$clustername/heartbeat/dead_threshold is the one
used to configure heartbeat dead threshold. Kernel has a default value
of it but user can set O2CB_HEARTBEAT_THRESHOLD in /etc/sysconfig/o2cb
to override it.
Commit 45b997737a80 ("ocfs2/cluster: use per-attribute show and store
methods") changed heartbeat dead threshold name while ocfs2-tools not,
so ocfs2-tools won't set this configure and default value is always
used. So revert it.

Thanks,
Junxiao.

> 
> One reason for doing this is to permit effecitive patch review.
> 
> Another reason is to permit others to decide whether the patch should
> be backported into -stable kernels.
> 
> Yet another reason is so that maintainers of other kernels can
> determine whether this patch will fix behaviour which their users are
> reporting.
> 
> Thanks.
> 

[Ocfs2-devel] [PATCH] ocfs2: o2hb: revert hb threshold to keep compatible

[Junxiao Bi]

On 03/29/2017 12:01 PM, Joseph Qi wrote:
> 
> 
> On 17/3/29 09:07, Junxiao Bi wrote:
>> On 03/29/2017 06:31 AM, Andrew Morton wrote:
>>> On Tue, 28 Mar 2017 09:40:45 +0800 Junxiao Bi <junxiao.bi@oracle.com>
>>> wrote:
>>>
>>>> Configfs is the interface for ocfs2-tools to set configure to
>>>> kernel. Change heartbeat dead threshold name in configfs will
>>>> cause compatible issue, so revert it.
>>>>
>>>> Fixes: 45b997737a80 ("ocfs2/cluster: use per-attribute show and
>>>> store methods")
>>> I don't get it.  45b997737a80 was merged nearly two years ago, so isn't
>>> it a bit late to fix compatibility issues?
>>>
>> This compatibility will not cause ocfs2 down, just some configure (hb
>> dead threshold) lose effect. If someone want to use the new kernel, they
>> should apply this fix.
> The threshold configuration file has default value in kernel, so it will
> only affect changing this value in user space.
Right. Thank you.

> 
> Thanks,
> Joseph
>>
>> Thanks,
>> Junxiao.
>>
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel at oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
>