$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@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)