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

[Martin Kletzander]

On 10/31/2012 12:44 AM, Eric Blake wrote:
> 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 :)
> 

I squashed both in, fixed the huge amount of typos, added the explicit
setting to false for the bools in qemuMonitorJSONGetKVMState(), double
checked and pushed.  Thanks very much.

Martin