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

Re: [libvirt] [PATCHv5 1/8] qemu: bulk stats: extend internal collection API



On 09/15/14 11:20, Peter Krempa wrote:
> On 09/15/14 10:48, Francesco Romani wrote:
>> Future patches which will implement more
>> bulk stats groups for QEMU will need to access
>> the connection object.
>>
>> To accomodate that, a few changes are needed:
>>
>> * enrich internal prototype to pass qemu driver object.
>> * add per-group flag to mark if one collector needs
>>   monitor access or not.
>> * if at least one collector of the requested stats
>>   needs monitor access, thus we must start a query job
>>   for each domain. The specific collectors will
>>   run nested monitor jobs inside that.
>> * although requested, monitor could be not available.
>>   pass a flag to workers to signal the availability
>>   of monitor, in order to gather as much data as
>>   is possible anyway.
>>
>> Signed-off-by: Francesco Romani <fromani redhat com>
>> ---
>>  src/qemu/qemu_driver.c | 62 +++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 54 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 73edda3..39e9d27 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
> 
>> @@ -17525,12 +17556,22 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
>>              !virConnectGetAllDomainStatsCheckACL(conn, dom->def))
>>              continue;
>>  
>> -        if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0)
>> -            goto cleanup;
>> +        if (HAVE_MONITOR(domflags) &&
>> +             qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0)
>> +            /* As it was never requested. Gather as much as possible anyway. */
>> +            domflags &= ~QEMU_DOMAIN_STATS_HAVE_MONITOR;
>> +
>> +        if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0)
>> +            goto endjob;
>>  
>>          if (tmp)
>>              tmpstats[nstats++] = tmp;
>>  
>> +        if (HAVE_MONITOR(domflags) && !qemuDomainObjEndJob(driver, dom)) {
>> +            dom = NULL;
>> +            goto cleanup;
> 
> I'd rather "continue;" here as a vanishing domain shouldn't be a problem
> for this code.
> 
>> +        }
>> +
>>          virObjectUnlock(dom);
>>          dom = NULL;
>>      }
> 
> ACK with the nit above fixed. (I made the change locally, thus no need
> to re-send this one).

Additionally I'll change the QEMU_DOMAIN_STATS_HAVE_MONITOR flag to
QEMU_DOMAIN_STATS_HAVE_JOB as the flag denotes exactly that fact.

Peter


Attachment: signature.asc
Description: OpenPGP digital signature


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