[pve-devel] [PATCH ha-manager 2/6] node status: abort update if not quorate

[Thomas Lamprecht]

On 11/08/2017 06:18 AM, Dietmar Maurer wrote:
>> If not quorate we must not use the node info as it is undefined
>>
>> Solves an issue where the current master marked all other nodes as
>> offline when the cluster file system was temporarily unavailable
>> (e.g., because of a restart triggered by an update).
>>
>> The quorum information was always passed to this sub, just not used
>> here.
> 
> I just found one call to that function:
> 
> src/PVE/HA/Manager.pm:    $ns->update($haenv->get_node_info());
> 
> It is a bit hard to see the '$quorate' parameter here. I suugest
> to use something like:
> 
> my ($node_info, $quorate) = $haenv->get_node_info();
> $ns->update(node_info, $quorate)
> 
> 

Yeah, I agree, if we would check inside this method then it this
suggestion would be definitively nicer to read.

On 11/08/2017 06:28 AM, Dietmar Maurer wrote:
> 
>> diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm
>> index 7e6e317..4ae28dd 100644
>> --- a/src/PVE/HA/NodeStatus.pm
>> +++ b/src/PVE/HA/NodeStatus.pm
>> @@ -117,7 +117,9 @@ my $set_node_state = sub {
>>  };
>>  
>>  sub update {
>> -    my ($self, $node_info) = @_;
>> +    my ($self, $node_info, $quorate) = @_;
>> +
>> +    return if !$quorate;
> 
> 
> I think we should not call this method when we are not quorate. So
> adding a quorate parameter makes no sense to me. Instead, avoid the
> call when not quorate?
> 

OK, sounds good to me.