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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list