public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Reiter <s.reiter@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 7/7] fuse_loop: handle unmap on crashed instance
Date: Wed,  7 Oct 2020 13:53:08 +0200	[thread overview]
Message-ID: <20201007115308.6275-8-s.reiter@proxmox.com> (raw)
In-Reply-To: <20201007115308.6275-1-s.reiter@proxmox.com>

If a fuse_loop instance dies suddenly (e.g. SIGKILL), the FUSE mount and
loop device assignment are left behind. We can determine this scenario
on specific unmap, when the PID file is either missing or contains a PID
of a non-running process, but the backing file and potentially loop
device are still there.

If that's the case, do an "emergency cleanup", by unassigning the
loopdev, calling 'fusermount -u' and then cleaning any leftover files
manually.

With this in place, pretty much any situation is now recoverable via
only the 'proxmox-backup-client' binary, by either calling 'unmap' with
or without parameters.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/tools/fuse_loop.rs | 50 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/src/tools/fuse_loop.rs b/src/tools/fuse_loop.rs
index 05d92525..ab733f27 100644
--- a/src/tools/fuse_loop.rs
+++ b/src/tools/fuse_loop.rs
@@ -236,7 +236,7 @@ pub fn cleanup_unused_run_files(filter_name: Option<String>) {
 
                 // clean leftover FUSE instances (e.g. user called 'losetup -d' or similar)
                 // does nothing if files are already stagnant (e.g. instance crashed etc...)
-                if let Ok(_) = unmap_from_backing(&path) {
+                if let Ok(_) = unmap_from_backing(&path, None) {
                     // we have reaped some leftover instance, tell the user
                     eprintln!(
                         "Cleaned up dangling mapping '{}': no loop device assigned",
@@ -280,19 +280,53 @@ fn get_backing_file(loopdev: &str) -> Result<String, Error> {
     Ok(backing_file.to_owned())
 }
 
-fn unmap_from_backing(backing_file: &Path) -> Result<(), Error> {
+// call in broken state: we found the mapping, but the client is already dead,
+// only thing to do is clean up what we can
+fn emerg_cleanup (loopdev: Option<&str>, mut backing_file: PathBuf) {
+    eprintln!(
+        "warning: found mapping with dead process ({:?}), attempting cleanup",
+        &backing_file
+    );
+
+    if let Some(loopdev) = loopdev {
+        let _ = loopdev::unassign(loopdev);
+    }
+
+    // killing the backing process does not cancel the FUSE mount automatically
+    let mut command = std::process::Command::new("fusermount");
+    command.arg("-u");
+    command.arg(&backing_file);
+    let _ = crate::tools::run_command(command, None);
+
+    let _ = remove_file(&backing_file);
+    backing_file.set_extension("pid");
+    let _ = remove_file(&backing_file);
+}
+
+fn unmap_from_backing(backing_file: &Path, loopdev: Option<&str>) -> Result<(), Error> {
     let mut pid_path = PathBuf::from(backing_file);
     pid_path.set_extension("pid");
 
-    let pid_str = read_to_string(&pid_path).map_err(|err|
-        format_err!("error reading pidfile {:?}: {}", &pid_path, err))?;
+    let pid_str = read_to_string(&pid_path).map_err(|err| {
+        if err.kind() == std::io::ErrorKind::NotFound {
+            emerg_cleanup(loopdev, backing_file.to_owned());
+        }
+        format_err!("error reading pidfile {:?}: {}", &pid_path, err)
+    })?;
     let pid = pid_str.parse::<i32>().map_err(|err|
         format_err!("malformed PID ({}) in pidfile - {}", pid_str, err))?;
 
     let pid = Pid::from_raw(pid);
 
     // send SIGINT to trigger cleanup and exit in target process
-    signal::kill(pid, Signal::SIGINT)?;
+    match signal::kill(pid, Signal::SIGINT) {
+        Ok(()) => {},
+        Err(nix::Error::Sys(nix::errno::Errno::ESRCH)) => {
+            emerg_cleanup(loopdev, backing_file.to_owned());
+            return Ok(());
+        },
+        Err(e) => return Err(e.into()),
+    }
 
     // block until unmap is complete or timeout
     let start = time::epoch_i64();
@@ -364,16 +398,16 @@ pub fn unmap_loopdev<S: AsRef<str>>(loopdev: S) -> Result<(), Error> {
     }
 
     let backing_file = get_backing_file(loopdev)?;
-    unmap_from_backing(Path::new(&backing_file))
+    unmap_from_backing(Path::new(&backing_file), Some(loopdev))
 }
 
 /// Try and unmap a running proxmox-backup-client instance from the given name
 pub fn unmap_name<S: AsRef<str>>(name: S) -> Result<(), Error> {
-    for (mapping, _) in find_all_mappings()? {
+    for (mapping, loopdev) in find_all_mappings()? {
         if mapping.ends_with(name.as_ref()) {
             let mut path = PathBuf::from(RUN_DIR);
             path.push(&mapping);
-            return unmap_from_backing(&path);
+            return unmap_from_backing(&path, loopdev.as_deref());
         }
     }
     Err(format_err!("no mapping for name '{}' found", name.as_ref()))
-- 
2.20.1





  parent reply	other threads:[~2020-10-07 11:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07 11:53 [pbs-devel] [PATCH 0/7] fuse_loop cleanups and named mappings Stefan Reiter
2020-10-07 11:53 ` [pbs-devel] [PATCH proxmox-backup 1/7] format: fix typo in function name Stefan Reiter
2020-10-07 11:53 ` [pbs-devel] [PATCH proxmox-backup 2/7] fuse_loop: add documentation Stefan Reiter
2020-10-07 11:53 ` [pbs-devel] [PATCH proxmox-backup 3/7] loopdev: add module doc Stefan Reiter
2020-10-07 11:53 ` [pbs-devel] [PATCH proxmox-backup 4/7] mount/map: use names for map/unmap for easier use Stefan Reiter
2020-10-07 11:53 ` [pbs-devel] [PATCH proxmox-backup 5/7] fuse_loop: add automatic cleanup of run files and dangling instances Stefan Reiter
2020-10-07 11:53 ` [pbs-devel] [PATCH proxmox-backup 6/7] fuse_loop: wait for instance to close after killing Stefan Reiter
2020-10-07 11:53 ` Stefan Reiter [this message]
2020-10-08  6:45 ` [pbs-devel] applied: [PATCH 0/7] fuse_loop cleanups and named mappings Dietmar Maurer

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=20201007115308.6275-8-s.reiter@proxmox.com \
    --to=s.reiter@proxmox.com \
    --cc=pbs-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal