Re: [PATCH] qemu-kvm: fix segfault when running kvm without /dev/kvm, falling back to non-accelerated mode

Marcelo Tosatti
2009-09-03T19:56:17Z

On Thu, Sep 03, 2009 at 12:31:33PM -0500, Dustin Kirkland wrote:
> qemu-kvm: fix segfault when running kvm without /dev/kvm, falling back
> to non-accelerated mode
>
> We're seeing segfaults on systems without access to /dev/kvm. It
> looks like the global kvm_allowed is being set just a little too late
> in vl.c. This patch moves the kvm initialization a bit higher in the
> vl.c main, just after options processing, and solves the segfaults.
> We're carrying this patch in Ubuntu 9.10 Alpha. Please apply
> upstream, or advise if and why this might not be the optimal solution.
>
> Signed-off-by: Dustin Kirkland <kirkland@canonical.com>

Dustin,

I think its safer to move it just after fork() from -daemonize, to
make sure no state initialized by kvm_init is lost in the child.

> Move the kvm_init() call a bit higher to fix a segfault when
> /dev/kvm is not available. The kvm_allowed global needs
> to be set correctly a little earlier.
>
> Signed-off-by: Dustin Kirkland <kirkland@canonical.com>
>
>
> --- qemu-kvm-0.11.0~rc1.orig/vl.c
> +++ qemu-kvm-0.11.0~rc1/vl.c
> @@ -5748,6 +5748,20 @@
> }
> }
>
> + if (kvm_enabled()) {
> + int ret;
> +
> + ret = kvm_init(smp_cpus);
> + if (ret < 0) {
> +#if defined(KVM_UPSTREAM) || defined(NO_CPU_EMULATION)
> + fprintf(stderr, "failed to initialize KVM\n");
> + exit(1);
> +#endif
> + fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
> + kvm_allowed = 0;
> + }
> + }
> +
> /* If no data_dir is specified then try to find it relative to the
> executable path. */
> if (!data_dir) {
> @@ -6008,20 +6022,6 @@
> }
> }
>
> - if (kvm_enabled()) {
> - int ret;
> -
> - ret = kvm_init(smp_cpus);
> - if (ret < 0) {
> -#if defined(KVM_UPSTREAM) || defined(NO_CPU_EMULATION)
> - fprintf(stderr, "failed to initialize KVM\n");
> - exit(1);
> -#endif
> - fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
> - kvm_allowed = 0;
> - }
> - }
> -
> if (monitor_device) {
> monitor_hd = qemu_chr_open("monitor", monitor_device, NULL);
> if (!monitor_hd) {