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

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

[Dustin Kirkland]

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

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>

[-- Attachment #2: 04_fix-no-kvm-segfault.patch --]
[-- Type: text/x-diff, Size: 1380 bytes --]

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

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

[Marcelo Tosatti]

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

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

[Mark McLoughlin]

On Thu, 2009-09-03 at 12:31 -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.

Ah discussion about an alternative fix for this fizzled out recently:

  http://www.mail-archive.com/kvm@vger.kernel.org/msg19890.html

Cheers,
Mark.

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

[Luiz Capitulino]

On Thu, 3 Sep 2009 12:31:33 -0500
Dustin Kirkland <kirkland@canonical.com> 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>

 Fixes the problem to me:

 Tested-by: Luiz Capitulino <lcapitulino@redhat.com>

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

[Dustin Kirkland]

On Thu, Sep 3, 2009 at 3:01 PM, Mark McLoughlin<markmc@redhat.com> wrote:
> On Thu, 2009-09-03 at 12:31 -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.
>
> Ah discussion about an alternative fix for this fizzled out recently:
>
>  http://www.mail-archive.com/kvm@vger.kernel.org/msg19890.html

Ah, thanks Mark.  In that thread, I found Daniel's suggestion the most
reasonable, and user-friendly:

On Mon, Jul 27, 2009 at 1:44 PM, Daniel P. Berrange<berrange@redhat.com> wrote:
> Well, we could go for logic like:
>
>  * No arg given          => try kvm, try kqemu, try tcg
>  * --accelmode arg given => try $arg, and fail if unavailable
>
> then libvirt would simply always supply --accelmode for all VMs,
> while people running qemu manually would get best available



:-Dustin

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

[Glauber Costa]

On Thu, Sep 3, 2009 at 6:48 PM, Dustin Kirkland<kirkland@canonical.com> wrote:
> On Thu, Sep 3, 2009 at 3:01 PM, Mark McLoughlin<markmc@redhat.com> wrote:
>> On Thu, 2009-09-03 at 12:31 -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.
>>
>> Ah discussion about an alternative fix for this fizzled out recently:
>>
>>  http://www.mail-archive.com/kvm@vger.kernel.org/msg19890.html
>
> Ah, thanks Mark.  In that thread, I found Daniel's suggestion the most
> reasonable, and user-friendly:
>
> On Mon, Jul 27, 2009 at 1:44 PM, Daniel P. Berrange<berrange@redhat.com> wrote:
>> Well, we could go for logic like:
>>
>>  * No arg given          => try kvm, try kqemu, try tcg
>>  * --accelmode arg given => try $arg, and fail if unavailable
>>
>> then libvirt would simply always supply --accelmode for all VMs,
>> while people running qemu manually would get best available
I sent some patches to do that, but they were incomplete, and I was
preempted by something else.
If you want, you can wait for my cycles to come back, or pick from where I left



-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

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

[Mark McLoughlin]

On Thu, 2009-09-03 at 19:18 -0300, Glauber Costa wrote:
> On Thu, Sep 3, 2009 at 6:48 PM, Dustin Kirkland<kirkland@canonical.com> wrote:
> > On Thu, Sep 3, 2009 at 3:01 PM, Mark McLoughlin<markmc@redhat.com> wrote:
> >> On Thu, 2009-09-03 at 12:31 -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.
> >>
> >> Ah discussion about an alternative fix for this fizzled out recently:
> >>
> >>  http://www.mail-archive.com/kvm@vger.kernel.org/msg19890.html
> >
> > Ah, thanks Mark.  In that thread, I found Daniel's suggestion the most
> > reasonable, and user-friendly:
> >
> > On Mon, Jul 27, 2009 at 1:44 PM, Daniel P. Berrange<berrange@redhat.com> wrote:
> >> Well, we could go for logic like:
> >>
> >>  * No arg given          => try kvm, try kqemu, try tcg
> >>  * --accelmode arg given => try $arg, and fail if unavailable
> >>
> >> then libvirt would simply always supply --accelmode for all VMs,
> >> while people running qemu manually would get best available
> I sent some patches to do that, but they were incomplete, and I was
> preempted by something else.
> If you want, you can wait for my cycles to come back, or pick from where I left

In the meantime, can we commit to stable-0.11 either Dustin's fix or
this:

  http://git.et.redhat.com/?p=qemu-fedora.git;a=commitdiff;h=aa1620047b

Cheers,
Mark.

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

[Dustin Kirkland]

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

On Fri, 2009-09-04 at 08:22 +0100, Mark McLoughlin wrote:
> On Thu, 2009-09-03 at 19:18 -0300, Glauber Costa wrote:
> > On Thu, Sep 3, 2009 at 6:48 PM, Dustin Kirkland<kirkland@canonical.com> wrote:
> > > On Thu, Sep 3, 2009 at 3:01 PM, Mark McLoughlin<markmc@redhat.com> wrote:
> > >> On Thu, 2009-09-03 at 12:31 -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.
> > >>
> > >> Ah discussion about an alternative fix for this fizzled out recently:
> > >>
> > >>  http://www.mail-archive.com/kvm@vger.kernel.org/msg19890.html
> > >
> > > Ah, thanks Mark.  In that thread, I found Daniel's suggestion the most
> > > reasonable, and user-friendly:
> > >
> > > On Mon, Jul 27, 2009 at 1:44 PM, Daniel P. Berrange<berrange@redhat.com> wrote:
> > >> Well, we could go for logic like:
> > >>
> > >>  * No arg given          => try kvm, try kqemu, try tcg
> > >>  * --accelmode arg given => try $arg, and fail if unavailable
> > >>
> > >> then libvirt would simply always supply --accelmode for all VMs,
> > >> while people running qemu manually would get best available
> > I sent some patches to do that, but they were incomplete, and I was
> > preempted by something else.
> > If you want, you can wait for my cycles to come back, or pick from where I left

Thanks for the pointer, Glauber.  My cycles a bit constrained too, but
I'll have a look when I get a chance.

> In the meantime, can we commit to stable-0.11 either Dustin's fix or
> this:
> 
>   http://git.et.redhat.com/?p=qemu-fedora.git;a=commitdiff;h=aa1620047b

+1.  We're looking for something agreeable in stable-0.11, that solves
the segfault and proceeds without VT acceleration.


-- 
:-Dustin

Dustin Kirkland
Canonical, LTD
kirkland@canonical.com
GPG: 1024D/83A61194

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

[Marcelo Tosatti]

On Fri, Sep 04, 2009 at 11:06:38AM -0500, Dustin Kirkland wrote:
> On Fri, 2009-09-04 at 08:22 +0100, Mark McLoughlin wrote:
> > On Thu, 2009-09-03 at 19:18 -0300, Glauber Costa wrote:
> > > On Thu, Sep 3, 2009 at 6:48 PM, Dustin Kirkland<kirkland@canonical.com> wrote:
> > > > On Thu, Sep 3, 2009 at 3:01 PM, Mark McLoughlin<markmc@redhat.com> wrote:
> > > >> On Thu, 2009-09-03 at 12:31 -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.
> > > >>
> > > >> Ah discussion about an alternative fix for this fizzled out recently:
> > > >>
> > > >>  http://www.mail-archive.com/kvm@vger.kernel.org/msg19890.html
> > > >
> > > > Ah, thanks Mark.  In that thread, I found Daniel's suggestion the most
> > > > reasonable, and user-friendly:
> > > >
> > > > On Mon, Jul 27, 2009 at 1:44 PM, Daniel P. Berrange<berrange@redhat.com> wrote:
> > > >> Well, we could go for logic like:
> > > >>
> > > >>  * No arg given          => try kvm, try kqemu, try tcg
> > > >>  * --accelmode arg given => try $arg, and fail if unavailable
> > > >>
> > > >> then libvirt would simply always supply --accelmode for all VMs,
> > > >> while people running qemu manually would get best available
> > > I sent some patches to do that, but they were incomplete, and I was
> > > preempted by something else.
> > > If you want, you can wait for my cycles to come back, or pick from where I left
> 
> Thanks for the pointer, Glauber.  My cycles a bit constrained too, but
> I'll have a look when I get a chance.
> 
> > In the meantime, can we commit to stable-0.11 either Dustin's fix or
> > this:
> > 
> >   http://git.et.redhat.com/?p=qemu-fedora.git;a=commitdiff;h=aa1620047b
> 
> +1.  We're looking for something agreeable in stable-0.11, that solves
> the segfault and proceeds without VT acceleration.

Dustin,

Can you please resend the patch with the suggestion i made earlier, for
stable-0.11?

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

[Dustin Kirkland]

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

On Fri, Sep 4, 2009 at 11:36 AM, Marcelo Tosatti<mtosatti@redhat.com> wrote:
> On Fri, Sep 04, 2009 at 11:06:38AM -0500, Dustin Kirkland wrote:
>> On Fri, 2009-09-04 at 08:22 +0100, Mark McLoughlin wrote:
>> > On Thu, 2009-09-03 at 19:18 -0300, Glauber Costa wrote:
>> > > On Thu, Sep 3, 2009 at 6:48 PM, Dustin Kirkland<kirkland@canonical.com> wrote:
>> > > > On Thu, Sep 3, 2009 at 3:01 PM, Mark McLoughlin<markmc@redhat.com> wrote:
>> > > >> On Thu, 2009-09-03 at 12:31 -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.
>> > > >>
>> > > >> Ah discussion about an alternative fix for this fizzled out recently:
>> > > >>
>> > > >>  http://www.mail-archive.com/kvm@vger.kernel.org/msg19890.html
>> > > >
>> > > > Ah, thanks Mark.  In that thread, I found Daniel's suggestion the most
>> > > > reasonable, and user-friendly:
>> > > >
>> > > > On Mon, Jul 27, 2009 at 1:44 PM, Daniel P. Berrange<berrange@redhat.com> wrote:
>> > > >> Well, we could go for logic like:
>> > > >>
>> > > >>  * No arg given          => try kvm, try kqemu, try tcg
>> > > >>  * --accelmode arg given => try $arg, and fail if unavailable
>> > > >>
>> > > >> then libvirt would simply always supply --accelmode for all VMs,
>> > > >> while people running qemu manually would get best available
>> > > I sent some patches to do that, but they were incomplete, and I was
>> > > preempted by something else.
>> > > If you want, you can wait for my cycles to come back, or pick from where I left
>>
>> Thanks for the pointer, Glauber.  My cycles a bit constrained too, but
>> I'll have a look when I get a chance.
>>
>> > In the meantime, can we commit to stable-0.11 either Dustin's fix or
>> > this:
>> >
>> >   http://git.et.redhat.com/?p=qemu-fedora.git;a=commitdiff;h=aa1620047b
>>
>> +1.  We're looking for something agreeable in stable-0.11, that solves
>> the segfault and proceeds without VT acceleration.
>
> Dustin,
>
> Can you please resend the patch with the suggestion i made earlier, for
> stable-0.11?

Sure, Marcelo.  It's attached.

I tested it, and it still does avoid the segfault.

Luiz, could you re-test this patch on your side too?

:-Dustin

[-- Attachment #2: fix-no-dev-kvm-segfault.patch --]
[-- Type: text/x-diff, Size: 1833 bytes --]

qemu-kvm: fix segfault when running kvm without /dev/kvm

qemu-kvm segfaults on systems without access to /dev/kvm.
The global kvm_allowed is being set too late in vl.c.

This patch moves the kvm initialization a bit higher in the
vl.c main, just after the daemonize fork.

This fix is intended to be a short term solution, solving the
segfaults.

In the longer term, the suggested approach requires a bit more
development and testing:
 * If no arg given          => try kvm, try kqemu, try tcg
 * If --accelmode arg given => try $arg, and fail if unavailable

Signed-off-by: Dustin Kirkland <kirkland@canonical.com>

diff --git a/vl.c b/vl.c
index db75470..26bced8 100644
--- a/vl.c
+++ b/vl.c
@@ -5831,6 +5831,20 @@ int main(int argc, char **argv, char **envp)
     }
 #endif
 
+    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;
+        }
+    }
+
 #ifdef CONFIG_KQEMU
     if (smp_cpus > 1)
         kqemu_allowed = 0;
@@ -6002,20 +6016,6 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
-    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) {

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

[Luiz Capitulino]

On Fri, 4 Sep 2009 14:38:30 -0500
Dustin Kirkland <kirkland@canonical.com> wrote:

> On Fri, Sep 4, 2009 at 11:36 AM, Marcelo Tosatti<mtosatti@redhat.com> wrote:
> > On Fri, Sep 04, 2009 at 11:06:38AM -0500, Dustin Kirkland wrote:
> >> On Fri, 2009-09-04 at 08:22 +0100, Mark McLoughlin wrote:
> >> > On Thu, 2009-09-03 at 19:18 -0300, Glauber Costa wrote:
> >> > > On Thu, Sep 3, 2009 at 6:48 PM, Dustin Kirkland<kirkland@canonical.com> wrote:
> >> > > > On Thu, Sep 3, 2009 at 3:01 PM, Mark McLoughlin<markmc@redhat.com> wrote:
> >> > > >> On Thu, 2009-09-03 at 12:31 -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.
> >> > > >>
> >> > > >> Ah discussion about an alternative fix for this fizzled out recently:
> >> > > >>
> >> > > >>  http://www.mail-archive.com/kvm@vger.kernel.org/msg19890.html
> >> > > >
> >> > > > Ah, thanks Mark.  In that thread, I found Daniel's suggestion the most
> >> > > > reasonable, and user-friendly:
> >> > > >
> >> > > > On Mon, Jul 27, 2009 at 1:44 PM, Daniel P. Berrange<berrange@redhat.com> wrote:
> >> > > >> Well, we could go for logic like:
> >> > > >>
> >> > > >>  * No arg given          => try kvm, try kqemu, try tcg
> >> > > >>  * --accelmode arg given => try $arg, and fail if unavailable
> >> > > >>
> >> > > >> then libvirt would simply always supply --accelmode for all VMs,
> >> > > >> while people running qemu manually would get best available
> >> > > I sent some patches to do that, but they were incomplete, and I was
> >> > > preempted by something else.
> >> > > If you want, you can wait for my cycles to come back, or pick from where I left
> >>
> >> Thanks for the pointer, Glauber.  My cycles a bit constrained too, but
> >> I'll have a look when I get a chance.
> >>
> >> > In the meantime, can we commit to stable-0.11 either Dustin's fix or
> >> > this:
> >> >
> >> >   http://git.et.redhat.com/?p=qemu-fedora.git;a=commitdiff;h=aa1620047b
> >>
> >> +1.  We're looking for something agreeable in stable-0.11, that solves
> >> the segfault and proceeds without VT acceleration.
> >
> > Dustin,
> >
> > Can you please resend the patch with the suggestion i made earlier, for
> > stable-0.11?
> 
> Sure, Marcelo.  It's attached.
> 
> I tested it, and it still does avoid the segfault.
> 
> Luiz, could you re-test this patch on your side too?

 I'm getting rejections, are you sure it's against upstream?

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

[Marcelo Tosatti]

On Fri, Sep 04, 2009 at 06:39:25PM -0300, Luiz Capitulino wrote:
> > Sure, Marcelo.  It's attached.
> > 
> > I tested it, and it still does avoid the segfault.
> > 
> > Luiz, could you re-test this patch on your side too?
> 
>  I'm getting rejections, are you sure it's against upstream?

Its againts 0.11-stable branch. Try this against master:

qemu-kvm segfaults on systems without access to /dev/kvm.
The global kvm_allowed is being set too late in vl.c.

This patch moves the kvm initialization a bit higher in the
vl.c main, just after the daemonize fork.

This fix is intended to be a short term solution, solving the
segfaults.

In the longer term, the suggested approach requires a bit more
development and testing:
 * If no arg given          => try kvm, try kqemu, try tcg
 * If --accelmode arg given => try $arg, and fail if unavailable

Signed-off-by: Dustin Kirkland <kirkland@canonical.com>

diff --git a/vl.c b/vl.c
index 9f03d85..3485ce6 100644
--- a/vl.c
+++ b/vl.c
@@ -5823,6 +5823,20 @@ int main(int argc, char **argv, char **envp)
         signal(SIGTTIN, SIG_IGN);
     }
 
+    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 (pid_file && qemu_create_pidfile(pid_file) != 0) {
         if (daemonize) {
             uint8_t status = 1;
@@ -5983,20 +5997,6 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
-    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) {

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

[Luiz Capitulino]

On Fri, 4 Sep 2009 18:46:23 -0300
Marcelo Tosatti <mtosatti@redhat.com> wrote:

> On Fri, Sep 04, 2009 at 06:39:25PM -0300, Luiz Capitulino wrote:
> > > Sure, Marcelo.  It's attached.
> > > 
> > > I tested it, and it still does avoid the segfault.
> > > 
> > > Luiz, could you re-test this patch on your side too?
> > 
> >  I'm getting rejections, are you sure it's against upstream?
> 
> Its againts 0.11-stable branch. 

 duh. :)

 Works for me on 0.11-stable:

 Tested-by: Luiz Capitulino <lcapitulino@redhat.com>