[PATCH] exit if we fail to initialize kvm

[PATCH] exit if we fail to initialize kvm

[Glauber Costa]

Falling back to tcg has proven to be evil through time. The option is to
do not try to act behind user's back, and quit the program completely if
we fail to initialize kvm. Right now, the only way to run tcg from our tree
becomes explicitly asking for it, with the -no-kvm option.

But it will change when upstream accepts the --accel option.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 vl.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index 30c4ff9..cab62cb 100644
--- a/vl.c
+++ b/vl.c
@@ -5926,12 +5926,8 @@ int main(int argc, char **argv, char **envp)
 
         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;
         }
     }
 
-- 
1.6.2.2

Re: [PATCH] exit if we fail to initialize kvm

[Alexander Graf]

On 28.07.2009, at 22:52, Glauber Costa <glommer@redhat.com> wrote:

> Falling back to tcg has proven to be evil through time. The option  
> is to
> do not try to act behind user's back, and quit the program  
> completely if
> we fail to initialize kvm. Right now, the only way to run tcg from  
> our tree
> becomes explicitly asking for it, with the -no-kvm option.

Full ack. I have a patch for that in the suse version for some time  
now, because it really annoyed me.

Alex

Re: [PATCH] exit if we fail to initialize kvm

[Alexander Graf]

On 28.07.2009, at 22:52, Glauber Costa <glommer@redhat.com> wrote:

> Falling back to tcg has proven to be evil through time. The option  
> is to
> do not try to act behind user's back, and quit the program  
> completely if
> we fail to initialize kvm. Right now, the only way to run tcg from  
> our tree
> becomes explicitly asking for it, with the -no-kvm option.

Well, actually there's one little difference: I tell the user to use - 
no-kvm if he really wants cpu emulation.

But simply failing is probably good enough.


Alex

Re: [PATCH] exit if we fail to initialize kvm

[Glauber Costa]

On Tue, Jul 28, 2009 at 11:15:19PM +0200, Alexander Graf wrote:
>
> On 28.07.2009, at 22:52, Glauber Costa <glommer@redhat.com> wrote:
>
>> Falling back to tcg has proven to be evil through time. The option is 
>> to
>> do not try to act behind user's back, and quit the program completely 
>> if
>> we fail to initialize kvm. Right now, the only way to run tcg from our 
>> tree
>> becomes explicitly asking for it, with the -no-kvm option.
>
> Well, actually there's one little difference: I tell the user to use - 
> no-kvm if he really wants cpu emulation.
>
> But simply failing is probably good enough.
With my patch, we won't fail if the user asked -no-kvm, because then we won't
even try to initialize kvm.

We only exit here, if we try, but fail

Re: [PATCH] exit if we fail to initialize kvm

[Alexander Graf]

On 28.07.2009, at 23:28, Glauber Costa <glommer@redhat.com> wrote:

> On Tue, Jul 28, 2009 at 11:15:19PM +0200, Alexander Graf wrote:
>>
>> On 28.07.2009, at 22:52, Glauber Costa <glommer@redhat.com> wrote:
>>
>>> Falling back to tcg has proven to be evil through time. The option  
>>> is
>>> to
>>> do not try to act behind user's back, and quit the program  
>>> completely
>>> if
>>> we fail to initialize kvm. Right now, the only way to run tcg from  
>>> our
>>> tree
>>> becomes explicitly asking for it, with the -no-kvm option.
>>
>> Well, actually there's one little difference: I tell the user to  
>> use -
>> no-kvm if he really wants cpu emulation.
>>
>> But simply failing is probably good enough.
> With my patch, we won't fail if the user asked -no-kvm, because then  
> we won't
> even try to initialize kvm.
>
> We only exit here, if we try, but fail
>

Right, the difference is that instead of saying "initializing kvm  
failed" it would give the user some advice on what happened and what  
to do next.

Alex

Re: [PATCH] exit if we fail to initialize kvm

[Alexander Graf]

On 28.07.2009, at 23:28, Glauber Costa wrote:

> On Tue, Jul 28, 2009 at 11:15:19PM +0200, Alexander Graf wrote:
>>
>> On 28.07.2009, at 22:52, Glauber Costa <glommer@redhat.com> wrote:
>>
>>> Falling back to tcg has proven to be evil through time. The option  
>>> is
>>> to
>>> do not try to act behind user's back, and quit the program  
>>> completely
>>> if
>>> we fail to initialize kvm. Right now, the only way to run tcg from  
>>> our
>>> tree
>>> becomes explicitly asking for it, with the -no-kvm option.
>>
>> Well, actually there's one little difference: I tell the user to  
>> use -
>> no-kvm if he really wants cpu emulation.
>>
>> But simply failing is probably good enough.
> With my patch, we won't fail if the user asked -no-kvm, because then  
> we won't
> even try to initialize kvm.
>
> We only exit here, if we try, but fail

This is the patch as I had it in kvm-86. It's really only about being  
helpful to the user.

Index: kvm-86/vl.c
===================================================================
--- kvm-86.orig/vl.c
+++ kvm-86/vl.c
@@ -5836,7 +5836,8 @@ int main(int argc, char **argv, char **e
  #ifdef USE_KVM
      if (kvm_enabled()) {
  	if (kvm_qemu_init() < 0) {
-	    fprintf(stderr, "Could not initialize KVM, will disable KVM  
support\n");
+	    fprintf(stderr, "Could not initialize KVM. Do you have kvm-amd  
or kvm-intel modprobe'd?\nIf you want to use CPU emulation, start with  
-no-kvm.\n");
+	    exit(1);
  #ifdef NO_CPU_EMULATION
  	    fprintf(stderr, "Compiled with --disable-cpu-emulation, exiting. 
\n");
  	    exit(1);


Alex

Re: [PATCH] exit if we fail to initialize kvm

[Jan Kiszka]

Alexander Graf wrote:
> 
> On 28.07.2009, at 23:28, Glauber Costa wrote:
> 
>> On Tue, Jul 28, 2009 at 11:15:19PM +0200, Alexander Graf wrote:
>>>
>>> On 28.07.2009, at 22:52, Glauber Costa <glommer@redhat.com> wrote:
>>>
>>>> Falling back to tcg has proven to be evil through time. The option is
>>>> to
>>>> do not try to act behind user's back, and quit the program completely
>>>> if
>>>> we fail to initialize kvm. Right now, the only way to run tcg from our
>>>> tree
>>>> becomes explicitly asking for it, with the -no-kvm option.
>>>
>>> Well, actually there's one little difference: I tell the user to use -
>>> no-kvm if he really wants cpu emulation.
>>>
>>> But simply failing is probably good enough.
>> With my patch, we won't fail if the user asked -no-kvm, because then
>> we won't
>> even try to initialize kvm.
>>
>> We only exit here, if we try, but fail
> 
> This is the patch as I had it in kvm-86. It's really only about being
> helpful to the user.
> 
> Index: kvm-86/vl.c
> ===================================================================
> --- kvm-86.orig/vl.c
> +++ kvm-86/vl.c
> @@ -5836,7 +5836,8 @@ int main(int argc, char **argv, char **e
>  #ifdef USE_KVM
>      if (kvm_enabled()) {
>      if (kvm_qemu_init() < 0) {
> -        fprintf(stderr, "Could not initialize KVM, will disable KVM
> support\n");
> +        fprintf(stderr, "Could not initialize KVM. Do you have kvm-amd
> or kvm-intel modprobe'd?\nIf you want to use CPU emulation, start with
> -no-kvm.\n");
> +        exit(1);
>  #ifdef NO_CPU_EMULATION
>          fprintf(stderr, "Compiled with --disable-cpu-emulation,
> exiting.\n");
>          exit(1);

Yes, that's a useful hint which should be included.

I just wonder now if/when qemu-kvm will switch over to the
kvm-by-default-off policy of upstream?

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [PATCH] exit if we fail to initialize kvm

[Avi Kivity]

On 07/29/2009 01:17 PM, Jan Kiszka wrote:
> I just wonder now if/when qemu-kvm will switch over to the
> kvm-by-default-off policy of upstream?
>    

That will surely inconvenience/surprise a lot of users.

A migration path could be:

- add -accel
- start warning when -accel is not used, encouraging people to use -accel
- time passes
- switch default to tcg (maybe unless invoked with kvm in argv[0])
- time passes
- remove warning

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] exit if we fail to initialize kvm

[Alexander Graf]

Avi Kivity wrote:
> On 07/29/2009 01:17 PM, Jan Kiszka wrote:
>> I just wonder now if/when qemu-kvm will switch over to the
>> kvm-by-default-off policy of upstream?
>>    
>
> That will surely inconvenience/surprise a lot of users.
>
> A migration path could be:
>
> - add -accel
> - start warning when -accel is not used, encouraging people to use -accel
> - time passes
> - switch default to tcg (maybe unless invoked with kvm in argv[0])
> - time passes
> - remove warning
>

I would prefer:

- add -accel (default to kvm)
- give a message to use -accel tcg when the user wants tcg
- make -accel go kvm,kqemu,tcg in qemu upstream
- have distros / management tools decide which accel they like

Alex

Re: [PATCH] exit if we fail to initialize kvm

[Avi Kivity]

On 07/29/2009 03:28 PM, Alexander Graf wrote:
> Avi Kivity wrote:
>    
>> On 07/29/2009 01:17 PM, Jan Kiszka wrote:
>>      
>>> I just wonder now if/when qemu-kvm will switch over to the
>>> kvm-by-default-off policy of upstream?
>>>
>>>        
>> That will surely inconvenience/surprise a lot of users.
>>
>> A migration path could be:
>>
>> - add -accel
>> - start warning when -accel is not used, encouraging people to use -accel
>> - time passes
>> - switch default to tcg (maybe unless invoked with kvm in argv[0])
>> - time passes
>> - remove warning
>>
>>      
>
> I would prefer:
>
> - add -accel (default to kvm)
> - give a message to use -accel tcg when the user wants tcg
> - make -accel go kvm,kqemu,tcg in qemu upstream
> - have distros / management tools decide which accel they like
>    

I like this better.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] exit if we fail to initialize kvm

[Jan Kiszka]

Alexander Graf wrote:
> Avi Kivity wrote:
>> On 07/29/2009 01:17 PM, Jan Kiszka wrote:
>>> I just wonder now if/when qemu-kvm will switch over to the
>>> kvm-by-default-off policy of upstream?
>>>    
>> That will surely inconvenience/surprise a lot of users.
>>
>> A migration path could be:
>>
>> - add -accel
>> - start warning when -accel is not used, encouraging people to use -accel
>> - time passes
>> - switch default to tcg (maybe unless invoked with kvm in argv[0])
>> - time passes
>> - remove warning
>>
> 
> I would prefer:
> 
> - add -accel (default to kvm)
> - give a message to use -accel tcg when the user wants tcg
> - make -accel go kvm,kqemu,tcg in qemu upstream

Upstream just dropped the behavior '-accel kqemu,tcg', so I'm not sure
if this is desired to return (though I'm surely not comparing kvm to
kqemu here).

> - have distros / management tools decide which accel they like

That's likely the key: explicit -accel.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [PATCH] exit if we fail to initialize kvm

[Glauber Costa]

On Wed, Jul 29, 2009 at 04:43:11PM +0200, Jan Kiszka wrote:
> Alexander Graf wrote:
> > Avi Kivity wrote:
> >> On 07/29/2009 01:17 PM, Jan Kiszka wrote:
> >>> I just wonder now if/when qemu-kvm will switch over to the
> >>> kvm-by-default-off policy of upstream?
> >>>    
> >> That will surely inconvenience/surprise a lot of users.
> >>
> >> A migration path could be:
> >>
> >> - add -accel
> >> - start warning when -accel is not used, encouraging people to use -accel
> >> - time passes
> >> - switch default to tcg (maybe unless invoked with kvm in argv[0])
> >> - time passes
> >> - remove warning
> >>
> > 
> > I would prefer:
> > 
> > - add -accel (default to kvm)
> > - give a message to use -accel tcg when the user wants tcg
> > - make -accel go kvm,kqemu,tcg in qemu upstream
> 
> Upstream just dropped the behavior '-accel kqemu,tcg', so I'm not sure
> if this is desired to return (though I'm surely not comparing kvm to
> kqemu here).
> 
> > - have distros / management tools decide which accel they like
> 
> That's likely the key: explicit -accel.
While I do understand the value of backwards compatibility, we did change
behaviour of a number of things in the past. Example: "qemu" would print a help message,
and now it runs without any disks. It changed people's script already.

It should probably be okay to drop all kvm,kqemu,whatever-related options in favour
of accel if we are doing this in a release boundary. 

Re: [PATCH] exit if we fail to initialize kvm

[Avi Kivity]

On 07/29/2009 06:50 PM, Glauber Costa wrote:While I do understand the 
value of backwards compatibility, we did change
> behaviour of a number of things in the past. Example: "qemu" would print a help message,
> and now it runs without any disks. It changed people's script already.
>    

There's a huge difference in those two cases.  I don't want people's 
guests to silently lose 90% of their performance.

> It should probably be okay to drop all kvm,kqemu,whatever-related options in favour
> of accel if we are doing this in a release boundary.

Maintaining some backward compatibility is helpful.

-- 
error compiling committee.c: too many arguments to function