[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 05/03/2016 02:39 AM, Shivaprasad bhat wrote:
> Thanks for the reply Cole.
> 
> On Tue, May 3, 2016 at 3:27 AM, Cole Robinson <crobinso 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 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. 
> 

Re-adding libvir-list to CC

I lightly grepped kernel sources. I think PPC is the only > 255 VCPU case, if
anything the other archs will be limited by their KVM support and not qemu
machine type support

- Cole


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]