On 08/28/14 14:48, Eric Blake wrote:
> On 08/28/2014 06:18 AM, Peter Krempa wrote:
>> Implement the API function for virDomainListGetStats and
>> virConnectGetAllDomainStats in a modular way and implement the
>> VIR_DOMAIN_STATS_STATE group of statistics.
>>
>> Although it may look like the function looks universal I'd rather not
>> expose it to other drivers as the coming stats groups are likely to do
>> qemu specific stuff to obtain the stats.
>> ---
>>
>> Notes:
>> Version 4:
>> - fixed handling and error checking of @stats
>> - domain filtering flags are now rejected when passing in a domain list
>
> Looks nicer. Thanks for putting up with me.
>
>
>> +static int
>> +qemuDomainGetStats(virConnectPtr conn,
>> + virDomainObjPtr dom,
>
>> +
>> + for (i = 0; qemuDomainGetStatsWorkers[i].func; i++) {
>> + if (stats & qemuDomainGetStatsWorkers[i].stats) {
>> + if (qemuDomainGetStatsWorkers[i].func(dom, tmp, &maxparams,
>
> These two if's could be combined into one with &&, for one less level of
> indentation (but I'm also okay if you leave it alone).
>
>> + flags) < 0)
>> + goto cleanup;
>> + }
>> + }
>> +
>> + if (!(tmp->dom = virGetDomain(conn, dom->def->name, dom->def->uuid)))
>
> Earlier versions had a comment about doing dom last for a reason. Not
> sure if you want to reinstate it; I can live without it.
I've added the comment before I found a way to non-error-destructively
free the domain object so now it isn't entirely true.
>
>
>> +}
>> +
>> +
>> +
>> +static int
>> +qemuConnectGetAllDomainStats(virConnectPtr conn,
>
> 3 blank lines is unusual.
>
Also not-easy-to spot when trying to find it :D.
> ACK.
>
Thanks pushed.
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list