[PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[Markus Armbruster]

We currently enable KVM by default, and when it's not available, we
print a message and fall back to TCG.  Option -enable-kvm is ignored.
Option -no-kvm suppresses KVM.

Upstream works differently: KVM is off by default, -enable-kvm
switches it on.  -enable-kvm terminates the process unsuccessfully if
KVM is not available.

upstream qemu   |  default  |-enable-kvm
----------------+-----------+-----------
KVM available   | disabled  |  enabled
KVM unavailable | disabled  |    fail

qemu-kvm        |  default  |-enable-kvm|  -no-kvm
----------------+-----------+-----------+----------
KVM available   |  enabled* |  enabled  |  disabled
KVM unavailable | disabled  | disabled* |  disabled

* differs from upstream

Users of qemu and qemu-kvm need to be aware of these differences to
enable / disable use of KVM reliably.  This is bothersome.

Consider -enable-kvm when KVM is unavailable: If the user expects
qemu-kvm behavior (fall back), but qemu fails, he'll likely be
surprised and unhappy.  If the user expects upstream behavior (fail),
but qemu-kvm falls back to TCG, the guest runs slow as molasses, and
the user will likely be confused and unhappy (unless he spots and
understands the "disable KVM" message).

Eventually, we'll sort this upstream with -accel (defaults tied to
machine type).  Until then, this patch reduces the difference to
upstream so that most users shouldn't need to be aware of them.

Make -enable-kvm behave just like in upstream: enable KVM, fail if not
available.  But retain current default behavior: enable KVM, fall back
to TCG.

qemu-kvm new    |  default  |-enable-kvm|  -no-kvm
----------------+-----------+-----------+-----------
KVM available   |  enabled* |  enabled  |  disabled
KVM unavailable | disabled  |    fail+  |  disabled

* differs from upstream
+ changed by this patch

Bonus fix: -no-kvm -enable-kvm now enables KVM.  Before, it disabled it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 vl.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/vl.c b/vl.c
index e3c8919..1958e01 100644
--- a/vl.c
+++ b/vl.c
@@ -247,7 +247,7 @@ static void *boot_set_opaque;
 static NotifierList exit_notifiers =
     NOTIFIER_LIST_INITIALIZER(exit_notifiers);
 
-int kvm_allowed = 1;
+int kvm_allowed = -1;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
 
@@ -2436,10 +2436,8 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_smbios:
                 do_smbios_option(optarg);
                 break;
-#ifdef OBSOLETE_KVM_IMPL
             case QEMU_OPTION_enable_kvm:
                 kvm_allowed = 1;
-#endif
                 break;
 	    case QEMU_OPTION_no_kvm:
 		kvm_allowed = 0;
@@ -2789,19 +2787,17 @@ int main(int argc, char **argv, char **envp)
     if (kvm_allowed) {
         int ret = kvm_init(smp_cpus);
         if (ret < 0) {
-#if defined(OBSOLETE_KVM_IMPL) || defined(CONFIG_NO_CPU_EMULATION)
-            if (!kvm_available()) {
-                printf("KVM not supported for this target\n");
-            } else {
-                fprintf(stderr, "failed to initialize KVM: %s\n", strerror(-ret));
+            if (kvm_allowed > 0) {
+                if (!kvm_available()) {
+                    printf("KVM not supported for this target\n");
+                } else {
+                    fprintf(stderr, "failed to initialize KVM: %s\n", strerror(-ret));
+                }
+                exit(1);
             }
-            exit(1);
-#endif
-#ifdef CONFIG_KVM
             fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
-            kvm_allowed = 0;
-#endif
         }
+        kvm_allowed = ret >= 0;
     }
 
     if (qemu_init_main_loop()) {
-- 
1.7.2.3

Re: [PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[Avi Kivity]

On 12/22/2010 02:33 PM, Markus Armbruster wrote:
> We currently enable KVM by default, and when it's not available, we
> print a message and fall back to TCG.  Option -enable-kvm is ignored.
> Option -no-kvm suppresses KVM.
>
> Upstream works differently: KVM is off by default, -enable-kvm
> switches it on.  -enable-kvm terminates the process unsuccessfully if
> KVM is not available.
>
> upstream qemu   |  default  |-enable-kvm
> ----------------+-----------+-----------
> KVM available   | disabled  |  enabled
> KVM unavailable | disabled  |    fail
>
> qemu-kvm        |  default  |-enable-kvm|  -no-kvm
> ----------------+-----------+-----------+----------
> KVM available   |  enabled* |  enabled  |  disabled
> KVM unavailable | disabled  | disabled* |  disabled
>
> * differs from upstream
>
> Users of qemu and qemu-kvm need to be aware of these differences to
> enable / disable use of KVM reliably.  This is bothersome.
>
> Consider -enable-kvm when KVM is unavailable: If the user expects
> qemu-kvm behavior (fall back), but qemu fails, he'll likely be
> surprised and unhappy.  If the user expects upstream behavior (fail),
> but qemu-kvm falls back to TCG, the guest runs slow as molasses, and
> the user will likely be confused and unhappy (unless he spots and
> understands the "disable KVM" message).
>
> Eventually, we'll sort this upstream with -accel (defaults tied to
> machine type).  Until then, this patch reduces the difference to
> upstream so that most users shouldn't need to be aware of them.
>
> Make -enable-kvm behave just like in upstream: enable KVM, fail if not
> available.  But retain current default behavior: enable KVM, fall back
> to TCG.
>
> qemu-kvm new    |  default  |-enable-kvm|  -no-kvm
> ----------------+-----------+-----------+-----------
> KVM available   |  enabled* |  enabled  |  disabled
> KVM unavailable | disabled  |    fail+  |  disabled
>
> * differs from upstream
> + changed by this patch
>
> Bonus fix: -no-kvm -enable-kvm now enables KVM.  Before, it disabled it.
>

Applied, thanks.

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

Re: [PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[Jan Kiszka]

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

Am 22.12.2010 13:33, Markus Armbruster wrote:
> We currently enable KVM by default, and when it's not available, we
> print a message and fall back to TCG.  Option -enable-kvm is ignored.
> Option -no-kvm suppresses KVM.
> 
> Upstream works differently: KVM is off by default, -enable-kvm
> switches it on.  -enable-kvm terminates the process unsuccessfully if
> KVM is not available.
> 
> upstream qemu   |  default  |-enable-kvm
> ----------------+-----------+-----------
> KVM available   | disabled  |  enabled
> KVM unavailable | disabled  |    fail
> 
> qemu-kvm        |  default  |-enable-kvm|  -no-kvm
> ----------------+-----------+-----------+----------
> KVM available   |  enabled* |  enabled  |  disabled
> KVM unavailable | disabled  | disabled* |  disabled
> 
> * differs from upstream
> 
> Users of qemu and qemu-kvm need to be aware of these differences to
> enable / disable use of KVM reliably.  This is bothersome.
> 
> Consider -enable-kvm when KVM is unavailable: If the user expects
> qemu-kvm behavior (fall back), but qemu fails, he'll likely be
> surprised and unhappy.  If the user expects upstream behavior (fail),
> but qemu-kvm falls back to TCG, the guest runs slow as molasses, and
> the user will likely be confused and unhappy (unless he spots and
> understands the "disable KVM" message).
> 
> Eventually, we'll sort this upstream with -accel (defaults tied to
> machine type).  Until then, this patch reduces the difference to
> upstream so that most users shouldn't need to be aware of them.
> 
> Make -enable-kvm behave just like in upstream: enable KVM, fail if not
> available.  But retain current default behavior: enable KVM, fall back
> to TCG.
> 
> qemu-kvm new    |  default  |-enable-kvm|  -no-kvm
> ----------------+-----------+-----------+-----------
> KVM available   |  enabled* |  enabled  |  disabled
> KVM unavailable | disabled  |    fail+  |  disabled
> 
> * differs from upstream
> + changed by this patch
> 
> Bonus fix: -no-kvm -enable-kvm now enables KVM.  Before, it disabled it.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  vl.c |   22 +++++++++-------------
>  1 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index e3c8919..1958e01 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -247,7 +247,7 @@ static void *boot_set_opaque;
>  static NotifierList exit_notifiers =
>      NOTIFIER_LIST_INITIALIZER(exit_notifiers);
>  
> -int kvm_allowed = 1;
> +int kvm_allowed = -1;
>  uint32_t xen_domid;
>  enum xen_mode xen_mode = XEN_EMULATE;
>  
> @@ -2436,10 +2436,8 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_smbios:
>                  do_smbios_option(optarg);
>                  break;
> -#ifdef OBSOLETE_KVM_IMPL
>              case QEMU_OPTION_enable_kvm:
>                  kvm_allowed = 1;
> -#endif
>                  break;
>  	    case QEMU_OPTION_no_kvm:
>  		kvm_allowed = 0;
> @@ -2789,19 +2787,17 @@ int main(int argc, char **argv, char **envp)
>      if (kvm_allowed) {
>          int ret = kvm_init(smp_cpus);
>          if (ret < 0) {
> -#if defined(OBSOLETE_KVM_IMPL) || defined(CONFIG_NO_CPU_EMULATION)
> -            if (!kvm_available()) {
> -                printf("KVM not supported for this target\n");
> -            } else {
> -                fprintf(stderr, "failed to initialize KVM: %s\n", strerror(-ret));
> +            if (kvm_allowed > 0) {
> +                if (!kvm_available()) {
> +                    printf("KVM not supported for this target\n");
> +                } else {
> +                    fprintf(stderr, "failed to initialize KVM: %s\n", strerror(-ret));
> +                }
> +                exit(1);
>              }
> -            exit(1);
> -#endif
> -#ifdef CONFIG_KVM
>              fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
> -            kvm_allowed = 0;
> -#endif
>          }
> +        kvm_allowed = ret >= 0;
>      }
>  
>      if (qemu_init_main_loop()) {

Does any qemu-kvm user rely on the automatic fallback to TCG if KVM
initialization fails? If not, then just set kvm_allowed to 1 in qemu-kvm
and leave the rest as upstream provides it. This fallback is really
annoying, specifically as the only point of qemu-kvm is, well, running
over KVM.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[Avi Kivity]

On 01/02/2011 12:15 PM, Jan Kiszka wrote:
> Does any qemu-kvm user rely on the automatic fallback to TCG if KVM
> initialization fails?

We don't know...

> If not, then just set kvm_allowed to 1 in qemu-kvm
> and leave the rest as upstream provides it. This fallback is really
> annoying, specifically as the only point of qemu-kvm is, well, running
> over KVM.

I agree, upstream's behaviour is better, and the proposed -accel is even 
better.  But we can't just change behaviour randomly, even if it's an 
improvement.

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

Re: [PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[David Mair]

On 01/02/2011 03:18 AM, Avi Kivity wrote:
> On 01/02/2011 12:15 PM, Jan Kiszka wrote:
>> Does any qemu-kvm user rely on the automatic fallback to TCG if KVM
>> initialization fails?
>
> We don't know...

Speaking as an actual user of the behaviour - I wouldn't say I became 
reliant on it when I had distro kernel updates that stopped my 
hand-built kvm modules from loading and I didn't notice, but in truth I 
was taking advantage of it on powerful enough hardware 4x3GHz/8GB. On my 
2x2.3GHz/3GB laptop it was easier just to boot my 1x600MHz/2GB laptop 
with a copy of XP rather than try a Win7 or XP VM without kvm.

>> If not, then just set kvm_allowed to 1 in qemu-kvm
>> and leave the rest as upstream provides it. This fallback is really
>> annoying, specifically as the only point of qemu-kvm is, well, running
>> over KVM.
>
> I agree, upstream's behaviour is better, and the proposed -accel is even
> better. But we can't just change behaviour randomly, even if it's an
> improvement.
>

If the behaviour was as described by Jan I would have been forced to put 
the effort into re-building kvm modules against my updated kernel sooner 
or intentionally adjust my qemu command line to disable kvm. You also 
make a good point but I would say in response that I was encouraged to 
tolerate a reduction in utility by the current behaviour rather than 
deal with what's really my own configuration error on my hosts and I 
think then it's an improvement for qemu-kvm with kvm_allowed != 0 to 
fail to load when kvm is not present. Your point comes down to the guy 
with "hosted services" on VMs that he guarantees to always be running 
(and is dependent on the current behaviour for it). He will think 
differently from me and be especially mad if he doesn't read the 
release-notes when he updates but it's not like he couldn't adjust the 
scripts he already starts qemu-kvm with to check for the error and retry 
with -no-kvm if appropriate...presumably upstream has an equally usable 
though non-specific error code to what happens in Markus's patch.

-- 
David.

Re: [PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[Richard W.M. Jones]

On Sun, Jan 02, 2011 at 11:15:33AM +0100, Jan Kiszka wrote:
> Does any qemu-kvm user rely on the automatic fallback to TCG if KVM
> initialization fails?

Yes, libguestfs does ...

... or at least we would like to be able to reliably request this from
the command line.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/

Re: [PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[Alexander Graf]

On 02.01.2011, at 11:18, Avi Kivity wrote:

> On 01/02/2011 12:15 PM, Jan Kiszka wrote:
>> Does any qemu-kvm user rely on the automatic fallback to TCG if KVM
>> initialization fails?
> 
> We don't know...
> 
>> If not, then just set kvm_allowed to 1 in qemu-kvm
>> and leave the rest as upstream provides it. This fallback is really
>> annoying, specifically as the only point of qemu-kvm is, well, running
>> over KVM.
> 
> I agree, upstream's behaviour is better, and the proposed -accel is even better.  But we can't just change behaviour randomly, even if it's an improvement.

Every SUSE released qemu-kvm binary since 10.3 or so disables the fallback already. There were just so many bug reports with random breakage or people frustrated over KVM's speed caused be the fallback that it didn't seem to be worth it.
IIRC I even sent a patch to disable the fallback back then - no idea what happened to it.


Alex

Re: [PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[Avi Kivity]

On 01/02/2011 09:52 PM, Richard W.M. Jones wrote:
> On Sun, Jan 02, 2011 at 11:15:33AM +0100, Jan Kiszka wrote:
> >  Does any qemu-kvm user rely on the automatic fallback to TCG if KVM
> >  initialization fails?
>
> Yes, libguestfs does ...
>
> ... or at least we would like to be able to reliably request this from
> the command line.

You can specify as many toppings as you like for your pizza with -accel.

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

Re: [PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[Richard W.M. Jones]

On Mon, Jan 03, 2011 at 10:28:12AM +0200, Avi Kivity wrote:
> On 01/02/2011 09:52 PM, Richard W.M. Jones wrote:
> >On Sun, Jan 02, 2011 at 11:15:33AM +0100, Jan Kiszka wrote:
> >>  Does any qemu-kvm user rely on the automatic fallback to TCG if KVM
> >>  initialization fails?
> >
> >Yes, libguestfs does ...
> >
> >... or at least we would like to be able to reliably request this from
> >the command line.
> 
> You can specify as many toppings as you like for your pizza with -accel.

Does that option now exist?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

Re: [PATCH v2] qemu-kvm: Switch to upstream -enable-kvm semantics

[Avi Kivity]

On 01/03/2011 01:18 PM, Richard W.M. Jones wrote:
> On Mon, Jan 03, 2011 at 10:28:12AM +0200, Avi Kivity wrote:
> >  On 01/02/2011 09:52 PM, Richard W.M. Jones wrote:
> >  >On Sun, Jan 02, 2011 at 11:15:33AM +0100, Jan Kiszka wrote:
> >  >>   Does any qemu-kvm user rely on the automatic fallback to TCG if KVM
> >  >>   initialization fails?
> >  >
> >  >Yes, libguestfs does ...
> >  >
> >  >... or at least we would like to be able to reliably request this from
> >  >the command line.
> >
> >  You can specify as many toppings as you like for your pizza with -accel.
>
> Does that option now exist?

Unfortunately not.

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