Fix: Aus-/Umhaengen behandelt Fehler robuster
- unmount_side() verschluckte einen fehlgeschlagenen build_target() kommentarlos und mountete stattdessen einen leeren Platzhalter aus - z. B. wenn owner_user zwischenzeitlich vom System geloescht wurde. Wird jetzt geloggt, bevor mit denselben Best-effort-Defaults weitergemacht wird. - unmount_pair() brach beim ersten `?` (auch bei einem reinen is_mounted()-Pruepffehler) komplett ab, statt die zweite Seite trotzdem zu versuchen - ein echt gemounteter Cloud-Anteil blieb dann unangetastet, wenn schon die Local-Pruefung fehlschlug. Beide Seiten werden jetzt unabhaengig voneinander versucht; der erste Fehler wird erst nach beiden Versuchen zurueckgegeben. - target::active_side() erkennt "die" aktive Seite ausschliesslich ueber den sichtbaren Symlink - ein Mount auf der jeweils anderen Seite (z. B. nach einem abgebrochenen Umschalten oder externem manuellen Mount) blieb dadurch fuer status/watch unsichtbar und wurde nie automatisch wieder ausgehaengt. cleanup_orphaned_mounts() raeumt einen solchen verwaisten Mount jetzt bei jedem Reconcile-Durchlauf best-effort auf. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+121
-7
@@ -14,7 +14,7 @@
|
||||
|
||||
use crate::config::{AppConfig, DrivePair, GlobalSettings, LocalSide};
|
||||
use crate::db::credentials::{CredentialStore, Side};
|
||||
use crate::error::Result;
|
||||
use crate::error::{Error, Result};
|
||||
use crate::mount::{self, lock, target};
|
||||
use crate::network::{self, address};
|
||||
|
||||
@@ -78,6 +78,7 @@ async fn reconcile_pair_inner(
|
||||
|
||||
let local_reachable = check_local_reachable(&pair.local, settings);
|
||||
let active = target::active_side(pair);
|
||||
cleanup_orphaned_mounts(pair, settings, active).await;
|
||||
|
||||
if local_reachable {
|
||||
if active == Some(Side::Local) {
|
||||
@@ -114,6 +115,55 @@ fn check_local_reachable(local: &LocalSide, settings: &GlobalSettings) -> bool {
|
||||
}
|
||||
}
|
||||
|
||||
/// Räumt einen Mount auf der jeweils NICHT aktiven Seite auf, falls einer besteht.
|
||||
///
|
||||
/// [`target::active_side`] identifiziert "die" aktive Seite ausschließlich über den sichtbaren
|
||||
/// Symlink - ein Mount auf der jeweils anderen Seite (z. B. weil eine vorherige
|
||||
/// `activate_symlink`-Aktivierung mitten im Umschalten abgebrochen wurde, oder weil extern
|
||||
/// manuell gemountet wurde) bliebe dadurch unbemerkt: `status`/`watch` sähen ihn nie, und er
|
||||
/// würde nie automatisch wieder ausgehängt. Best-effort (nur geloggt, nicht propagiert) - ein
|
||||
/// hier fehlschlagendes Aufräumen darf den eigentlichen Reconcile-Schritt nicht blockieren.
|
||||
async fn cleanup_orphaned_mounts(pair: &DrivePair, settings: &GlobalSettings, active: Option<Side>) {
|
||||
for side in [Side::Local, Side::Cloud] {
|
||||
if Some(side) == active {
|
||||
continue;
|
||||
}
|
||||
let dir = target::backing_dir(pair, side);
|
||||
match mount::state::is_mounted(&dir) {
|
||||
Ok(true) => {
|
||||
logger_ctdra::warn(
|
||||
"reconcile",
|
||||
&format!(
|
||||
"pair '{}': found an orphaned mount on the inactive side '{}' (not \
|
||||
reflected by the active symlink) - unmounting it",
|
||||
pair.id,
|
||||
side.as_str()
|
||||
),
|
||||
);
|
||||
if let Err(e) = unmount_side(pair, settings, side).await {
|
||||
logger_ctdra::warn(
|
||||
"reconcile",
|
||||
&format!(
|
||||
"pair '{}': could not clean up orphaned mount on side '{}': {e}",
|
||||
pair.id,
|
||||
side.as_str()
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(false) => {}
|
||||
Err(e) => logger_ctdra::warn(
|
||||
"reconcile",
|
||||
&format!(
|
||||
"pair '{}': could not check mount state for side '{}': {e}",
|
||||
pair.id,
|
||||
side.as_str()
|
||||
),
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Mountet `new_side` zuerst, flippt danach den sichtbaren Symlink, und hängt erst zum
|
||||
/// Schluss `old_active` (falls vorhanden und verschieden) aus. Diese Reihenfolge stellt
|
||||
/// sicher, dass der sichtbare `pair.mount_point` nie auf ein gerade ausgehängtes oder noch
|
||||
@@ -169,15 +219,70 @@ async fn mount_side(
|
||||
pub async fn unmount_pair(pair: &DrivePair, settings: &GlobalSettings) -> Result<Action> {
|
||||
let _guard = lock::acquire(&pair.id).await?;
|
||||
|
||||
let Some(side) = target::active_side(pair) else {
|
||||
return Ok(Action::NoOp);
|
||||
};
|
||||
unmount_side(pair, settings, side).await?;
|
||||
Ok(Action::NoOp)
|
||||
// Beide Seiten werden unabhängig voneinander versucht - ein Fehler (auch ein
|
||||
// `is_mounted`-Prüffehler) bei der ersten Seite darf die zweite nicht ungeprüft
|
||||
// überspringen. Der erste aufgetretene Fehler wird nach beiden Versuchen zurückgegeben,
|
||||
// damit der Aufrufer weiterhin erkennt, dass etwas fehlgeschlagen ist.
|
||||
let mut first_err: Option<Error> = None;
|
||||
for side in [Side::Local, Side::Cloud] {
|
||||
let dir = target::backing_dir(pair, side);
|
||||
match mount::state::is_mounted(&dir) {
|
||||
Ok(true) => {
|
||||
if let Err(e) = unmount_side(pair, settings, side).await
|
||||
&& first_err.is_none()
|
||||
{
|
||||
first_err = Some(e);
|
||||
}
|
||||
}
|
||||
Ok(false) => {}
|
||||
Err(e) => {
|
||||
logger_ctdra::warn(
|
||||
"reconcile",
|
||||
&format!(
|
||||
"pair '{}': could not check mount state for side '{}': {e} - still \
|
||||
attempting the other side",
|
||||
pair.id,
|
||||
side.as_str()
|
||||
),
|
||||
);
|
||||
if first_err.is_none() {
|
||||
first_err = Some(e);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
match first_err {
|
||||
Some(e) => Err(e),
|
||||
None => Ok(Action::NoOp),
|
||||
}
|
||||
}
|
||||
|
||||
async fn unmount_side(pair: &DrivePair, settings: &GlobalSettings, side: Side) -> Result<()> {
|
||||
let mount_target = target::build_target(pair, settings, side)?;
|
||||
let mount_target = target::build_target(pair, settings, side).unwrap_or_else(|e| {
|
||||
logger_ctdra::warn(
|
||||
"reconcile",
|
||||
&format!(
|
||||
"pair '{}': could not fully resolve the mount target for side '{}' while \
|
||||
unmounting ({e}) - unmounting its backing directory anyway with best-effort \
|
||||
defaults",
|
||||
pair.id,
|
||||
side.as_str()
|
||||
),
|
||||
);
|
||||
crate::mount::MountTarget {
|
||||
pair_id: pair.id.clone(),
|
||||
side,
|
||||
source: String::new(),
|
||||
mount_point: target::backing_dir(pair, side),
|
||||
options: vec![],
|
||||
invocation: match pair.context {
|
||||
crate::config::MountContext::System => crate::mount::MountInvocation::Direct,
|
||||
crate::config::MountContext::User => crate::mount::MountInvocation::ViaFstab,
|
||||
},
|
||||
owner_user: pair.owner_user.clone(),
|
||||
}
|
||||
});
|
||||
mount::backend_for(target::side_kind(pair, side)).unmount(&mount_target)
|
||||
}
|
||||
|
||||
@@ -228,4 +333,13 @@ mod tests {
|
||||
assert_ne!(t.mount_point, pair.mount_point);
|
||||
assert_eq!(t.mount_point, target::backing_dir(&pair, Side::Local));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unmount_pair_noop_when_neither_side_mounted() {
|
||||
let pair = sample_pair(MountKind::Smb, MountKind::WebDav);
|
||||
let action = unmount_pair(&pair, &GlobalSettings::default())
|
||||
.await
|
||||
.expect("unmount");
|
||||
assert!(matches!(action, Action::NoOp));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user