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

2009-09-04 Thread Luiz Capitulino
On Fri, 4 Sep 2009 18:46:23 -0300
Marcelo Tosatti  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 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2009-09-04 Thread 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 

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) {
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2009-09-04 Thread Luiz Capitulino
On Fri, 4 Sep 2009 14:38:30 -0500
Dustin Kirkland  wrote:

> On Fri, Sep 4, 2009 at 11:36 AM, Marcelo Tosatti 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 wrote:
> >> > > > On Thu, Sep 3, 2009 at 3:01 PM, Mark McLoughlin 
> >> > > > 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 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?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html