qemu-kvm: Switch to upstream -enable-kvm semantics

PATCH Applied Markus Armbruster 6 participants 10 messages Dec 22, 2010 – Jan 3, 2011 KVM

Share & tools

Markus Armbruster

author

2010-12-22 12:33 UTC #0 lore raw

PATCH v2 1/1 qemu-kvm: Switch to upstream -enable-kvm semantics

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 <[email protected]>

---
 vl.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

vl.c +9 ~~-13~~

index e3c8919..1958e01 100644
@@ -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()) {

Signature · 1 line

1.7.2.3

Avi Kivity

Applied

2010-12-30 10:21 UTC #1 lore raw

Quoted 40 lines · 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
----------------+-----------+-----------+-----------

Quoted 8 lines

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.

Signature · 1 line

error compiling committee.c: too many arguments to function

Jan Kiszka

2011-01-02 10:15 UTC #2 lore raw

In reply to Markus Armbruster

Quoted 95 lines · 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 <[email protected]>
---
 vl.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)
vl.c +8 -9
index e3c8919..1958e01 100644
@@ -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);

vl.c int main(int argc, char **argv, char **envp) · line 2803

Quoted 10 lines

vl.c +1 -4

-#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

Avi Kivity

2011-01-02 10:18 UTC #3 lore raw

Quoted 2 lines · Jan Kiszka

Does any qemu-kvm user rely on the automatic fallback to TCG if KVM initialization fails?

We don't know...

Quoted 4 lines

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.

Signature · 1 line

error compiling committee.c: too many arguments to function

David Mair

2011-01-02 18:37 UTC #4 lore raw

Quoted 5 lines · Avi Kivity

Quoted 2 lines · Jan Kiszka
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.

Quoted 9 lines

Quoted 4 lines
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.

Signature · 1 line

David.

Alexander Graf

2011-01-03 00:06 UTC #5 lore raw

In reply to Avi Kivity

Quoted 4 lines · Avi Kivity

Quoted 2 lines · Jan Kiszka
Does any qemu-kvm user rely on the automatic fallback to TCG if KVM initialization fails?

Quoted 8 lines

We don't know...
Quoted 4 lines
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

Richard W.M. Jones

2011-01-02 19:52 UTC #6 lore raw

In reply to Jan Kiszka

Quoted 2 lines · Jan Kiszka

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.

Signature · 4 lines

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/

Avi Kivity

2011-01-03 08:28 UTC #7 lore raw

Quoted 8 lines · Richard W.M. Jones

Quoted 2 lines · Jan Kiszka
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.

Signature · 1 line

error compiling committee.c: too many arguments to function

Richard W.M. Jones

2011-01-03 11:18 UTC #8 lore raw

Quoted 11 lines · Avi Kivity

Quoted 8 lines · Richard W.M. Jones
Quoted 2 lines · Jan Kiszka
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.

Signature · 4 lines

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

Avi Kivity

2011-01-03 11:41 UTC #9 lore raw

Quoted 6 lines · Richard W.M. Jones

Quoted 5 lines · Avi Kivity
Quoted 4 lines · Richard W.M. Jones
Quoted 2 lines · Jan Kiszka
Does any qemu-kvm user rely on the automatic fallback to TCG if KVM initialization fails?

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
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.

Signature · 1 line

error compiling committee.c: too many arguments to function