all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH proxmox-offline-mirror v2 3/5] pool: unlink_file: fix check for empty directory
Date: Tue,  9 Jul 2024 12:47:03 +0200	[thread overview]
Message-ID: <20240709104705.1681395-4-s.ivanov@proxmox.com> (raw)
In-Reply-To: <20240709104705.1681395-1-s.ivanov@proxmox.com>

path.is_empty() checks for the empty-path, not an empty directory [0].
as the check that the path is below the link_dir happens anyways in
the if we can directly call std::fs::remove_dir (which is even safer
than the std::fs::remove_dir_all call used in pool::remove_dir()).

the oversight seems to have been in place since the intial commit. I
ran across the issue when removing many snapshots of a Debian Bookworm
repository, syncing this to a medium, and still having a vast amount
of empty directories left behind (as debian has one directory per
package), which in turn increases the sync run-time.

[0] https://docs.rs/nix/latest/nix/trait.NixPath.html#tymethod.is_empty

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/pool.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/pool.rs b/src/pool.rs
index 611dcc9..b4f2a6a 100644
--- a/src/pool.rs
+++ b/src/pool.rs
@@ -428,11 +428,11 @@ impl PoolLockGuard<'_> {
         while let Some(parent) = path.parent() {
             path = parent;
 
-            if !self.pool.path_in_link_dir(path) || !path.is_empty() {
+            if !self.pool.path_in_link_dir(path) || path.read_dir()?.next().is_some() {
                 break;
             }
 
-            remove_dir(path)?;
+            std::fs::remove_dir(path)?;
         }
 
         Ok(())
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


  parent reply	other threads:[~2024-07-09 10:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-09 10:47 [pve-devel] [PATCH proxmox-offline-mirror v2 0/5] remove snapshot directories vanished on source also on medium Stoiko Ivanov
2024-07-09 10:47 ` [pve-devel] [PATCH proxmox-offline-mirror v2 1/5] bump proxmox-apt to 0.11 and adapt to changes Stoiko Ivanov
2024-07-09 10:47 ` [pve-devel] [PATCH proxmox-offline-mirror v2 2/5] pool: drop superfluous check for impossible path combination Stoiko Ivanov
2024-07-09 10:47 ` Stoiko Ivanov [this message]
2024-07-09 10:47 ` [pve-devel] [PATCH proxmox-offline-mirror v2 4/5] pool: gc: remove empty directories under link_dir Stoiko Ivanov
2024-07-09 10:47 ` [pve-devel] [PATCH proxmox-offline-mirror v2 5/5] pool: remove unused imports Stoiko Ivanov
2024-07-10 10:42 ` [pve-devel] applied-series: [PATCH proxmox-offline-mirror v2 0/5] remove snapshot directories vanished on source also on medium Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240709104705.1681395-4-s.ivanov@proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal