Re: [libvirt] [Qemu-devel] [PATCH] kvm: warn if num cpus is greater than num recommended

Am 22.08.2013 18:12, schrieb Eduardo Habkost: > > On 22/08/2013, at 12:39, Andrew Jones <drjones(a)redhat.com&gt; wrote: > >> The comment in kvm_max_vcpus() states that it's using the recommended >> procedure from the kernel API documentation to get the max number >> of vcpus that kvm supports. It is, but by always returning the >> maximum number supported. The maximum number should only be used >> for development purposes. qemu should check KVM_CAP_NR_VCPUS for >> the recommended number of vcpus. This patch adds a warning if a user >> specifies a number of cpus between the recommended and max. >> >> Signed-off-by: Andrew Jones <drjones(a)redhat.com&gt; > > CCing libvir-list. It is probably interesting for libvirt to expose or warn > about the recommended VCPU limit somehow, and in this case a simple > warning on stderr won't be enough. > >> --- >> kvm-all.c | 45 +++++++++++++++++++++++++++------------------ >> 1 file changed, 27 insertions(+), 18 deletions(-) >> >> diff --git a/kvm-all.c b/kvm-all.c >> index 716860f617455..9092e13ae60ea 100644 >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -1313,24 +1313,24 @@ static int kvm_irqchip_create(KVMState *s) >> return 0; >> } >> >> -static int kvm_max_vcpus(KVMState *s) >> +/* Find number of supported CPUs using the recommended >> + * procedure from the kernel API documentation to cope with >> + * older kernels that may be missing capabilities. >> + */ >> +static int kvm_recommended_vcpus(KVMState *s) >> { >> int ret; >> >> - /* Find number of supported CPUs using the recommended >> - * procedure from the kernel API documentation to cope with >> - * older kernels that may be missing capabilities. >> - */ >> - ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS); >> - if (ret) { >> - return ret; >> - } >> ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS); >> - if (ret) { >> - return ret; >> - } >> + return (ret) ? ret : 4; >> +} >> >> - return 4; >> +static int kvm_max_vcpus(KVMState *s) >> +{ >> + int ret; >> + >> + ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS); >> + return (ret) ? ret : kvm_recommended_vcpus(s); >> } >> >> int kvm_init(void) >> @@ -1383,12 +1383,21 @@ int kvm_init(void) >> goto err; >> } >> >> - max_vcpus = kvm_max_vcpus(s); >> + max_vcpus = kvm_recommended_vcpus(s); >> if (smp_cpus > max_vcpus) { >> - ret = -EINVAL; >> - fprintf(stderr, "Number of SMP cpus requested (%d) exceeds max >> cpus " >> - "supported by KVM (%d)\n", smp_cpus, max_vcpus); >> - goto err; >> + fprintf(stderr, >> + "Warning: Number of SMP cpus requested (%d) exceeds " >> + "recommended cpus supported by KVM (%d)\n", >> + smp_cpus, max_vcpus); >> + >> + max_vcpus = kvm_max_vcpus(s); >> + if (smp_cpus > max_vcpus) { >> + ret = -EINVAL; >> + fprintf(stderr, "Number of SMP cpus requested (%d) exceeds " >> + "max cpus supported by KVM (%d)\n", >> + smp_cpus, max_vcpus); >> + goto err; >> + } Should at least the fatal one use the new error_report()?

>> } >> >> s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0); I notice that only checks in kvm_init() based on smp_cpus are touched herein. Should we add similar checks to CPU hot-add code and thus possibly move that into some per-vCPU code path?

Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg