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

[Jan Kiszka <jan.kiszka@web.de>]

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