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