From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 485DB1FF0E4 for ; Tue, 28 Jul 2026 15:59:52 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id DDCD221499; Tue, 28 Jul 2026 15:59:49 +0200 (CEST) Message-ID: Date: Tue, 28 Jul 2026 15:59:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Christian Ebner Subject: Re: [PATCH proxmox-backup 2/3] report: use shared proxmox-system-report crate To: Erik Fastermann , pbs-devel@lists.proxmox.com, pdm-devel@lists.proxmox.com References: <20260714083156.99794-1-e.fastermann@proxmox.com> <20260714083156.99794-3-e.fastermann@proxmox.com> Content-Language: en-US, de-DE In-Reply-To: <20260714083156.99794-3-e.fastermann@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1785247148633 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.156 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: IB72GVAB3RWJJ3DCDKW3RUO57QIEGZE4 X-Message-ID-Hash: IB72GVAB3RWJJ3DCDKW3RUO57QIEGZE4 X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Lukas Wagner X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 7/14/26 10:31 AM, Erik Fastermann wrote: > Move the common report logic to the proxmox-system-report crate and > keep only the PBS specific files, commands and the Datastores > function here. > > User-visible output changes: > - df now also shows the filesystem type (df -h -T), matching PDM > - adds the IPv4 and IPv6 route tables (ip -4/-6 route show), > previously PDM only > > The section order (FILES, COMMANDS, FUNCTIONS) and all product > specific entries are unchanged. > > Suggested-by: Lukas Wagner > Suggested-by: Christian Ebner > Signed-off-by: Erik Fastermann > --- > Cargo.toml | 2 + > src/server/report.rs | 213 +++---------------------------------------- > 2 files changed, 15 insertions(+), 200 deletions(-) > > diff --git a/Cargo.toml b/Cargo.toml > index a625370cf..fb7904d38 100644 > --- a/Cargo.toml > +++ b/Cargo.toml > @@ -94,6 +94,7 @@ proxmox-sortable-macro = "1" > proxmox-subscription = { version = "1.0.2", features = [ "api-types" ] } > proxmox-syslog-api = { version = "1.1", features = [ "impl" ] } > proxmox-sys = "1" > +proxmox-system-report = "0.1" nit: as motivated in patch 1, lets start with 1.0 here.... also, this requires a build dependency in debian/control on the corresponding dev package, so it is pulled in by apt. > proxmox-systemd = "1.0.1" > proxmox-tfa = { version = "6.0.3", features = [ "api", "api-types" ] } > proxmox-time = "2" > @@ -252,6 +253,7 @@ proxmox-sortable-macro.workspace = true > proxmox-subscription.workspace = true > proxmox-syslog-api.workspace = true > proxmox-sys = { workspace = true, features = [ "timer" ] } > +proxmox-system-report.workspace = true > proxmox-systemd.workspace = true > proxmox-tfa.workspace = true > proxmox-time.workspace = true > diff --git a/src/server/report.rs b/src/server/report.rs > index 297718d2e..442a8573d 100644 > --- a/src/server/report.rs > +++ b/src/server/report.rs > @@ -1,33 +1,7 @@ > -use std::fmt::Write; > -use std::path::Path; > -use std::process::Command; > +use proxmox_system_report::{CommandSpec, FileGroup, FunctionMapping}; > > -use proxmox_network_api::NetworkInterfaceType; > - > -fn get_top_processes() -> String { > - let (exe, args) = ("top", vec!["-b", "-c", "-w512", "-n", "1", "-o", "TIME"]); > - let output = Command::new(exe).args(&args).output(); > - let output = match output { > - Ok(output) => String::from_utf8_lossy(&output.stdout).to_string(), > - Err(err) => err.to_string(), > - }; > - let output = output.lines().take(30).collect::>().join("\n"); > - format!("$ `{exe} {}`\n```\n{output}\n```", args.join(" ")) > -} > - > -fn files() -> Vec<(&'static str, Vec<&'static str>)> { > +fn files() -> Vec { nit: maybe better call this pbs_specific_files() or product_specific_files(), just to make it clearer that this is only part of what's to be generated. > vec![ > - ( > - "General System Info", > - vec![ > - "/etc/hostname", > - "/etc/hosts", > - "/etc/network/interfaces", > - "/etc/apt/sources.list", > - "/etc/apt/sources.list.d/", > - "/proc/pressure/", > - ], > - ), > ( > "Datastores & Remotes", > vec!["/etc/proxmox-backup/datastore.cfg"], > @@ -65,192 +39,31 @@ fn files() -> Vec<(&'static str, Vec<&'static str>)> { > ] > } > > -fn commands() -> Vec<(&'static str, Vec<&'static str>)> { > +fn commands() -> Vec { nit: same as above for files() > vec![ > - // ("", vec![]) > ("date", vec!["-R"]), > ("proxmox-backup-manager", vec!["versions", "--verbose"]), > ("proxmox-backup-manager", vec!["subscription", "get"]), > ("proxmox-backup-manager", vec!["ldap", "list"]), > ("proxmox-backup-manager", vec!["openid", "list"]), > - ("proxmox-boot-tool", vec!["status"]), > - ("df", vec!["-h"]), > - ( > - "lsblk", > - vec![ > - "--ascii", > - "-M", > - "-o", > - "+HOTPLUG,ROTA,PHY-SEC,FSTYPE,MODEL,TRAN", > - ], > - ), > - ("bash", vec!["-c", "ls -l /dev/disk/by-*/"]), > - ("zpool", vec!["status"]), > - ("zfs", vec!["list"]), > - ("zarcstat", vec![]), > - ("dmidecode", vec!["-t", "bios"]), > - ("lscpu", vec![]), > - ("lspci", vec!["-nnk"]), > - ("ip", vec!["-details", "-statistics", "a"]), > ] > } > > -fn dynamic_commands() -> Vec<(&'static str, Vec)> { > - let mut commands = Vec::new(); > - > - match proxmox_network_api::config() { > - Ok((config, _)) => { > - for (name, iface) in config.interfaces { > - if iface.interface_type == NetworkInterfaceType::Eth { > - commands.push(("ethtool", vec![name])); > - } > - } > - } > - Err(err) => { > - eprintln!("failed to query network interfaces: {err}"); > - } > - } > - > - commands > -} > - > -// (description, function()) > -type FunctionMapping = (&'static str, fn() -> String); > - > fn function_calls() -> Vec { nit: same as above for files() and commands()... > - vec![ > - ("Datastores", || { > - let config = match pbs_config::datastore::config() { > - Ok((config, _digest)) => config, > - _ => return String::from("could not read datastore config"), > - }; > - > - let mut list = Vec::new(); > - for store in config.sections.keys() { > - list.push(store.as_str()); > - } > - format!("```\n{}\n```", list.join(", ")) > - }), > - ("System Load & Uptime", get_top_processes), > - ] > -} > - > -fn get_file_content(file: impl AsRef) -> String { > - use proxmox_sys::fs::file_read_optional_string; > - let content = match file_read_optional_string(&file) { > - Ok(Some(content)) => content, > - Ok(None) => String::from("# file does not exist"), > - Err(err) => err.to_string(), > - }; > - let file_name = file.as_ref().display(); > - format!("`$ cat '{file_name}'`\n```\n{}\n```", content.trim_end()) > -} > - > -fn get_directory_content(path: impl AsRef) -> String { > - let read_dir_iter = match std::fs::read_dir(&path) { > - Ok(iter) => iter, > - Err(err) => { > - return format!( > - "`$ cat '{}*'`\n```\n# read dir failed - {err}\n```", > - path.as_ref().display(), > - ); > - } > - }; > - let mut out = String::new(); > - let mut first = true; > - for entry in read_dir_iter { > - let entry = match entry { > - Ok(entry) => entry, > - Err(err) => { > - let _ = writeln!(out, "error during read-dir - {err}"); > - continue; > - } > + vec![("Datastores", || { > + let config = match pbs_config::datastore::config() { > + Ok((config, _digest)) => config, > + _ => return String::from("could not read datastore config"), > }; > - let path = entry.path(); > - if path.is_file() { > - if first { > - let _ = writeln!(out, "{}", get_file_content(path)); > - first = false; > - } else { > - let _ = writeln!(out, "\n{}", get_file_content(path)); > - } > - } else { > - let _ = writeln!(out, "skipping sub-directory `{}`", path.display()); > - } > - } > - out > -} > > -fn get_command_output(exe: &str, args: &Vec<&str>) -> String { > - let output = Command::new(exe) > - .env("PROXMOX_OUTPUT_NO_BORDER", "1") > - .args(args) > - .output(); > - let output = match output { > - Ok(output) => { > - let mut out = String::from_utf8_lossy(&output.stdout) > - .trim_end() > - .to_string(); > - let stderr = String::from_utf8_lossy(&output.stderr) > - .trim_end() > - .to_string(); > - if !stderr.is_empty() { > - let _ = writeln!(out, "\n```\nSTDERR:\n```\n{stderr}"); > - } > - out > + let mut list = Vec::new(); > + for store in config.sections.keys() { > + list.push(store.as_str()); > } > - Err(err) => err.to_string(), > - }; > - format!("$ `{exe} {}`\n```\n{output}\n```", args.join(" ")) > + format!("```\n{}\n```", list.join(", ")) > + })] > } > > pub fn generate_report() -> String { > - let file_contents = files() > - .iter() > - .map(|group| { > - let (group, files) = group; > - let group_content = files > - .iter() > - .map(|file_name| { > - let path = Path::new(file_name); > - if path.is_dir() { > - get_directory_content(path) > - } else { > - get_file_content(file_name) > - } > - }) > - .collect::>() > - .join("\n\n"); > - > - format!("### {group}\n\n{group_content}") > - }) > - .collect::>() > - .join("\n\n"); > - > - let static_command_outputs = commands() > - .into_iter() > - .map(|(command, args)| get_command_output(command, &args)); > - > - let dynamic_command_outputs = dynamic_commands().into_iter().map(|(command, args)| { > - let args = args.iter().map(String::as_str).collect(); > - get_command_output(command, &args) > - }); > - > - let command_outputs = static_command_outputs > - .chain(dynamic_command_outputs) > - .collect::>() > - .join("\n\n"); > - > - let function_outputs = function_calls() > - .iter() > - .map(|(desc, function)| { > - let output = function(); > - format!("#### {desc}\n{}\n", output.trim_end()) > - }) > - .collect::>() > - .join("\n\n"); > - > - format!( > - "## FILES\n\n{file_contents}\n## COMMANDS\n\n{command_outputs}\n## FUNCTIONS\n\n{function_outputs}\n" > - ) > + proxmox_system_report::generate_report(files(), commands(), function_calls()) > }