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@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