10:34 a.m.
$SUBJ: "Introduce" and "NO_WAIT"
On 06/07/2018 07:59 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1552092
If there's a long running job it might cause us to wait 30
seconds before we give up acquiring job. This may cause trouble
s/job/the job/
s/may cause trouble/is problematic/
to interactive applications that fetch stats repeatedly every few
seconds.
Solution is to introduce
The solution is...
VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT flag which tries to
acquire job but does not wait if acquiring failed.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
include/libvirt/libvirt-domain.h | 1 +
src/libvirt-domain.c | 10 ++++++++++
src/qemu/qemu_driver.c | 15 ++++++++++++---
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index da773b76cb..1a1d34620d 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2055,6 +2055,7 @@ typedef enum {
VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF = VIR_CONNECT_LIST_DOMAINS_SHUTOFF,
VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER = VIR_CONNECT_LIST_DOMAINS_OTHER,
+ VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT = 1 << 29, /* ignore stalled
domains */
"stalled"? How about "don't wait on other jobs"
VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING = 1 << 30, /*
include backing chain for block stats */
VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS = 1U << 31, /* enforce
requested stats */
} virConnectGetAllDomainStatsFlags;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index d44b553c74..906b097adb 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11502,6 +11502,11 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* fields for offline domains if the statistics are meaningful only for a
* running domain.
*
+ * Passing VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT in
+ * @flags means when libvirt is unable to fetch stats for any of
+ * the domains (for whatever reason) such domain is silently
+ * ignored.
+ *
So we're adding this for both capabilities and...
* Similarly to virConnectListAllDomains, @flags can contain various
flags to
* filter the list of domains to provide stats for.
*
@@ -11586,6 +11591,11 @@ virConnectGetAllDomainStats(virConnectPtr conn,
* fields for offline domains if the statistics are meaningful only for a
* running domain.
*
+ * Passing VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT in
+ * @flags means when libvirt is unable to fetch stats for any of
+ * the domains (for whatever reason) such domain is silently
+ * ignored.
+ *
...stats in the API documentation, but...
BTW: the domain isn't silently ignored, instead only a subset of
statistics is returned for the domain. That subset being statistics
that don't involve querying the underlying hypervisor.
* Note that any of the domain list filtering flags in @flags may be
rejected
* by this function.
*
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 38ea865ce3..28338de05f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20446,6 +20446,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
...only adding the check in the DomainStats?
virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE |
+ VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT |
VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
@@ -20480,9 +20481,17 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
virObjectLock(vm);
- if (HAVE_JOB(privflags) &&
- qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) == 0)
- domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
Existing, but if BeginJob fails for some "other" reason, then one
wonders how much of the next steps actually happen. Furthermore, we
don't clear the LastError.
+ if (HAVE_JOB(privflags)) {
+ int rv;
+
+ if (flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT)
+ rv = qemuDomainObjBeginJobInstant(driver, vm, QEMU_JOB_QUERY);
Let's assume rv = -1 for a moment and it's the last domain that caused
the failure - does that mean the caller then re...
+ else
+ rv = qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY);
+
+ if (rv == 0)
+ domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
to add to my comment above, if rv < 0, then should we clear the last
error since this code doesn't seem to care...
John
+ }
/* else: without a job it's still possible to gather some data */
if (flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING)