[libvirt] [PATCH 0/8 v2] Report disk latency info

This patch series introduces a new API to report the disk latency related information, which is supported by upstream QEMU just a few days ago (commit c488c7f649, Thu Aug 25). Per previous dicussion on the ABI compatiblity, and API design principle, the new API is defined with style: int virDomainBlockStatsFlags (virDomainPtr dom, const char *path, virTypeParamsPtr params, int *nparams, unsigned int flags) V1 - V2: * Use virTypedParameter instead of creating a new struct for the new API. * Update comments for the API, (Eric's suggestion). [PATCH 1/8] latency: Define new public API and structure [PATCH 2/8] latency: Define the internal driver callback [PATCH 3/8] latency: Implemente the public API [PATCH 4/8] latency: Wire up the remote protocol [PATCH 5/8] latency: Update monitor functions for new latency fields [PATCH 6/8] latency: Implemente internal API for qemu driver [PATCH 7/8] latency: Expose the new API for Python binding [PATCH 8/8] latency: Update cmdBlkStats to use new API Regards, Osier

--- include/libvirt/libvirt.h.in | 84 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 2 files changed, 85 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8864865..735ea94 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -574,6 +574,85 @@ struct _virDomainBlockStats { */ typedef virDomainBlockStatsStruct *virDomainBlockStatsPtr; + +/** + * VIR_DOMAIN_BLOCK_STATS_FIELD_LENGTH: + * + * Macro providing the field length of virDomainBlockStatsFlagsStruct + */ +#define VIR_DOMAIN_BLOCK_STATS_FIELD_LENGTH 80 + +/** + * VIR_DOMAIN_BLOCK_STATS_READ_BYTES: + * + * Macro represents the total number of read bytes of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_READ_BYTES "rd_bytes" + +/** + * VIR_DOMAIN_BLOCK_STATS_READ_REQ: + * + * Macro represents the total read requests of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_READ_REQ "rd_operations" + +/** + * VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES: + * + * Macro represents the total time spend on cache reads in + * nano-seconds of the block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES "rd_total_times" + +/** + * VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES: + * + * Macro represents the total number of write bytes of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES "wr_bytes" + +/** + * VIR_DOMAIN_BLOCK_STATS_WRITE_REQ: + * + * Macro represents the total write requests of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_WRITE_REQ "wr_operations" + +/** + * VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES: + * + * Macro represents the total time spend on cache writes in + * nano-seconds of the block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES "wr_total_times" + +/** + * VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ: + * + * Macro represents the total flush requests of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ "flush_operations" + +/** + * VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES: + * + * Macro represents the total time spend on cache flushing in + * nano-seconds of the block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES "flush_total_times" + +/** + * VIR_DOMAIN_BLOCK_STATS_ERRS: + * + * In Xen this returns the mysterious 'oo_req' + */ +#define VIR_DOMAIN_BLOCK_STATS_ERRS "errs" + /** * virDomainInterfaceStats: * @@ -1178,6 +1257,11 @@ int virDomainBlockStats (virDomainPtr dom, const char *path, virDomainBlockStatsPtr stats, size_t size); +int virDomainBlockStatsFlags (virDomainPtr dom, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); int virDomainInterfaceStats (virDomainPtr dom, const char *path, virDomainInterfaceStatsPtr stats, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 169c3ee..dc5a80b 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -483,6 +483,7 @@ LIBVIRT_0.9.4 { LIBVIRT_0.9.5 { global: virDomainMigrateGetMaxSpeed; + virDomainBlockStatsFlags; } LIBVIRT_0.9.4; # .... define new API here using predicted next version number .... -- 1.7.6

On Mon, Sep 05, 2011 at 04:38:28PM +0800, Osier Yang wrote:
--- include/libvirt/libvirt.h.in | 84 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 2 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8864865..735ea94 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -574,6 +574,85 @@ struct _virDomainBlockStats { */ typedef virDomainBlockStatsStruct *virDomainBlockStatsPtr;
+ +/** + * VIR_DOMAIN_BLOCK_STATS_FIELD_LENGTH: + * + * Macro providing the field length of virDomainBlockStatsFlagsStruct + */ +#define VIR_DOMAIN_BLOCK_STATS_FIELD_LENGTH 80 + +/** + * VIR_DOMAIN_BLOCK_STATS_READ_BYTES: + * + * Macro represents the total number of read bytes of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_READ_BYTES "rd_bytes" + +/** + * VIR_DOMAIN_BLOCK_STATS_READ_REQ: + * + * Macro represents the total read requests of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_READ_REQ "rd_operations" + +/** + * VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES: + * + * Macro represents the total time spend on cache reads in + * nano-seconds of the block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES "rd_total_times" + +/** + * VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES: + * + * Macro represents the total number of write bytes of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES "wr_bytes" + +/** + * VIR_DOMAIN_BLOCK_STATS_WRITE_REQ: + * + * Macro represents the total write requests of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_WRITE_REQ "wr_operations" + +/** + * VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES: + * + * Macro represents the total time spend on cache writes in + * nano-seconds of the block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES "wr_total_times" + +/** + * VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ: + * + * Macro represents the total flush requests of the + * block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ "flush_operations" + +/** + * VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES: + * + * Macro represents the total time spend on cache flushing in + * nano-seconds of the block device. + */ +#define VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES "flush_total_times" + +/** + * VIR_DOMAIN_BLOCK_STATS_ERRS: + * + * In Xen this returns the mysterious 'oo_req' + */ +#define VIR_DOMAIN_BLOCK_STATS_ERRS "errs" + /** * virDomainInterfaceStats: * @@ -1178,6 +1257,11 @@ int virDomainBlockStats (virDomainPtr dom, const char *path, virDomainBlockStatsPtr stats, size_t size); +int virDomainBlockStatsFlags (virDomainPtr dom, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); int virDomainInterfaceStats (virDomainPtr dom, const char *path, virDomainInterfaceStatsPtr stats, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 169c3ee..dc5a80b 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -483,6 +483,7 @@ LIBVIRT_0.9.4 { LIBVIRT_0.9.5 { global: virDomainMigrateGetMaxSpeed; + virDomainBlockStatsFlags; } LIBVIRT_0.9.4;
# .... define new API here using predicted next version number .... -- 1.7.6
ACK now, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- src/driver.h | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/driver.h b/src/driver.h index 21b2bd3..fc7a931 100644 --- a/src/driver.h +++ b/src/driver.h @@ -348,6 +348,13 @@ typedef int const char *path, struct _virDomainBlockStats *stats); typedef int + (*virDrvDomainBlockStatsFlags) + (virDomainPtr domain, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); +typedef int (*virDrvDomainInterfaceStats) (virDomainPtr domain, const char *path, @@ -806,6 +813,7 @@ struct _virDriver { virDrvDomainMigratePerform domainMigratePerform; virDrvDomainMigrateFinish domainMigrateFinish; virDrvDomainBlockStats domainBlockStats; + virDrvDomainBlockStatsFlags domainBlockStatsFlags; virDrvDomainInterfaceStats domainInterfaceStats; virDrvDomainMemoryStats domainMemoryStats; virDrvDomainBlockPeek domainBlockPeek; -- 1.7.6

On Mon, Sep 05, 2011 at 04:38:29PM +0800, Osier Yang wrote:
--- src/driver.h | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/driver.h b/src/driver.h index 21b2bd3..fc7a931 100644 --- a/src/driver.h +++ b/src/driver.h @@ -348,6 +348,13 @@ typedef int const char *path, struct _virDomainBlockStats *stats); typedef int + (*virDrvDomainBlockStatsFlags) + (virDomainPtr domain, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); +typedef int (*virDrvDomainInterfaceStats) (virDomainPtr domain, const char *path, @@ -806,6 +813,7 @@ struct _virDriver { virDrvDomainMigratePerform domainMigratePerform; virDrvDomainMigrateFinish domainMigrateFinish; virDrvDomainBlockStats domainBlockStats; + virDrvDomainBlockStatsFlags domainBlockStatsFlags; virDrvDomainInterfaceStats domainInterfaceStats; virDrvDomainMemoryStats domainMemoryStats; virDrvDomainBlockPeek domainBlockPeek;
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- src/libvirt.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 71 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 4d80e2f..a921e4d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6442,6 +6442,77 @@ error: } /** + * virDomainBlockStatsFlags: + * @dom: pointer to domain object + * @path: path to the block device + * @params: pointer to block stats parameter object + * (return value) + * @nparams: pointer to number of block stats + * @flags: unused, always passes 0 + * + * This function is to get block stats parameters for block + * devices attached to the domain. + * + * The @path is the name of the block device. Get this + * by calling virDomainGetXMLDesc and finding the <target dev='...'> + * attribute within //domain/devices/disk. (For example, "xvda"). + * + * Domains may have more than one block device. To get stats for + * each you should make multiple calls to this function. + * + * The @params array will be filled with the value equal to the number of + * parameters suggested by @nparams. + * + * As the value of @nparams is dynamic, call the API setting @nparams to 0 and + * @params as NULL, the API returns the number of parameters supported by the + * HV by updating @nparams on SUCCESS. (Note that block device of different type + * might support different parameters numbers, so it might be necessary to compute + * @nparams for each block device type). The caller should then allocate @params + * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API + * again. See virDomainGetMemoryParameters for more details. + * + * Returns -1 in case of error, 0 in case of success. + */ +int virDomainBlockStatsFlags (virDomainPtr dom, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "path=%s, params=%p, nparams=%d, flags=%x", + path, params, nparams ? *nparams : -1, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (!path || (nparams == NULL) || (*nparams < 0)) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + conn = dom->conn; + + if (conn->driver->domainBlockStatsFlags) { + int ret; + ret = conn->driver->domainBlockStatsFlags(dom, path, params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + + +/** * virDomainInterfaceStats: * @dom: pointer to the domain object * @path: path to the interface -- 1.7.6

On Mon, Sep 05, 2011 at 04:38:30PM +0800, Osier Yang wrote:
--- src/libvirt.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 4d80e2f..a921e4d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6442,6 +6442,77 @@ error: }
/** + * virDomainBlockStatsFlags: + * @dom: pointer to domain object + * @path: path to the block device + * @params: pointer to block stats parameter object
objects (plural)
+ * (return value) + * @nparams: pointer to number of block stats
need to describe the behaviour of allocation, i.e. who allocate, and what happens if the caller doesn't allocate enough parameters
+ * @flags: unused, always passes 0 + * + * This function is to get block stats parameters for block + * devices attached to the domain. + * + * The @path is the name of the block device. Get this + * by calling virDomainGetXMLDesc and finding the <target dev='...'> + * attribute within //domain/devices/disk. (For example, "xvda"). + * + * Domains may have more than one block device. To get stats for + * each you should make multiple calls to this function. + * + * The @params array will be filled with the value equal to the number of + * parameters suggested by @nparams.
still unclear if it is too much or too many
+ * As the value of @nparams is dynamic, call the API setting @nparams to 0 and + * @params as NULL, the API returns the number of parameters supported by the + * HV by updating @nparams on SUCCESS. (Note that block device of different type + * might support different parameters numbers, so it might be necessary to compute + * @nparams for each block device type). The caller should then allocate @params + * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API + * again. See virDomainGetMemoryParameters for more details.
tend to disagree, we should document the behaviour here, not reference another function docuemntation, copy as needed
+ * Returns -1 in case of error, 0 in case of success. + */ +int virDomainBlockStatsFlags (virDomainPtr dom, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "path=%s, params=%p, nparams=%d, flags=%x", + path, params, nparams ? *nparams : -1, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (!path || (nparams == NULL) || (*nparams < 0)) {
okay we accept param == NULL here,
+ virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + conn = dom->conn; + + if (conn->driver->domainBlockStatsFlags) { + int ret; + ret = conn->driver->domainBlockStatsFlags(dom, path, params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + + +/** * virDomainInterfaceStats: * @dom: pointer to the domain object * @path: path to the interface
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- daemon/remote.c | 69 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 64 ++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 19 +++++++++++- 3 files changed, 151 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d5ead81..38bbb10 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -933,6 +933,75 @@ cleanup: } static int +remoteDispatchDomainBlockStatsFlags(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_block_stats_flags_args *args, + remote_domain_block_stats_flags_ret *ret) +{ + virTypedParameterPtr params = NULL; + virDomainPtr dom = NULL; + int i; + const char *path = args->path; + int nparams = args->nparams; + unsigned int flags; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + flags = args->flags; + + if (nparams > REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; + } + if (VIR_ALLOC_N(params, nparams) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virDomainBlockStatsFlags(dom, path, params, &nparams, flags) < 0) + goto cleanup; + + /* In this case, we need to send back the number of parameters + * supported + */ + if (args->nparams == 0) { + ret->nparams = nparams; + goto success; + } + + /* Serialise the block stats. */ + if (remoteSerializeTypedParameters(params, nparams, + &ret->params.params_val, + &ret->params.params_len) < 0) + goto cleanup; + +success: + rv = 0; + +cleanup: + if (rv < 0) { + virNetMessageSaveError(rerr); + if (ret->params.params_val) { + for (i = 0; i < nparams; i++) + VIR_FREE(ret->params.params_val[i].field); + VIR_FREE(ret->params.params_val); + } + } + VIR_FREE(params); + return rv; +} + +static int remoteDispatchDomainMemoryPeek(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 783c404..9d34b7e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1361,6 +1361,69 @@ cleanup: } static int +remoteDomainBlockStatsFlags(virDomainPtr domain, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_stats_flags_args args; + remote_domain_block_stats_flags_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + args.nparams = *nparams; + args.path = (char *) path; + args.flags = flags; + + memset (&ret, 0, sizeof ret); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS, + (xdrproc_t) xdr_remote_domain_block_stats_flags_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_block_stats_flags_ret, (char *) &ret) == -1) + goto done; + + /* Check the length of the returned list carefully. */ + if (ret.params.params_len > REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX || + ret.params.params_len > *nparams) { + remoteError(VIR_ERR_RPC, "%s", + _("remoteDomainBlockStatsFlags: " + "returned number of stats exceeds limit")); + goto cleanup; + } + + /* Handle the case when the caller does not know the number of stats + * and is asking for the number of stats supported + */ + if (*nparams == 0) { + *nparams = ret.nparams; + rv = 0; + goto cleanup; + } + + *nparams = ret.params.params_len; + + /* Deserialise the result. */ + if (remoteDeserializeTypedParameters(ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, + params, + nparams) < 0) + goto cleanup; + + rv = 0; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_block_stats_flags_ret, + (char *) &ret); +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetMemoryParameters (virDomainPtr domain, virTypedParameterPtr params, int *nparams, unsigned int flags) @@ -4307,6 +4370,7 @@ static virDriver remote_driver = { .domainMigratePerform = remoteDomainMigratePerform, /* 0.3.2 */ .domainMigrateFinish = remoteDomainMigrateFinish, /* 0.3.2 */ .domainBlockStats = remoteDomainBlockStats, /* 0.3.2 */ + .domainBlockStatsFlags = remoteDomainBlockStatsFlags, /* 0.9.5 */ .domainInterfaceStats = remoteDomainInterfaceStats, /* 0.3.2 */ .domainMemoryStats = remoteDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = remoteDomainBlockPeek, /* 0.4.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 676570e..7214b6f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -131,6 +131,9 @@ const REMOTE_NODE_CPU_STATS_MAX = 16; /* Upper limit on list of node memory stats. */ const REMOTE_NODE_MEMORY_STATS_MAX = 16; +/* Upper limit on list of block stats. */ +const REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX = 16; + /* Upper limit on number of NUMA cells */ const REMOTE_NODE_MAX_CELLS = 1024; @@ -331,6 +334,7 @@ struct remote_node_get_memory_stats { unsigned hyper value; }; + /*----- Calls. -----*/ /* For each call we may have a 'remote_CALL_args' and 'remote_CALL_ret' @@ -544,6 +548,18 @@ struct remote_domain_block_stats_ret { /* insert@2 */ hyper errs; }; +struct remote_domain_block_stats_flags_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + int nparams; + unsigned int flags; +}; + +struct remote_domain_block_stats_flags_ret { + remote_typed_param params<REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX>; + int nparams; +}; + struct remote_domain_interface_stats_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2486,7 +2502,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_PULL = 240, /* autogen autogen */ REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242 /* autogen autogen */ + REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, /* autogen autogen */ + REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.6

On Mon, Sep 05, 2011 at 04:38:31PM +0800, Osier Yang wrote:
--- daemon/remote.c | 69 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 64 ++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 19 +++++++++++- 3 files changed, 151 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index d5ead81..38bbb10 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -933,6 +933,75 @@ cleanup: }
static int +remoteDispatchDomainBlockStatsFlags(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_block_stats_flags_args *args, + remote_domain_block_stats_flags_ret *ret) +{ + virTypedParameterPtr params = NULL; + virDomainPtr dom = NULL; + int i; + const char *path = args->path; + int nparams = args->nparams; + unsigned int flags; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + flags = args->flags; + + if (nparams > REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; + } + if (VIR_ALLOC_N(params, nparams) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virDomainBlockStatsFlags(dom, path, params, &nparams, flags) < 0) + goto cleanup; + + /* In this case, we need to send back the number of parameters + * supported + */ + if (args->nparams == 0) { + ret->nparams = nparams; + goto success; + } + + /* Serialise the block stats. */ + if (remoteSerializeTypedParameters(params, nparams, + &ret->params.params_val, + &ret->params.params_len) < 0) + goto cleanup; + +success: + rv = 0; + +cleanup: + if (rv < 0) { + virNetMessageSaveError(rerr); + if (ret->params.params_val) { + for (i = 0; i < nparams; i++) + VIR_FREE(ret->params.params_val[i].field); + VIR_FREE(ret->params.params_val); + } + } + VIR_FREE(params); + return rv; +} + +static int remoteDispatchDomainMemoryPeek(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 783c404..9d34b7e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1361,6 +1361,69 @@ cleanup: }
static int +remoteDomainBlockStatsFlags(virDomainPtr domain, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_stats_flags_args args; + remote_domain_block_stats_flags_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + args.nparams = *nparams; + args.path = (char *) path; + args.flags = flags; + + memset (&ret, 0, sizeof ret); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS, + (xdrproc_t) xdr_remote_domain_block_stats_flags_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_block_stats_flags_ret, (char *) &ret) == -1) + goto done; + + /* Check the length of the returned list carefully. */ + if (ret.params.params_len > REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX || + ret.params.params_len > *nparams) { + remoteError(VIR_ERR_RPC, "%s", + _("remoteDomainBlockStatsFlags: " + "returned number of stats exceeds limit")); + goto cleanup; + } + + /* Handle the case when the caller does not know the number of stats + * and is asking for the number of stats supported + */ + if (*nparams == 0) { + *nparams = ret.nparams; + rv = 0; + goto cleanup; + } + + *nparams = ret.params.params_len; + + /* Deserialise the result. */ + if (remoteDeserializeTypedParameters(ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, + params, + nparams) < 0) + goto cleanup; + + rv = 0; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_block_stats_flags_ret, + (char *) &ret); +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetMemoryParameters (virDomainPtr domain, virTypedParameterPtr params, int *nparams, unsigned int flags) @@ -4307,6 +4370,7 @@ static virDriver remote_driver = { .domainMigratePerform = remoteDomainMigratePerform, /* 0.3.2 */ .domainMigrateFinish = remoteDomainMigrateFinish, /* 0.3.2 */ .domainBlockStats = remoteDomainBlockStats, /* 0.3.2 */ + .domainBlockStatsFlags = remoteDomainBlockStatsFlags, /* 0.9.5 */ .domainInterfaceStats = remoteDomainInterfaceStats, /* 0.3.2 */ .domainMemoryStats = remoteDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = remoteDomainBlockPeek, /* 0.4.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 676570e..7214b6f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -131,6 +131,9 @@ const REMOTE_NODE_CPU_STATS_MAX = 16; /* Upper limit on list of node memory stats. */ const REMOTE_NODE_MEMORY_STATS_MAX = 16;
+/* Upper limit on list of block stats. */ +const REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX = 16; + /* Upper limit on number of NUMA cells */ const REMOTE_NODE_MAX_CELLS = 1024;
@@ -331,6 +334,7 @@ struct remote_node_get_memory_stats { unsigned hyper value; };
+ /*----- Calls. -----*/
/* For each call we may have a 'remote_CALL_args' and 'remote_CALL_ret' @@ -544,6 +548,18 @@ struct remote_domain_block_stats_ret { /* insert@2 */ hyper errs; };
+struct remote_domain_block_stats_flags_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + int nparams; + unsigned int flags; +}; + +struct remote_domain_block_stats_flags_ret { + remote_typed_param params<REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX>; + int nparams; +}; + struct remote_domain_interface_stats_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2486,7 +2502,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_PULL = 240, /* autogen autogen */
REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242 /* autogen autogen */ + REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, /* autogen autogen */ + REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243 /* skipgen skipgen */
/* * Notice how the entries are grouped in sets of 10 ?
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The mainly changes are: 1) Update qemuMonitorGetBlockStatsInfo and it's children (Text/JSON) functions to return the value of new latency fields. 2) Add new function qemuMonitorGetBlockStatsParamsNumber, which is to count how many parameters the underlying QEMU supports. 3) Update virDomainBlockStats in src/qemu/qemu_driver.c to be compatible with the changes by 1). --- src/qemu/qemu_driver.c | 4 ++ src/qemu/qemu_monitor.c | 35 ++++++++++++ src/qemu/qemu_monitor.h | 6 ++ src/qemu/qemu_monitor_json.c | 124 +++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_monitor_json.h | 6 ++ src/qemu/qemu_monitor_text.c | 121 +++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_monitor_text.h | 6 ++ 7 files changed, 291 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7028d72..c5809d2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7224,8 +7224,12 @@ qemudDomainBlockStats (virDomainPtr dom, disk->info.alias, &stats->rd_req, &stats->rd_bytes, + NULL, &stats->wr_req, &stats->wr_bytes, + NULL, + NULL, + NULL, &stats->errs); qemuDomainObjExitMonitor(driver, vm); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..92631ae 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1201,8 +1201,12 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs) { int ret; @@ -1217,16 +1221,47 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, if (mon->json) ret = qemuMonitorJSONGetBlockStatsInfo(mon, devname, rd_req, rd_bytes, + rd_total_times, wr_req, wr_bytes, + wr_total_times, + flush_req, + flush_total_times, errs); else ret = qemuMonitorTextGetBlockStatsInfo(mon, devname, rd_req, rd_bytes, + rd_total_times, wr_req, wr_bytes, + wr_total_times, + flush_req, + flush_total_times, errs); return ret; } +/* Return 0 and update @nparams with the number of block stats + * QEMU supports if success. Return -1 if failure. + */ +int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams) +{ + int ret; + VIR_DEBUG("mon=%p nparams=%p", mon, nparams); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONGetBlockStatsParamsNumber(mon, nparams); + else + ret = qemuMonitorTextGetBlockStatsParamsNumber(mon, nparams); + + return ret; +} + int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..1b9d98d 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -212,9 +212,15 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs); +int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams); int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *devname, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4ceb536..0cc4702 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1318,8 +1318,12 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs) { int ret; @@ -1330,7 +1334,17 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; virJSONValuePtr devices; - *rd_req = *rd_bytes = *wr_req = *wr_bytes = *errs = 0; + *rd_req = *rd_bytes = -1; + *wr_req = *wr_bytes = *errs = -1; + + if (rd_total_times) + *rd_total_times = -1; + if (wr_total_times) + *wr_total_times = -1; + if (flush_req) + *flush_req = -1; + if (flush_total_times) + *flush_total_times = -1; if (!cmd) return -1; @@ -1396,6 +1410,15 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, "rd_operations"); goto cleanup; } + if (rd_total_times && + virJSONValueObjectHasKey(stats, "rd_total_times_ns") && + (virJSONValueObjectGetNumberLong(stats, "rd_total_times_ns", + rd_total_times) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "rd_total_times_ns"); + goto cleanup; + } if (virJSONValueObjectGetNumberLong(stats, "wr_bytes", wr_bytes) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), @@ -1408,6 +1431,33 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, "wr_operations"); goto cleanup; } + if (wr_total_times && + virJSONValueObjectHasKey(stats, "wr_total_times_ns") && + (virJSONValueObjectGetNumberLong(stats, "wr_total_times_ns", + wr_total_times) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "wr_total_times_ns"); + goto cleanup; + } + if (flush_req && + virJSONValueObjectHasKey(stats, "flush_operations") && + (virJSONValueObjectGetNumberLong(stats, "flush_operations", + flush_req) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "flush_operations"); + goto cleanup; + } + if (flush_total_times && + virJSONValueObjectHasKey(stats, "flush_total_times_ns") && + (virJSONValueObjectGetNumberLong(stats, "flush_total_times_ns", + flush_total_times) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "flush_total_times_ns"); + goto cleanup; + } } if (!found) { @@ -1424,6 +1474,78 @@ cleanup: } +int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams) +{ + int ret, i, num = 0; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats", + NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr devices = NULL; + virJSONValuePtr dev = NULL; + virJSONValuePtr stats = NULL; + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + if (ret < 0) + goto cleanup; + ret = -1; + + devices = virJSONValueObjectGet(reply, "return"); + if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats reply was missing device list")); + goto cleanup; + } + + dev = virJSONValueArrayGet(devices, 0); + + if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats device entry was not in expected format")); + goto cleanup; + } + + if ((stats = virJSONValueObjectGet(dev, "stats")) == NULL || + stats->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats stats entry was not in expected format")); + goto cleanup; + } + + for (i = 0 ; i < stats->data.object.npairs; i++) { + const char *key = stats->data.object.pairs[i].key; + + if (STREQ(key, "rd_bytes") || + STREQ(key, "rd_operations") || + STREQ(key, "rd_total_times_ns") || + STREQ(key, "wr_bytes") || + STREQ(key, "wr_operations") || + STREQ(key, "wr_total_times_ns") || + STREQ(key, "flush_operations") || + STREQ(key, "flush_total_times_ns")) { + num++; + } else { + /* wr_highest_offset is parsed by qemuMonitorJSONGetBlockExtent. */ + if (STRNEQ(key, "wr_highest_offset")) + VIR_DEBUG("Missed block stat: %s", key); + } + } + + *nparams = num; + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..ec9fecb 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -64,9 +64,15 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs); +int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams); int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 854ee7f..9bbb8b0 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -708,8 +708,12 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs) { char *info = NULL; @@ -736,11 +740,17 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } - *rd_req = -1; - *rd_bytes = -1; - *wr_req = -1; - *wr_bytes = -1; - *errs = -1; + *rd_req = *rd_bytes = -1; + *wr_req = *wr_bytes = *errs = -1; + + if (rd_total_times) + *rd_total_times = -1; + if (wr_total_times) + *wr_total_times = -1; + if (flush_req) + *flush_req = -1; + if (flush_total_times) + *flush_total_times = -1; /* The output format for both qemu & KVM is: * blockdevice: rd_bytes=% wr_bytes=% rd_operations=% wr_operations=% @@ -768,23 +778,44 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, while (*p) { if (STRPREFIX (p, "rd_bytes=")) { - p += 9; + p += strlen("rd_bytes="); if (virStrToLong_ll (p, &dummy, 10, rd_bytes) == -1) VIR_DEBUG ("error reading rd_bytes: %s", p); } else if (STRPREFIX (p, "wr_bytes=")) { - p += 9; + p += strlen("wr_bytes="); if (virStrToLong_ll (p, &dummy, 10, wr_bytes) == -1) VIR_DEBUG ("error reading wr_bytes: %s", p); } else if (STRPREFIX (p, "rd_operations=")) { - p += 14; + p += strlen("rd_operations="); if (virStrToLong_ll (p, &dummy, 10, rd_req) == -1) VIR_DEBUG ("error reading rd_req: %s", p); } else if (STRPREFIX (p, "wr_operations=")) { - p += 14; + p += strlen("wr_operations="); if (virStrToLong_ll (p, &dummy, 10, wr_req) == -1) VIR_DEBUG ("error reading wr_req: %s", p); - } else + } else if (rd_total_times && + STRPREFIX (p, "rd_total_times_ns=")) { + p += strlen("rd_total_times_ns="); + if (virStrToLong_ll (p, &dummy, 10, rd_total_times) == -1) + VIR_DEBUG ("error reading rd_total_times: %s", p); + } else if (wr_total_times && + STRPREFIX (p, "wr_total_times_ns=")) { + p += strlen("wr_total_times_ns="); + if (virStrToLong_ll (p, &dummy, 10, wr_total_times) == -1) + VIR_DEBUG ("error reading wr_total_times: %s", p); + } else if (flush_req && + STRPREFIX (p, "flush_operations=")) { + p += strlen("flush_operations="); + if (virStrToLong_ll (p, &dummy, 10, flush_req) == -1) + VIR_DEBUG ("error reading flush_req: %s", p); + } else if (flush_total_times && + STRPREFIX (p, "flush_total_times_ns=")) { + p += strlen("flush_total_times_ns="); + if (virStrToLong_ll (p, &dummy, 10, flush_total_times) == -1) + VIR_DEBUG ("error reading flush_total_times: %s", p); + } else { VIR_DEBUG ("unknown block stat near %s", p); + } /* Skip to next label. */ p = strchr (p, ' '); @@ -810,6 +841,76 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, return ret; } +int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams) +{ + char *info = NULL; + int ret = -1; + int num = 0; + const char *p, *eol; + + if (qemuMonitorHMPCommand (mon, "info blockstats", &info) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("'info blockstats' command failed")); + goto cleanup; + } + + /* If the command isn't supported then qemu prints the supported + * info commands, so the output starts "info ". Since this is + * unlikely to be the name of a block device, we can use this + * to detect if qemu supports the command. + */ + if (strstr(info, "\ninfo ")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", + _("'info blockstats' not supported by this qemu")); + goto cleanup; + } + + /* The output format for both qemu & KVM is: + * blockdevice: rd_bytes=% wr_bytes=% rd_operations=% wr_operations=% + * (repeated for each block device) + * where '%' is a 64 bit number. + */ + p = info; + + eol = strchr (p, '\n'); + if (!eol) + eol = p + strlen (p); + + /* Skip the device name and following ":", and spaces (e.g. + * "floppy0: ") + */ + p = strchr(p, ' '); + p++; + + while (*p) { + if (STRPREFIX (p, "rd_bytes=") || + STRPREFIX (p, "wr_bytes=") || + STRPREFIX (p, "rd_operations=") || + STRPREFIX (p, "wr_operations=") || + STRPREFIX (p, "rd_total_times_ns=") || + STRPREFIX (p, "wr_total_times_ns=") || + STRPREFIX (p, "flush_operations=") || + STRPREFIX (p, "flush_total_times_ns=")) { + num++; + } else { + VIR_DEBUG ("unknown block stat near %s", p); + } + + /* Skip to next label. */ + p = strchr (p, ' '); + if (!p || p >= eol) break; + p++; + } + + *nparams = num; + ret = 0; + + cleanup: + VIR_FREE(info); + return ret; +} int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *devname ATTRIBUTE_UNUSED, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b250738..de6fbcc 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -61,9 +61,15 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs); +int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams); int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent); -- 1.7.6

On Mon, Sep 05, 2011 at 04:38:32PM +0800, Osier Yang wrote:
The mainly changes are:
1) Update qemuMonitorGetBlockStatsInfo and it's children (Text/JSON) functions to return the value of new latency fields. 2) Add new function qemuMonitorGetBlockStatsParamsNumber, which is to count how many parameters the underlying QEMU supports. 3) Update virDomainBlockStats in src/qemu/qemu_driver.c to be compatible with the changes by 1). --- src/qemu/qemu_driver.c | 4 ++ src/qemu/qemu_monitor.c | 35 ++++++++++++ src/qemu/qemu_monitor.h | 6 ++ src/qemu/qemu_monitor_json.c | 124 +++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_monitor_json.h | 6 ++ src/qemu/qemu_monitor_text.c | 121 +++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_monitor_text.h | 6 ++ 7 files changed, 291 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7028d72..c5809d2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7224,8 +7224,12 @@ qemudDomainBlockStats (virDomainPtr dom, disk->info.alias, &stats->rd_req, &stats->rd_bytes, + NULL, &stats->wr_req, &stats->wr_bytes, + NULL, + NULL, + NULL, &stats->errs); qemuDomainObjExitMonitor(driver, vm);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..92631ae 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1201,8 +1201,12 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs) { int ret; @@ -1217,16 +1221,47 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, if (mon->json) ret = qemuMonitorJSONGetBlockStatsInfo(mon, devname, rd_req, rd_bytes, + rd_total_times, wr_req, wr_bytes, + wr_total_times, + flush_req, + flush_total_times, errs); else ret = qemuMonitorTextGetBlockStatsInfo(mon, devname, rd_req, rd_bytes, + rd_total_times, wr_req, wr_bytes, + wr_total_times, + flush_req, + flush_total_times, errs); return ret; }
+/* Return 0 and update @nparams with the number of block stats + * QEMU supports if success. Return -1 if failure. + */ +int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams) +{ + int ret; + VIR_DEBUG("mon=%p nparams=%p", mon, nparams); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONGetBlockStatsParamsNumber(mon, nparams); + else + ret = qemuMonitorTextGetBlockStatsParamsNumber(mon, nparams); + + return ret; +} + int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..1b9d98d 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -212,9 +212,15 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs); +int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams);
int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *devname, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4ceb536..0cc4702 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1318,8 +1318,12 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs) { int ret; @@ -1330,7 +1334,17 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; virJSONValuePtr devices;
- *rd_req = *rd_bytes = *wr_req = *wr_bytes = *errs = 0; + *rd_req = *rd_bytes = -1; + *wr_req = *wr_bytes = *errs = -1; + + if (rd_total_times) + *rd_total_times = -1; + if (wr_total_times) + *wr_total_times = -1; + if (flush_req) + *flush_req = -1; + if (flush_total_times) + *flush_total_times = -1;
if (!cmd) return -1; @@ -1396,6 +1410,15 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, "rd_operations"); goto cleanup; } + if (rd_total_times && + virJSONValueObjectHasKey(stats, "rd_total_times_ns") && + (virJSONValueObjectGetNumberLong(stats, "rd_total_times_ns", + rd_total_times) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "rd_total_times_ns"); + goto cleanup; + } if (virJSONValueObjectGetNumberLong(stats, "wr_bytes", wr_bytes) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), @@ -1408,6 +1431,33 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, "wr_operations"); goto cleanup; } + if (wr_total_times && + virJSONValueObjectHasKey(stats, "wr_total_times_ns") && + (virJSONValueObjectGetNumberLong(stats, "wr_total_times_ns", + wr_total_times) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "wr_total_times_ns"); + goto cleanup; + } + if (flush_req && + virJSONValueObjectHasKey(stats, "flush_operations") && + (virJSONValueObjectGetNumberLong(stats, "flush_operations", + flush_req) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "flush_operations"); + goto cleanup; + } + if (flush_total_times && + virJSONValueObjectHasKey(stats, "flush_total_times_ns") && + (virJSONValueObjectGetNumberLong(stats, "flush_total_times_ns", + flush_total_times) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "flush_total_times_ns"); + goto cleanup; + } }
if (!found) { @@ -1424,6 +1474,78 @@ cleanup: }
+int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams) +{ + int ret, i, num = 0; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats", + NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr devices = NULL; + virJSONValuePtr dev = NULL; + virJSONValuePtr stats = NULL; + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + if (ret < 0) + goto cleanup; + ret = -1; + + devices = virJSONValueObjectGet(reply, "return"); + if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats reply was missing device list")); + goto cleanup; + } + + dev = virJSONValueArrayGet(devices, 0); + + if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats device entry was not in expected format")); + goto cleanup; + } + + if ((stats = virJSONValueObjectGet(dev, "stats")) == NULL || + stats->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats stats entry was not in expected format")); + goto cleanup; + } + + for (i = 0 ; i < stats->data.object.npairs; i++) { + const char *key = stats->data.object.pairs[i].key; + + if (STREQ(key, "rd_bytes") || + STREQ(key, "rd_operations") || + STREQ(key, "rd_total_times_ns") || + STREQ(key, "wr_bytes") || + STREQ(key, "wr_operations") || + STREQ(key, "wr_total_times_ns") || + STREQ(key, "flush_operations") || + STREQ(key, "flush_total_times_ns")) { + num++; + } else { + /* wr_highest_offset is parsed by qemuMonitorJSONGetBlockExtent. */ + if (STRNEQ(key, "wr_highest_offset")) + VIR_DEBUG("Missed block stat: %s", key); + } + } + + *nparams = num; + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..ec9fecb 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -64,9 +64,15 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs); +int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams); int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 854ee7f..9bbb8b0 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -708,8 +708,12 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs) { char *info = NULL; @@ -736,11 +740,17 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; }
- *rd_req = -1; - *rd_bytes = -1; - *wr_req = -1; - *wr_bytes = -1; - *errs = -1; + *rd_req = *rd_bytes = -1; + *wr_req = *wr_bytes = *errs = -1; + + if (rd_total_times) + *rd_total_times = -1; + if (wr_total_times) + *wr_total_times = -1; + if (flush_req) + *flush_req = -1; + if (flush_total_times) + *flush_total_times = -1;
/* The output format for both qemu & KVM is: * blockdevice: rd_bytes=% wr_bytes=% rd_operations=% wr_operations=% @@ -768,23 +778,44 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon,
while (*p) { if (STRPREFIX (p, "rd_bytes=")) { - p += 9; + p += strlen("rd_bytes="); if (virStrToLong_ll (p, &dummy, 10, rd_bytes) == -1) VIR_DEBUG ("error reading rd_bytes: %s", p); } else if (STRPREFIX (p, "wr_bytes=")) { - p += 9; + p += strlen("wr_bytes="); if (virStrToLong_ll (p, &dummy, 10, wr_bytes) == -1) VIR_DEBUG ("error reading wr_bytes: %s", p); } else if (STRPREFIX (p, "rd_operations=")) { - p += 14; + p += strlen("rd_operations="); if (virStrToLong_ll (p, &dummy, 10, rd_req) == -1) VIR_DEBUG ("error reading rd_req: %s", p); } else if (STRPREFIX (p, "wr_operations=")) { - p += 14; + p += strlen("wr_operations="); if (virStrToLong_ll (p, &dummy, 10, wr_req) == -1) VIR_DEBUG ("error reading wr_req: %s", p); - } else + } else if (rd_total_times && + STRPREFIX (p, "rd_total_times_ns=")) { + p += strlen("rd_total_times_ns="); + if (virStrToLong_ll (p, &dummy, 10, rd_total_times) == -1) + VIR_DEBUG ("error reading rd_total_times: %s", p); + } else if (wr_total_times && + STRPREFIX (p, "wr_total_times_ns=")) { + p += strlen("wr_total_times_ns="); + if (virStrToLong_ll (p, &dummy, 10, wr_total_times) == -1) + VIR_DEBUG ("error reading wr_total_times: %s", p); + } else if (flush_req && + STRPREFIX (p, "flush_operations=")) { + p += strlen("flush_operations="); + if (virStrToLong_ll (p, &dummy, 10, flush_req) == -1) + VIR_DEBUG ("error reading flush_req: %s", p); + } else if (flush_total_times && + STRPREFIX (p, "flush_total_times_ns=")) { + p += strlen("flush_total_times_ns="); + if (virStrToLong_ll (p, &dummy, 10, flush_total_times) == -1) + VIR_DEBUG ("error reading flush_total_times: %s", p); + } else { VIR_DEBUG ("unknown block stat near %s", p); + }
/* Skip to next label. */ p = strchr (p, ' '); @@ -810,6 +841,76 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, return ret; }
+int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams) +{ + char *info = NULL; + int ret = -1; + int num = 0; + const char *p, *eol; + + if (qemuMonitorHMPCommand (mon, "info blockstats", &info) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("'info blockstats' command failed")); + goto cleanup; + } + + /* If the command isn't supported then qemu prints the supported + * info commands, so the output starts "info ". Since this is + * unlikely to be the name of a block device, we can use this + * to detect if qemu supports the command. + */ + if (strstr(info, "\ninfo ")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", + _("'info blockstats' not supported by this qemu")); + goto cleanup; + } + + /* The output format for both qemu & KVM is: + * blockdevice: rd_bytes=% wr_bytes=% rd_operations=% wr_operations=% + * (repeated for each block device) + * where '%' is a 64 bit number. + */ + p = info; + + eol = strchr (p, '\n'); + if (!eol) + eol = p + strlen (p); + + /* Skip the device name and following ":", and spaces (e.g. + * "floppy0: ") + */ + p = strchr(p, ' '); + p++; + + while (*p) { + if (STRPREFIX (p, "rd_bytes=") || + STRPREFIX (p, "wr_bytes=") || + STRPREFIX (p, "rd_operations=") || + STRPREFIX (p, "wr_operations=") || + STRPREFIX (p, "rd_total_times_ns=") || + STRPREFIX (p, "wr_total_times_ns=") || + STRPREFIX (p, "flush_operations=") || + STRPREFIX (p, "flush_total_times_ns=")) { + num++; + } else { + VIR_DEBUG ("unknown block stat near %s", p); + } + + /* Skip to next label. */ + p = strchr (p, ' '); + if (!p || p >= eol) break; + p++; + } + + *nparams = num; + ret = 0; + + cleanup: + VIR_FREE(info); + return ret; +}
int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *devname ATTRIBUTE_UNUSED, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b250738..de6fbcc 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -61,9 +61,15 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, long long *rd_bytes, + long long *rd_total_times, long long *wr_req, long long *wr_bytes, + long long *wr_total_times, + long long *flush_req, + long long *flush_total_times, long long *errs); +int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, + int *nparams); int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent);
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- src/qemu/qemu_driver.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 189 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c5809d2..a8b2b6d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7243,6 +7243,194 @@ cleanup: return ret; } +static int +qemudDomainBlockStatsFlags (virDomainPtr dom, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i, tmp, ret = -1; + virDomainObjPtr vm; + virDomainDiskDefPtr disk = NULL; + qemuDomainObjPrivatePtr priv; + long long rd_req, rd_bytes, wr_req, wr_bytes, rd_total_times; + long long wr_total_times, flush_req, flush_total_times, errs; + + virCheckFlags(0, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (*nparams != 0) { + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(path, vm->def->disks[i]->dst)) { + disk = vm->def->disks[i]; + break; + } + } + + if (!disk) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("invalid path: %s"), path); + goto cleanup; + } + + if (!disk->info.alias) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("missing disk device alias name for %s"), disk->dst); + goto cleanup; + } + } + + priv = vm->privateData; + VIR_DEBUG("priv=%p, params=%p, flags=%x", priv, params, flags); + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + tmp = *nparams; + ret = qemuMonitorGetBlockStatsParamsNumber(priv->mon, nparams); + + if (tmp == 0) { + qemuDomainObjExitMonitor(driver, vm); + goto endjob; + } + + ret = qemuMonitorGetBlockStatsInfo(priv->mon, + disk->info.alias, + &rd_req, + &rd_bytes, + &rd_total_times, + &wr_req, + &wr_bytes, + &wr_total_times, + &flush_req, + &flush_total_times, + &errs); + + qemuDomainObjExitMonitor(driver, vm); + + if (ret < 0) + goto endjob; + + /* Field 'errs' is meaningless for QEMU, won't set it. */ + for (i = 0; i < *nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + + switch (i) { + case 0: /* fill write_bytes here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field write bytes too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = wr_bytes; + break; + + case 1: /* fill wr_operations here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field write requests too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = wr_req; + break; + + case 2: /* fill read_bytes here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_BYTES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field read bytes too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = rd_bytes; + break; + + case 3: /* fill rd_operations here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_REQ) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field read requests too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = rd_req; + break; + + case 4: /* fill flush_operations here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field flush requests too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = flush_req; + break; + + case 5: /* fill wr_total_times_ns here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field write total times too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = wr_total_times; + break; + + case 6: /* fill rd_total_times_ns here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field read total times too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = rd_total_times; + break; + + case 7: /* fill flush_total_times_ns here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field flush total times too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = flush_total_times; + break; + + default: + break; + /* should not hit here */ + } + } + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + #ifdef __linux__ static int qemudDomainInterfaceStats (virDomainPtr dom, @@ -9984,6 +10172,7 @@ static virDriver qemuDriver = { .domainSetSchedulerParametersFlags = qemuSetSchedulerParametersFlags, /* 0.9.2 */ .domainMigratePerform = qemudDomainMigratePerform, /* 0.5.0 */ .domainBlockStats = qemudDomainBlockStats, /* 0.4.1 */ + .domainBlockStatsFlags = qemudDomainBlockStatsFlags, /* 0.9.5 */ .domainInterfaceStats = qemudDomainInterfaceStats, /* 0.4.1 */ .domainMemoryStats = qemudDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = qemudDomainBlockPeek, /* 0.4.4 */ -- 1.7.6

On Mon, Sep 05, 2011 at 04:38:33PM +0800, Osier Yang wrote:
--- src/qemu/qemu_driver.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 189 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c5809d2..a8b2b6d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7243,6 +7243,194 @@ cleanup: return ret; }
+static int +qemudDomainBlockStatsFlags (virDomainPtr dom, + const char *path, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i, tmp, ret = -1; + virDomainObjPtr vm; + virDomainDiskDefPtr disk = NULL; + qemuDomainObjPrivatePtr priv; + long long rd_req, rd_bytes, wr_req, wr_bytes, rd_total_times; + long long wr_total_times, flush_req, flush_total_times, errs; + + virCheckFlags(0, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (*nparams != 0) { + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(path, vm->def->disks[i]->dst)) { + disk = vm->def->disks[i]; + break; + } + } + + if (!disk) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("invalid path: %s"), path); + goto cleanup; + } + + if (!disk->info.alias) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("missing disk device alias name for %s"), disk->dst); + goto cleanup; + } + } + + priv = vm->privateData; + VIR_DEBUG("priv=%p, params=%p, flags=%x", priv, params, flags); + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + tmp = *nparams; + ret = qemuMonitorGetBlockStatsParamsNumber(priv->mon, nparams); + + if (tmp == 0) { + qemuDomainObjExitMonitor(driver, vm); + goto endjob; + } + + ret = qemuMonitorGetBlockStatsInfo(priv->mon, + disk->info.alias, + &rd_req, + &rd_bytes, + &rd_total_times, + &wr_req, + &wr_bytes, + &wr_total_times, + &flush_req, + &flush_total_times, + &errs); + + qemuDomainObjExitMonitor(driver, vm); + + if (ret < 0) + goto endjob; + + /* Field 'errs' is meaningless for QEMU, won't set it. */ + for (i = 0; i < *nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + + switch (i) { + case 0: /* fill write_bytes here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field write bytes too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = wr_bytes; + break; + + case 1: /* fill wr_operations here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field write requests too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = wr_req; + break; + + case 2: /* fill read_bytes here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_BYTES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field read bytes too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = rd_bytes; + break; + + case 3: /* fill rd_operations here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_REQ) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field read requests too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = rd_req; + break; + + case 4: /* fill flush_operations here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field flush requests too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = flush_req; + break; + + case 5: /* fill wr_total_times_ns here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field write total times too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = wr_total_times; + break; + + case 6: /* fill rd_total_times_ns here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field read total times too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = rd_total_times; + break; + + case 7: /* fill flush_total_times_ns here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field flush total times too long for destination")); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_LLONG; + param->value.l = flush_total_times; + break; + + default: + break; + /* should not hit here */ + } + } + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + #ifdef __linux__ static int qemudDomainInterfaceStats (virDomainPtr dom, @@ -9984,6 +10172,7 @@ static virDriver qemuDriver = { .domainSetSchedulerParametersFlags = qemuSetSchedulerParametersFlags, /* 0.9.2 */ .domainMigratePerform = qemudDomainMigratePerform, /* 0.5.0 */ .domainBlockStats = qemudDomainBlockStats, /* 0.4.1 */ + .domainBlockStatsFlags = qemudDomainBlockStatsFlags, /* 0.9.5 */ .domainInterfaceStats = qemudDomainInterfaceStats, /* 0.4.1 */ .domainMemoryStats = qemudDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = qemudDomainBlockPeek, /* 0.4.4 */
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- python/generator.py | 1 + python/libvirt-override-api.xml | 7 +++ python/libvirt-override.c | 85 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index cc253cf..d17fb26 100755 --- a/python/generator.py +++ b/python/generator.py @@ -373,6 +373,7 @@ skip_impl = ( 'virNodeGetMemoryStats', 'virDomainGetBlockJobInfo', 'virDomainMigrateGetMaxSpeed', + 'virDomainBlockStatsFlags', ) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 1cf115c..3013e46 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -128,6 +128,13 @@ <arg name='domain' type='virDomainPtr' info='a domain object'/> <arg name='path' type='char *' info='the path for the block device'/> </function> + <function name='virDomainBlockStatsFlags' file='python'> + <info>Extracts block device statistics parameters of a running domain</info> + <return type='virTypedParameterPtr' info='None in case of error, returns a dictionary of params'/> + <arg name='domain' type='virDomainPtr' info='pointer to domain object'/> + <arg name='path' type='char *' info='the path for the block device'/> + <arg name='flags' type='int' info='flags (unused; pass 0)'/> + </function> <function name='virDomainInterfaceStats' file='python'> <info>Extracts interface device statistics for a domain</info> <return type='virDomainInterfaceStats' info='a tuple of statistics'/> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index b020342..d65423d 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -100,6 +100,90 @@ libvirt_virDomainBlockStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { } static PyObject * +libvirt_virDomainBlockStatsFlags(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + int i_retval; + int nparams = 0, i; + unsigned int flags; + virTypedParameterPtr params; + const char *path; + + if (!PyArg_ParseTuple(args, (char *)"Ozi:virDomainBlockStatsFlags", + &pyobj_domain, &path, &flags)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainBlockStatsFlags(domain, path, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) + return VIR_PY_NONE; + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return VIR_PY_NONE; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainBlockStatsFlags(domain, path, params, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + free(params); + return VIR_PY_NONE; + } + + /* convert to a Python tuple of long objects */ + if ((info = PyDict_New()) == NULL) { + free(params); + return VIR_PY_NONE; + } + + for (i = 0 ; i < nparams ; i++) { + PyObject *key, *val; + + switch (params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong((long)params[i].value.i); + break; + + case VIR_TYPED_PARAM_UINT: + val = PyInt_FromLong((long)params[i].value.ui); + break; + + case VIR_TYPED_PARAM_LLONG: + val = PyLong_FromLongLong((long long)params[i].value.l); + break; + + case VIR_TYPED_PARAM_ULLONG: + val = PyLong_FromLongLong((long long)params[i].value.ul); + break; + + case VIR_TYPED_PARAM_DOUBLE: + val = PyFloat_FromDouble((double)params[i].value.d); + break; + + case VIR_TYPED_PARAM_BOOLEAN: + val = PyBool_FromLong((long)params[i].value.b); + break; + + default: + free(params); + Py_DECREF(info); + return VIR_PY_NONE; + } + + key = libvirt_constcharPtrWrap(params[i].field); + PyDict_SetItem(info, key, val); + } + + free(params); + return(info); +} + + +static PyObject * libvirt_virDomainInterfaceStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; PyObject *pyobj_domain; @@ -4605,6 +4689,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainGetAutostart", libvirt_virDomainGetAutostart, METH_VARARGS, NULL}, {(char *) "virNetworkGetAutostart", libvirt_virNetworkGetAutostart, METH_VARARGS, NULL}, {(char *) "virDomainBlockStats", libvirt_virDomainBlockStats, METH_VARARGS, NULL}, + {(char *) "virDomainBlockStatsFlags", libvirt_virDomainBlockStatsFlags, METH_VARARGS, NULL}, {(char *) "virDomainInterfaceStats", libvirt_virDomainInterfaceStats, METH_VARARGS, NULL}, {(char *) "virDomainMemoryStats", libvirt_virDomainMemoryStats, METH_VARARGS, NULL}, {(char *) "virNodeGetCellsFreeMemory", libvirt_virNodeGetCellsFreeMemory, METH_VARARGS, NULL}, -- 1.7.6

On Mon, Sep 05, 2011 at 04:38:34PM +0800, Osier Yang wrote:
--- python/generator.py | 1 + python/libvirt-override-api.xml | 7 +++ python/libvirt-override.c | 85 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 0 deletions(-)
diff --git a/python/generator.py b/python/generator.py index cc253cf..d17fb26 100755 --- a/python/generator.py +++ b/python/generator.py @@ -373,6 +373,7 @@ skip_impl = ( 'virNodeGetMemoryStats', 'virDomainGetBlockJobInfo', 'virDomainMigrateGetMaxSpeed', + 'virDomainBlockStatsFlags', )
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 1cf115c..3013e46 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -128,6 +128,13 @@ <arg name='domain' type='virDomainPtr' info='a domain object'/> <arg name='path' type='char *' info='the path for the block device'/> </function> + <function name='virDomainBlockStatsFlags' file='python'> + <info>Extracts block device statistics parameters of a running domain</info> + <return type='virTypedParameterPtr' info='None in case of error, returns a dictionary of params'/> + <arg name='domain' type='virDomainPtr' info='pointer to domain object'/> + <arg name='path' type='char *' info='the path for the block device'/> + <arg name='flags' type='int' info='flags (unused; pass 0)'/> + </function> <function name='virDomainInterfaceStats' file='python'> <info>Extracts interface device statistics for a domain</info> <return type='virDomainInterfaceStats' info='a tuple of statistics'/> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index b020342..d65423d 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -100,6 +100,90 @@ libvirt_virDomainBlockStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { }
static PyObject * +libvirt_virDomainBlockStatsFlags(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + int i_retval; + int nparams = 0, i; + unsigned int flags; + virTypedParameterPtr params; + const char *path; + + if (!PyArg_ParseTuple(args, (char *)"Ozi:virDomainBlockStatsFlags", + &pyobj_domain, &path, &flags)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainBlockStatsFlags(domain, path, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) + return VIR_PY_NONE; + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return VIR_PY_NONE; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainBlockStatsFlags(domain, path, params, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + free(params); + return VIR_PY_NONE; + } + + /* convert to a Python tuple of long objects */ + if ((info = PyDict_New()) == NULL) { + free(params); + return VIR_PY_NONE; + } + + for (i = 0 ; i < nparams ; i++) { + PyObject *key, *val; + + switch (params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong((long)params[i].value.i); + break; + + case VIR_TYPED_PARAM_UINT: + val = PyInt_FromLong((long)params[i].value.ui); + break; + + case VIR_TYPED_PARAM_LLONG: + val = PyLong_FromLongLong((long long)params[i].value.l); + break; + + case VIR_TYPED_PARAM_ULLONG: + val = PyLong_FromLongLong((long long)params[i].value.ul); + break; + + case VIR_TYPED_PARAM_DOUBLE: + val = PyFloat_FromDouble((double)params[i].value.d); + break; + + case VIR_TYPED_PARAM_BOOLEAN: + val = PyBool_FromLong((long)params[i].value.b); + break; + + default: + free(params); + Py_DECREF(info); + return VIR_PY_NONE; + } + + key = libvirt_constcharPtrWrap(params[i].field); + PyDict_SetItem(info, key, val); + } + + free(params); + return(info); +} + + +static PyObject * libvirt_virDomainInterfaceStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; PyObject *pyobj_domain; @@ -4605,6 +4689,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainGetAutostart", libvirt_virDomainGetAutostart, METH_VARARGS, NULL}, {(char *) "virNetworkGetAutostart", libvirt_virNetworkGetAutostart, METH_VARARGS, NULL}, {(char *) "virDomainBlockStats", libvirt_virDomainBlockStats, METH_VARARGS, NULL}, + {(char *) "virDomainBlockStatsFlags", libvirt_virDomainBlockStatsFlags, METH_VARARGS, NULL}, {(char *) "virDomainInterfaceStats", libvirt_virDomainInterfaceStats, METH_VARARGS, NULL}, {(char *) "virDomainMemoryStats", libvirt_virDomainMemoryStats, METH_VARARGS, NULL}, {(char *) "virNodeGetCellsFreeMemory", libvirt_virNodeGetCellsFreeMemory, METH_VARARGS, NULL},
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The modified function fallbacks to use virDomainBlockStats if virDomainBlockStatsFlags is not supported by the hypervisor driver. If the new API is supported, it will be invoked instead of the old API. --- tools/virsh.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 84 insertions(+), 20 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c7240e5..ea41221 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1070,6 +1070,9 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; const char *name = NULL, *device = NULL; struct _virDomainBlockStats stats; + virTypedParameterPtr params = NULL; + int rc, nparams = 0; + bool ret = false; if (!vshConnectionUsability (ctl, ctl->conn)) return false; @@ -1077,34 +1080,95 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain (ctl, cmd, &name))) return false; - if (vshCommandOptString (cmd, "device", &device) <= 0) { - virDomainFree(dom); - return false; - } + if (vshCommandOptString (cmd, "device", &device) <= 0) + goto cleanup; - if (virDomainBlockStats (dom, device, &stats, sizeof stats) == -1) { - vshError(ctl, _("Failed to get block stats %s %s"), name, device); - virDomainFree(dom); - return false; - } + rc = virDomainBlockStatsFlags(dom, device, NULL, &nparams, 0); - if (stats.rd_req >= 0) - vshPrint (ctl, "%s rd_req %lld\n", device, stats.rd_req); + /* It might fail when virDomainBlockStatsFlags is not + * supported on older libvirt, fallback to use virDomainBlockStats + * then. + */ + if (rc < 0) { + if (last_error->code != VIR_ERR_NO_SUPPORT) { + virshReportError(ctl); + goto cleanup; + } else { + virFreeError(last_error); + last_error = NULL; - if (stats.rd_bytes >= 0) - vshPrint (ctl, "%s rd_bytes %lld\n", device, stats.rd_bytes); + if (virDomainBlockStats (dom, device, &stats, + sizeof stats) == -1) { + vshError(ctl, _("Failed to get block stats %s %s"), + name, device); + goto cleanup; + } + + if (stats.rd_req >= 0) + vshPrint (ctl, "%s rd_req %lld\n", device, stats.rd_req); - if (stats.wr_req >= 0) - vshPrint (ctl, "%s wr_req %lld\n", device, stats.wr_req); + if (stats.rd_bytes >= 0) + vshPrint (ctl, "%s rd_bytes %lld\n", device, stats.rd_bytes); - if (stats.wr_bytes >= 0) - vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes); + if (stats.wr_req >= 0) + vshPrint (ctl, "%s wr_req %lld\n", device, stats.wr_req); - if (stats.errs >= 0) - vshPrint (ctl, "%s errs %lld\n", device, stats.errs); + if (stats.wr_bytes >= 0) + vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes); + if (stats.errs >= 0) + vshPrint (ctl, "%s errs %lld\n", device, stats.errs); + } + } else { + params = vshMalloc(ctl, sizeof(*params) * nparams); + memset(params, 0, sizeof(*params) * nparams); + + if (virDomainBlockStatsFlags (dom, device, params, &nparams, 0) < 0) { + vshError(ctl, _("Failed to get block stats %s %s"), name, device); + goto cleanup; + } + + int i; + /* XXX: The output sequence will be different. */ + for (i = 0; i < nparams; i++) { + switch(params[i].type) { + case VIR_TYPED_PARAM_INT: + vshPrint (ctl, "%s %s %d\n", device, + params[i].field, params[i].value.i); + break; + case VIR_TYPED_PARAM_UINT: + vshPrint (ctl, "%s %s %u\n", device, + params[i].field, params[i].value.ui); + break; + case VIR_TYPED_PARAM_LLONG: + vshPrint (ctl, "%s %s %lld\n", device, + params[i].field, params[i].value.l); + break; + case VIR_TYPED_PARAM_ULLONG: + vshPrint (ctl, "%s %s %llu\n", device, + params[i].field, params[i].value.ul); + break; + case VIR_TYPED_PARAM_DOUBLE: + vshPrint (ctl, "%s %s %f\n", device, + params[i].field, params[i].value.d); + break; + case VIR_TYPED_PARAM_BOOLEAN: + vshPrint (ctl, "%s %s %s\n", device, + params[i].field, params[i].value.b ? _("yes") : _("no")); + break; + default: + vshError(ctl, _("unimplemented block statistics parameter type")); + } + + } + } + + ret = true; + +cleanup: + VIR_FREE(params); virDomainFree(dom); - return true; + return ret; } /* "domifstat" command -- 1.7.6

On Mon, Sep 05, 2011 at 04:38:35PM +0800, Osier Yang wrote:
The modified function fallbacks to use virDomainBlockStats if virDomainBlockStatsFlags is not supported by the hypervisor driver. If the new API is supported, it will be invoked instead of the old API. --- tools/virsh.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 84 insertions(+), 20 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index c7240e5..ea41221 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1070,6 +1070,9 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; const char *name = NULL, *device = NULL; struct _virDomainBlockStats stats; + virTypedParameterPtr params = NULL; + int rc, nparams = 0; + bool ret = false;
if (!vshConnectionUsability (ctl, ctl->conn)) return false; @@ -1077,34 +1080,95 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain (ctl, cmd, &name))) return false;
- if (vshCommandOptString (cmd, "device", &device) <= 0) { - virDomainFree(dom); - return false; - } + if (vshCommandOptString (cmd, "device", &device) <= 0) + goto cleanup;
- if (virDomainBlockStats (dom, device, &stats, sizeof stats) == -1) { - vshError(ctl, _("Failed to get block stats %s %s"), name, device); - virDomainFree(dom); - return false; - } + rc = virDomainBlockStatsFlags(dom, device, NULL, &nparams, 0);
- if (stats.rd_req >= 0) - vshPrint (ctl, "%s rd_req %lld\n", device, stats.rd_req); + /* It might fail when virDomainBlockStatsFlags is not + * supported on older libvirt, fallback to use virDomainBlockStats + * then. + */ + if (rc < 0) { + if (last_error->code != VIR_ERR_NO_SUPPORT) { + virshReportError(ctl); + goto cleanup; + } else { + virFreeError(last_error); + last_error = NULL;
- if (stats.rd_bytes >= 0) - vshPrint (ctl, "%s rd_bytes %lld\n", device, stats.rd_bytes); + if (virDomainBlockStats (dom, device, &stats, + sizeof stats) == -1) { + vshError(ctl, _("Failed to get block stats %s %s"), + name, device); + goto cleanup; + } + + if (stats.rd_req >= 0) + vshPrint (ctl, "%s rd_req %lld\n", device, stats.rd_req);
- if (stats.wr_req >= 0) - vshPrint (ctl, "%s wr_req %lld\n", device, stats.wr_req); + if (stats.rd_bytes >= 0) + vshPrint (ctl, "%s rd_bytes %lld\n", device, stats.rd_bytes);
- if (stats.wr_bytes >= 0) - vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes); + if (stats.wr_req >= 0) + vshPrint (ctl, "%s wr_req %lld\n", device, stats.wr_req);
- if (stats.errs >= 0) - vshPrint (ctl, "%s errs %lld\n", device, stats.errs); + if (stats.wr_bytes >= 0) + vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes);
+ if (stats.errs >= 0) + vshPrint (ctl, "%s errs %lld\n", device, stats.errs); + } + } else { + params = vshMalloc(ctl, sizeof(*params) * nparams); + memset(params, 0, sizeof(*params) * nparams); + + if (virDomainBlockStatsFlags (dom, device, params, &nparams, 0) < 0) { + vshError(ctl, _("Failed to get block stats %s %s"), name, device); + goto cleanup; + } + + int i; + /* XXX: The output sequence will be different. */ + for (i = 0; i < nparams; i++) { + switch(params[i].type) { + case VIR_TYPED_PARAM_INT: + vshPrint (ctl, "%s %s %d\n", device, + params[i].field, params[i].value.i); + break; + case VIR_TYPED_PARAM_UINT: + vshPrint (ctl, "%s %s %u\n", device, + params[i].field, params[i].value.ui); + break; + case VIR_TYPED_PARAM_LLONG: + vshPrint (ctl, "%s %s %lld\n", device, + params[i].field, params[i].value.l); + break; + case VIR_TYPED_PARAM_ULLONG: + vshPrint (ctl, "%s %s %llu\n", device, + params[i].field, params[i].value.ul); + break; + case VIR_TYPED_PARAM_DOUBLE: + vshPrint (ctl, "%s %s %f\n", device, + params[i].field, params[i].value.d); + break; + case VIR_TYPED_PARAM_BOOLEAN: + vshPrint (ctl, "%s %s %s\n", device, + params[i].field, params[i].value.b ? _("yes") : _("no")); + break; + default: + vshError(ctl, _("unimplemented block statistics parameter type")); + } + + } + } + + ret = true; + +cleanup: + VIR_FREE(params); virDomainFree(dom); - return true; + return ret; }
the title should indicate it's only for virsh I hope the change of order for returned values won't introduce regressions... ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

于 2011年09月06日 11:59, Daniel Veillard 写道:
On Mon, Sep 05, 2011 at 04:38:35PM +0800, Osier Yang wrote:
The modified function fallbacks to use virDomainBlockStats if virDomainBlockStatsFlags is not supported by the hypervisor driver. If the new API is supported, it will be invoked instead of the old API. --- tools/virsh.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 84 insertions(+), 20 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index c7240e5..ea41221 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1070,6 +1070,9 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; const char *name = NULL, *device = NULL; struct _virDomainBlockStats stats; + virTypedParameterPtr params = NULL; + int rc, nparams = 0; + bool ret = false;
if (!vshConnectionUsability (ctl, ctl->conn)) return false; @@ -1077,34 +1080,95 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain (ctl, cmd,&name))) return false;
- if (vshCommandOptString (cmd, "device",&device)<= 0) { - virDomainFree(dom); - return false; - } + if (vshCommandOptString (cmd, "device",&device)<= 0) + goto cleanup;
- if (virDomainBlockStats (dom, device,&stats, sizeof stats) == -1) { - vshError(ctl, _("Failed to get block stats %s %s"), name, device); - virDomainFree(dom); - return false; - } + rc = virDomainBlockStatsFlags(dom, device, NULL,&nparams, 0);
- if (stats.rd_req>= 0) - vshPrint (ctl, "%s rd_req %lld\n", device, stats.rd_req); + /* It might fail when virDomainBlockStatsFlags is not + * supported on older libvirt, fallback to use virDomainBlockStats + * then. + */ + if (rc< 0) { + if (last_error->code != VIR_ERR_NO_SUPPORT) { + virshReportError(ctl); + goto cleanup; + } else { + virFreeError(last_error); + last_error = NULL;
- if (stats.rd_bytes>= 0) - vshPrint (ctl, "%s rd_bytes %lld\n", device, stats.rd_bytes); + if (virDomainBlockStats (dom, device,&stats, + sizeof stats) == -1) { + vshError(ctl, _("Failed to get block stats %s %s"), + name, device); + goto cleanup; + } + + if (stats.rd_req>= 0) + vshPrint (ctl, "%s rd_req %lld\n", device, stats.rd_req);
- if (stats.wr_req>= 0) - vshPrint (ctl, "%s wr_req %lld\n", device, stats.wr_req); + if (stats.rd_bytes>= 0) + vshPrint (ctl, "%s rd_bytes %lld\n", device, stats.rd_bytes);
- if (stats.wr_bytes>= 0) - vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes); + if (stats.wr_req>= 0) + vshPrint (ctl, "%s wr_req %lld\n", device, stats.wr_req);
- if (stats.errs>= 0) - vshPrint (ctl, "%s errs %lld\n", device, stats.errs); + if (stats.wr_bytes>= 0) + vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes);
+ if (stats.errs>= 0) + vshPrint (ctl, "%s errs %lld\n", device, stats.errs); + } + } else { + params = vshMalloc(ctl, sizeof(*params) * nparams); + memset(params, 0, sizeof(*params) * nparams); + + if (virDomainBlockStatsFlags (dom, device, params,&nparams, 0)< 0) { + vshError(ctl, _("Failed to get block stats %s %s"), name, device); + goto cleanup; + } + + int i; + /* XXX: The output sequence will be different. */ + for (i = 0; i< nparams; i++) { + switch(params[i].type) { + case VIR_TYPED_PARAM_INT: + vshPrint (ctl, "%s %s %d\n", device, + params[i].field, params[i].value.i); + break; + case VIR_TYPED_PARAM_UINT: + vshPrint (ctl, "%s %s %u\n", device, + params[i].field, params[i].value.ui); + break; + case VIR_TYPED_PARAM_LLONG: + vshPrint (ctl, "%s %s %lld\n", device, + params[i].field, params[i].value.l); + break; + case VIR_TYPED_PARAM_ULLONG: + vshPrint (ctl, "%s %s %llu\n", device, + params[i].field, params[i].value.ul); + break; + case VIR_TYPED_PARAM_DOUBLE: + vshPrint (ctl, "%s %s %f\n", device, + params[i].field, params[i].value.d); + break; + case VIR_TYPED_PARAM_BOOLEAN: + vshPrint (ctl, "%s %s %s\n", device, + params[i].field, params[i].value.b ? _("yes") : _("no")); + break; + default: + vshError(ctl, _("unimplemented block statistics parameter type")); + } + + } + } + + ret = true; + +cleanup: + VIR_FREE(params); virDomainFree(dom); - return true; + return ret; }
the title should indicate it's only for virsh I hope the change of order for returned values won't introduce regressions...
ACK
Daniel
I pushed series, with title of this patch changed into: latency: Update virsh command domblkstat to use new API Thanks ! Regards Osier
participants (2)
-
Daniel Veillard
-
Osier Yang