Re: [PATCHv4] qemu: Implement bulk stats API and one of the stats groups to return

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

 



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

[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]