On Fri, 2016-06-24 at 20:34 +0530, Shivaprasad G Bhat wrote:
The qemu limit and host limit both should be considered for the domain vcpu max limits. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 11 ++++++++--- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_driver.c | 2 +- tests/domaincapstest.c | 3 ++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 01466fc..ff5ad19 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -38,6 +38,7 @@ #include "virbitmap.h" #include "virnodesuspend.h" #include "virnuma.h" +#include "virhostcpu.h" #include "qemu_monitor.h" #include "virstring.h" #include "qemu_hostdev.h" @@ -4336,16 +4337,20 @@ int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, virQEMUCapsPtr qemuCaps, virFirmwarePtr *firmwares, - size_t nfirmwares) + size_t nfirmwares, + virDomainVirtType virttype)
I missed this the first time around, but passing virttype here is kinda pointless as it can be retrieved from domCaps->virttype. However, that would cause the test suite to depend on the host state, /dev/kvm in particular). So ACK to this approach for now, but there should be a follow-up patch that updates domaincapstest to access a mocked /dev/kvm and gets rid of this extra parameter.
{ virDomainCapsOSPtr os = &domCaps->os; virDomainCapsDeviceDiskPtr disk = &domCaps->disk; virDomainCapsDeviceHostdevPtr hostdev = &domCaps->hostdev; virDomainCapsDeviceGraphicsPtr graphics = &domCaps->graphics; virDomainCapsDeviceVideoPtr video = &domCaps->video; - int maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, domCaps->machine); - domCaps->maxvcpus = maxvcpus; + domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, domCaps->machine);
Line's too long, should have been split into two.
+ if (virttype == VIR_DOMAIN_VIRT_KVM) { + int hostmaxvcpus = virHostCPUGetKVMMaxVCPUs(); + domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus); + }
I mentioned during the first round of reviews that the value returned by virHostCPUGetKVMMaxVCPUs() should be checked, because it could be <0 if /dev/kvm could not be accessed. ACK, I've squashed in the following trivial diff and pushed the patch. diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ff5ad19..28d5321 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4346,10 +4346,12 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, virDomainCapsDeviceGraphicsPtr graphics = &domCaps->graphics; virDomainCapsDeviceVideoPtr video = &domCaps->video; - domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, domCaps->machine); + domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, + domCaps->machine); if (virttype == VIR_DOMAIN_VIRT_KVM) { int hostmaxvcpus = virHostCPUGetKVMMaxVCPUs(); - domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus); + if (hostmaxvcpus >= 0) + domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus); } if (virQEMUCapsFillDomainOSCaps(os, firmwares, nfirmwares) < 0 || -- Andrea Bolognani / Red Hat / Virtualization