misc: vmm: add streamlined logging to every event #7484#7505
misc: vmm: add streamlined logging to every event #7484#7505JuKu wants to merge 1 commit intocloud-hypervisor:mainfrom
Conversation
|
I'm not sure if this is really what we want to do - what's your use case for this? |
There was a problem hiding this comment.
Generally I think it is beneficial if we unify the usage of event! by adding it to every API event and pair it with a matching info!, therefore I created #7484 as you already noticed. Thanks for addressing this!
You missed one aspect of the ticket tho. A few event! calls are missing for some VM events and you didn't add them yet unfortunately. For example, vm_add_fs doesn't emit an event but I think it should. (I think this is just missing so far and was never left out intentionally?).
Further, there is a typo in the commit message. Its vmm not vvm. Please read and accept our contributing guidelines regarding several style and commit checks.
| } | ||
|
|
||
| fn reset(&mut self) -> Option<Arc<dyn VirtioInterrupt>> { | ||
| info!("vdpa reset: id={}", self.id); |
There was a problem hiding this comment.
This is not next to the event! line. It could be that we log here but not emit an event in the following as we fail in the following lines and return early
| pub const HANDLED_SIGNALS: [i32; 2] = [SIGTERM, SIGINT]; | ||
|
|
||
| /// Get VM identifier (UUID if available, otherwise process ID) | ||
| fn vm_id(&self) -> String { |
There was a problem hiding this comment.
I don't think this information in its current form is useful at all.
One Cloud Hypervisor instance can only run exactly one VM. Giving names and UUIDs to VMMs should be up to the management layer (i.e., libvirt). I'm in favor of removing this.
|
|
||
| fn vm_pause(&mut self) -> result::Result<(), VmError> { | ||
| let state = self.vm.as_ref().and_then(|vm| vm.get_state().ok()); | ||
| info!("request to pause VM: {}, state={:?}", self.vm_id(), state); |
There was a problem hiding this comment.
| info!("request to pause VM: {}, state={:?}", self.vm_id(), state); | |
| info!("request to pause VM: old_state={:?}", state); |
No need to print any VM id here. We can only stop the VM, not any VM as there is exactly one.
|
|
||
| fn vm_resume(&mut self) -> result::Result<(), VmError> { | ||
| let state = self.vm.as_ref().and_then(|vm| vm.get_state().ok()); | ||
| info!("request to resume VM: {}, state={:?}", self.vm_id(), state); |
| } | ||
|
|
||
| /// Get VM identifier (UUID if available, otherwise process ID) | ||
| fn vm_id(&self) -> String { |
phip1611
changed the title
|
I think we should close this PR as #7512 has landed now and that has a generalised solution to what I think this PR was trying to achieve? |
yes, sounds good! |
3 participants
Add streamlined logging to every event.
#7484