[libvirt] [PATCH 0/2] util: Small cleanups
What it says on the tin. Andrea Bolognani (2): util: Drop obsolete compatibility code util: Only define /dev/kvm path once src/util/virhostcpu.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) -- 2.7.4
All operating system releases we support include these definitions. --- src/util/virhostcpu.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index fed5be7..a34d983 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -65,17 +65,6 @@ VIR_LOG_INIT("util.hostcpu"); #define KVM_DEVICE "/dev/kvm" -/* 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 - #if defined(__FreeBSD__) || defined(__APPLE__) static int -- 2.7.4
On Fri, Jun 24, 2016 at 07:34:37PM +0200, Andrea Bolognani wrote:
All operating system releases we support include these definitions.
Do we have a list of supported OSes somewhere? :) Also, it seems the virHostCPUGetKVMMaxVCPUs function is built unconditionally, so it might need to be wrapped in HAVE_LINUX_KVM_H. Have you tried building it on non-Linux?
--- src/util/virhostcpu.c | 11 ----------- 1 file changed, 11 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index fed5be7..a34d983 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -65,17 +65,6 @@ VIR_LOG_INIT("util.hostcpu");
#define KVM_DEVICE "/dev/kvm"
-/* 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 -
Introduced in libvirt by: commit 04e9e1b2a63f4255d6fddbd5990a0a0cde0ccf1b Author: Richard W.M. Jones <rjones@redhat.com> CommitDate: 2008-09-17 14:07:49 +0000 configure.in, src/qemu_driver.h, src/qemu_driver.c: KVM can determine max VCPUs at runtime (Guido Günther). git describe: v0.4.4-212-g04e9e1b contains: v0.4.6~17 And in kernel by: commit f725230af9ea03f6cc6f4a90e87aa428df46ec19 Author: Avi Kivity <avi@qumranet.com> CommitDate: 2008-04-27 11:53:23 +0300 KVM: Add API to retrieve the number of supported vcpus per vm Signed-off-by: Avi Kivity <avi@qumranet.com> git describe: v2.6.25-5338-gf725230 contains: v2.6.26-rc1~1028^2~110 So I think we're safe to drop the redefinitions if we have linux/kvm.h. Jan
On Sat, 2016-06-25 at 10:19 +0200, Ján Tomko wrote:
On Fri, Jun 24, 2016 at 07:34:37PM +0200, Andrea Bolognani wrote:
All operating system releases we support include these definitions. Do we have a list of supported OSes somewhere? :)
RHEL6 and OSs released around that time. I don't think it's been written down anywhere, but we dropped support for anything older a while back.
Also, it seems the virHostCPUGetKVMMaxVCPUs function is built unconditionally, so it might need to be wrapped in HAVE_LINUX_KVM_H. Have you tried building it on non-Linux?
You're right, it doesn't. I'll fix it :) -- Andrea Bolognani / Red Hat / Virtualization
Remove the local kvmpath variable from virHostCPUGetThreadsPerSubcore() and use the global KVM_DEVICE define instead. --- src/util/virhostcpu.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index a34d983..055fa1f 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1238,7 +1238,6 @@ int virHostCPUGetThreadsPerSubcore(virArch arch) { int threads_per_subcore = 0; - const char *kvmpath = "/dev/kvm"; int kvmfd; if (ARCH_IS_PPC64(arch)) { @@ -1248,17 +1247,17 @@ virHostCPUGetThreadsPerSubcore(virArch arch) * b. the kvm module might not be installed or enabled * In either case, falling back to the subcore-unaware thread * counting logic is the right thing to do */ - if (!virFileExists(kvmpath)) + if (!virFileExists(KVM_DEVICE)) goto out; - if ((kvmfd = open(kvmpath, O_RDONLY)) < 0) { + if ((kvmfd = open(KVM_DEVICE, O_RDONLY)) < 0) { /* This can happen when running as a regular user if * permissions are tight enough, in which case erroring out * is better than silently falling back and reporting * different nodeinfo depending on the user */ virReportSystemError(errno, _("Failed to open '%s'"), - kvmpath); + KVM_DEVICE); threads_per_subcore = -1; goto out; } -- 2.7.4
On Fri, Jun 24, 2016 at 07:34:38PM +0200, Andrea Bolognani wrote:
Remove the local kvmpath variable from virHostCPUGetThreadsPerSubcore() and use the global KVM_DEVICE define instead. --- src/util/virhostcpu.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
ACK Jan
On Sat, 2016-06-25 at 10:19 +0200, Ján Tomko wrote:
On Fri, Jun 24, 2016 at 07:34:38PM +0200, Andrea Bolognani wrote:
Remove the local kvmpath variable from virHostCPUGetThreadsPerSubcore() and use the global KVM_DEVICE define instead. --- src/util/virhostcpu.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
ACK
... and safe for freeze? :) -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Jun 27, 2016 at 01:15:56PM +0200, Andrea Bolognani wrote:
On Sat, 2016-06-25 at 10:19 +0200, Ján Tomko wrote:
On Fri, Jun 24, 2016 at 07:34:38PM +0200, Andrea Bolognani wrote:
Remove the local kvmpath variable from virHostCPUGetThreadsPerSubcore() and use the global KVM_DEVICE define instead. --- src/util/virhostcpu.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
ACK
... and safe for freeze? :)
Safe? Yes. But it does not fix anything so I don't see the point of pushing it during the freeze. Jan
On Mon, 2016-06-27 at 15:17 +0200, Ján Tomko wrote:
Remove the local kvmpath variable from virHostCPUGetThreadsPerSubcore() and use the global KVM_DEVICE define instead. --- src/util/virhostcpu.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
ACK
... and safe for freeze? :)
Safe? Yes.
But it does not fix anything so I don't see the point of pushing it during the freeze.
Fair enough, I'll push it after release. Thanks! -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Ján Tomko