From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH] qemu-kvm: fix segfault when running kvm without
	/dev/kvm, falling back to non-accelerated mode
Date: Thu, 3 Sep 2009 16:55:10 -0300
Message-ID: <20090903195510.GA11497@amt.cnet>
References: <d9c105ea0909031031g463aa731j846dd73d4c158ebc@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org
To: Dustin Kirkland <kirkland@canonical.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:8335 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1756196AbZICT4R (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 3 Sep 2009 15:56:17 -0400
Content-Disposition: inline
In-Reply-To: <d9c105ea0909031031g463aa731j846dd73d4c158ebc@mail.gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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) {