-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[vmm]: support sandbox cgroup management in the host #52
Conversation
8b00a9a
to
a2aa43a
Compare
d5f61f4
to
06de8dc
Compare
14704f1
to
5f89b25
Compare
bfa8f96
to
9b9f8f4
Compare
vmm/sandbox/src/sandbox.rs
Outdated
@@ -168,6 +172,24 @@ where | |||
return Err(Error::AlreadyExist("sandbox".to_string())); | |||
} | |||
|
|||
let mut sandbox_cgroups = SandboxCgroup::default(); | |||
let sandbox_cgroup_path = get_sandbox_cgroup_parent_path(&s.sandbox).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can not assume that all sandbox has parent cgroup path defined. I think we can not unwrap here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix it!
vmm/sandbox/src/vm.rs
Outdated
@@ -70,6 +70,8 @@ pub trait VM: Serialize + Sync + Send { | |||
async fn ping(&self) -> Result<()>; | |||
fn socket_address(&self) -> String; | |||
async fn wait_channel(&self) -> Option<Receiver<(u32, i128)>>; | |||
async fn get_vcpu_threads(&self) -> Result<VcpuThreads>; | |||
fn get_virtiofsd_pid(&self) -> Option<u32>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is too implementation specific, maybe we can add a method of name "pids()" and return a Vec of all pids related to this virtual machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I create a new struct Pids
to encapsulate the vmm related processes pids, now it only contains the vmm_pid
& virtiofsd_pid
filed. Currently, it only includes the vmm_pid
and virtiofsd_pid
fields.
In the future, if we need to add more processes pid info, we can simply add new fields to the Struct Pids
while maintaining consistency with the pids()
trait interface.
vmm/sandbox/src/vm.rs
Outdated
@@ -70,6 +70,8 @@ pub trait VM: Serialize + Sync + Send { | |||
async fn ping(&self) -> Result<()>; | |||
fn socket_address(&self) -> String; | |||
async fn wait_channel(&self) -> Option<Receiver<(u32, i128)>>; | |||
async fn get_vcpu_threads(&self) -> Result<VcpuThreads>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall wen change the name of this method to "vcpus()" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, vcpus
func name looks more clearly.
36fe09d
to
f33b0a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fix: kuasar-io#51 reason: 1. limit the vm process resource usage in the specified cgroup dir 2. support pod_overhead conception actually, which limits the non-vcpu type threads of vm process into pod_overhead cgroup dir and set the pod_overhead resources limit in this cgroup dir. Signed-off-by: flyflypeng <flyflyflypeng@gmail.com>
fix: #51
reason: