2 cosmetic nits inline:
On Fri, 8 Nov 2024 10:32:58 +0100
Dominik Csapak <d.csa...@proxmox.com> wrote:
> 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.
>
> Also takes now an optional error list that can be ignored. If
> encountering such an error, returns success instead of failure.
suggestion:
Now also takes an optional list of errors that can be ignored.
>
> Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
> ---
> src/PVE/SysFSTools.pm | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/src/PVE/SysFSTools.pm b/src/PVE/SysFSTools.pm
> index 0bde6d7..156fee6 100644
> --- a/src/PVE/SysFSTools.pm
> +++ b/src/PVE/SysFSTools.pm
> @@ -211,17 +211,31 @@ sub check_iommu_support{
> return PVE::Tools::dir_glob_regex('/sys/class/iommu/', "[^\.].*");
> }
>
> +# writes $buf into $filename
> +# returns success when encountering an error from the given $ignore_list,
> e.g. EEXIST
I somehow read this as only condition when it returns success (also not
sure if the comment is needed or this gets clear when looking through the
code)?
maybe:
# writes $buf into $filename, returns false and warns on errors not listed in
the optional $ignore_list
?
> sub file_write {
> - my ($filename, $buf) = @_;
> + my ($filename, $buf, $ignore_list) = @_;
if you do:
$ignore_list //= [];
here...
>
> 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 = $!) {
> + if (defined($ignore_list)) {
.. this nesting can be omitted.
> + for my $to_ignore ($ignore_list->@*) {
> + if ($err == $to_ignore) {
> + return 1;
> + }
the inner if could be shorted a bit to:
return 1 if ($err == $to_ignore);
> + }
> + }
> + warn "error writing '$buf' to '$filename': $err\n";
> + }
> +
> + return 0;
> }
>
> sub pci_device_info {
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
