Re: [libvirt] [PATCH 2/4] Rename and move kvmGetMaxVCPUs to utils and extend it
On Wed, 2016-06-15 at 09:56 +0000, Shivaprasad G Bhat wrote:
This function needs to be used at two different places and make it
global
now. Also, extend it to return the NR_CPUs when needed.
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 52 +---------------------------------------------
src/util/virhostcpu.c | 37 +++++++++++++++++++++++++++++++++
src/util/virhostcpu.h | 7 ++++++
4 files changed, 46 insertions(+), 51 deletions(-)
This should really be two patches: one moving the function
to its new location while marking it as a private exported
symbol, and the other one chaging its signature and
behavior.
Actually, make it three patches - see below.
-/* add definitions missing in older linux/kvm.h */
-#ifndef KVMIO
-# define KVMIO 0xAE
-#endif
-#ifndef KVM_CHECK_EXTENSION
-# define KVM_CHECK_EXTENSION _IO(KVMIO, 0x03)
-#endif
-#ifndef KVM_CAP_NR_VCPUS
-# define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */
-#endif
We no longer support any distribution old enough not to have
these definitions, so you can just drop them (as a separate
patch).
+int
+virHostCPUGetKVMVCPUs(virHostCPUKVMWrapperFlags flag)
+{
+ int fd;
+ int ret;
+
+ if ((fd = open(KVM_DEVICE, O_RDONLY)) < 0) {
+ virReportSystemError(errno, _("Unable to open %s"), KVM_DEVICE);
+ return -1;
+ }
+
+#ifdef KVM_CAP_MAX_VCPUS
+ /* at first try KVM_CAP_MAX_VCPUS to determine the maximum count */
+ if (flag & VIR_HOSTCPU_KVM_MAXVCPUS &&
+ (ret = ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS)) > 0)
+ goto cleanup;
+#endif /* KVM_CAP_MAX_VCPUS */
+
+ /* as a fallback get KVM_CAP_NR_VCPUS (the recommended maximum number of
+ * vcpus). Note that on most machines this is set to 160. */
+ if ((flag & VIR_HOSTCPU_KVM_MAXVCPUS || flag & VIR_HOSTCPU_KVM_NR_VCPUS)
&&
+ (ret = ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS)) > 0)
+ goto cleanup;
You dropped a blank line here, and the "goto" is not indented
properly
+ /* if KVM_CAP_NR_VCPUS doesn't exist either, kernel
documentation states
+ * that 4 should be used as the maximum number of cpus */
+ ret = 4;
[...]
+typedef enum {
+ VIR_HOSTCPU_KVM_MAXVCPUS = (1 << 0),
+ VIR_HOSTCPU_KVM_NR_VCPUS = (1 << 1),
+} virHostCPUKVMWrapperFlags;
+
+int virHostCPUGetKVMVCPUs(virHostCPUKVMWrapperFlags flag);
The naming for the function, and especially for the enum and
its values, is IMHO kinda awkward after the change... We could
have something like
virHostCPUGetKVMMaxVCPUs()
virHostCPUGetKVMRecommendedVCPUs()
instead. The attached patch, which applies cleanly on top of
your series, shows what I have in mind :)
--
Andrea Bolognani
Software Engineer - Virtualization Team
Attachments:
- 0001-util-Introduce-virHostCPUGetKVM-Max-Recommended-VCPU.patch (text/x-patch — 5.1 KB)