Storage IDs in Proxmox VE may contain dots, which makes a simple rename adding a '.old' extension impossible without potential breakage. The storage RRD is grouped by nodes, so to fix it, create a '.old' directory for each node and move migrated RRD files there.
If a previous migration with the logic before this change is detected, the old logic will be kept to avoid picking up '.old' RRD files created by that previous migration. Signed-off-by: Fiona Ebner <f.eb...@proxmox.com> --- src/main.rs | 47 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/src/main.rs b/src/main.rs index fb58d3a..03a84b8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -317,7 +317,10 @@ fn mv_old(file: &str) -> Result<()> { } /// Colllect all RRD files in the provided directory -fn collect_rrd_files(location: &PathBuf) -> Result<Vec<(CString, OsString)>> { +fn collect_rrd_files( + location: &PathBuf, + include_files_with_extension: bool, +) -> Result<Vec<(CString, OsString)>> { let mut files: Vec<(CString, OsString)> = Vec::new(); let contents = match fs::read_dir(location) { @@ -331,7 +334,7 @@ fn collect_rrd_files(location: &PathBuf) -> Result<Vec<(CString, OsString)>> { contents .filter(|f| f.is_ok()) .map(|f| f.unwrap().path()) - .filter(|f| f.is_file() && f.extension().is_none()) + .filter(|f| f.is_file() && (include_files_with_extension || f.extension().is_none())) .for_each(|file| { let path = CString::new(file.as_path().as_os_str().as_bytes()) .expect("Could not convert path to CString."); @@ -416,7 +419,7 @@ fn migrate_guests( println!("Migrating RRD metrics data for virtual guests…"); println!("Using {threads} thread(s)"); - let guest_source_files = collect_rrd_files(&source_dir_guests)?; + let guest_source_files = collect_rrd_files(&source_dir_guests, false)?; if guest_source_files.is_empty() { println!("No guest metrics to migrate"); @@ -518,7 +521,7 @@ fn migrate_nodes( std::fs::create_dir(&target_dir_nodes)?; } - let node_source_files = collect_rrd_files(&source_dir_nodes)?; + let node_source_files = collect_rrd_files(&source_dir_nodes, false)?; let mut no_migration_err = true; for file in node_source_files { @@ -579,17 +582,25 @@ fn migrate_storage( let mut no_migration_err = true; // storage has another layer of directories per node over which we need to iterate + // Storage IDs may contain dots, so the old RRD files are moved to a .old directory per node + // rather than renamed themselves. fs::read_dir(&source_dir_storage)? .filter(|f| f.is_ok()) .map(|f| f.unwrap().path()) - .filter(|f| f.is_dir()) + .filter(|f| f.is_dir() && f.extension().is_none()) .try_for_each(|node| { let mut source_storage_subdir = source_dir_storage.clone(); source_storage_subdir.push(node.file_name().unwrap()); + let source_storage_subdir_old = source_storage_subdir.as_path().with_extension("old"); + let mut target_storage_subdir = target_dir_storage.clone(); target_storage_subdir.push(node.file_name().unwrap()); + // If already migrated using the old rename logic, don't try to migrate with new logic. + let migrated_using_old_rename = + target_storage_subdir.exists() && !source_storage_subdir_old.exists(); + if !target_storage_subdir.exists() && migrate { fs::create_dir(target_storage_subdir.as_path())?; let metadata = target_storage_subdir.metadata()?; @@ -598,7 +609,18 @@ fn migrate_storage( fs::set_permissions(&target_storage_subdir, permissions)?; } - let storage_source_files = collect_rrd_files(&source_storage_subdir)?; + if !source_storage_subdir_old.exists() && migrate && !migrated_using_old_rename { + fs::create_dir(source_storage_subdir_old.as_path())?; + let metadata = source_storage_subdir_old.metadata()?; + let mut permissions = metadata.permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&source_storage_subdir_old, permissions)?; + } + + // Storage IDs may contain dots, so need to consider all extensions. + // If old logic was used already before, don't use new logic. + let storage_source_files = + collect_rrd_files(&source_storage_subdir, !migrated_using_old_rename)?; for file in storage_source_files { println!( "Migrating metrics for storage '{}/{}'", @@ -609,6 +631,7 @@ fn migrate_storage( ); let full_path = file.0.clone().into_string().unwrap(); + let target_path = source_storage_subdir_old.join(file.1.clone()); match do_rrd_migration( file, &target_storage_subdir, @@ -617,7 +640,12 @@ fn migrate_storage( force, ) { Ok(()) => { - mv_old(full_path.as_str())?; + // Keep using old logic if already migrated with old logic. + if migrated_using_old_rename { + mv_old(full_path.as_str())?; + } else { + fs::rename(full_path, target_path)?; + } } Err(err) => { eprintln!("{err}"); // includes information messages, so just print. @@ -625,6 +653,11 @@ fn migrate_storage( } } } + + if source_storage_subdir.read_dir()?.next().is_none() { + fs::remove_dir(source_storage_subdir)?; + } + Ok::<(), Error>(()) })?; -- 2.47.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel