From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7E0B79A94C for ; Wed, 10 May 2023 15:23:30 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5905728E30 for ; Wed, 10 May 2023 15:23:00 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 10 May 2023 15:22:59 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id F2FAC47FB2 for ; Wed, 10 May 2023 15:22:58 +0200 (CEST) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Wed, 10 May 2023 15:22:58 +0200 Message-Id: <20230510132258.3350401-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.015 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [zip.rs] Subject: [pbs-devel] [PATCH proxmox] compression: zip_directory: improve error handling X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 May 2023 13:23:30 -0000 when zipping a directory, our intention was to skip over files that cannot be zipped (e.g. the file can't be read/vanished/etc.), so we ignored errors and simply logged it. but when 'add_entry' fails, we will never actually restore, since every error there is fatal to the point that the zip cannot be finished thats because we take the 'target' sink out of self, and only insert it again after all writes succeeded. so if an error occurs in between 'target' is not put into self again (and never will be) and the zip cannot be finished (even if we would catch all those intermediate errors and restore 'target', we don't know in which state the output was, so we're unable to finish a valid zip) to fix that, split the actual 'add_entry' part there out of the async move block and treat its errors always as fatal without this, we generate heaps of log lines even after an error occurred, and can never recover Signed-off-by: Dominik Csapak --- note that i know we use 'eprintln' instead of the log macro, but it's not part of the fix, and i don't know if we want to add the log crate as dep here. we only use this function currently in the restore daemon for downloading a directory proxmox-compression/src/zip.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/proxmox-compression/src/zip.rs b/proxmox-compression/src/zip.rs index c9ed80e..6d457fb 100644 --- a/proxmox-compression/src/zip.rs +++ b/proxmox-compression/src/zip.rs @@ -643,7 +643,7 @@ where let entry_path = entry.path().to_owned(); let encoder = &mut encoder; - if let Err(err) = async move { + match async move { let entry_path_no_base = entry.path().strip_prefix(base_path)?; let metadata = entry.metadata()?; let mtime = match metadata @@ -659,22 +659,27 @@ where if entry.file_type().is_file() { let file = tokio::fs::File::open(entry.path()).await?; let ze = ZipEntry::new(entry_path_no_base, mtime, mode, true); - encoder.add_entry(ze, Some(file)).await?; + Ok(Some((ze, Some(file)))) } else if entry.file_type().is_dir() { let ze = ZipEntry::new(entry_path_no_base, mtime, mode, false); let content: Option = None; - encoder.add_entry(ze, content).await?; + Ok(Some((ze, content))) + } else { + // ignore other file types + Ok::<_, Error>(None) } - // ignore other file types - Ok::<(), Error>(()) } .await { - eprintln!( - "zip: error encoding file or directory '{}': {}", - entry_path.display(), - err - ); + Ok(Some((ze, content))) => encoder.add_entry(ze, content).await?, + Ok(None) => {} + Err(err) => { + eprintln!( + "zip: error encoding file or directory '{}': {}", + entry_path.display(), + err + ); + } } } -- 2.30.2