Add -kvm and -vcpu options to show information about VMs and VCPUs #130
Conversation
…VCPUs of each VM. Signed-off-by: Siddhi Katage <[email protected]>
oracle-contributor-agreement
bot
added
the
OCA Verified
| def get_vm_list(prog: Program) -> Iterable[Object]: | ||
| """ | ||
| Returns list of Object of type ``struct kvm * | ||
| """ | ||
| vm_list = [] | ||
| try: | ||
| vm_list = list( | ||
| list_for_each_entry( | ||
| "struct kvm", prog["vm_list"].address_of_(), "vm_list" | ||
| ) | ||
| ) | ||
| except KeyError: | ||
| print("No VMs running") | ||
|
|
||
| return vm_list |
There was a problem hiding this comment.
How about naming this for_each_vm() and having it return Iterator[Object].
| def get_vm_list(prog: Program) -> Iterable[Object]: | |
| """ | |
| Returns list of Object of type ``struct kvm * | |
| """ | |
| vm_list = [] | |
| try: | |
| vm_list = list( | |
| list_for_each_entry( | |
| "struct kvm", prog["vm_list"].address_of_(), "vm_list" | |
| ) | |
| ) | |
| except KeyError: | |
| print("No VMs running") | |
| return vm_list | |
| def for_each_vm(prog: Program) -> Iterable[Object]: | |
| """ | |
| Iterates over all ``struct kvm * | |
| """ | |
| try: | |
| yield from list_for_each_entry( | |
| "struct kvm", prog["vm_list"].address_of_(), "vm_list" | |
| ) | |
| except KeyError: | |
| print("No VMs running") |
| KVM_VCPU_STATE = { | ||
| 0: "RUNNABLE", | ||
| 1: "UNINITIALIZED", | ||
| 2: "INIT_RECEIVED", | ||
| 3: "HALTED", | ||
| 4: "SIPI_RECEIVED", | ||
| 5: "STOPPED", | ||
| 6: "CHECK_STOP", | ||
| 7: "OPERATING", | ||
| 8: "LOAD", | ||
| } |
There was a problem hiding this comment.
I see that these are defined in the code as #define. Since they're userspace API, they should not change, so it's totally fine to hardcode them. However, could you make this an enum? Also, maybe to help readers understand, it would be good to mention that in a comment that these have the prefix KVM_MP_STATE_ in the Linux code?
| KVM_VCPU_STATE = { | |
| 0: "RUNNABLE", | |
| 1: "UNINITIALIZED", | |
| 2: "INIT_RECEIVED", | |
| 3: "HALTED", | |
| 4: "SIPI_RECEIVED", | |
| 5: "STOPPED", | |
| 6: "CHECK_STOP", | |
| 7: "OPERATING", | |
| 8: "LOAD", | |
| } | |
| enum KvmVcpuState: | |
| """Defined in include/uapi/linux/kvm.h, with prefix KVM_MP_STATE_""" | |
| RUNNABLE = 0 | |
| UNINITIALIZED = 1 | |
| INIT_RECEIVED = 2 | |
| HALTED = 3 | |
| SIPI_RECEIVED = 4 | |
| STOPPED = 5 | |
| CHECK_STOP = 6 | |
| OPERATING = 7 | |
| LOAD = 8 |
When you need to look up the name by from an integer value, you can do KvmVcpuState(value).name, rather than KVM_VCPU_STATE[value].
| @@ -93,6 +286,32 @@ class VirtUtil(CorelensModule): | |||
|
|
|||
| name = "virt" | |||
|
|
|||
| def add_args(self, parser: argparse.ArgumentParser) -> None: | |||
| parser.add_argument( | |||
| "-kvm", | |||
There was a problem hiding this comment.
To make everything consistent in corelens, please use two hyphens to introduce a long option (e.g. --kvm, --vcpu. But -p can stay the same.)
| if args.show_platform: | ||
| show_platform(prog) | ||
| elif args.list_vm: | ||
| print_vm_list(prog) | ||
| elif args.vcpu_list: | ||
| print_vcpu_list(prog) |
There was a problem hiding this comment.
Is there any reason not to just print all of these by default?
I wouldn't expect more than a few hundred vCPU or VM even on a large machine, so I don't think there's a huge performance cost to enabling everything by default. Plus, that means default corelens reports will have the information automatically. (If we make them opt-in via CLI flags, then we need to specify defaults for corelens to use: see the NVME module).
| if args.show_platform: | |
| show_platform(prog) | |
| elif args.list_vm: | |
| print_vm_list(prog) | |
| elif args.vcpu_list: | |
| print_vcpu_list(prog) | |
| show_platform(prog) | |
| print_vm_list(prog) | |
| print_vcpu_list(prog) |
| for vm in vm_list: | ||
| if has_member(vm, "vcpus"): | ||
| struct_vcpu = vm.vcpus | ||
| for i in range(len(struct_vcpu)): | ||
| if hex(struct_vcpu[i].value_()) == "0x0": | ||
| break | ||
| vcpu = hex(struct_vcpu[i].value_()) | ||
| id = struct_vcpu[i].vcpu_id.value_() | ||
| if has_member(struct_vcpu[i], "vcpu_idx"): | ||
| idx = struct_vcpu[i].vcpu_idx.value_() | ||
| else: | ||
| idx = i | ||
| arch = hex(struct_vcpu[i].arch.address_of_()) | ||
| stat = hex(struct_vcpu[i].stat.address_of_()) | ||
| if has_member(struct_vcpu[i], "stats_id"): | ||
| stat_id = struct_vcpu[i].stats_id.string_().decode("utf-8") | ||
| else: | ||
| # The UEK5 does not have kvm_vcpu.stats_id structure member. Print None instead. | ||
| stat_id = None | ||
| state = struct_vcpu[i].arch.mp_state.value_() | ||
| cpu = struct_vcpu[i].cpu.value_() | ||
| if has_member(struct_vcpu[i], "wait"): | ||
| task = hex(struct_vcpu[i].wait.task.value_()) | ||
| else: | ||
| # The UEK5 does not have kvm_vcpu.wait structure member. Print None instead. | ||
| task = None | ||
| rows.append( | ||
| [ | ||
| hex(vm.value_()), | ||
| vcpu, | ||
| id, | ||
| idx, | ||
| arch, | ||
| stat, | ||
| stat_id, | ||
| KVM_VCPU_STATE[state], | ||
| cpu, | ||
| str(task), | ||
| ] | ||
| ) | ||
| # For UEK-NEXT: | ||
| else: | ||
| for _, entry in xa_for_each(vm.vcpu_array.address_of_()): | ||
| struct_vcpu = cast("struct kvm_vcpu *", entry) | ||
| if hex(struct_vcpu.value_()) == "0x0": | ||
| break | ||
| vcpu = hex(struct_vcpu.value_()) | ||
| id = struct_vcpu.vcpu_id.value_() | ||
| idx = struct_vcpu.vcpu_idx.value_() | ||
| arch = hex(struct_vcpu.arch.address_of_()) | ||
| stat = hex(struct_vcpu.stat.address_of_()) | ||
| stat_id = struct_vcpu.stats_id.string_().decode("utf-8") | ||
| state = struct_vcpu.arch.mp_state.value_() | ||
| cpu = struct_vcpu.cpu.value_() | ||
| task = hex(struct_vcpu.wait.task.value_()) | ||
|
|
||
| rows.append( | ||
| [ | ||
| hex(vm.value_()), | ||
| vcpu, | ||
| id, | ||
| idx, | ||
| arch, | ||
| stat, | ||
| stat_id, | ||
| KVM_VCPU_STATE[state], | ||
| cpu, | ||
| str(task), | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Rather than having two distinct loops with a lot of duplicate code, we could clean this up. Let's define a for_each_vcpu() function like below:
def for_each_vcpu(vm: Object) -> Iterator[Object]:
if has_member(vm, "vcpus"):
vcpu_iterator = iter(vm.vcpus)
else:
vcpu_iterator = (
cast("struct kvm_vcpu *", e)
for _, e in xa_for_each(vm.vcpu_array.address_of_())
)
for struct_vcpu in vcpu_iterator:
if struct_vcpu.value_() == 0:
break
yield struct_vcpuThen you can use that helper here to consolidate, so that you don't repeat the printing code. Plus we get a nice helper for others to use.
|
This is looking good, thank you for this, and sorry for the delay in review. Would you mind including |
|
Siddhi, virtutil is used to detected hypervisor type like virt-what. I would like dedicated module for kvm to support for this. |
No description provided.