Re: [PATCH 1/2] qemu: Enhance QMP detection of KVM state

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

 



On 10/28/2012 06:55 AM, Martin Kletzander wrote:
> When there is no 'qemu-kvm' binary and the emulator used for a machine
> is, for example, 'qemu-system-x86_64' that, by default, runs without
> kvm enabled, libvirt still supplies '-no-kvm' option to this process,
> even though it does not recognize such option (making the start of a
> domain fail in that case).
> 
> This patch adds QMP querying for KVM state using 'query-kvm' state,
> but does not set any of QEMU_CAPS_KVM and QEMU_CAPS_ENABLE_KVM flags.
> That functionality is done in different patch in order to be able to
> compare two possibilities and chose the better one without looking at
> the part of the code that's exactly the same for both of them (this
> patch).

> 
> +static int
> +qemuCapsProbeQMPKVMState(qemuCapsPtr caps,
> +                         qemuMonitorPtr mon)
> +{
> +    bool enabled = false;
> +    bool present = false;
> +
> +    if (!qemuCapsGet(caps, QEMU_CAPS_KVM))
> +        return 0;
> +
> +    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. */

Since this cute comment disappears in either 2/2 approach, I would
probably rather see this "series" squashed into a single commit when
finally going upstream.  That would also mean deleting the second
paragraph of the commit message.


> +int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon,
> +                               bool *enabled,
> +                               bool *present)
> +{
> +    int ret;
> +    virJSONValuePtr cmd = NULL;
> +    virJSONValuePtr reply = NULL;
> +    virJSONValuePtr data = NULL;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("query-kvm", NULL)))
> +        return -1;
> +
> +    ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +
> +    if (ret == 0) {
> +        if (qemuMonitorJSONHasError(reply, "CommandNotFound"))
> +            goto cleanup;

This relies on the caller to pre-initialize *enabled and *present to
false; it might be better to explicitly repeat that setting here so that
this function guarantees that the values are always correct on
successful return even if the caller forgot to initialize.  But right
now, the only caller happens to pre-initialize, so it's not a show-stopper.

ACK, once I pick which of the 2/2 variants I like best :)

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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]