On 09/15/2014 09:42 AM, Peter Krempa wrote:
> From: Francesco Romani <fromani@xxxxxxxxxx>
>
> This patch implements the VIR_DOMAIN_STATS_BLOCK group of statistics.
>
> To do so, an helper function to get the block stats of all the disks of
s/an helper/a helper/ (the "h" in helper is pronounced, and that's the
key for which article to use; remember "a half of an hour")
> a domain is added.
>
> Signed-off-by: Francesco Romani <fromani@xxxxxxxxxx>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> @@ -21635,6 +21635,26 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
> * "net.<num>.tx.errs" - transmission errors as unsigned long long.
> * "net.<num>.tx.drop" - transmit packets dropped as unsigned long long.
> *
> + * VIR_DOMAIN_STATS_BLOCK: Return block devices statistics.
Hmm. My work on allocation watermark makes me wonder if we'll
eventually want to report stats on backing files (possibly as a new
stats group or only with a flag). I also wonder if this series is
missing stats reporting for block jobs. But the nice thing about the
API is that it is extensible so we can add more later as we find a need
for things in clients.
> + * The typed parameter keys are in this format:
> + * "block.count" - number of block devices on this domain
> + * as unsigned int.
> + * "block.<num>.name" - name of the block device <num> as string.
> + * matches the name of the block device.
Is this the "<target dev='vda'/>" name? Management can then map that
name back to a file path or network disk source, but it still might be
worth being just a bit more verbose on what a block's name really entails.
>
> +/* expects a LL, but typed parameter must be ULL */
> +#define QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, num, name, value) \
> +do { \
> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
> + "block.%zu.%s", num, name); \
> + if (value >= 0 && virTypedParamsAddULLong(&(record)->params, \
> + &(record)->nparams, \
> + maxparams, \
> + param_name, \
> + value) < 0) \
> + goto cleanup; \
> +} while (0)
Just noticing that we aren't very consistent on whether multi-line
macros line up the \ in a final column vs. this style of one space per
line. Doesn't really matter too much.
> + nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL,
> + stats, nstats);
> +
> + qemuDomainObjExitMonitor(driver, dom);
> +
> + if (nstats < 0) {
> + virResetLastError();
> + ret = 0; /* still ok, again go ahead silently */
Another case where we may be polluting the log.
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list