From ffa5a93c7588f6e13f07cf22dff50ef65d4b16d0 Mon Sep 17 00:00:00 2001
From: tiann <twsxtd@gmail.com>
Date: Sun, 26 Feb 2023 11:44:08 +0800
Subject: [PATCH] ksud: fix mount and remount order

---
 userspace/ksud/src/event.rs | 16 +++++++++-------
 userspace/ksud/src/mount.rs | 27 ++++++++++++++++++---------
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/userspace/ksud/src/event.rs b/userspace/ksud/src/event.rs
index a9129854..dcc63fc4 100644
--- a/userspace/ksud/src/event.rs
+++ b/userspace/ksud/src/event.rs
@@ -25,8 +25,8 @@ fn mount_partition(partition: &str, lowerdir: &mut Vec<String>) -> Result<()> {
     let result = stock_mount.umount();
     if result.is_err() {
         let remount_result = stock_mount.remount();
-        if remount_result.is_err() {
-            log::error!("remount stock mount of failed: {:?}", remount_result);
+        if let Err(e) = remount_result {
+            log::error!("remount stock failed: {:?}", e);
         }
         bail!("umount stock mount of failed: {:?}", result);
     }
@@ -40,11 +40,13 @@ fn mount_partition(partition: &str, lowerdir: &mut Vec<String>) -> Result<()> {
 
     let result = mount::mount_overlay(&lowerdir, &lowest_dir);
 
-    if result.is_ok() && stock_mount.remount().is_err() {
-        // if mount overlay ok but stock remount failed, we should umount overlay
-        warn!("remount stock mount of failed, umount overlay {lowest_dir} now");
-        if mount::umount_dir(&lowest_dir).is_err() {
-            warn!("umount overlay {lowest_dir} failed");
+    if let Err(e) = stock_mount.remount() {
+        if result.is_ok() {
+            // if mount overlay ok but stock remount failed, we should umount overlay
+            warn!("remount stock failed: {:?}, umount overlay {lowest_dir}", e);
+            if mount::umount_dir(&lowest_dir).is_err() {
+                warn!("umount overlay {lowest_dir} failed");
+            }
         }
     }
 
diff --git a/userspace/ksud/src/mount.rs b/userspace/ksud/src/mount.rs
index 2ef13bf2..b9bd3549 100644
--- a/userspace/ksud/src/mount.rs
+++ b/userspace/ksud/src/mount.rs
@@ -275,16 +275,18 @@ impl StockMount {
     }
 
     fn get_target_mounts(&self) -> Vec<&proc_mounts::MountInfo> {
-        let mounts = self
+        let mut mounts = self
             .mountlist
             .destination_starts_with(std::path::Path::new(&self.mnt))
-            .filter(|m| m.fstype != "overlay" && m.fstype != "rootfs");
-        mounts.collect()
+            .filter(|m| m.fstype != "overlay" && m.fstype != "rootfs")
+            .collect::<Vec<_>>();
+        mounts.sort_by(|a, b| b.dest.cmp(&a.dest)); // inverse order
+        mounts
     }
 
     pub fn umount(&self) -> Result<()> {
         let mounts = self.get_target_mounts();
-        log::info!("stock mount for {} : {:?}", self.mnt, mounts);
+        log::info!("umount stock for {} : {:?}", self.mnt, mounts);
         for m in mounts {
             let dst = m
                 .dest
@@ -293,11 +295,14 @@ impl StockMount {
             umount_dir(dst)?;
             log::info!("umount: {:?}", m);
         }
+        log::info!("umount stock succeed!");
         Ok(())
     }
 
     pub fn remount(&self) -> Result<()> {
-        let mounts = self.get_target_mounts();
+        let mut mounts = self.get_target_mounts();
+        mounts.reverse(); // remount it in order
+        log::info!("remount stock for {} : {:?}", self.mnt, mounts);
         for m in mounts {
             let src = std::fs::canonicalize(&m.source)?;
 
@@ -310,16 +315,20 @@ impl StockMount {
             let fstype = m.fstype.as_str();
             let options = m.options.join(",");
 
-            log::info!("mount: {:?}", m);
-            std::process::Command::new("mount")
+            log::info!("begin remount: {src} -> {dst}");
+            let result = std::process::Command::new("mount")
                 .arg("-t")
                 .arg(fstype)
                 .arg("-o")
                 .arg(options)
                 .arg(src)
                 .arg(dst)
-                .status()
-                .with_context(|| format!("Failed to mount {:?}", m))?;
+                .status();
+            if let Err(e) = result {
+                log::error!("remount failed: {}", e);
+            } else {
+                log::info!("remount {src} -> {dst} succeed!");
+            }
         }
         Ok(())
     }