Am 22.08.2013 18:12, schrieb Eduardo Habkost:
>
> On 22/08/2013, at 12:39, Andrew Jones <drjones@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
