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 665336DCBC for ; Mon, 28 Mar 2022 08:30:38 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5467C2298F for ; Mon, 28 Mar 2022 08:30:08 +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 id 2140A22982 for ; Mon, 28 Mar 2022 08:30:07 +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 ECA2D425F0 for ; Mon, 28 Mar 2022 08:30:06 +0200 (CEST) Message-ID: <013ab17d-2ebf-aaca-0ce5-9eeabccba9c4@proxmox.com> Date: Mon, 28 Mar 2022 08:30:04 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:99.0) Gecko/20100101 Thunderbird/99.0 Content-Language: en-US To: Proxmox Backup Server development discussion , Dominik Csapak References: <20220325123816.3865356-1-d.csapak@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20220325123816.3865356-1-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.056 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) 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 - Subject: Re: [pbs-devel] [PATCH proxmox-backup] rest-server: cleanup_old_tasks: 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: Mon, 28 Mar 2022 06:30:38 -0000 On 25.03.22 13:38, Dominik Csapak wrote: > by not bubbling up most errors, and continuing on. this avoids that we > stop cleaning up because e.g. one directory was missing. continuing is OK but if an those errors are encountered I'd still return an Err(), and not an Ok(), at the end to make any caller easily distinguish the full OK or error case. If we expect such errors to happen there's something wrong with the general approach, then the known false positives should be filtered. > > Signed-off-by: Dominik Csapak > --- > proxmox-rest-server/src/worker_task.rs | 25 ++++++++++++++++++++----- > 1 file changed, 20 insertions(+), 5 deletions(-) > > diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs > index 643e1872..619b8e9d 100644 > --- a/proxmox-rest-server/src/worker_task.rs > +++ b/proxmox-rest-server/src/worker_task.rs > @@ -270,18 +270,33 @@ pub fn cleanup_old_tasks(compressed: bool) -> Result<(), Error> { > for i in 0..256 { > let mut path = setup.taskdir.clone(); > path.push(format!("{:02X}", i)); > - for file in std::fs::read_dir(path)? { > - let file = file?; > + let files = match std::fs::read_dir(path) { > + Ok(files) => files, > + Err(_) => continue, // does not exist or no permissions hmm, silently ignoring ENOENT is one thing, but I'd not do so for other errors, at least print a note/warning? > + }; > + for file in files { > + let file = match file { > + Ok(file) => file, > + Err(err) => { > + log::error!("task cleanup: could not check some file in {}: {}", i, err); > + continue; > + } > + }; > let path = file.path(); > > - let modified = get_modified(file) > - .map_err(|err| format_err!("error getting mtime for {:?}: {}", path, err))?; > + let modified = match get_modified(file) { > + Ok(modified) => modified, > + Err(err) => { > + log::error!("task cleanup: error getting mtime for {:?}: {}", path, err); > + continue; > + } > + }; > > if modified < cutoff_time { > match std::fs::remove_file(path) { > Ok(()) => {}, > Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}, > - Err(err) => bail!("could not remove file: {}", err), > + Err(err) => log::error!("task cleanup: could not remove file: {}", err), > } > } > }