[pve-devel] [PATCH RFC 2/3] implement start flag

[Thomas Lamprecht]

On 11/14/2016 12:23 PM, Dietmar Maurer wrote:
> Signed-off-by: Dietmar Maurer <dietmar at proxmox.com>
> ---
>   src/PVE/HA/Manager.pm | 29 ++++++++++++++++++++---------
>   1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index e58fc0b..ab8e81d 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -290,7 +290,8 @@ my $recover_fenced_service = sub {
>   
>   	# $sd *is normally read-only*, fencing is the exception
>   	$cd->{node} = $sd->{node} = $recovery_node;
> -	&$change_service_state($self, $sid, 'started', node => $recovery_node);
> +	&$change_service_state($self, $sid, $cd->{start} ? 'started' : 'request_stop',
> +			       node => $recovery_node);
>       } else {
>   	# no possible node found, cannot recover
>   	$haenv->log('err', "recovering service '$sid' from fenced node " .
> @@ -379,10 +380,12 @@ sub manage {
>       # add new service
>       foreach my $sid (sort keys %$sc) {
>   	next if $ss->{$sid}; # already there
> +	my $cd = $sc->{$sid};
>   	$haenv->log('info', "adding new service '$sid' on node '$sc->{$sid}->{node}'");
>   	# assume we are running to avoid relocate running service at add
> -	$ss->{$sid} = { state => 'started', node => $sc->{$sid}->{node},
> -			uid => compute_new_uuid('started') };
> +	my $state = $cd->{start} ? 'started' : 'request_stop';
> +	$ss->{$sid} = { state => $state, node => $cd->{node},
> +			uid => compute_new_uuid($state) };
>       }
>   
>       # remove stale service from manager state
> @@ -432,7 +435,7 @@ sub manage {
>   
>   		my $lrm_mode = $sd->{node} ? $lrm_modes->{$sd->{node}} : undef;
>   		# unfreeze
> -		&$change_service_state($self, $sid, 'started')
> +		&$change_service_state($self, $sid, $cd->{start} ? 'started' : 'request_stop')
>   		    if $lrm_mode && $lrm_mode eq 'active';
>   
>   	    } elsif ($last_state eq 'error') {
> @@ -518,7 +521,8 @@ sub next_state_migrate_relocate {
>       # check result from LRM daemon
>       if ($lrm_res) {
>   	my $exit_code = $lrm_res->{exit_code};
> -	my $req_state = $cd->{state} eq 'enabled' ? 'started' : 'request_stop';
> +	my $req_state = ($cd->{state} eq 'enabled' && $cd->{start}) ?
> +	    'started' : 'request_stop';
>   	if ($exit_code == SUCCESS) {
>   	    &$change_service_state($self, $sid, $req_state, node => $sd->{target});
>   	    return;
> @@ -579,10 +583,17 @@ sub next_state_stopped {
>   	return;
>       }
>   
> +    if ($ns->node_is_offline_delayed($sd->{node})) {
> +	&$change_service_state($self, $sid, 'fence');
> +	return;
> +    }
> +
>       if ($cd->{state} eq 'enabled') {
> -	# simply mark it started, if it's on the wrong node
> -	# next_state_started will fix that for us
> -	&$change_service_state($self, $sid, 'started', node => $sd->{node});
> +	if ($cd->{start}) {
> +	    # simply mark it started, if it's on the wrong node
> +	    # next_state_started will fix that for us
> +	    &$change_service_state($self, $sid, 'started', node => $sd->{node});
> +	}
>   	return;
>       }
>   
> @@ -613,7 +624,7 @@ sub next_state_started {
>   	return;
>       }
>   	
> -    if ($cd->{state} eq 'disabled') {
> +    if ($cd->{state} eq 'disabled' || !$cd->{start} ) {

Disabling is now both disable from recovery and stop?
This seems not completely intuitive with the new start flag, imo.
(users already had problems previously with grasping what we meant here)

my idea to this problem was to make a strict separation between
* resource should be started/stopped
* resource should be recovered

This changes design quite a bit, but would reflect also wishes from 
users on user mailing list discussion.
So not quite sure.

We could decouple the state from the resource start flag also later (5.0 
?) so this is OK for me in the meanwhile.

>   	&$change_service_state($self, $sid, 'request_stop');
>   	return;
>       }