Introduce VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT

[libvirt] [PATCH v2 4/5] Introduce VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT

Posted by Michal Privoznik 7 years ago

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 the job. This is problematic
to interactive applications that fetch stats repeatedly every few
seconds.

The solution is to introduce
VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT 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 |  2 ++
 src/libvirt-domain.c             | 12 ++++++++++++
 src/qemu/qemu_driver.c           | 15 ++++++++++++---
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 3ef7c24528..796f2e1408 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2055,6 +2055,8 @@ 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_NOWAIT = 1 << 29, /* report statistics that can be obtained
+                                                           immediately without any blocking */
     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 4a899f31c8..c71f2e6877 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11502,6 +11502,12 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * fields for offline domains if the statistics are meaningful only for a
  * running domain.
  *
+ * Passing VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT in
+ * @flags means when libvirt is unable to fetch stats for any of
+ * the domains (for whatever reason) only a subset of statistics
+ * is returned for the domain.  That subset being statistics that
+ * don't involve querying the underlying hypervisor.
+ *
  * Similarly to virConnectListAllDomains, @flags can contain various flags to
  * filter the list of domains to provide stats for.
  *
@@ -11586,6 +11592,12 @@ virConnectGetAllDomainStats(virConnectPtr conn,
  * fields for offline domains if the statistics are meaningful only for a
  * running domain.
  *
+ * Passing VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT in
+ * @flags means when libvirt is unable to fetch stats for any of
+ * the domains (for whatever reason) 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 42069ee617..35038889a3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20428,6 +20428,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
     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_NOWAIT |
                   VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
                   VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
 
@@ -20462,9 +20463,17 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
 
         virObjectLock(vm);
 
-        if (HAVE_JOB(privflags) &&
-            qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) == 0)
-            domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
+        if (HAVE_JOB(privflags)) {
+            int rv;
+
+            if (flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT)
+                rv = qemuDomainObjBeginJobNowait(driver, vm, QEMU_JOB_QUERY);
+            else
+                rv = qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY);
+
+            if (rv == 0)
+                domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
+        }
         /* else: without a job it's still possible to gather some data */
 
         if (flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING)
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 4/5] Introduce VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT

Posted by John Ferlan 7 years ago


On 06/15/2018 04:18 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 the job. This is problematic
> to interactive applications that fetch stats repeatedly every few
> seconds.
> 
> The solution is to introduce
> VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT 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 |  2 ++
>  src/libvirt-domain.c             | 12 ++++++++++++
>  src/qemu/qemu_driver.c           | 15 ++++++++++++---
>  3 files changed, 26 insertions(+), 3 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

Is the 30 seconds qemu specific?  Could drop it from the commit message
- if you feel so compelled.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 4/5] Introduce VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT

Posted by Michal Prívozník 7 years ago

On 06/19/2018 01:25 AM, John Ferlan wrote:
> 
> 
> On 06/15/2018 04:18 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 the job. This is problematic
>> to interactive applications that fetch stats repeatedly every few
>> seconds.
>>
>> The solution is to introduce
>> VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT 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 |  2 ++
>>  src/libvirt-domain.c             | 12 ++++++++++++
>>  src/qemu/qemu_driver.c           | 15 ++++++++++++---
>>  3 files changed, 26 insertions(+), 3 deletions(-)
>>
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> John
> 
> Is the 30 seconds qemu specific?  Could drop it from the commit message
> - if you feel so compelled.
> 

Yes & no. Each driver has its own timeout but all set it to 30 seconds:

#define LXC_JOB_WAIT_TIME (1000ull * 30)
#define LIBXL_JOB_WAIT_TIME (1000ull * 30)
#define VZ_JOB_WAIT_TIME (1000 * 30)
#define QEMU_JOB_WAIT_TIME (1000ull * 30)

Therefore I rather keep 30seconds in the commit message.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 1/5] qemu_domain: Document qemuDomainObjBeginJob
[libvirt] [PATCH v2 2/5] qemuDomainObjBeginJobInternal: Remove spurious @ret assignment
[libvirt] [PATCH v2 3/5] qemu_domain: Introduce qemuDomainObjBeginJobNowait
[libvirt] [PATCH v2 4/5] Introduce VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT
[libvirt] [PATCH v2 5/5] virsh: Introduce --nowait to domstats