[pve-devel] [PATCH common v3 1/2] sysfstools: file_write: extend with logging and ignore list

[Thomas Lamprecht]

Am 11.11.24 um 09:12 schrieb Dominik Csapak:
> the actual error and path is useful to know when trying to debug or
> figure out what did not work, so warn here if there was an error.
> 
> Now also takes an optional error list that can be ignored. If
> encountering such an error, returns success instead of failure.
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> changes from v2:
> * enhance comment
> * improve commit message
> * shorten return statement
> * remove indentation by default initializing $ignore_list to []
> 
>  src/PVE/SysFSTools.pm | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/src/PVE/SysFSTools.pm b/src/PVE/SysFSTools.pm
> index 0bde6d7..0aeff5f 100644
> --- a/src/PVE/SysFSTools.pm
> +++ b/src/PVE/SysFSTools.pm
> @@ -211,17 +211,28 @@ sub check_iommu_support{
>      return PVE::Tools::dir_glob_regex('/sys/class/iommu/', "[^\.].*");
>  }
>  
> +# writes $buf into $filename, returns false and warns on errors not listed in the optional $ignore_list
> +# error to ignore come from the POSIX module e.g. 'EEXIST'
>  sub file_write {
> -    my ($filename, $buf) = @_;
> +    my ($filename, $buf, $ignore_list) = @_;

The parameter name could be better, i.e.: ignore _what_ list?

But actually I'd prefer having an $allow_existing error, this is too generic for
my taste, at least for this use case.

Alternatively return undef and relay the $! if it's set and allow the call-site
to decide, this feels like a sort of mixed approach; if the case for EEXIST is
common enough having a wrapper here that handles that explicitly, and note that
one then also can check $! like a hash, i.e. `$!{EEXIST}`, so no need to include
POSIX then.

> +    $ignore_list //= [];
>  
>      my $fh = IO::File->new($filename, "w");
>      return undef if !$fh;
>  
> -    my $res = defined(syswrite($fh, $buf)) ? 1 : 0;
> -
> +    my $res = syswrite($fh, $buf);
>      $fh->close();
>  
> -    return $res;
> +    if (defined($res)) {
> +	return 1;
> +    } elsif (my $err = $!) {
> +	for my $to_ignore ($ignore_list->@*) {
> +	    return 1 if $err == $to_ignore;
> +	}
> +	warn "error writing '$buf' to '$filename': $err\n";
> +    }
> +
> +    return 0;
>  }
>  
>  sub pci_device_info {