virt: add support for new metrics #2168
Conversation
| int domain_state = 0; | ||
| int domain_reason = 0; | ||
|
|
||
| int status = virDomainGetState(domain, &domain_state, &domain_reason, 0); |
There was a problem hiding this comment.
I believe a valid value for the domain_reason is NULL... we should check this before submitting this value
Please see https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetState
There was a problem hiding this comment.
virDomainGetState API can be called with NULL as a domain_state, if one doesn't wish to retrieve domain reason i.e. virDomainGetState(domain, &domain_state, NULL, 0); .
In this patch domain_state variable is created on the stack, and then its address is passed to libvirt API. The value received from the API is an integer, thus I don't see any valid reason to make extra check.
|
Yes, and there are more changes coming, so to prevent duplicate work please wait a bit, or perhaps you can bundle your efforts with @fromanirh. |
|
Hi, just to notify that I don't have any more RFEs in the pipeline, we added all the metrics we needed. |
|
@fromanirh good to know, thanks for the information. |
pszczerx
force-pushed
the
feat_libvirt_upstream
branch
from
a283b60
to
b1d354f
Compare
Add support for new libvirt metrics: - Performance monitoring events - Domain state & reason - Percentage CPU usage - CPU pinning (affinity) - Disk errors - File system info - Job stats Change-Id: I233fb18653d218c7e906a157743606c6818f9feb Signed-off-by: Przemyslaw Szczerbik <przemyslawx.szczerbik@intel.com>
pszczerx
force-pushed
the
feat_libvirt_upstream
branch
from
b1d354f
to
9d534e1
Compare
|
@rubenk, PR has been rebased and slightly reworked (added support for recently added ExtraStats logic, domain state changed from metric to notification). I'm looking forward to hearing your comments. |
|
Thanks again @pszczerx! |
Extend virt plugin functionality to support new statistics:
Leverage recently added 'ExtraStats' logic to allow user to enable or disable new metrics.
Plugin decodes domain state and reason enum values to human-readable string format, which will make it easier to interpret notifications.
Known limitation:
Missing CPU utilization metric is expected in some cases. In order to compute %CPU, we require 2 samples of CPU time. However, connection to libvirt is periodically refreshed, by default every 60 seconds. This interval can be changed with RefreshInterval virt plugin configuration option. Whenever connection is refreshed, we lose previous readings of CPU time, which means that we are unable to dispatch %CPU at next dispatch. With default settings - Plugin Interval 10 sec and RefreshInterval 60 sec CPU utilization will be missing every 6th dispatch. We are already working on solution for this issue, fix coming soon.