All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] kvm: warn if num cpus is greater than num recommended
@ 2013-08-23 13:24 ` Andrew Jones
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Jones @ 2013-08-23 13:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: kvm, pbonzini, gleb, mtosatti, ehabkost, afaerber, libvir-list

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.

v2:
Incorporate tests for max_cpus, which specifies the maximum number
of hotpluggable cpus. An additional note is that the message for
the fail case was slightly changed, 'exceeds max cpus' to
'exceeds the maximum cpus'. If this is unacceptable change for
users like libvirt, then I'll need to spin a v3.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 kvm-all.c | 69 ++++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index a2d49786365e3..021f5f47e53da 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1322,24 +1322,20 @@ 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;
-    }
+    int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
+    return (ret) ? ret : 4;
+}
 
-    return 4;
+static int kvm_max_vcpus(KVMState *s)
+{
+    int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
+    return (ret) ? ret : kvm_recommended_vcpus(s);
 }
 
 int kvm_init(void)
@@ -1347,11 +1343,19 @@ int kvm_init(void)
     static const char upgrade_note[] =
         "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
         "(see http://sourceforge.net/projects/kvm).\n";
+    struct {
+        const char *name;
+        int num;
+    } num_cpus[] = {
+        { "SMP",          smp_cpus },
+        { "hotpluggable", max_cpus },
+        { NULL, }
+    }, *nc = num_cpus;
+    int soft_vcpus_limit, hard_vcpus_limit;
     KVMState *s;
     const KVMCapabilityInfo *missing_cap;
     int ret;
     int i;
-    int max_vcpus;
 
     s = g_malloc0(sizeof(KVMState));
 
@@ -1392,19 +1396,26 @@ int kvm_init(void)
         goto err;
     }
 
-    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;
-    }
+    /* check the vcpu limits */
+    soft_vcpus_limit = kvm_recommended_vcpus(s);
+    hard_vcpus_limit = kvm_max_vcpus(s);
 
-    if (max_cpus > max_vcpus) {
-        ret = -EINVAL;
-        fprintf(stderr, "Number of hotpluggable cpus requested (%d) exceeds max cpus "
-                "supported by KVM (%d)\n", max_cpus, max_vcpus);
-        goto err;
+    while (nc->name) {
+        if (nc->num > soft_vcpus_limit) {
+            fprintf(stderr,
+                    "Warning: Number of %s cpus requested (%d) exceeds "
+                    "the recommended cpus supported by KVM (%d)\n",
+                    nc->name, nc->num, soft_vcpus_limit);
+
+            if (nc->num > hard_vcpus_limit) {
+                ret = -EINVAL;
+                fprintf(stderr, "Number of %s cpus requested (%d) exceeds "
+                        "the maximum cpus supported by KVM (%d)\n",
+                        nc->name, nc->num, hard_vcpus_limit);
+                goto err;
+            }
+        }
+        nc++;
     }
 
     s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2] kvm: warn if num cpus is greater than num recommended
@ 2013-08-23 13:24 ` Andrew Jones
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Jones @ 2013-08-23 13:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: kvm, gleb, libvir-list, mtosatti, pbonzini, afaerber, ehabkost

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.

v2:
Incorporate tests for max_cpus, which specifies the maximum number
of hotpluggable cpus. An additional note is that the message for
the fail case was slightly changed, 'exceeds max cpus' to
'exceeds the maximum cpus'. If this is unacceptable change for
users like libvirt, then I'll need to spin a v3.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 kvm-all.c | 69 ++++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index a2d49786365e3..021f5f47e53da 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1322,24 +1322,20 @@ 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;
-    }
+    int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
+    return (ret) ? ret : 4;
+}
 
-    return 4;
+static int kvm_max_vcpus(KVMState *s)
+{
+    int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
+    return (ret) ? ret : kvm_recommended_vcpus(s);
 }
 
 int kvm_init(void)
@@ -1347,11 +1343,19 @@ int kvm_init(void)
     static const char upgrade_note[] =
         "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
         "(see http://sourceforge.net/projects/kvm).\n";
+    struct {
+        const char *name;
+        int num;
+    } num_cpus[] = {
+        { "SMP",          smp_cpus },
+        { "hotpluggable", max_cpus },
+        { NULL, }
+    }, *nc = num_cpus;
+    int soft_vcpus_limit, hard_vcpus_limit;
     KVMState *s;
     const KVMCapabilityInfo *missing_cap;
     int ret;
     int i;
-    int max_vcpus;
 
     s = g_malloc0(sizeof(KVMState));
 
@@ -1392,19 +1396,26 @@ int kvm_init(void)
         goto err;
     }
 
-    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;
-    }
+    /* check the vcpu limits */
+    soft_vcpus_limit = kvm_recommended_vcpus(s);
+    hard_vcpus_limit = kvm_max_vcpus(s);
 
-    if (max_cpus > max_vcpus) {
-        ret = -EINVAL;
-        fprintf(stderr, "Number of hotpluggable cpus requested (%d) exceeds max cpus "
-                "supported by KVM (%d)\n", max_cpus, max_vcpus);
-        goto err;
+    while (nc->name) {
+        if (nc->num > soft_vcpus_limit) {
+            fprintf(stderr,
+                    "Warning: Number of %s cpus requested (%d) exceeds "
+                    "the recommended cpus supported by KVM (%d)\n",
+                    nc->name, nc->num, soft_vcpus_limit);
+
+            if (nc->num > hard_vcpus_limit) {
+                ret = -EINVAL;
+                fprintf(stderr, "Number of %s cpus requested (%d) exceeds "
+                        "the maximum cpus supported by KVM (%d)\n",
+                        nc->name, nc->num, hard_vcpus_limit);
+                goto err;
+            }
+        }
+        nc++;
     }
 
     s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [libvirt] [PATCH v2] kvm: warn if num cpus is greater than num recommended
  2013-08-23 13:24 ` [Qemu-devel] " Andrew Jones
@ 2013-08-28  1:30   ` Eric Blake
  -1 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2013-08-28  1:30 UTC (permalink / raw)
  To: Andrew Jones
  Cc: ehabkost, kvm, libvir-list, mtosatti, qemu-devel, pbonzini, afaerber


[-- Attachment #1.1: Type: text/plain, Size: 1579 bytes --]

On 08/23/2013 07:24 AM, Andrew Jones 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.
> 
> v2:
> Incorporate tests for max_cpus, which specifies the maximum number
> of hotpluggable cpus. An additional note is that the message for
> the fail case was slightly changed, 'exceeds max cpus' to
> 'exceeds the maximum cpus'. If this is unacceptable change for
> users like libvirt, then I'll need to spin a v3.

A quick grep of libvirt does not show any dependence on the particular
wording "exceeds max cpus", so you are probably fine changing that.

What I'm more worried about is what number is libvirt supposed to show
to the end user, and should libvirt enforce the lower recommended max,
or the larger kernel absolute max?  Which of the two values does the QMP
'MachineInfo' type return in its 'cpu-max' field during the
'query-machines' command?  Should we be modifying QMP to return both
values, so that libvirt can also expose the logic to the end user of
allowing a recommended vs. larger development max?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [libvirt] [PATCH v2] kvm: warn if num cpus is greater than num recommended
@ 2013-08-28  1:30   ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2013-08-28  1:30 UTC (permalink / raw)
  To: Andrew Jones
  Cc: ehabkost, kvm, libvir-list, mtosatti, qemu-devel, pbonzini, afaerber

[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]

On 08/23/2013 07:24 AM, Andrew Jones 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.
> 
> v2:
> Incorporate tests for max_cpus, which specifies the maximum number
> of hotpluggable cpus. An additional note is that the message for
> the fail case was slightly changed, 'exceeds max cpus' to
> 'exceeds the maximum cpus'. If this is unacceptable change for
> users like libvirt, then I'll need to spin a v3.

A quick grep of libvirt does not show any dependence on the particular
wording "exceeds max cpus", so you are probably fine changing that.

What I'm more worried about is what number is libvirt supposed to show
to the end user, and should libvirt enforce the lower recommended max,
or the larger kernel absolute max?  Which of the two values does the QMP
'MachineInfo' type return in its 'cpu-max' field during the
'query-machines' command?  Should we be modifying QMP to return both
values, so that libvirt can also expose the logic to the end user of
allowing a recommended vs. larger development max?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [libvirt] [PATCH v2] kvm: warn if num cpus is greater than num recommended
  2013-08-28  1:30   ` [Qemu-devel] " Eric Blake
@ 2013-08-28  7:45     ` Andrew Jones
  -1 siblings, 0 replies; 14+ messages in thread
From: Andrew Jones @ 2013-08-28  7:45 UTC (permalink / raw)
  To: Eric Blake
  Cc: qemu-devel, kvm, libvir-list, mtosatti, pbonzini, afaerber, ehabkost



----- Original Message -----
> On 08/23/2013 07:24 AM, Andrew Jones 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.
> > 
> > v2:
> > Incorporate tests for max_cpus, which specifies the maximum number
> > of hotpluggable cpus. An additional note is that the message for
> > the fail case was slightly changed, 'exceeds max cpus' to
> > 'exceeds the maximum cpus'. If this is unacceptable change for
> > users like libvirt, then I'll need to spin a v3.
> 
> A quick grep of libvirt does not show any dependence on the particular
> wording "exceeds max cpus", so you are probably fine changing that.
> 
> What I'm more worried about is what number is libvirt supposed to show
> to the end user, and should libvirt enforce the lower recommended max,
> or the larger kernel absolute max?  Which of the two values does the QMP
> 'MachineInfo' type return in its 'cpu-max' field during the
> 'query-machines' command?  Should we be modifying QMP to return both
> values, so that libvirt can also expose the logic to the end user of
> allowing a recommended vs. larger development max?
> 

Machine definitions maintain yet another 'max_cpus'. And it appears that
qmp would return that value. It would probably be best if it returned
max(qemu_machine.max_cpus, kvm_max_cpus) though.
 
I'm starting to think that we should just keep things simple for most of
the virt stack by sticking to enforcing the larger developer max. And
then on a production kernel we should just compile KVM_MAX_VCPUS =
KVM_SOFT_MAX_VCPUS and be done with it. With that thought, this patch
could be dropped too. The alternative seems to be supporting a run-time
selectable experimental mode throughout the whole virt stack.

drew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [libvirt] [PATCH v2] kvm: warn if num cpus is greater than num recommended
@ 2013-08-28  7:45     ` Andrew Jones
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Jones @ 2013-08-28  7:45 UTC (permalink / raw)
  To: Eric Blake
  Cc: ehabkost, kvm, libvir-list, mtosatti, qemu-devel, pbonzini, afaerber



----- Original Message -----
> On 08/23/2013 07:24 AM, Andrew Jones 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.
> > 
> > v2:
> > Incorporate tests for max_cpus, which specifies the maximum number
> > of hotpluggable cpus. An additional note is that the message for
> > the fail case was slightly changed, 'exceeds max cpus' to
> > 'exceeds the maximum cpus'. If this is unacceptable change for
> > users like libvirt, then I'll need to spin a v3.
> 
> A quick grep of libvirt does not show any dependence on the particular
> wording "exceeds max cpus", so you are probably fine changing that.
> 
> What I'm more worried about is what number is libvirt supposed to show
> to the end user, and should libvirt enforce the lower recommended max,
> or the larger kernel absolute max?  Which of the two values does the QMP
> 'MachineInfo' type return in its 'cpu-max' field during the
> 'query-machines' command?  Should we be modifying QMP to return both
> values, so that libvirt can also expose the logic to the end user of
> allowing a recommended vs. larger development max?
> 

Machine definitions maintain yet another 'max_cpus'. And it appears that
qmp would return that value. It would probably be best if it returned
max(qemu_machine.max_cpus, kvm_max_cpus) though.
 
I'm starting to think that we should just keep things simple for most of
the virt stack by sticking to enforcing the larger developer max. And
then on a production kernel we should just compile KVM_MAX_VCPUS =
KVM_SOFT_MAX_VCPUS and be done with it. With that thought, this patch
could be dropped too. The alternative seems to be supporting a run-time
selectable experimental mode throughout the whole virt stack.

drew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [libvirt] [PATCH v2] kvm: warn if num cpus is greater than num recommended
  2013-08-28  7:45     ` [Qemu-devel] " Andrew Jones
@ 2013-08-28 12:33       ` Eric Blake
  -1 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2013-08-28 12:33 UTC (permalink / raw)
  To: Andrew Jones
  Cc: qemu-devel, kvm, libvir-list, mtosatti, pbonzini, afaerber, ehabkost

[-- Attachment #1: Type: text/plain, Size: 1783 bytes --]

On 08/28/2013 01:45 AM, Andrew Jones wrote:
>> What I'm more worried about is what number is libvirt supposed to show
>> to the end user, and should libvirt enforce the lower recommended max,
>> or the larger kernel absolute max?  Which of the two values does the QMP
>> 'MachineInfo' type return in its 'cpu-max' field during the
>> 'query-machines' command?  Should we be modifying QMP to return both
>> values, so that libvirt can also expose the logic to the end user of
>> allowing a recommended vs. larger development max?
>>
> 
> Machine definitions maintain yet another 'max_cpus'. And it appears that
> qmp would return that value. It would probably be best if it returned
> max(qemu_machine.max_cpus, kvm_max_cpus) though.
>  
> I'm starting to think that we should just keep things simple for most of
> the virt stack by sticking to enforcing the larger developer max. And
> then on a production kernel we should just compile KVM_MAX_VCPUS =
> KVM_SOFT_MAX_VCPUS and be done with it. With that thought, this patch
> could be dropped too. The alternative seems to be supporting a run-time
> selectable experimental mode throughout the whole virt stack.

Indeed - if it is a number you are unwilling to support, don't compile
it into the kernel in the first place.  Allowing arbitrary limits that
are lower than the maximum imply policy, and policy implies touching the
stack (because someone, somewhere in the stack, will have good reason
for setting policy different than the lowest layer); fix the maximum
instead, and the whole stack complies without having to worry about
policy.  IMO, this is a case where fewer knobs is better.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [libvirt] [PATCH v2] kvm: warn if num cpus is greater than num recommended
@ 2013-08-28 12:33       ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2013-08-28 12:33 UTC (permalink / raw)
  To: Andrew Jones
  Cc: ehabkost, kvm, libvir-list, mtosatti, qemu-devel, pbonzini, afaerber

[-- Attachment #1: Type: text/plain, Size: 1783 bytes --]

On 08/28/2013 01:45 AM, Andrew Jones wrote:
>> What I'm more worried about is what number is libvirt supposed to show
>> to the end user, and should libvirt enforce the lower recommended max,
>> or the larger kernel absolute max?  Which of the two values does the QMP
>> 'MachineInfo' type return in its 'cpu-max' field during the
>> 'query-machines' command?  Should we be modifying QMP to return both
>> values, so that libvirt can also expose the logic to the end user of
>> allowing a recommended vs. larger development max?
>>
> 
> Machine definitions maintain yet another 'max_cpus'. And it appears that
> qmp would return that value. It would probably be best if it returned
> max(qemu_machine.max_cpus, kvm_max_cpus) though.
>  
> I'm starting to think that we should just keep things simple for most of
> the virt stack by sticking to enforcing the larger developer max. And
> then on a production kernel we should just compile KVM_MAX_VCPUS =
> KVM_SOFT_MAX_VCPUS and be done with it. With that thought, this patch
> could be dropped too. The alternative seems to be supporting a run-time
> selectable experimental mode throughout the whole virt stack.

Indeed - if it is a number you are unwilling to support, don't compile
it into the kernel in the first place.  Allowing arbitrary limits that
are lower than the maximum imply policy, and policy implies touching the
stack (because someone, somewhere in the stack, will have good reason
for setting policy different than the lowest layer); fix the maximum
instead, and the whole stack complies without having to worry about
policy.  IMO, this is a case where fewer knobs is better.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: warn if num cpus is greater than num recommended
  2013-08-23 13:24 ` [Qemu-devel] " Andrew Jones
@ 2013-09-01  9:46   ` Gleb Natapov
  -1 siblings, 0 replies; 14+ messages in thread
From: Gleb Natapov @ 2013-09-01  9:46 UTC (permalink / raw)
  To: Andrew Jones
  Cc: qemu-devel, kvm, pbonzini, mtosatti, ehabkost, afaerber, libvir-list

On Fri, Aug 23, 2013 at 03:24:37PM +0200, Andrew Jones 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.
> 
> v2:
> Incorporate tests for max_cpus, which specifies the maximum number
> of hotpluggable cpus. An additional note is that the message for
> the fail case was slightly changed, 'exceeds max cpus' to
> 'exceeds the maximum cpus'. If this is unacceptable change for
> users like libvirt, then I'll need to spin a v3.
> 
Looks good to me. Any ACKs, objections?

> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  kvm-all.c | 69 ++++++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index a2d49786365e3..021f5f47e53da 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1322,24 +1322,20 @@ 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;
> -    }
> +    int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
> +    return (ret) ? ret : 4;
> +}
>  
> -    return 4;
> +static int kvm_max_vcpus(KVMState *s)
> +{
> +    int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> +    return (ret) ? ret : kvm_recommended_vcpus(s);
>  }
>  
>  int kvm_init(void)
> @@ -1347,11 +1343,19 @@ int kvm_init(void)
>      static const char upgrade_note[] =
>          "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
>          "(see http://sourceforge.net/projects/kvm).\n";
> +    struct {
> +        const char *name;
> +        int num;
> +    } num_cpus[] = {
> +        { "SMP",          smp_cpus },
> +        { "hotpluggable", max_cpus },
> +        { NULL, }
> +    }, *nc = num_cpus;
> +    int soft_vcpus_limit, hard_vcpus_limit;
>      KVMState *s;
>      const KVMCapabilityInfo *missing_cap;
>      int ret;
>      int i;
> -    int max_vcpus;
>  
>      s = g_malloc0(sizeof(KVMState));
>  
> @@ -1392,19 +1396,26 @@ int kvm_init(void)
>          goto err;
>      }
>  
> -    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;
> -    }
> +    /* check the vcpu limits */
> +    soft_vcpus_limit = kvm_recommended_vcpus(s);
> +    hard_vcpus_limit = kvm_max_vcpus(s);
>  
> -    if (max_cpus > max_vcpus) {
> -        ret = -EINVAL;
> -        fprintf(stderr, "Number of hotpluggable cpus requested (%d) exceeds max cpus "
> -                "supported by KVM (%d)\n", max_cpus, max_vcpus);
> -        goto err;
> +    while (nc->name) {
> +        if (nc->num > soft_vcpus_limit) {
> +            fprintf(stderr,
> +                    "Warning: Number of %s cpus requested (%d) exceeds "
> +                    "the recommended cpus supported by KVM (%d)\n",
> +                    nc->name, nc->num, soft_vcpus_limit);
> +
> +            if (nc->num > hard_vcpus_limit) {
> +                ret = -EINVAL;
> +                fprintf(stderr, "Number of %s cpus requested (%d) exceeds "
> +                        "the maximum cpus supported by KVM (%d)\n",
> +                        nc->name, nc->num, hard_vcpus_limit);
> +                goto err;
> +            }
> +        }
> +        nc++;
>      }
>  
>      s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
> -- 
> 1.8.1.4

--
			Gleb.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2] kvm: warn if num cpus is greater than num recommended
@ 2013-09-01  9:46   ` Gleb Natapov
  0 siblings, 0 replies; 14+ messages in thread
From: Gleb Natapov @ 2013-09-01  9:46 UTC (permalink / raw)
  To: Andrew Jones
  Cc: ehabkost, kvm, libvir-list, mtosatti, qemu-devel, pbonzini, afaerber

On Fri, Aug 23, 2013 at 03:24:37PM +0200, Andrew Jones 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.
> 
> v2:
> Incorporate tests for max_cpus, which specifies the maximum number
> of hotpluggable cpus. An additional note is that the message for
> the fail case was slightly changed, 'exceeds max cpus' to
> 'exceeds the maximum cpus'. If this is unacceptable change for
> users like libvirt, then I'll need to spin a v3.
> 
Looks good to me. Any ACKs, objections?

> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  kvm-all.c | 69 ++++++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index a2d49786365e3..021f5f47e53da 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1322,24 +1322,20 @@ 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;
> -    }
> +    int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
> +    return (ret) ? ret : 4;
> +}
>  
> -    return 4;
> +static int kvm_max_vcpus(KVMState *s)
> +{
> +    int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> +    return (ret) ? ret : kvm_recommended_vcpus(s);
>  }
>  
>  int kvm_init(void)
> @@ -1347,11 +1343,19 @@ int kvm_init(void)
>      static const char upgrade_note[] =
>          "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
>          "(see http://sourceforge.net/projects/kvm).\n";
> +    struct {
> +        const char *name;
> +        int num;
> +    } num_cpus[] = {
> +        { "SMP",          smp_cpus },
> +        { "hotpluggable", max_cpus },
> +        { NULL, }
> +    }, *nc = num_cpus;
> +    int soft_vcpus_limit, hard_vcpus_limit;
>      KVMState *s;
>      const KVMCapabilityInfo *missing_cap;
>      int ret;
>      int i;
> -    int max_vcpus;
>  
>      s = g_malloc0(sizeof(KVMState));
>  
> @@ -1392,19 +1396,26 @@ int kvm_init(void)
>          goto err;
>      }
>  
> -    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;
> -    }
> +    /* check the vcpu limits */
> +    soft_vcpus_limit = kvm_recommended_vcpus(s);
> +    hard_vcpus_limit = kvm_max_vcpus(s);
>  
> -    if (max_cpus > max_vcpus) {
> -        ret = -EINVAL;
> -        fprintf(stderr, "Number of hotpluggable cpus requested (%d) exceeds max cpus "
> -                "supported by KVM (%d)\n", max_cpus, max_vcpus);
> -        goto err;
> +    while (nc->name) {
> +        if (nc->num > soft_vcpus_limit) {
> +            fprintf(stderr,
> +                    "Warning: Number of %s cpus requested (%d) exceeds "
> +                    "the recommended cpus supported by KVM (%d)\n",
> +                    nc->name, nc->num, soft_vcpus_limit);
> +
> +            if (nc->num > hard_vcpus_limit) {
> +                ret = -EINVAL;
> +                fprintf(stderr, "Number of %s cpus requested (%d) exceeds "
> +                        "the maximum cpus supported by KVM (%d)\n",
> +                        nc->name, nc->num, hard_vcpus_limit);
> +                goto err;
> +            }
> +        }
> +        nc++;
>      }
>  
>      s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
> -- 
> 1.8.1.4

--
			Gleb.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: warn if num cpus is greater than num recommended
  2013-08-23 13:24 ` [Qemu-devel] " Andrew Jones
@ 2013-09-01 22:46   ` Marcelo Tosatti
  -1 siblings, 0 replies; 14+ messages in thread
From: Marcelo Tosatti @ 2013-09-01 22:46 UTC (permalink / raw)
  To: Andrew Jones
  Cc: qemu-devel, kvm, pbonzini, gleb, ehabkost, afaerber, libvir-list

On Fri, Aug 23, 2013 at 03:24:37PM +0200, Andrew Jones 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.
> 
> v2:
> Incorporate tests for max_cpus, which specifies the maximum number
> of hotpluggable cpus. An additional note is that the message for
> the fail case was slightly changed, 'exceeds max cpus' to
> 'exceeds the maximum cpus'. If this is unacceptable change for
> users like libvirt, then I'll need to spin a v3.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  kvm-all.c | 69 ++++++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index a2d49786365e3..021f5f47e53da 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1322,24 +1322,20 @@ 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;
> -    }
> +    int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
> +    return (ret) ? ret : 4;
> +}
>  
> -    return 4;
> +static int kvm_max_vcpus(KVMState *s)
> +{
> +    int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> +    return (ret) ? ret : kvm_recommended_vcpus(s);
>  }
>  
>  int kvm_init(void)
> @@ -1347,11 +1343,19 @@ int kvm_init(void)
>      static const char upgrade_note[] =
>          "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
>          "(see http://sourceforge.net/projects/kvm).\n";
> +    struct {
> +        const char *name;
> +        int num;
> +    } num_cpus[] = {
> +        { "SMP",          smp_cpus },
> +        { "hotpluggable", max_cpus },
> +        { NULL, }
> +    }, *nc = num_cpus;
> +    int soft_vcpus_limit, hard_vcpus_limit;
>      KVMState *s;
>      const KVMCapabilityInfo *missing_cap;
>      int ret;
>      int i;
> -    int max_vcpus;
>  
>      s = g_malloc0(sizeof(KVMState));
>  
> @@ -1392,19 +1396,26 @@ int kvm_init(void)
>          goto err;
>      }
>  
> -    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;
> -    }
> +    /* check the vcpu limits */
> +    soft_vcpus_limit = kvm_recommended_vcpus(s);
> +    hard_vcpus_limit = kvm_max_vcpus(s);
>  
> -    if (max_cpus > max_vcpus) {
> -        ret = -EINVAL;
> -        fprintf(stderr, "Number of hotpluggable cpus requested (%d) exceeds max cpus "
> -                "supported by KVM (%d)\n", max_cpus, max_vcpus);
> -        goto err;
> +    while (nc->name) {
> +        if (nc->num > soft_vcpus_limit) {
> +            fprintf(stderr,
> +                    "Warning: Number of %s cpus requested (%d) exceeds "
> +                    "the recommended cpus supported by KVM (%d)\n",
> +                    nc->name, nc->num, soft_vcpus_limit);
> +
> +            if (nc->num > hard_vcpus_limit) {
> +                ret = -EINVAL;
> +                fprintf(stderr, "Number of %s cpus requested (%d) exceeds "
> +                        "the maximum cpus supported by KVM (%d)\n",
> +                        nc->name, nc->num, hard_vcpus_limit);
> +                goto err;
> +            }
> +        }
> +        nc++;
>      }
>  
>      s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
> -- 
> 1.8.1.4

ACK


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2] kvm: warn if num cpus is greater than num recommended
@ 2013-09-01 22:46   ` Marcelo Tosatti
  0 siblings, 0 replies; 14+ messages in thread
From: Marcelo Tosatti @ 2013-09-01 22:46 UTC (permalink / raw)
  To: Andrew Jones
  Cc: kvm, gleb, libvir-list, qemu-devel, pbonzini, afaerber, ehabkost

On Fri, Aug 23, 2013 at 03:24:37PM +0200, Andrew Jones 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.
> 
> v2:
> Incorporate tests for max_cpus, which specifies the maximum number
> of hotpluggable cpus. An additional note is that the message for
> the fail case was slightly changed, 'exceeds max cpus' to
> 'exceeds the maximum cpus'. If this is unacceptable change for
> users like libvirt, then I'll need to spin a v3.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  kvm-all.c | 69 ++++++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index a2d49786365e3..021f5f47e53da 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1322,24 +1322,20 @@ 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;
> -    }
> +    int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
> +    return (ret) ? ret : 4;
> +}
>  
> -    return 4;
> +static int kvm_max_vcpus(KVMState *s)
> +{
> +    int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> +    return (ret) ? ret : kvm_recommended_vcpus(s);
>  }
>  
>  int kvm_init(void)
> @@ -1347,11 +1343,19 @@ int kvm_init(void)
>      static const char upgrade_note[] =
>          "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
>          "(see http://sourceforge.net/projects/kvm).\n";
> +    struct {
> +        const char *name;
> +        int num;
> +    } num_cpus[] = {
> +        { "SMP",          smp_cpus },
> +        { "hotpluggable", max_cpus },
> +        { NULL, }
> +    }, *nc = num_cpus;
> +    int soft_vcpus_limit, hard_vcpus_limit;
>      KVMState *s;
>      const KVMCapabilityInfo *missing_cap;
>      int ret;
>      int i;
> -    int max_vcpus;
>  
>      s = g_malloc0(sizeof(KVMState));
>  
> @@ -1392,19 +1396,26 @@ int kvm_init(void)
>          goto err;
>      }
>  
> -    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;
> -    }
> +    /* check the vcpu limits */
> +    soft_vcpus_limit = kvm_recommended_vcpus(s);
> +    hard_vcpus_limit = kvm_max_vcpus(s);
>  
> -    if (max_cpus > max_vcpus) {
> -        ret = -EINVAL;
> -        fprintf(stderr, "Number of hotpluggable cpus requested (%d) exceeds max cpus "
> -                "supported by KVM (%d)\n", max_cpus, max_vcpus);
> -        goto err;
> +    while (nc->name) {
> +        if (nc->num > soft_vcpus_limit) {
> +            fprintf(stderr,
> +                    "Warning: Number of %s cpus requested (%d) exceeds "
> +                    "the recommended cpus supported by KVM (%d)\n",
> +                    nc->name, nc->num, soft_vcpus_limit);
> +
> +            if (nc->num > hard_vcpus_limit) {
> +                ret = -EINVAL;
> +                fprintf(stderr, "Number of %s cpus requested (%d) exceeds "
> +                        "the maximum cpus supported by KVM (%d)\n",
> +                        nc->name, nc->num, hard_vcpus_limit);
> +                goto err;
> +            }
> +        }
> +        nc++;
>      }
>  
>      s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
> -- 
> 1.8.1.4

ACK

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] kvm: warn if num cpus is greater than num recommended
  2013-08-23 13:24 ` [Qemu-devel] " Andrew Jones
@ 2013-09-03  8:54   ` Gleb Natapov
  -1 siblings, 0 replies; 14+ messages in thread
From: Gleb Natapov @ 2013-09-03  8:54 UTC (permalink / raw)
  To: Andrew Jones
  Cc: qemu-devel, kvm, pbonzini, mtosatti, ehabkost, afaerber, libvir-list

On Fri, Aug 23, 2013 at 03:24:37PM +0200, Andrew Jones 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.
> 
> v2:
> Incorporate tests for max_cpus, which specifies the maximum number
> of hotpluggable cpus. An additional note is that the message for
> the fail case was slightly changed, 'exceeds max cpus' to
> 'exceeds the maximum cpus'. If this is unacceptable change for
> users like libvirt, then I'll need to spin a v3.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Applied, thanks.

> ---
>  kvm-all.c | 69 ++++++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index a2d49786365e3..021f5f47e53da 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1322,24 +1322,20 @@ 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;
> -    }
> +    int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
> +    return (ret) ? ret : 4;
> +}
>  
> -    return 4;
> +static int kvm_max_vcpus(KVMState *s)
> +{
> +    int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> +    return (ret) ? ret : kvm_recommended_vcpus(s);
>  }
>  
>  int kvm_init(void)
> @@ -1347,11 +1343,19 @@ int kvm_init(void)
>      static const char upgrade_note[] =
>          "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
>          "(see http://sourceforge.net/projects/kvm).\n";
> +    struct {
> +        const char *name;
> +        int num;
> +    } num_cpus[] = {
> +        { "SMP",          smp_cpus },
> +        { "hotpluggable", max_cpus },
> +        { NULL, }
> +    }, *nc = num_cpus;
> +    int soft_vcpus_limit, hard_vcpus_limit;
>      KVMState *s;
>      const KVMCapabilityInfo *missing_cap;
>      int ret;
>      int i;
> -    int max_vcpus;
>  
>      s = g_malloc0(sizeof(KVMState));
>  
> @@ -1392,19 +1396,26 @@ int kvm_init(void)
>          goto err;
>      }
>  
> -    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;
> -    }
> +    /* check the vcpu limits */
> +    soft_vcpus_limit = kvm_recommended_vcpus(s);
> +    hard_vcpus_limit = kvm_max_vcpus(s);
>  
> -    if (max_cpus > max_vcpus) {
> -        ret = -EINVAL;
> -        fprintf(stderr, "Number of hotpluggable cpus requested (%d) exceeds max cpus "
> -                "supported by KVM (%d)\n", max_cpus, max_vcpus);
> -        goto err;
> +    while (nc->name) {
> +        if (nc->num > soft_vcpus_limit) {
> +            fprintf(stderr,
> +                    "Warning: Number of %s cpus requested (%d) exceeds "
> +                    "the recommended cpus supported by KVM (%d)\n",
> +                    nc->name, nc->num, soft_vcpus_limit);
> +
> +            if (nc->num > hard_vcpus_limit) {
> +                ret = -EINVAL;
> +                fprintf(stderr, "Number of %s cpus requested (%d) exceeds "
> +                        "the maximum cpus supported by KVM (%d)\n",
> +                        nc->name, nc->num, hard_vcpus_limit);
> +                goto err;
> +            }
> +        }
> +        nc++;
>      }
>  
>      s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
> -- 
> 1.8.1.4

--
			Gleb.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2] kvm: warn if num cpus is greater than num recommended
@ 2013-09-03  8:54   ` Gleb Natapov
  0 siblings, 0 replies; 14+ messages in thread
From: Gleb Natapov @ 2013-09-03  8:54 UTC (permalink / raw)
  To: Andrew Jones
  Cc: ehabkost, kvm, libvir-list, mtosatti, qemu-devel, pbonzini, afaerber

On Fri, Aug 23, 2013 at 03:24:37PM +0200, Andrew Jones 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.
> 
> v2:
> Incorporate tests for max_cpus, which specifies the maximum number
> of hotpluggable cpus. An additional note is that the message for
> the fail case was slightly changed, 'exceeds max cpus' to
> 'exceeds the maximum cpus'. If this is unacceptable change for
> users like libvirt, then I'll need to spin a v3.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Applied, thanks.

> ---
>  kvm-all.c | 69 ++++++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index a2d49786365e3..021f5f47e53da 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1322,24 +1322,20 @@ 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;
> -    }
> +    int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
> +    return (ret) ? ret : 4;
> +}
>  
> -    return 4;
> +static int kvm_max_vcpus(KVMState *s)
> +{
> +    int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> +    return (ret) ? ret : kvm_recommended_vcpus(s);
>  }
>  
>  int kvm_init(void)
> @@ -1347,11 +1343,19 @@ int kvm_init(void)
>      static const char upgrade_note[] =
>          "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
>          "(see http://sourceforge.net/projects/kvm).\n";
> +    struct {
> +        const char *name;
> +        int num;
> +    } num_cpus[] = {
> +        { "SMP",          smp_cpus },
> +        { "hotpluggable", max_cpus },
> +        { NULL, }
> +    }, *nc = num_cpus;
> +    int soft_vcpus_limit, hard_vcpus_limit;
>      KVMState *s;
>      const KVMCapabilityInfo *missing_cap;
>      int ret;
>      int i;
> -    int max_vcpus;
>  
>      s = g_malloc0(sizeof(KVMState));
>  
> @@ -1392,19 +1396,26 @@ int kvm_init(void)
>          goto err;
>      }
>  
> -    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;
> -    }
> +    /* check the vcpu limits */
> +    soft_vcpus_limit = kvm_recommended_vcpus(s);
> +    hard_vcpus_limit = kvm_max_vcpus(s);
>  
> -    if (max_cpus > max_vcpus) {
> -        ret = -EINVAL;
> -        fprintf(stderr, "Number of hotpluggable cpus requested (%d) exceeds max cpus "
> -                "supported by KVM (%d)\n", max_cpus, max_vcpus);
> -        goto err;
> +    while (nc->name) {
> +        if (nc->num > soft_vcpus_limit) {
> +            fprintf(stderr,
> +                    "Warning: Number of %s cpus requested (%d) exceeds "
> +                    "the recommended cpus supported by KVM (%d)\n",
> +                    nc->name, nc->num, soft_vcpus_limit);
> +
> +            if (nc->num > hard_vcpus_limit) {
> +                ret = -EINVAL;
> +                fprintf(stderr, "Number of %s cpus requested (%d) exceeds "
> +                        "the maximum cpus supported by KVM (%d)\n",
> +                        nc->name, nc->num, hard_vcpus_limit);
> +                goto err;
> +            }
> +        }
> +        nc++;
>      }
>  
>      s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
> -- 
> 1.8.1.4

--
			Gleb.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-09-03  8:56 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-23 13:24 [PATCH v2] kvm: warn if num cpus is greater than num recommended Andrew Jones
2013-08-23 13:24 ` [Qemu-devel] " Andrew Jones
2013-08-28  1:30 ` [libvirt] " Eric Blake
2013-08-28  1:30   ` [Qemu-devel] " Eric Blake
2013-08-28  7:45   ` Andrew Jones
2013-08-28  7:45     ` [Qemu-devel] " Andrew Jones
2013-08-28 12:33     ` Eric Blake
2013-08-28 12:33       ` [Qemu-devel] " Eric Blake
2013-09-01  9:46 ` Gleb Natapov
2013-09-01  9:46   ` [Qemu-devel] " Gleb Natapov
2013-09-01 22:46 ` Marcelo Tosatti
2013-09-01 22:46   ` [Qemu-devel] " Marcelo Tosatti
2013-09-03  8:54 ` Gleb Natapov
2013-09-03  8:54   ` [Qemu-devel] " Gleb Natapov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.