From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 057B11FF15D for ; Thu, 17 Oct 2024 12:22:51 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EC18BD1CC; Thu, 17 Oct 2024 12:23:22 +0200 (CEST) Date: Thu, 17 Oct 2024 12:22:45 +0200 From: Christoph Heiss Cc: Proxmox VE development discussion Message-ID: References: <20240723082925.934603-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240723082925.934603-1-d.csapak@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.480 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment MISSING_HEADERS 1.021 Missing To: header RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [RFC PATCH common] sysfstools: file_write: properly catch errors X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Tested this together with the Nvidia vGPU series [0], so please consider this also: Tested-by: Christoph Heiss [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 > --- > > 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@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel