[pve-devel] [RFC PATCH common] sysfstools: file_write: properly catch errors

[Christoph Heiss]

Tested this together with the Nvidia vGPU series [0], so please consider
this also:

Tested-by: Christoph Heiss <c.heiss at proxmox.com>

[0] https://lore.proxmox.com/pve-devel/ul3faxwxsi45hk4e44cl2fw4jomoncy53owx3you2wuvo7pymf@dchu53bqfa3p/T/#m2526c3a031aacc2f52080977febd432b4ab2efcf

On Tue, Jul 23, 2024 at 10:29:25AM GMT, Dominik Csapak wrote:
> since `print` is doing buffered IO, we don't always get an error there,
> even if the underlying write does not work.
>
> To properly catch that, do an unbuffered `syswrite` which circumvents
> all buffers and writes directly to the file handle.
>
> We aren't actually interested in the specific error here, but only if
> the write was successful or not.
>
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>
> Note: this is heavily used when doing PCI passthrough, and it seems we
> did not catch as many errors as we thought. Maybe we should introduce an
> 'noerr' parameter and pass that on all current code paths, since i fear
> that there are many situations where the sysfs write had failed but it
> still works because we ignored it and qemu/the kernel do the right thing
> anyway.

Actually catching errors here is definitely the right thing to do, IMHO.
But as for not breaking existing callers, maybe a `warn` statement would
be sensible?

That way broken call-sites can be caught in the future and fixed as
necessary. So a `noerr` parameter and `warn` if that is unset.

>
>  src/PVE/SysFSTools.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/PVE/SysFSTools.pm b/src/PVE/SysFSTools.pm
> index 57f0ac8..8eb9f2e 100644
> --- a/src/PVE/SysFSTools.pm
> +++ b/src/PVE/SysFSTools.pm
> @@ -198,7 +198,7 @@ sub file_write {
>      my $fh = IO::File->new($filename, "w");
>      return undef if !$fh;
>
> -    my $res = print $fh $buf;
> +    my $res = defined(syswrite($fh, $buf)) ? 1 : 0;
>
>      $fh->close();
>
> --
> 2.39.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>