Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] vl.c: Fix max_cpus

[Stefan Hajnoczi]

On Wed, Jul 25, 2012 at 12:11 PM,  <address@hidden> wrote:
> From: Dunrong Huang <address@hidden>
>
> The VCPU count limit in kernel now is 254, defined by KVM_MAX_VCPUS
> in kernel's header files. But the count limit in QEMU is 255,
> so QEMU will failed to start if user passes "-enable-kvm" and "-smp 255"
> to it.
>
> Exit QEMU with an error if KVM is enabled and number of SMP cpus requested
> exceeds KVM_MAX_VCPUS.
>
> Signed-off-by: Dunrong Huang <address@hidden>
> ---
> v1 -> v2:
> Checking if the number of smp cpus requested exceeds KVM limit count
> if and only if kvm is enabled.
>
>  vl.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 8904db1..cdd1c96 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -169,6 +169,11 @@ int main(int argc, char **argv)
>
>  #define MAX_VIRTIO_CONSOLES 1
>
> +/* KVM_MAX_VCPUS defined in kernel's header files */
> +#ifndef KVM_MAX_VCPUS
> +#define KVM_MAX_VCPUS 254
> +#endif
> +
>  static const char *data_dir;
>  const char *bios_name = NULL;
>  enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
> @@ -3348,6 +3353,12 @@ int main(int argc, char **argv, char **envp)
>
>      configure_accelerator();
>
> +    if (kvm_enabled() && smp_cpus > KVM_MAX_VCPUS) {
> +        fprintf(stderr, "Number of SMP cpus requested (%d) exceeds max cpus "
> +                "supported by KVM (%d)\n", smp_cpus, KVM_MAX_VCPUS);
> +        exit(1);
> +    }

After a little discussion on IRC, two points emerged:
1. Use KVM_CAP_MAX_VCPUS to query the max number of vcpus at runtime.
2. We should fail gracefully when ioctl() fails.

In other words, using the KVM_MAX_VCPUS value from userspace isn't a
good idea.  Imagine what happens when the user upgrades their kernel
without recompiling QEMU.  If the KVM_MAX_VCPUS value increased in the
kernel QEMU would not know.

Please either drop this patch completely or at least using
KVM_CAP_MAX_VCPUS so we fetch the maximum value at runtime.

Stefan