Re: [libvirt] [PATCH] ppc64: get the maxvcpus from the qemu caps instead of /dev/kvm

Thanks for the reply Cole. On Tue, May 3, 2016 at 3:27 AM, Cole Robinson <crobinso(a)redhat.com <mailto:crobinso@redhat.com>> wrote: On 05/02/2016 09:14 AM, Shivaprasad G Bhat wrote: > On PPC64, the KVM_MAX_VCPUS is defined to be 1024 where as qemu has > MAX_CPUMASK_BITS defined at 255 in include/sysemu/sysemu.h. > > virsh domacapabilities and virsh maxvcpus --type kvm return different > maxvcpus values and is confusing as to know what actually works. > > Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com <mailto:sbhat@linux.vnet.ibm.com>> > --- > src/qemu/qemu_driver.c | 28 ++++++++++++++++++++++++++-- > 1 file changed, 26 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 3d0c7c8..d84fc47 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -1255,10 +1255,34 @@ static int qemuConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) > > > static int > -kvmGetMaxVCPUs(void) > +kvmGetMaxVCPUs(virConnectPtr conn) > { > int fd; > int ret; > + virArch arch = virArchFromHost(); > + virQEMUCapsPtr qemuCaps = NULL; > + virQEMUDriverPtr driver = conn->privateData; > + > + const char *machine; > + > + if (ARCH_IS_PPC64(arch)) { > + if (!(qemuCaps = virQEMUCapsCacheLookupByArch(driver->qemuCapsCache, > + arch))) { > + virReportError(VIR_ERR_INVALID_ARG, > + _("unable to find any emulator to serve '%s' " > + "architecture"), virArchToString(arch)); > + return -1; > + } > + > + if (!(machine = virQEMUCapsGetDefaultMachine(qemuCaps))) { > + virObjectUnref(qemuCaps); > + return -1; > + } > + > + ret = virQEMUCapsGetMachineMaxCpus(qemuCaps, machine); > + virObjectUnref(qemuCaps); > + return ret; > + } > I don't really like this. The MaxVCPUs API sucks: it doesn't take enough parameters to give an accurate picture of what the max VCPUs supported for your desired config is. And that's basically unfixable. Hacking in a one off fix for PPC64 here doesn't seem like a good idea. If you really want to 'fix' this I'd suggest one or more of: - Reflect /dev/kvm maxvcpus in the capabilities XML somehow - Extend 'virsh maxvcpus' to take os_type+virt_type+emulator+machine_type, look up the capabilities XML, correlate that with maxvcpus output/new capabilities XML, and give a real value for the desired config - Change the maxvcpus API to consistently to return capabilities output, or cap the return value to the largest reported in capabilities output, and do it consistently for all architectures Extending maxvcpus API would make it more or less same as domcapabilities and I am not sure if that is necessary. I feel just reporting the maxvcpus to show the max possible vcpus for the default machine & arch & binary is a good idea as that is what the API intends to return today. I think the third option here what you suggested seems more logical. The maxvcpus supposed to be "minimum" of what comes out of capabilities or /dev/kvm not the "maximum". I am not sure of how the /dev/kvm and capabilties outputs differ on all archs. For x86 I saw them to be consistent. And on PPC it was different. Hope having a generic approach to report the "minimum" is good for all archs. Being unsure was the reason limiting my checks to PPC alone. I'll take the third approach as suggested.