geomind_code/my_hypervisor
0
0
Fork 0
Code Issues 7 Pull requests 2 Projects Releases 5 Packages Wiki Activity Actions

Make garbage collection ownership-aware #71

Merged
rawan merged 9 commits from development_owned_gc into development 2026-03-31 09:59:48 +00:00
Conversation 3 Commits 9 Files changed 14 +754 -49
rawan commented 2026-03-26 11:59:04 +00:00
Member
Copy link

#45

#45
update docs
All checks were successful
Unit and Integration Test / test (push) Successful in 2m17s
Details
326317920a
test: integration test
All checks were successful
Unit and Integration Test / test (push) Successful in 1m49s
Details
516771b6a3
fix: remove refresh status on dry_run
All checks were successful
Unit and Integration Test / test (push) Successful in 1m52s
Details
bcb6bd7bee
fix: only clear successfully removed resources from ownership
All checks were successful
Unit and Integration Test / test (push) Successful in 1m51s
Details
101aafc36b
fix(gc): preserve network ownership during GC for precise cleanup
All checks were successful
Unit and Integration Test / test (push) Successful in 1m52s
Details
f3af70c3d0
salmaelsoly commented 2026-03-29 10:23:52 +00:00
Member
Copy link

can u resolve conflicts

can u resolve conflicts
rawan changed title from Make garbage collection ownership-aware to WIP: Make garbage collection ownership-aware 2026-03-29 11:04:04 +00:00
Merge branch 'development' into development_owned_gc
All checks were successful
Unit and Integration Test / test (push) Successful in 2m24s
Details
82b76b5f17
rawan changed title from WIP: Make garbage collection ownership-aware to Make garbage collection ownership-aware 2026-03-30 08:38:22 +00:00
salmaelsoly requested changes 2026-03-30 10:32:02 +00:00
Dismissed
crates/my_hypervisor-cli/src/commands/doctor.rs
@ -140,0 +140,4 @@
gc_cleanup_heuristic().await?;
} else {
gc_cleanup(args.dry_run).await?;
}
salmaelsoly commented 2026-03-30 09:24:29 +00:00
Member
Copy link

are these flags mutually exclusive? because something like that --gc --force-heuristic --dry-run will ignore the dry run, if this is the behaviour needed u can use https://docs.rs/clap/latest/clap/struct.Arg.html#method.conflicts_with
if it is not then we should expect dry run for the heuristic gc as well

are these flags mutually exclusive? because something like that `--gc --force-heuristic --dry-run` will ignore the dry run, if this is the behaviour needed u can use https://docs.rs/clap/latest/clap/struct.Arg.html#method.conflicts_with if it is not then we should expect dry run for the heuristic gc as well
salmaelsoly commented 2026-03-30 10:14:24 +00:00
Member
Copy link

as well as my_hypervisor doctor --force-heuristic
without --gc ignores force heuristic the same case will be for --dry run u can use this https://docs.rs/clap/latest/clap/struct.Arg.html#method.required

as well as `my_hypervisor doctor --force-heuristic` without --gc ignores force heuristic the same case will be for --dry run u can use this https://docs.rs/clap/latest/clap/struct.Arg.html#method.required
rawan marked this conversation as resolved
crates/my_hypervisor-cli/src/commands/doctor.rs Outdated
@ -146,0 +180,4 @@
prefix,
result.actions.len()
);
}
salmaelsoly commented 2026-03-30 09:33:50 +00:00
Member
Copy link

should we print here crashed vms cleaned? if it is not used can we remove it?

should we print here crashed vms cleaned? if it is not used can we remove it?
rawan marked this conversation as resolved
crates/my_hypervisor-lib/src/vm/manager.rs Outdated
@ -1175,0 +1261,4 @@
.cloned()
.collect()
};
salmaelsoly commented 2026-03-30 10:04:43 +00:00
Member
Copy link
let states: Vec<RuntimeState> = self.state_store                                                                           
      .list()                                                                                                                
      .into_iter()                                                                                                           
      .filter(|s| {                                                                                                          
          matches!(s.status, VmState::Stopped | VmState::Failed)                                                             
              || (dry_run && s.status == VmState::Running                                                                    
                  && s.vmm_pid > 0 && !process_running(s.vmm_pid))                                                           
      })                                                          
      .cloned()                                                                                                              
      .collect();

instead of duplicated filtering logic

```rust let states: Vec<RuntimeState> = self.state_store .list() .into_iter() .filter(|s| { matches!(s.status, VmState::Stopped | VmState::Failed) || (dry_run && s.status == VmState::Running && s.vmm_pid > 0 && !process_running(s.vmm_pid)) }) .cloned() .collect(); ``` instead of duplicated filtering logic
rawan marked this conversation as resolved
crates/my_hypervisor-lib/src/vm/manager.rs Outdated
@ -1175,0 +1270,4 @@
if state.vm_config.network.mode != NetworkMode::None
&& !state.vm_config.network.port_forwards.is_empty()
{
warnings.push(GcWarning {
salmaelsoly commented 2026-03-30 10:07:23 +00:00
Member
Copy link

the vm may have tap device configured with no port forwarding configs so in this case there is no warning is it intended behaviour?

the vm may have tap device configured with no port forwarding configs so in this case there is no warning is it intended behaviour?
rawan marked this conversation as resolved
crates/my_hypervisor-lib/src/vm/manager.rs Outdated
@ -1175,0 +1311,4 @@
});
}
Ok(o) => {
let stderr = String::from_utf8_lossy(&o.stderr);
salmaelsoly commented 2026-03-30 10:29:21 +00:00
Member
Copy link

we can have helper function gc_delete_resource() that run the delete command with passing args and check whether it succeeded or not for taps and iptables

we can have helper function gc_delete_resource() that run the delete command with passing args and check whether it succeeded or not for taps and iptables
rawan marked this conversation as resolved
crates/my_hypervisor-lib/src/vm/manager.rs Outdated
@ -1175,0 +1329,4 @@
// iptables rules
for rule_str in &ownership.iptables_rules {
let reason = format!("VM {} ({})", state.name, state.status);
if dry_run {
salmaelsoly commented 2026-03-30 10:23:35 +00:00
Member
Copy link

reason can be formatted once for a vm since it using vm name and status no need to redo with each rule

reason can be formatted once for a vm since it using vm name and status no need to redo with each rule
rawan marked this conversation as resolved
fix: refactor gc_owned_resources, fixed gc cli args relations, added convenient prints and unit tests
All checks were successful
Unit and Integration Test / test (push) Successful in 2m24s
Details
72aee3ded7
rawan merged commit f9d7a43916 into development 2026-03-31 09:59:48 +00:00
rawan deleted branch development_owned_gc 2026-03-31 09:59:48 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
salmaelsoly
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
geomind_code/my_hypervisor!71
No description provided.