From 64aa56235557f5e5cc8094e9ff561138e8b27080 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 11 Aug 2021 15:20:25 +1000 Subject: [PATCH] agent: Correct mount point creation mount_storage() first makes sure the mount point for the storage volume exists. It uses fs::create_dir_all() in the case of 9p or virtiofs volumes otherwise ensure_destination_exists(). But.. ensure_destination_exists() boils down to an fs::create_dir_all() in most cases anyway. The only case it doesn't is for a bind fstype, where it creates a file instead of a directory. But, that's not correct anyway because we need to create either a file or a directory depending on the source of the bind mount, which ensure_destination_exists() doesn't know. The 9p/virtiofs paths also check if the mountpoint exists before calling fs::create_dir_all(), which is unnecessary (fs::create_dir_all already handles that case). mount_storage() does have the information to know what we need to create, so have it explicitly call ensure_destination_file_exists() for the bind mount to a non-directory case, and fs::create_dir_all() in all other cases. fixes #2390 Signed-off-by: David Gibson --- src/agent/src/mount.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/agent/src/mount.rs b/src/agent/src/mount.rs index 3bcc46a819..ba2cf5f6d2 100644 --- a/src/agent/src/mount.rs +++ b/src/agent/src/mount.rs @@ -439,17 +439,14 @@ fn mount_storage(logger: &Logger, storage: &Storage) -> Result<()> { return Ok(()); } - match storage.fstype.as_str() { - DRIVER_9P_TYPE | DRIVER_VIRTIOFS_TYPE => { - let dest_path = Path::new(storage.mount_point.as_str()); - if !dest_path.exists() { - fs::create_dir_all(dest_path).context("Create mount destination failed")?; - } - } - _ => { - ensure_destination_exists(storage.mount_point.as_str(), storage.fstype.as_str())?; - } + let mount_path = Path::new(&storage.mount_point); + let src_path = Path::new(&storage.source); + if storage.fstype == "bind" && !src_path.is_dir() { + ensure_destination_file_exists(mount_path) + } else { + fs::create_dir_all(mount_path).map_err(anyhow::Error::from) } + .context("Could not create mountpoint")?; let options_vec = storage.options.to_vec(); let options_vec = options_vec.iter().map(String::as_str).collect();