[libvirt] [PATCH v2 5/5] cputune_event: queue the event for cputune updates

Pavel Hrdina phrdina at redhat.com
Mon Sep 15 12:59:21 UTC 2014


On 09/12/2014 10:31 PM, John Ferlan wrote:
>
> s//"something to describe what's being done!"
>
> Might be nice to see what the event would look like when it's triggered.
> May help in that documentation effort in the future.
>
> On 09/10/2014 08:08 AM, Pavel Hrdina wrote:
>> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>> ---
>>   src/qemu/qemu_cgroup.c | 18 ++++++++++++-
>>   src/qemu/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 90 insertions(+), 1 deletion(-)
>>
>
> So this is only going to occur as asked in 4/4 in v1 when/if the
> cputune.shares value is adjusted during qemuProcessStart.
>

Yes, that's right.

> Also events are only triggered when someone uses the api that virsh
> vcpupin, emulatorpin, and schedinfo(set) call.  Which is "ok' by me for
> the onlist iothreadpin changes which don't yet have a similar
> iothreadpin api.  Meaning my other series shouldn't be affected
>
>

[..]

>> @@ -4569,6 +4575,18 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
>>
>>           if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>>               goto cleanup;
>> +
>> +        if (virSnprintf(paramField, "vcpu%d", vcpu) < 0) {
>
> As pointed out for 1/5 - this really could just be an snprintf()
>
>
>> +            goto cleanup;
>> +        }
>
> The extra {} ^^^  and vvv are probably not necessary
>

[..]

>> @@ -4833,6 +4860,14 @@ qemuDomainPinEmulator(virDomainPtr dom,
>>
>>           if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>>               goto cleanup;
>> +
>> +        str = virBitmapFormat(pcpumap);
>> +        if (virTypedParamsAddString(&eventParams, &eventNparams,
>> +                                    &eventMaxparams, "emulatorpin", str) < 0) {
>> +            goto cleanup;
>> +        }
>
> ditto on the {}
>

[..]

>> @@ -9205,12 +9272,18 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>>           vmdef = NULL;
>>       }
>>
>> +
>
> Extraneous line
>
>
> ACK with the adjustments mentioned.  I think you should just skip the
> virSnprintf for this series and add one in another series to encompass
> all callers (and of course add the 'rule' like Eric suggests in 1/5
>

Will fix the brackets and the extra line, thanks.

Pavel

>
> John
>




More information about the libvir-list mailing list