[pve-devel] [PATCH cluster] add qdevice status api call

[Thomas Lamprecht]

On 6/28/19 5:42 PM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
>  data/PVE/API2/ClusterConfig.pm | 55 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 

while we talked already off-list regarding this, a few comments below,
before I, or you :P, forget it.

> diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
> index e7142b5..81041dc 100644
> --- a/data/PVE/API2/ClusterConfig.pm
> +++ b/data/PVE/API2/ClusterConfig.pm
> @@ -12,6 +12,8 @@ use PVE::Cluster;
>  use PVE::APIClient::LWP;
>  use PVE::Corosync;
>  
> +use IO::Socket::UNIX;
> +
>  use base qw(PVE::RESTHandler);
>  
>  my $clusterconf = "/etc/pve/corosync.conf";
> @@ -69,6 +71,7 @@ __PACKAGE__->register_method({
>  	    { name => 'nodes' },
>  	    { name => 'totem' },
>  	    { name => 'join' },
> +	    { name => 'qdevice' },
>  	];
>  
>  	return $result;
> @@ -552,4 +555,56 @@ __PACKAGE__->register_method({
>  	return $totem_cfg;
>      }});
>  
> +__PACKAGE__->register_method ({
> +    name => 'status',
> +    path => 'qdevice',
> +    method => 'GET',
> +    description => 'Get QDevice and configuration.',
> +    permissions => {
> +	check => ['perm', '/', [ 'Sys.Audit' ]],
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {},
> +    },
> +    returns => {
> +	type => "object",
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $conf = PVE::Cluster::cfs_read_file('corosync.conf');
> +	my $qdevice_cfg = $conf->{main}->{quorum}->{device} // {};

above two variables are not used anywhere.

> +	my $socket_path="/var/run/corosync-qdevice/corosync-qdevice.sock";

return {} if -S $socket_path; 
? To catch systems without a running qdevice gracefully.

also, use space separation between variable name and initialization:
my $socket_path = "/var...

> +
> +	my $qdevice_socket = IO::Socket::UNIX->new(
> +	    Type => SOCK_STREAM,
> +	    Peer => $socket_path,
> +	);
> +
> +	print $qdevice_socket "status verbose\n";
> +	my @qdevice_matches = (
> +	    "Algorithm",
> +	    "Echo reply",
> +	    "Last poll call",
> +	    "Model",
> +	    "QNetd host",
> +	    "State",
> +	    "Tie-breaker",
> +	);
> +	my $keys = { map { $_ => 1 } @qdevice_matches };
Why not directly do a
my $qdev_keys => {
    "Algorithm" => 1,
    "Echo Reply" => 1,
    ...
};

I mean above works too, so if you got any reason keep it, but IMO one
does gains much with it.

> +	my $result = {};
> +	while (my $line = <$qdevice_socket>) {
> +	    chomp $line;
> +	    next if $line =~ /^\s/;
> +	    if ($line =~ /^(.*?)\s*:\s*(.*)$/) {
> +		$result->{$1} = $2 if $keys->{$1};
> +	    }
> +	}
> +
> +	return $result;
> +    }});
> +#TODO: possibly add setup and remove methods
> +
> +
>  1;
>