[libvirt] [PATCH 2/2] getstats: add block.n.source stat

[Peter Krempa]

On 11/25/14 16:43, Eric Blake wrote:
> On 11/25/2014 02:10 AM, Peter Krempa wrote:
>> On 11/25/14 06:21, Eric Blake wrote:
>>> I noticed that for an offline domain, 'virsh domstats --block $dom'
>>> was producing just the domain name, with no stats.  But the older
>>> 'virsh domblkinfo' works just fine on offline domains.  Furthermore,
>>> I'm about to make block stats optionally more complex to cover
>>> backing chains, where block.count will no longer equal the number
>>> of <disks> for a domain.  For these reasons, it is nicer if the
>>> statistics output always includes minimal information on every
>>> resource being described, with enough to correlate back to host
>>> resources, and even when some statistics are available only on a
>>> running domain.
>>>
> 
>>> + * "block.<num>.source" - string describing the source of block device <num>,
>>> + *                        such as the host path of a file or device.
>>
>> Fair enough as long as we document that we only return it for
>> non-network sources. We had quite a few problems with network sources so
>> I'd rather not expose it for those due to possible ambiguity.
> 
> Sure, I can do that.  Maybe I should then name it block.<num>.path or
> block.<num>.file, to make it obvious that it is only a file name?  And I
> suppose we could always add other fields later if it turns out to be
> useful on network devices, but I won't worry about it for now.

I'd go with path. File is not quite right with physical devices/lvs used
as source.

> 
>>>   *
>>>   * Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags makes
>>>   * the function return error in case some of the stat types in @stats were
>>> - * not recognized by the daemon.
>>> + * not recognized by the daemon.  However, even with this flag, a hypervisor
>>> + * may omit individual fields within a group if the information is not
>>> + * available; as an extreme example, a supported group may produce zero
>>> + * fields for offline domains if the statistics are meaningful only for a
>>> + * running domain.
>>>   *
>>
>> The code changes are not entirely related to this doc change.
> 
> Should I split it into a separate patch, then?

I think it would make more sense.

> 
>>> -        QEMU_ADD_NAME_PARAM(record, maxparams, "block", i, disk->dst);
>>> +        QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", i, disk->dst);
>>> +        if (disk->src && disk->src->path)
>>
>> && virStorageSourceIsLocalStorage(disk->src)
> 
> Indeed, that fits with your request to limit to files.
> 

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141125/e4b919c0/attachment-0001.sig>