mirror of
https://github.com/LibreQoE/LibreQoS.git
synced 2026-10-25 14:20:05 -05:00
Fix topology duplicate and stale locks
This commit is contained in:
1 parent
341d2fa39a
commit
5726826735
3 files changed
+50
-11
No files matched your search
@@ -673,9 +673,10 @@ fn build_shaping_inputs(
|
||||
}
|
||||
let conflicts = duplicate_circuit_shape_conflicts(circuit, device);
|
||||
if !conflicts.is_empty() {
|
||||
errors.push(format!(
|
||||
"Circuit '{}' had conflicting circuit-level fields across ShapedDevices.csv rows while building shaping_inputs.json: {}.",
|
||||
warnings.push(format!(
|
||||
"Circuit '{}' had conflicting circuit-level fields across ShapedDevices.csv rows; using the first row's values and ignoring the duplicate row's circuit-level values for device '{}': {}.",
|
||||
device.circuit_id,
|
||||
device.device_id,
|
||||
conflicts.join(", ")
|
||||
));
|
||||
}
|
||||
|
||||
@@ -228,7 +228,7 @@
|
||||
|
||||
|
||||
#[test]
|
||||
fn shaping_inputs_reject_duplicate_circuit_shape_conflicts() {
|
||||
fn shaping_inputs_warn_and_keep_first_duplicate_circuit_shape() {
|
||||
let lqos_directory = unique_temp_dir("lqos-topology-duplicate-circuit-shape");
|
||||
let config = Config {
|
||||
lqos_directory: lqos_directory.to_string_lossy().to_string(),
|
||||
@@ -269,17 +269,28 @@
|
||||
})),
|
||||
};
|
||||
|
||||
let err = build_shaping_inputs(&config, &artifacts)
|
||||
.expect_err("duplicate circuit conflicts should fail shaping inputs");
|
||||
let message = err.to_string();
|
||||
let shaping_inputs = build_shaping_inputs(&config, &artifacts)
|
||||
.expect("duplicate circuit conflicts should keep the first row")
|
||||
.expect("ShapedDevices.csv should produce shaping inputs");
|
||||
let message = shaping_inputs.warnings.join("\n");
|
||||
|
||||
assert!(message.contains("conflicting circuit-level fields"));
|
||||
assert!(message.contains("ignoring the duplicate row's circuit-level values"));
|
||||
assert!(message.contains("Download Min Mbps"));
|
||||
assert!(message.contains("Upload Min Mbps"));
|
||||
assert!(message.contains("Download Max Mbps"));
|
||||
assert!(message.contains("Upload Max Mbps"));
|
||||
assert!(message.contains("Comment"));
|
||||
assert!(message.contains("sqm"));
|
||||
assert_eq!(shaping_inputs.circuits.len(), 1);
|
||||
let circuit = &shaping_inputs.circuits[0];
|
||||
assert_eq!(circuit.download_min_mbps, 10.0);
|
||||
assert_eq!(circuit.upload_min_mbps, 10.0);
|
||||
assert_eq!(circuit.download_max_mbps, 100.0);
|
||||
assert_eq!(circuit.upload_max_mbps, 100.0);
|
||||
assert_eq!(circuit.comment, "first");
|
||||
assert_eq!(circuit.sqm_override.as_deref(), Some("cake"));
|
||||
assert_eq!(circuit.devices.len(), 2);
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -178,7 +178,7 @@ struct LockMetadata {
|
||||
impl LockMetadata {
|
||||
fn current(operation: &str) -> Self {
|
||||
Self {
|
||||
pid: unsafe { getpid() },
|
||||
pid: std::process::id() as i32,
|
||||
process: current_process_name(),
|
||||
operation: Some(sanitize_lock_field(operation)),
|
||||
created_unix: SystemTime::now()
|
||||
@@ -411,6 +411,17 @@ impl ProcessFileLock {
|
||||
if actual_start_ticks != expected_start_ticks {
|
||||
return false;
|
||||
}
|
||||
} else if let Some(recorded_process) = metadata
|
||||
.process
|
||||
.as_deref()
|
||||
.filter(|process| *process != "unknown")
|
||||
{
|
||||
let Some(current_process) = process_comm_for_pid(metadata.pid) else {
|
||||
return true;
|
||||
};
|
||||
if current_process != recorded_process {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
let Some(needle) = process_name_contains else {
|
||||
@@ -617,7 +628,8 @@ fn sanitize_lock_field(value: &str) -> String {
|
||||
mod tests {
|
||||
use super::{
|
||||
LockMetadata, ProcessFileLock, ProcessLockConfig, ProcessLockError,
|
||||
process_argv0_from_cmdline_bytes, process_name_matches, process_start_ticks_for_pid,
|
||||
process_argv0_from_cmdline_bytes, process_comm_for_pid, process_name_matches,
|
||||
process_start_ticks_for_pid,
|
||||
};
|
||||
use std::{
|
||||
fs::{create_dir_all, read_to_string, remove_dir_all, write},
|
||||
@@ -763,6 +775,19 @@ mod tests {
|
||||
assert!(!ProcessFileLock::is_lock_valid(&metadata, None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_metadata_rejects_a_reused_pid_with_a_different_process_name() {
|
||||
let metadata = LockMetadata {
|
||||
pid: std::process::id() as i32,
|
||||
process: Some("previous-lock-holder".to_string()),
|
||||
operation: Some("save operator overrides".to_string()),
|
||||
created_unix: Some(1_800_000_000),
|
||||
process_start_ticks: None,
|
||||
};
|
||||
|
||||
assert!(!ProcessFileLock::is_lock_valid(&metadata, None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn metadata_without_start_ticks_round_trips() {
|
||||
let metadata = LockMetadata {
|
||||
@@ -819,11 +844,13 @@ mod tests {
|
||||
let dir = unique_test_dir();
|
||||
create_dir_all(&dir).expect("failed to create temp test dir");
|
||||
let path = dir.join("test.lock");
|
||||
let process = process_comm_for_pid(std::process::id() as i32)
|
||||
.expect("current process should expose its command name");
|
||||
write(
|
||||
&path,
|
||||
format!(
|
||||
"pid={}\nprocess=test-holder\noperation=save overrides\ncreated_unix=1800000000\n",
|
||||
std::process::id()
|
||||
"pid={}\nprocess={process}\noperation=save overrides\ncreated_unix=1800000000\n",
|
||||
std::process::id(),
|
||||
),
|
||||
)
|
||||
.expect("failed to write temp lock");
|
||||
@@ -836,7 +863,7 @@ mod tests {
|
||||
|
||||
assert_eq!(code, "TEST_LOCKED");
|
||||
assert!(message.contains("pid="));
|
||||
assert!(message.contains("process=test-holder"));
|
||||
assert!(message.contains(&format!("process={process}")));
|
||||
assert!(message.contains("operation=save overrides"));
|
||||
assert!(message.contains("created_unix=1800000000"));
|
||||
|
||||
|
||||
Reference in new issue
Block a user