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 5FB0F6A41C for ; Thu, 16 Sep 2021 12:04:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 57B8B23A96 for ; Thu, 16 Sep 2021 12:04:34 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 7928023A8B for ; Thu, 16 Sep 2021 12:04:33 +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 4CBBD448D3 for ; Thu, 16 Sep 2021 12:04:33 +0200 (CEST) Date: Thu, 16 Sep 2021 12:04:27 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20210906105755.2651203-1-d.csapak@proxmox.com> <20210906105755.2651203-6-d.csapak@proxmox.com> In-Reply-To: <<20210906105755.2651203-6-d.csapak@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1631785829.yhqn3y6em3.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.368 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup 05/12] backup/datastore: prevent protected snapshots to be removed 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, 16 Sep 2021 10:04:34 -0000 On September 6, 2021 12:57 pm, Dominik Csapak wrote: > by throwing an error for remove_backup_dir, and skipping for > remove_backup_group >=20 > Signed-off-by: Dominik Csapak > --- > src/backup/datastore.rs | 37 +++++++++++++++++++++++++------------ > 1 file changed, 25 insertions(+), 12 deletions(-) >=20 > diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs > index 7986c328..03f0a744 100644 > --- a/src/backup/datastore.rs > +++ b/src/backup/datastore.rs > @@ -270,8 +270,9 @@ impl DataStore { > full_path > } > =20 > - /// Remove a complete backup group including all snapshots > - pub fn remove_backup_group(&self, backup_group: &BackupGroup) -> Re= sult<(), Error> { > + /// Remove a complete backup group including all snapshots, returns = true > + /// if all snapshots were removed, and false if some were protected > + pub fn remove_backup_group(&self, backup_group: &BackupGroup) -> Re= sult { > =20 > let full_path =3D self.group_path(backup_group); > =20 > @@ -279,22 +280,30 @@ impl DataStore { > =20 > log::info!("removing backup group {:?}", full_path); > =20 > + let mut removed_all =3D true; > + > // remove all individual backup dirs first to ensure nothing is = using them > for snap in backup_group.list_backups(&self.base_path())? { could also first iterate and check for protected status, and skip=20 removal of any snapshot entirely if we find a protected snapshot? it would still require the re-check in case the protection status=20 changed in the meantime, since that is not guarded by any lock atm, and=20 even if it were, it would be a snapshot level lock, and we can't hold=20 all of those for the whole group here ;) alternatively (since the remove group call bails anyway if a protected=20 snapshot was skipped), we could bail directly here when encountering the=20 first protected snapshot to simplify matters a bit? > + if snap.backup_dir.is_protected(self.base_path()) { > + removed_all =3D false; > + continue; > + } > self.remove_backup_dir(&snap.backup_dir, false)?; > } > =20 > - // no snapshots left, we can now safely remove the empty folder > - std::fs::remove_dir_all(&full_path) > - .map_err(|err| { > - format_err!( > - "removing backup group directory {:?} failed - {}", > - full_path, > - err, > - ) > - })?; > + if removed_all { > + // no snapshots left, we can now safely remove the empty fol= der > + std::fs::remove_dir_all(&full_path) > + .map_err(|err| { > + format_err!( > + "removing backup group directory {:?} failed - {= }", > + full_path, > + err, > + ) > + })?; > + } > =20 > - Ok(()) > + Ok(removed_all) > } > =20 > /// Remove a backup directory including all content > @@ -308,6 +317,10 @@ impl DataStore { > _manifest_guard =3D self.lock_manifest(backup_dir)?; > } > =20 > + if backup_dir.is_protected(self.base_path()) { > + bail!("cannot remove protected snapshot"); > + } > + > log::info!("removing backup snapshot {:?}", full_path); > std::fs::remove_dir_all(&full_path) > .map_err(|err| { > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel >=20 >=20 >=20