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

[Eric Blake]

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.

>>   *
>>   * 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?

>> -        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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

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