runtime-rs: ch: Implement minimal implementation for missing thread/pid APIs #8710
Conversation
Add implementations for the following `Hypervisor` trait methods which simply return the same details as the `get_vmm_master_tid()` method: - `get_thread_ids()` - `get_pids()` Fixes: kata-containers#6438. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
| Ok(VcpuThreadIds::default()) | ||
| let mut vcpus = HashMap::new(); | ||
|
|
||
| let vcpu = 0; |
There was a problem hiding this comment.
@jodh-intel is this always 0?
There was a problem hiding this comment.
@cmaf - well, I'm not sure what else we could do here. Thoughts?
There was a problem hiding this comment.
@jodh-intel This api is meant to return all the tids corresponding to all the vcpus that the VM uses.
qemu has an qmp api query-cpus-fast that returns the details about the vcpus: https://www.qemu.org/docs/master/system/cpu-hotplug.html
I am not sure if such an api exists or has been added to clh since the go implementation.
cc @likebreath Can you let us know about this?
Else we may have to follow the same method that we used for the go-implementation i.e walk through the /proc/pid tree:
https://github.com/kata-containers/kata-containers/blob/main/src/runtime/virtcontainers/clh.go#L760
There was a problem hiding this comment.
qemu has an qmp api
query-cpus-fastthat returns the details about the vcpus: https://www.qemu.org/docs/master/system/cpu-hotplug.htmlI am not sure if such an api exists or has been added to clh since the go implementation. cc @likebreath Can you let us know about this?
@amshinde is right. Cloud Hypervisor does not provide such pid information at this time. Of course, please feel free to send feature requests to the CLH community.
One clarification questions: what kind of PID information does kata want from VMM?
| @@ -688,7 +698,9 @@ impl CloudHypervisorInner { | |||
| } | |||
|
|
|||
| pub(crate) async fn get_pids(&self) -> Result<Vec<u32>> { | |||
| Ok(Vec::<u32>::new()) | |||
| let pid = self.get_vmm_master_tid().await?; | |||
There was a problem hiding this comment.
@jodh-intel We should probably return the virtio-fs deamon pid here as well.
There was a problem hiding this comment.
I just checked the golang implementation for the GetPids API.
Its interesting that the qemu implementation returns both hypervisor+virtiofs pids, whereas clh just returns the hypervisor pid.
Point to note is that golang has a separate API for the virtiofs deamon:
https://github.com/kata-containers/kata-containers/blob/main/src/runtime/virtcontainers/hypervisor.go#L1108
This is missing from current rust hypervisor plugin API. It would not apply to dragonball anyway as it does not run a separate virtio-fs deamon.
Also, dragonball returns all the child pids in its implementation:
https://github.com/kata-containers/kata-containers/blob/main/src/runtime-rs/crates/hypervisor/src/dragonball/inner_hypervisor.rs#L110
This should also include virtio-fs thread id.
So in conclusion, I feel that with runtime-rs+clh, we should return hypervisor_pid + virtiofs_pid as well.
FYI, the only place I see the golang GetPids api being called is for metrics:
https://github.com/kata-containers/kata-containers/blob/main/src/runtime/virtcontainers/sandbox_metrics.go#L135
There was a problem hiding this comment.
Hi @amshinde - I'm not sure that's needed since with the golang runtime, the CH driver spawns the virtiofsd but with runtime-rs, the runtime itself does that, not the CH driver.
There was a problem hiding this comment.
Ok, it makes sense to not include virtio-fs then
jodh-intel
changed the title
|
This PR is a minimal implementation. We can't return the full details since CH doesn't provide that information currently. I've raised cloud-hypervisor/cloud-hypervisor#6103 and #8799 for the "phase 2" implementation 😄 |
There was a problem hiding this comment.
@jodh-intel I am approving this, lets open a separate PR to handle multiple VCPUs in a separate PR similar to the golang implementation.
jodh-intel
added
safe-to-test
|
/test |
Add implementations for the following
Hypervisortrait methods which simply return the same details as theget_vmm_master_tid()method:get_thread_ids()get_pids()Fixes: #6438.