[pve-devel] [PATCH cluster 1/2] cluster: improve error handling when reading files

[Thomas Lamprecht]

On 10/11/2017 09:36 AM, Wolfgang Bumiller wrote:
> When querying file contents via IPC we return undef if the
> file does not exist, but also on any other error. This is
> potentially problematic as the ipcc_send_rec() xs function
> returns undef on actual errors as well, while setting $!
> (errno).
> 
> It's better to die in cases other than ENOENT. Before this,
> pvesr would assume an empty replication config and an empty
> vm list if pmxcfs wasn't running, which could then clear out
> the entire local replication state file.
> ---
> This patch alone already fixes the core issue with replication
> states mentioned in the commit message. The the 2nd patch and
> the pve-guest-common part are there mostly to avoid race
> conditions between the two non-failing queries (reading the
> config and querying the vm list).
> 
>  data/PVE/Cluster.pm | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
> index 84e3cbf..05f144f 100644
> --- a/data/PVE/Cluster.pm
> +++ b/data/PVE/Cluster.pm
> @@ -2,7 +2,7 @@ package PVE::Cluster;
>  
>  use strict;
>  use warnings;
> -use POSIX qw(EEXIST);
> +use POSIX qw(EEXIST ENOENT);
>  use File::stat qw();
>  use Socket;
>  use Storable qw(dclone);
> @@ -402,7 +402,8 @@ my $ipcc_get_config = sub {
>      my $bindata = pack "Z*", $path;
>      my $res = PVE::IPCC::ipcc_send_rec(6, $bindata);
>      if (!defined($res)) {
> -	return undef if ($! != 0);
> +	return undef if $! == ENOENT;
> +	die "$!\n" if $!;
>  	return '';
>      }
>  
> 

A comment there could be nice too, IMO.
That said:

Reviewed-by: Thomas Lamprecht <t.lamprecht at proxmox.com>