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@xxxxxxxxxxxxxxxxxx>
> ---
> 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 TeamFrom 03ebb82c095cf756d411cc69bec6e9636079b792 Mon Sep 17 00:00:00 2001
From: Andrea Bolognani <abologna@xxxxxxxxxx>
Date: Wed, 22 Jun 2016 16:18:05 +0200
Subject: [PATCH] util: Introduce virHostCPUGetKVM{Max,Recommended}VCPUs()
---
src/libvirt_private.syms | 3 ++-
src/qemu/qemu_capabilities.c | 4 ++--
src/qemu/qemu_driver.c | 2 +-
src/util/virhostcpu.c | 52 ++++++++++++++++++++++++++++++++++----------
src/util/virhostcpu.h | 8 ++-----
5 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4d48f0b..0b62134 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1072,7 +1072,8 @@ virLogManagerNew;
nodeCapsInitNUMA;
nodeGetInfo;
virHostCPUGetCount;
-virHostCPUGetKVMVCPUs;
+virHostCPUGetKVMMaxVCPUs;
+virHostCPUGetKVMRecommendedVCPUs;
virHostCPUGetMap;
virHostCPUGetOnlineBitmap;
virHostCPUGetPresentBitmap;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5cb54c2..f554ba0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4349,8 +4349,8 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, domCaps->machine);
if (virttype == VIR_DOMAIN_VIRT_KVM) {
- hostmaxvcpus = virHostCPUGetKVMVCPUs(VIR_HOSTCPU_KVM_MAXVCPUS);
- domCaps->suggestedvcpus = virHostCPUGetKVMVCPUs(VIR_HOSTCPU_KVM_NR_VCPUS);
+ hostmaxvcpus = virHostCPUGetKVMMaxVCPUs();
+ domCaps->suggestedvcpus = virHostCPUGetKVMRecommendedVCPUs();
domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus);
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1adfa23..2db23ad 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1290,7 +1290,7 @@ qemuConnectGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, const char *type)
return 16;
if (STRCASEEQ(type, "kvm"))
- return virHostCPUGetKVMVCPUs(VIR_HOSTCPU_KVM_MAXVCPUS);
+ return virHostCPUGetKVMMaxVCPUs();
if (STRCASEEQ(type, "kqemu"))
return 1;
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index c5fcd41..86d6d42 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1303,8 +1303,9 @@ virHostCPUGetThreadsPerSubcore(virArch arch ATTRIBUTE_UNUSED)
# define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */
#endif
+#ifdef KVM_CAP_MAX_VCPUS
int
-virHostCPUGetKVMVCPUs(virHostCPUKVMWrapperFlags flag)
+virHostCPUGetKVMMaxVCPUs(void)
{
int fd;
int ret;
@@ -1314,20 +1315,47 @@ virHostCPUGetKVMVCPUs(virHostCPUKVMWrapperFlags flag)
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)
+ /* Ask the kernel for the max number of VCPUs */
+ if ((ret = ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS)) > 0)
goto cleanup;
+
+ /* If we didn't get a usable reply, fall back to the recommended
+ * number of VCPUs instead */
+ ret = virHostCPUGetKVMRecommendedVCPUs();
+
+ cleanup:
+ VIR_FORCE_CLOSE(fd);
+ return ret;
+}
+#else
+int
+virHostCPUGetKVMMaxVCPUs(void)
+{
+ /* Before KVM_CAP_MAX_VCPUS, the max number of VCPUs could be
+ * obtained using the KVM_CAP_NR_VCPUS capability, which returns
+ * the number of recommended VCPUs on modern kernels */
+ return virHostCPUGetKVMRecommendedVCPUs()
+}
#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;
- /* if KVM_CAP_NR_VCPUS doesn't exist either, kernel documentation states
- * that 4 should be used as the maximum number of cpus */
+int
+virHostCPUGetKVMRecommendedVCPUs(void)
+{
+ int fd;
+ int ret;
+
+ if ((fd = open(KVM_DEVICE, O_RDONLY)) < 0) {
+ virReportSystemError(errno, _("Unable to open %s"), KVM_DEVICE);
+ return -1;
+ }
+
+ /* Obtain the recommended number of VCPUs. On most machines, this
+ * will be 160 */
+ if ((ret = ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS)) > 0)
+ goto cleanup;
+
+ /* If KVM_CAP_NR_VCPUS doesn't return a usable value, kernel
+ * documentation states that 4 should be used */
ret = 4;
cleanup:
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index c1b855a..08786a4 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -51,11 +51,7 @@ int virHostCPUGetInfo(virArch hostarch,
unsigned int *cores,
unsigned int *threads);
-typedef enum {
- VIR_HOSTCPU_KVM_MAXVCPUS = (1 << 0),
- VIR_HOSTCPU_KVM_NR_VCPUS = (1 << 1),
-} virHostCPUKVMWrapperFlags;
-
-int virHostCPUGetKVMVCPUs(virHostCPUKVMWrapperFlags flag);
+int virHostCPUGetKVMMaxVCPUs(void);
+int virHostCPUGetKVMRecommendedVCPUs(void);
#endif /* __VIR_HOSTCPU_H__*/
--
2.7.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list