[libvirt] [PATCH 0/8] 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: typedef struct _virDomainBlockStatsFlags virDomainBlockStatsFlagsStruct; typedef virDomainBlockStatsFlagsStruct *virDomainBlockStatsFlagsPtr; struct _virDomainBlockStatsFlags { char field[VIR_DOMAIN_BLOCK_STATS_FIELD_LENGTH]; long long value; }; int virDomainBlockStatsFlags (virDomainPtr dom, const char *path, virDomainBlockStatsFlagsPtr params, int *nparams, unsigned int flags) Other points: 1) With the new API, output of virsh command "domblkstat" is different, this might affect the existed scripts. But considering we are even introducing new feilds, so this can be fine? 2) qemuMonitorJSONGetBlockStatsInfo set "*errs = 0" before, it will cause "domblkstat" always print things like "vda errs 0". However, QEMU doesn't support this field. Fixed it in these patches (*errs = -1). And new API qemuDomainBlockStatsFlags won't even set a field for "errs". 3) Is it deserved to update gendispatch.pl to generate remote codes for functions take argument like "virNodeCPUStatsPtr params", "virNodeMemoryStatsPtr params"? All of these argument points to structs has same structure. Perhaps we can define an alias for these structs and generate the remote codes just like for "virTypedParameterPtr params". [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 | 111 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++ 2 files changed, 116 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 53a2f7d..5474f63 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -574,6 +574,112 @@ 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" + + +/** + * virDomainBlockStatsFlagsStruct: + * + * Struct filled by virDomainBlockStatsFlags() providing information + * about the block device. + * + * Hypervisors may return a field set to ((long long)-1) which indicates + * that the hypervisor does not support that statistic. + * + * NB. Here 'long long' means 64 bit integer. + */ +typedef struct _virDomainBlockStatsFlags virDomainBlockStatsFlagsStruct; + +struct _virDomainBlockStatsFlags { + char field[VIR_DOMAIN_BLOCK_STATS_FIELD_LENGTH]; + long long value; +}; + +/** + * virDomainBlockStatsFlagsPtr: + * + * A pointer to a virDomainBlockStatsFlagsStruct structure + */ +typedef virDomainBlockStatsFlagsStruct *virDomainBlockStatsFlagsPtr; + + /** * virDomainInterfaceStats: * @@ -1169,6 +1275,11 @@ int virDomainBlockStats (virDomainPtr dom, const char *path, virDomainBlockStatsPtr stats, size_t size); +int virDomainBlockStatsFlags (virDomainPtr dom, + const char *path, + virDomainBlockStatsFlagsPtr 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 c2b6666..61af4b5 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -480,4 +480,9 @@ LIBVIRT_0.9.4 { virDomainBlockPull; } LIBVIRT_0.9.3; +LIBVIRT_0.9.5 { + global: + virDomainBlockStatsFlags; +} LIBVIRT_0.9.4; + # .... define new API here using predicted next version number .... -- 1.7.6
On Wed, Aug 31, 2011 at 04:26:06PM +0800, Osier Yang wrote:
--- include/libvirt/libvirt.h.in | 111 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++ 2 files changed, 116 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 53a2f7d..5474f63 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -574,6 +574,112 @@ 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" + + +/** + * virDomainBlockStatsFlagsStruct: + * + * Struct filled by virDomainBlockStatsFlags() providing information + * about the block device. + * + * Hypervisors may return a field set to ((long long)-1) which indicates + * that the hypervisor does not support that statistic. + * + * NB. Here 'long long' means 64 bit integer. + */ +typedef struct _virDomainBlockStatsFlags virDomainBlockStatsFlagsStruct; + +struct _virDomainBlockStatsFlags { + char field[VIR_DOMAIN_BLOCK_STATS_FIELD_LENGTH]; + long long value; +}; + +/** + * virDomainBlockStatsFlagsPtr: + * + * A pointer to a virDomainBlockStatsFlagsStruct structure + */ +typedef virDomainBlockStatsFlagsStruct *virDomainBlockStatsFlagsPtr; + + /** * virDomainInterfaceStats: * @@ -1169,6 +1275,11 @@ int virDomainBlockStats (virDomainPtr dom, const char *path, virDomainBlockStatsPtr stats, size_t size); +int virDomainBlockStatsFlags (virDomainPtr dom, + const char *path, + virDomainBlockStatsFlagsPtr 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 c2b6666..61af4b5 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -480,4 +480,9 @@ LIBVIRT_0.9.4 { virDomainBlockPull; } LIBVIRT_0.9.3;
+LIBVIRT_0.9.5 { + global: + virDomainBlockStatsFlags; +} LIBVIRT_0.9.4; + # .... define new API here using predicted next version number .... -- 1.7.6
ACK for the API, 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/
On 09/02/2011 04:06 AM, Daniel Veillard wrote:
On Wed, Aug 31, 2011 at 04:26:06PM +0800, Osier Yang wrote:
--- include/libvirt/libvirt.h.in | 111 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++ 2 files changed, 116 insertions(+), 0 deletions(-)
+/** + * virDomainBlockStatsFlagsStruct: + * + * Struct filled by virDomainBlockStatsFlags() providing information + * about the block device. + * + * Hypervisors may return a field set to ((long long)-1) which indicates + * that the hypervisor does not support that statistic. + * + * NB. Here 'long long' means 64 bit integer. + */ +typedef struct _virDomainBlockStatsFlags virDomainBlockStatsFlagsStruct; + +struct _virDomainBlockStatsFlags { + char field[VIR_DOMAIN_BLOCK_STATS_FIELD_LENGTH]; + long long value; +};
Are we positive that all useful block stats will always be in long long format, or should we reuse virTypedParameter here to also allow other typed objects in the future? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Sep 02, 2011 at 08:05:14AM -0600, Eric Blake wrote:
On 09/02/2011 04:06 AM, Daniel Veillard wrote:
On Wed, Aug 31, 2011 at 04:26:06PM +0800, Osier Yang wrote:
--- include/libvirt/libvirt.h.in | 111 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++ 2 files changed, 116 insertions(+), 0 deletions(-)
+/** + * virDomainBlockStatsFlagsStruct: + * + * Struct filled by virDomainBlockStatsFlags() providing information + * about the block device. + * + * Hypervisors may return a field set to ((long long)-1) which indicates + * that the hypervisor does not support that statistic. + * + * NB. Here 'long long' means 64 bit integer. + */ +typedef struct _virDomainBlockStatsFlags virDomainBlockStatsFlagsStruct; + +struct _virDomainBlockStatsFlags { + char field[VIR_DOMAIN_BLOCK_STATS_FIELD_LENGTH]; + long long value; +};
Are we positive that all useful block stats will always be in long long format, or should we reuse virTypedParameter here to also allow other typed objects in the future?
Ouch, very good point ! We can't guess so we should not take the risk ! we really ought to align to the other APIs and use virTypedParameter 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 80d6628..05693e3 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, + virDomainBlockStatsFlagsPtr params, + int *nparams, + unsigned int flags); +typedef int (*virDrvDomainInterfaceStats) (virDomainPtr domain, const char *path, @@ -801,6 +808,7 @@ struct _virDriver { virDrvDomainMigratePerform domainMigratePerform; virDrvDomainMigrateFinish domainMigrateFinish; virDrvDomainBlockStats domainBlockStats; + virDrvDomainBlockStatsFlags domainBlockStatsFlags; virDrvDomainInterfaceStats domainInterfaceStats; virDrvDomainMemoryStats domainMemoryStats; virDrvDomainBlockPeek domainBlockPeek; -- 1.7.6
On Wed, Aug 31, 2011 at 04:26:07PM +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 80d6628..05693e3 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, + virDomainBlockStatsFlagsPtr params, + int *nparams, + unsigned int flags); +typedef int (*virDrvDomainInterfaceStats) (virDomainPtr domain, const char *path, @@ -801,6 +808,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 | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 69 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 65a099b..f83c019 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6394,6 +6394,75 @@ 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. 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, + virDomainBlockStatsFlagsPtr 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 Wed, Aug 31, 2011 at 04:26:08PM +0800, Osier Yang wrote:
--- src/libvirt.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 65a099b..f83c019 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6394,6 +6394,75 @@ 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. 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, + virDomainBlockStatsFlagsPtr 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
A priori reading those informations is secure, so we don't need to check for read-only connection, looks fine, 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/
On 09/02/2011 04:10 AM, Daniel Veillard wrote:
On Wed, Aug 31, 2011 at 04:26:08PM +0800, Osier Yang wrote:
--- src/libvirt.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 69 insertions(+), 0 deletions(-)
+ * + * Domains may have more than one block device. To get stats for + * each you should make multiple calls to this function.
Also mention that different blocks within the same domain may have a different number of stats - that is, @nparams that is sufficient for one block might not grab all the available stats of another block, so it must be recomputed for each block. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- daemon/remote.c | 81 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 69 +++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 23 +++++++++++- 3 files changed, 172 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 0f088c6..84784cb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -935,6 +935,87 @@ 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) +{ + virDomainBlockStatsFlagsPtr 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_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. */ + ret->params.params_len = nparams; + if (VIR_ALLOC_N(ret->params.params_val, nparams) < 0) + goto no_memory; + + for (i = 0; i < nparams; ++i) { + /* remoteDispatchClientRequest will free this: */ + ret->params.params_val[i].field = strdup(params[i].field); + if (ret->params.params_val[i].field == NULL) + goto no_memory; + + ret->params.params_val[i].value = params[i].value; + } + +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; + +no_memory: + virReportOOMError(); + goto cleanup; +} + +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 603d589..5d3d4dd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1361,6 +1361,74 @@ cleanup: } static int +remoteDomainBlockStatsFlags(virDomainPtr domain, + const char *path, + virDomainBlockStatsFlagsPtr params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_stats_flags_args args; + remote_domain_block_stats_flags_ret ret; + int i = -1; + 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_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. */ + for (i = 0; i < *nparams; ++i) { + if (virStrcpyStatic(params[i].field, ret.params.params_val[i].field) == NULL) { + remoteError(VIR_ERR_INTERNAL_ERROR, + _("Stats %s too big for destination"), + ret.params.params_val[i].field); + goto cleanup; + } + params[i].value = ret.params.params_val[i].value; + } + + 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 +4375,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 8f68808..dcd3c09 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_MAX = 16; + /* Upper limit on number of NUMA cells */ const REMOTE_NODE_MAX_CELLS = 1024; @@ -331,6 +334,11 @@ struct remote_node_get_memory_stats { unsigned hyper value; }; +struct remote_domain_block_stats_flags { + remote_nonnull_string field; + hyper value; +}; + /*----- Calls. -----*/ /* For each call we may have a 'remote_CALL_args' and 'remote_CALL_ret' @@ -544,6 +552,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_domain_block_stats_flags params<REMOTE_DOMAIN_BLOCK_STATS_MAX>; + int nparams; +}; + struct remote_domain_interface_stats_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2475,7 +2495,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_JOB_SET_SPEED = 239, /* autogen autogen */ REMOTE_PROC_DOMAIN_BLOCK_PULL = 240, /* autogen autogen */ - REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 242 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.6
On Wed, Aug 31, 2011 at 04:26:09PM +0800, Osier Yang wrote:
--- daemon/remote.c | 81 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 69 +++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 23 +++++++++++- 3 files changed, 172 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 0f088c6..84784cb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -935,6 +935,87 @@ 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) +{ + virDomainBlockStatsFlagsPtr 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_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. */ + ret->params.params_len = nparams; + if (VIR_ALLOC_N(ret->params.params_val, nparams) < 0) + goto no_memory; + + for (i = 0; i < nparams; ++i) { + /* remoteDispatchClientRequest will free this: */ + ret->params.params_val[i].field = strdup(params[i].field); + if (ret->params.params_val[i].field == NULL) + goto no_memory; + + ret->params.params_val[i].value = params[i].value; + } + +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; + +no_memory: + virReportOOMError(); + goto cleanup; +} + +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 603d589..5d3d4dd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1361,6 +1361,74 @@ cleanup: }
static int +remoteDomainBlockStatsFlags(virDomainPtr domain, + const char *path, + virDomainBlockStatsFlagsPtr params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_stats_flags_args args; + remote_domain_block_stats_flags_ret ret; + int i = -1; + 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_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. */ + for (i = 0; i < *nparams; ++i) { + if (virStrcpyStatic(params[i].field, ret.params.params_val[i].field) == NULL) { + remoteError(VIR_ERR_INTERNAL_ERROR, + _("Stats %s too big for destination"), + ret.params.params_val[i].field); + goto cleanup; + } + params[i].value = ret.params.params_val[i].value; + } + + 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 +4375,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 8f68808..dcd3c09 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_MAX = 16; + /* Upper limit on number of NUMA cells */ const REMOTE_NODE_MAX_CELLS = 1024;
@@ -331,6 +334,11 @@ struct remote_node_get_memory_stats { unsigned hyper value; };
+struct remote_domain_block_stats_flags { + remote_nonnull_string field; + hyper value; +}; + /*----- Calls. -----*/
/* For each call we may have a 'remote_CALL_args' and 'remote_CALL_ret' @@ -544,6 +552,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_domain_block_stats_flags params<REMOTE_DOMAIN_BLOCK_STATS_MAX>; + int nparams; +}; + struct remote_domain_interface_stats_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2475,7 +2495,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_JOB_SET_SPEED = 239, /* autogen autogen */ REMOTE_PROC_DOMAIN_BLOCK_PULL = 240, /* autogen autogen */
- REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 242 /* 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 f21122d..03fadae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6823,8 +6823,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 715b26e..77a2545 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 4bd761d..fab5e19 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 Wed, Aug 31, 2011 at 04:26:10PM +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).
I'm not sure we should extend the existing set of function used by GetBlockStats instead of creating new functions asking/looking for the full set of values. On one hand it's good to share the code, but on the other hand if this leads to more round-trip between libvirtd and qemu I would rather keep them separate. I.e. we should pay the extra cost only when asking for the extra data. 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月02日 18:17, Daniel Veillard 写道:
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). I'm not sure we should extend the existing set of function used by GetBlockStats instead of creating new functions asking/looking for the full set of values. On one hand it's good to share the code, but on the other hand if this leads to more round-trip between
On Wed, Aug 31, 2011 at 04:26:10PM +0800, Osier Yang wrote: libvirtd and qemu I would rather keep them separate. I.e. we should pay the extra cost only when asking for the extra data.
Daniel
I considered to extend the existing functions to get the params number, such as add a new flag argument to the function. But after thinking a while, I think it's more clear to keep them as seperate functions. And on the other hand, yes, it costs much on the wire between qemu and libvirtd, we only want the extra info when needs. So personally I'd perfer to keep them seperate. Thanks, Osier
--- src/qemu/qemu_driver.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 187 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 03fadae..2c2bf56 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6842,6 +6842,192 @@ cleanup: return ret; } +static int +qemudDomainBlockStatsFlags (virDomainPtr dom, + const char *path, + virDomainBlockStatsFlagsPtr 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++) { + virDomainBlockStatsFlagsPtr 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->value = 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->value = 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->value = 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->value = 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->value = 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->value = 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->value = 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->value = 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, @@ -9489,6 +9675,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
于 2011年08月31日 16:26, Osier Yang 写道:
--- src/qemu/qemu_driver.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 187 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 03fadae..2c2bf56 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6842,6 +6842,192 @@ cleanup: return ret; }
+static int +qemudDomainBlockStatsFlags (virDomainPtr dom,
s/qemud/qemu/ This will be updated
+ const char *path, + virDomainBlockStatsFlagsPtr 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++) { + virDomainBlockStatsFlagsPtr 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->value = 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->value = 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->value = 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->value = 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->value = 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->value = 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->value = 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->value = 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, @@ -9489,6 +9675,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 */
--- python/generator.py | 1 + python/libvirt-override-api.xml | 7 +++++ python/libvirt-override.c | 55 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index 97434ed..85ee614 100755 --- a/python/generator.py +++ b/python/generator.py @@ -372,6 +372,7 @@ skip_impl = ( 'virNodeGetCPUStats', 'virNodeGetMemoryStats', 'virDomainGetBlockJobInfo', + 'virDomainBlockStatsFlags', ) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 2fa5eed..091932f 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='virDomainBlockStatsFlagsPtr' 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 b5650e2..01ffa7e 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -100,6 +100,60 @@ 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; + virDomainBlockStatsFlagsPtr 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; + + val = PyLong_FromLongLong((long long)params[i].value); + 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; @@ -4582,6 +4636,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 Wed, Aug 31, 2011 at 04:26:12PM +0800, Osier Yang wrote:
--- python/generator.py | 1 + python/libvirt-override-api.xml | 7 +++++ python/libvirt-override.c | 55 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 0 deletions(-)
diff --git a/python/generator.py b/python/generator.py index 97434ed..85ee614 100755 --- a/python/generator.py +++ b/python/generator.py @@ -372,6 +372,7 @@ skip_impl = ( 'virNodeGetCPUStats', 'virNodeGetMemoryStats', 'virDomainGetBlockJobInfo', + 'virDomainBlockStatsFlags', )
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 2fa5eed..091932f 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='virDomainBlockStatsFlagsPtr' 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 b5650e2..01ffa7e 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -100,6 +100,60 @@ 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; + virDomainBlockStatsFlagsPtr 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; + + val = PyLong_FromLongLong((long long)params[i].value); + 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; @@ -4582,6 +4636,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 | 78 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 58 insertions(+), 20 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 15b9bdd..cd01528 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; + virDomainBlockStatsFlagsPtr params = NULL; + int rc, nparams = 0; + bool ret = false; if (!vshConnectionUsability (ctl, ctl->conn)) return false; @@ -1077,34 +1080,69 @@ 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 (virDomainBlockStats (dom, device, &stats, + sizeof stats) == -1) { + vshError(ctl, _("Failed to get block stats %s %s"), + name, device); + goto cleanup; + } - if (stats.rd_bytes >= 0) - vshPrint (ctl, "%s rd_bytes %lld\n", device, stats.rd_bytes); + 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++) { + if (params[i].value >= 0) + vshPrint (ctl, "%s %s %lld\n", device, params[i].field, + params[i].value); + } + } + + ret = true; + +cleanup: + VIR_FREE(params); virDomainFree(dom); - return true; + return ret; } /* "domifstat" command -- 1.7.6
On Wed, Aug 31, 2011 at 04:26:13PM +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.
Again I would use a new command and keep the existing code as is, 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月02日 18:21, Daniel Veillard 写道:
On Wed, Aug 31, 2011 at 04:26:13PM +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. Again I would use a new command and keep the existing code as is,
Daniel
What does "again" here mean? as I couldn't find another place which introduce new code. :-) And per the new API has some same fields with old API, and seems all of virsh commands try to fallback to old API if new API is introduced. Osier
On Fri, Sep 02, 2011 at 07:23:38PM +0800, Osier Yang wrote:
于 2011年09月02日 18:21, Daniel Veillard 写道:
On Wed, Aug 31, 2011 at 04:26:13PM +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. Again I would use a new command and keep the existing code as is,
Daniel
What does "again" here mean? as I couldn't find another place which introduce new code. :-)
Well I would not modify cmdDomblkstat and instead create a new command and a new function for the new API
And per the new API has some same fields with old API, and seems all of virsh commands try to fallback to old API if new API is introduced.
yes but the new API will provide informations in a different order and is potentially more expensive, so I'm not sure I really want to use the new API for the old command Maybe something like "domblkfullstat"... But if someone else disagrees with me I'm fine being in the minority :-) 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/
On 09/02/2011 05:34 AM, Daniel Veillard wrote:
On Fri, Sep 02, 2011 at 07:23:38PM +0800, Osier Yang wrote:
于 2011年09月02日 18:21, Daniel Veillard 写道:
On Wed, Aug 31, 2011 at 04:26:13PM +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. Again I would use a new command and keep the existing code as is,
Daniel
What does "again" here mean? as I couldn't find another place which introduce new code. :-)
Well I would not modify cmdDomblkstat and instead create a new command and a new function for the new API
And per the new API has some same fields with old API, and seems all of virsh commands try to fallback to old API if new API is introduced.
yes but the new API will provide informations in a different order and is potentially more expensive, so I'm not sure I really want to use the new API for the old command Maybe something like "domblkfullstat"...
But if someone else disagrees with me I'm fine being in the minority :-)
Personally, I'd like to keep a single virsh command for both APIs, just like 'virsh migrate' handles both virDomainMigrate and virDomainMigrate2. I don't know if it's better to default to the old or the new API, but it is easy enough to provide a flag that swaps the default to call the alternate API. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Sep 02, 2011 at 08:08:58AM -0600, Eric Blake wrote:
On 09/02/2011 05:34 AM, Daniel Veillard wrote:
On Fri, Sep 02, 2011 at 07:23:38PM +0800, Osier Yang wrote:
于 2011年09月02日 18:21, Daniel Veillard 写道: Well I would not modify cmdDomblkstat and instead create a new command and a new function for the new API
And per the new API has some same fields with old API, and seems all of virsh commands try to fallback to old API if new API is introduced.
yes but the new API will provide informations in a different order and is potentially more expensive, so I'm not sure I really want to use the new API for the old command Maybe something like "domblkfullstat"...
But if someone else disagrees with me I'm fine being in the minority :-)
Personally, I'd like to keep a single virsh command for both APIs, just like 'virsh migrate' handles both virDomainMigrate and virDomainMigrate2. I don't know if it's better to default to the old or the new API, but it is easy enough to provide a flag that swaps the default to call the alternate API.
Okay, I surrender, I don't have a strong opinion for the virsh command option :-) 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/
participants (3)
-
Daniel Veillard -
Eric Blake -
Osier Yang