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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list