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 UTF8SMTPS id 992FA6BFDB for ; Thu, 28 Jan 2021 13:26:40 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id 88B632DA28 for ; Thu, 28 Jan 2021 13:26:40 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (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 firstgate.proxmox.com (Proxmox) with UTF8SMTPS id A82E52DA1D for ; Thu, 28 Jan 2021 13:26:39 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id 6529446109 for ; Thu, 28 Jan 2021 13:26:39 +0100 (CET) To: pbs-devel@lists.proxmox.com References: <20210127134314.22530-1-d.csapak@proxmox.com> <1611818626.6jkzjorqup.astroid@nora.none> From: Dominik Csapak Message-ID: <0c7984b2-e88a-1a87-c388-95d539b85cf1@proxmox.com> Date: Thu, 28 Jan 2021 13:26:38 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:85.0) Gecko/20100101 Thunderbird/85.0 MIME-Version: 1.0 In-Reply-To: <1611818626.6jkzjorqup.astroid@nora.none> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.156 Adjusted score from AWL reputation of From: address KAM_ASCII_DIVIDERS 0.8 Spam that uses ascii formatting tricks 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) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Subject: Re: [pbs-devel] [RFC proxmox-backup] backup/backup_info: ignore vanished backup dirs during listing 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: Thu, 28 Jan 2021 12:26:40 -0000 On 1/28/21 8:34 AM, Fabian Grünbichler wrote: > when looking at the call sites of list_backup_files, this seems wrong - > e.g. when listing a BackupGroup's backups, such a no-longer-existing > snapshot is now returned but has no files. when creating a new > BackupInfo from a BackupDir, or listing a BackupInfo's files, it returns > Ok but contains no files. > > the decision whether to ignore such an error must be made at the call > sites of the call sites above, since in most cases I'd want to ignore > the snapshot and not just the file listing.. > > list_snapshot_files already throws an error if it can't read the > manifest, list_snapshots and list_groups seem to include it with no > files? > yeah you are right, we should probably catch this wider up in the call chain, but do we really want to downcast the anyhow errors wider up to nix errors? or do we want to have either an extra error type? or do we maybe want to make the return type an Result>, Error> to differentiate between 'no files' and 'vanished' ? > On January 27, 2021 2:43 pm, Dominik Csapak wrote: >> this had the effect that that a snapshot listing errored out when called >> during a prune action >> >> we now ignore such errors and simply return no files >> >> also add some context for the error if one happens >> >> Signed-off-by: Dominik Csapak >> --- >> maybe we also want to filter such snapshots out one level higher? >> so that we do not show 'empty' snapshots >> >> src/backup/backup_info.rs | 28 ++++++++++++++++++++++------ >> 1 file changed, 22 insertions(+), 6 deletions(-) >> >> diff --git a/src/backup/backup_info.rs b/src/backup/backup_info.rs >> index 5ff1a6f8..dba3fea5 100644 >> --- a/src/backup/backup_info.rs >> +++ b/src/backup/backup_info.rs >> @@ -1,4 +1,7 @@ >> -use crate::tools; >> +use crate::tools::{ >> + self, >> + fs::FileIterOps, >> +}; >> >> use anyhow::{bail, format_err, Error}; >> use std::os::unix::io::RawFd; >> @@ -339,11 +342,24 @@ impl BackupInfo { >> fn list_backup_files(dirfd: RawFd, path: &P) -> Result, Error> { >> let mut files = vec![]; >> >> - tools::scandir(dirfd, path, &BACKUP_FILE_REGEX, |_, filename, file_type| { >> - if file_type != nix::dir::Type::File { return Ok(()); } >> - files.push(filename.to_owned()); >> - Ok(()) >> - })?; >> + match tools::fs::read_subdir(dirfd, path) { >> + Ok(iter) => { >> + for entry in iter.filter_file_name_regex(&BACKUP_FILE_REGEX) { >> + let entry = entry?; >> + let file_type = entry.file_type().ok_or_else(|| format_err!("unable to detect file type"))?; >> + if file_type == nix::dir::Type::File { >> + // ok because we filtered with BACKUP_FILE_REGEX >> + let filename = unsafe { entry.file_name_utf8_unchecked() }; >> + files.push(filename.to_owned()); >> + } >> + } >> + }, >> + Err(nix::Error::Sys(errno)) if errno == nix::errno::Errno::ENOENT => { >> + // ignore vanished backup dirs >> + eprintln!("backup dir vanished during listing"); >> + }, >> + Err(err) => bail!("error listing backup files: {}", err), >> + } >> >> Ok(files) >> } >> -- >> 2.20.1 >> >> >> >> _______________________________________________ >> pbs-devel mailing list >> pbs-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel >> >> >> > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > >