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

PATCH Superseded Markus Armbruster 5 participants 12 messages Dec 15, 2010 – Dec 21, 2010 KVM

Share & tools

Markus Armbruster

author

2010-12-15 15:50 UTC #0 lore raw

PATCH 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
----------------+-----------+-----------

KVM available   |  enabled* |  enabled
KVM unavailable | 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).

Switch to upstream semantics: KVM off by default, -enable-kvm switches it on, and when it can't, it's fatal.

Having to enable KVM explicitly is annoying, but the proper place to address that is upstream.

Signed-off-by Markus Armbruster <[email protected]>

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

vl.c +1 -9

index e3c8919..87e88c2 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 = 0;
 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,18 +2787,12 @@ 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));
             }
             exit(1);
-#endif
-#ifdef CONFIG_KVM
-            fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
-            kvm_allowed = 0;
-#endif
         }
     }

Signature · 1 line

1.7.2.3

Anthony Liguori

2010-12-15 17:31 UTC #1 lore raw

Quoted 30 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
----------------+-----------+-----------
KVM available   |  enabled* |  enabled
KVM unavailable | 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).

Quoted 8 lines

Switch to upstream semantics: KVM off by default, -enable-kvm switches it on, and when it can't, it's fatal.
Having to enable KVM explicitly is annoying, but the proper place to address that is upstream.
Signed-off-by Markus Armbruster<[email protected]>

Backwards compatibility is going to kill us if we try to make this change.

Current qemu-kvm behavior:

default: -accel kvm,tcg
-no-kvm: -accel tcg
-enable-kvm: -accel kvm,tcg

Current upstream behavior

default: -accel tcg
-enable-kvm: -accel kvm

I think we should tie `-accel' to the machine type. For qemu-kvm, a different default machine type should be used than upstream qemu (it really should be a configure switch).

For pc', the default -accel' behavior should remain 'tcg'. For kvmpc', the default -accel' behavior should be 'kvm,tcg'.

-no-kvm should be deprecated. -enable-kvm should also be deprecated in favor of the `-accel' option.

In the short term, it would be a good idea to modify qemu-kvm to switch the -enable-kvm semantics to match upstream (fail if KVM isn't available). Adding an alias for 'kvmpc' upstream and qemu-kvm and making qemu-kvm default to 'kvmpc' would be helpful for management tools too.

Regards,

Anthony Liguori

Quoted 48 lines

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

vl.c +1 -9

index e3c8919..87e88c2 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 = 0;
  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,18 +2787,12 @@ 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));
              }
              exit(1);
-#endif
-#ifdef CONFIG_KVM
-            fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
-            kvm_allowed = 0;
-#endif
          }
      }

Markus Armbruster

author

2010-12-15 17:57 UTC #2 lore raw

Quoted 57 lines · Anthony Liguori

Quoted 30 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
----------------+-----------+-----------
KVM available   |  enabled* |  enabled
KVM unavailable | 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).
Quoted 8 lines
Switch to upstream semantics: KVM off by default, -enable-kvm switches it on, and when it can't, it's fatal.
Having to enable KVM explicitly is annoying, but the proper place to address that is upstream.
Signed-off-by Markus Armbruster<[email protected]>
Backwards compatibility is going to kill us if we try to make this change.
Current qemu-kvm behavior:
default: -accel kvm,tcg
(quoted diff) +0 -2
-no-kvm: -accel tcg
-enable-kvm: -accel kvm,tcg
Current upstream behavior
default: -accel tcg
-enable-kvm: -accel kvm
I think we should tie `-accel' to the machine type. For qemu-kvm, a different default machine type should be used than upstream qemu (it really should be a configure switch).

Quoted 5 lines

For pc', the default -accel' behavior should remain 'tcg'. For kvmpc', the default `-accel' behavior should be 'kvm,tcg'.
-no-kvm should be deprecated. -enable-kvm should also be deprecated in favor of the `-accel' option.

I'm fine with -accel and deprecating the old options. But until we have that:

Quoted 3 lines

In the short term, it would be a good idea to modify qemu-kvm to switch the -enable-kvm semantics to match upstream (fail if KVM isn't available).

That's what my patch does.

Additionally, it changes the default to match upstream: KVM disabled.

What do you want changed in my patch?

Quoted 3 lines

Adding an alias for 'kvmpc' upstream and qemu-kvm and
making qemu-kvm default to 'kvmpc' would be helpful for management tools too.

That would address "Having to enable KVM explicitly is annoying".

Avi Kivity

2010-12-21 15:16 UTC #3 lore raw

Quoted 9 lines · Markus Armbruster

Quoted 3 lines
In the short term, it would be a good idea to modify qemu-kvm to switch the -enable-kvm semantics to match upstream (fail if KVM isn't available).
That's what my patch does.
Additionally, it changes the default to match upstream: KVM disabled.
What do you want changed in my patch?

The 'Additionally' bit. qemu-kvm users rely on the default enabling kvm. Likely they don't rely on -enable-kvm failing is kvm is not available (and indeed, they likely expect it to match upstream). So the patch should only change behaviour when -enable-kvm is specified.

Signature · 1 line

error compiling committee.c: too many arguments to function

Markus Armbruster

author

2010-12-21 15:41 UTC #4 lore raw

Quoted 7 lines · Avi Kivity

Quoted 6 lines · Markus Armbruster
Quoted 3 lines
In the short term, it would be a good idea to modify qemu-kvm to switch the -enable-kvm semantics to match upstream (fail if KVM isn't available).
That's what my patch does.

Quoted 8 lines

Quoted 3 lines
Additionally, it changes the default to match upstream: KVM disabled.
What do you want changed in my patch?
The 'Additionally' bit. qemu-kvm users rely on the default enabling kvm. Likely they don't rely on -enable-kvm failing is kvm is not available (and indeed, they likely expect it to match upstream). So the patch should only change behaviour when -enable-kvm is specified.

Like this?

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  |    fail   |  disabled

* differs from upstream

Avi Kivity

2010-12-21 15:48 UTC #5 lore raw

Quoted 25 lines · Markus Armbruster

Quoted 7 lines · Avi Kivity
Quoted 6 lines · Markus Armbruster
Quoted 3 lines
In the short term, it would be a good idea to modify qemu-kvm to switch the -enable-kvm semantics to match upstream (fail if KVM isn't available).
That's what my patch does.
Quoted 8 lines
Quoted 3 lines
Additionally, it changes the default to match upstream: KVM disabled.
What do you want changed in my patch?
The 'Additionally' bit. qemu-kvm users rely on the default enabling kvm. Likely they don't rely on -enable-kvm failing is kvm is not available (and indeed, they likely expect it to match upstream). So the patch should only change behaviour when -enable-kvm is specified.
Like this?
upstream qemu | default |-enable-kvm
----------------+-----------+-----------
KVM available   | disabled  |  enabled
KVM unavailable | disabled  |    fail

Quoted 6 lines

qemu-kvm | default |-enable-kvm| -no-kvm
----------------+-----------+-----------+-----------
KVM available   |  enabled* |  enabled  |  disabled
KVM unavailable | disabled  |    fail   |  disabled
* differs from upstream

Yes.

Signature · 1 line

error compiling committee.c: too many arguments to function

Markus Armbruster

author

2010-12-21 16:05 UTC #6 lore raw

Quoted 26 lines · Avi Kivity

Quoted 25 lines · Markus Armbruster
Quoted 7 lines · Avi Kivity
Quoted 6 lines · Markus Armbruster
Quoted 3 lines
In the short term, it would be a good idea to modify qemu-kvm to switch the -enable-kvm semantics to match upstream (fail if KVM isn't available).
That's what my patch does.
Quoted 8 lines
Quoted 3 lines
Additionally, it changes the default to match upstream: KVM disabled.
What do you want changed in my patch?
The 'Additionally' bit. qemu-kvm users rely on the default enabling kvm. Likely they don't rely on -enable-kvm failing is kvm is not available (and indeed, they likely expect it to match upstream). So the patch should only change behaviour when -enable-kvm is specified.
Like this?
upstream qemu | default |-enable-kvm
----------------+-----------+-----------
KVM available   | disabled  |  enabled
KVM unavailable | disabled  |    fail

Quoted 8 lines

Quoted 6 lines
qemu-kvm | default |-enable-kvm| -no-kvm
----------------+-----------+-----------+-----------
KVM available   |  enabled* |  enabled  |  disabled
KVM unavailable | disabled  |    fail   |  disabled
* differs from upstream
Yes.

Thanks. I'll cook up a new patch.

Richard W.M. Jones

2010-12-21 16:00 UTC #7 lore raw

In reply to Markus Armbruster

Quoted 7 lines · Markus Armbruster

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

Quoted 6 lines

qemu-kvm | default |-enable-kvm| -no-kvm
----------------+-----------+-----------+-----------
KVM available   |  enabled* |  enabled  |  disabled
KVM unavailable | disabled  |    fail   |  disabled
* differs from upstream

libguestfs wants "best effort" behaviour, and libvirt wants "KVM or die" behaviour.

Avi, can you comment on whether just opening /dev/kvm O_RDWR is a reasonable way to detect if KVM is available?

Markus, any idea when we might get the -accel option appearing in released versions of qemu/KVM?

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

Richard W.M. Jones

2010-12-21 16:02 UTC #8 lore raw

Quoted 2 lines · Richard W.M. Jones

Markus, any idea when we might get the -accel option appearing in released versions of qemu/KVM?

Sorry, I thought this email wasn't going out to a public list. I should be more careful next time.

I'll say instead: We really need both the -accel option and a working system of qemu capabilities. Both would be hugely helpful making it easier to work with qemu.

Rich.

Signature · 4 lines

Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v

Markus Armbruster

author

2010-12-21 16:07 UTC #9 lore raw

In reply to Richard W.M. Jones

Quoted 13 lines · Richard W.M. Jones

Quoted 12 lines · Markus Armbruster
Like this?
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  |    fail   |  disabled

Quoted 4 lines

Quoted 1 line
* differs from upstream
libguestfs wants "best effort" behaviour, and libvirt wants "KVM or die" behaviour.

For what it's worth, default gives you exactly that with qemu-kvm. Maybe that's good enough, on the theory that if you have KVM, you most likely have libguestfs using qemu-kvm.

Quoted 5 lines

Avi, can you comment on whether just opening /dev/kvm O_RDWR is a reasonable way to detect if KVM is available?
Markus, any idea when we might get the -accel option appearing in released versions of qemu/KVM?

No idea. Anthony?

Anthony Liguori

2010-12-21 16:56 UTC #10 lore raw

Quoted 27 lines · Markus Armbruster

Quoted 14 lines · Richard W.M. Jones
Quoted 12 lines · Markus Armbruster
Like this?
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  |    fail   |  disabled
Quoted 5 lines
Quoted 2 lines
* differs from upstream
libguestfs wants "best effort" behaviour, and libvirt wants "KVM or die" behaviour.
For what it's worth, default gives you exactly that with qemu-kvm. Maybe that's good enough, on the theory that if you have KVM, you most likely have libguestfs using qemu-kvm.

Quoted 8 lines

Quoted 6 lines
Avi, can you comment on whether just opening /dev/kvm O_RDWR is a reasonable way to detect if KVM is available?
Markus, any idea when we might get the -accel option appearing in released versions of qemu/KVM?
No idea. Anthony?

I see no problem with 0.15 if someone cooks up a patch.

Regards,

Anthony Liguori

Alexander Graf

2010-12-21 17:25 UTC #11 lore raw

Quoted 31 lines · Anthony Liguori

Quoted 30 lines · Markus Armbruster
Quoted 14 lines · Richard W.M. Jones
Quoted 12 lines · Markus Armbruster
Like this?
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  |    fail   |  disabled
Quoted 5 lines
Quoted 2 lines
* differs from upstream
libguestfs wants "best effort" behaviour, and libvirt wants "KVM or die" behaviour.
For what it's worth, default gives you exactly that with qemu-kvm. Maybe that's good enough, on the theory that if you have KVM, you most likely have libguestfs using qemu-kvm.
Quoted 3 lines
Avi, can you comment on whether just opening /dev/kvm O_RDWR is a reasonable way to detect if KVM is available?

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Markus, any idea when we might get the -accel option appearing in released versions of qemu/KVM?
No idea. Anthony?
I see no problem with 0.15 if someone cooks up a patch.

Didn't Anthony do one? What happened to the Xen patch set anyways? :)

Alex