[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/2 v1] qemu: Fix SW emulated machines



On 10/28/2012 06:55 AM, Martin Kletzander wrote:
> This patch fixes building a command-line for QEMU machines without KVM
> acceleration and is based on following assumptions:
> 
>  - QEMU_CAPS_KVM flag means that QEMU is running KVM accelerated
>    machines by default (without explicitely requesting that using a

s/explicitely/explicitly/

>    command-line option).  It is the closest to the true according to

s/true/truth/

>    the code with the only exception being the comment next to the
>    flag, so it's fixed in this patch as well.
> 
>  - QEMU_CAPS_ENABLE_KVM flag means that QEMU is, by default, running
>    without KVM acceleration and in case we need KVM acceleration it
>    needs to be explicitely instructed to do so.  This is partially

s/explicitely/explicitly/

>    true for the past (this option essentially means that QEMU
>    recognizes the '-enable-kvm' option, even though it's almost the
>    same).

I've looked at both variants, and like this one better (if only because
of upgrade scenarios, since we save qemu capability flags in an older
libvirtd and then re-read those flags when restarting a newer libvirtd,
and expect the flags to have the same meaning no matter whether the
older or newer libvirtd was the original source for those flags).

> ---
>  src/qemu/qemu_capabilities.c | 13 +++++++++----
>  src/qemu/qemu_capabilities.h |  2 +-
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index fe462e9..aad366d 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2039,9 +2039,15 @@ qemuCapsProbeQMPKVMState(qemuCapsPtr caps,
>      if (qemuMonitorGetKVMState(mon, &enabled, &present) < 0)
>          return -1;
> 
> -    /* Youre right, this code does nothing, you must have checked out
> -     * some weird commit.  Go back to your room and think about what
> -     * you've done, young (wo)man. */
> +    /* Until now, the QEMU_CAPS_KVM was set according to the QEMU

Reads awkwardly.  Maybe:

The QEMU_CAPS_KVM flag was initially set according to...

> +     * reporting the recongnition of 'query-kvm' QMP command, but the

s/recongnition/recognition/

> +     * flag means whether the KVM is enabled by default and should be
> +     * disabled in case we want SW emulated machine, so let's fix that
> +     * if it's true. */
> +    if (!enabled) {
> +        qemuCapsClear(caps, QEMU_CAPS_KVM);
> +        qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM);
> +    }
> 
>      return 0;
>  }
> @@ -2177,7 +2183,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
>      qemuCapsSet(caps, QEMU_CAPS_DRIVE_SERIAL);
>      qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_UNIX);
>      qemuCapsSet(caps, QEMU_CAPS_CHARDEV);
> -    qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM);
>      qemuCapsSet(caps, QEMU_CAPS_MONITOR_JSON);
>      qemuCapsSet(caps, QEMU_CAPS_BALLOON);
>      qemuCapsSet(caps, QEMU_CAPS_DEVICE);
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 988dfdb..9c5fd0f 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -45,7 +45,7 @@ enum qemuCapsFlags {
>      QEMU_CAPS_MIGRATE_QEMU_TCP   = 10, /* have qemu tcp migration */
>      QEMU_CAPS_MIGRATE_QEMU_EXEC  = 11, /* have qemu exec migration */
>      QEMU_CAPS_DRIVE_CACHE_V2     = 12, /* cache= flag wanting new v2 values */
> -    QEMU_CAPS_KVM                = 13, /* Whether KVM is compiled in */
> +    QEMU_CAPS_KVM                = 13, /* Whether KVM is enabled by default */

ACK with typos fixed; per my comments in 1/2, I'd squash this into that
patch and push it all as one commit.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]