[libvirt] [PATCH 2/6] qemu: use query-cpus-fast in JSON monitor

Viktor Mihajlovski posted 6 patches 7 years, 2 months ago
[libvirt] [PATCH 2/6] qemu: use query-cpus-fast in JSON monitor
Posted by Viktor Mihajlovski 7 years, 2 months ago
Use query-cpus-fast instead of query-cpus if supported by QEMU.
Based on the QEMU_CAPS_QUERY_CPUS_FAST capability.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
---
 src/qemu/qemu_monitor.c      | 10 ++++++++--
 src/qemu/qemu_monitor_json.c | 26 +++++++++++++++++---------
 src/qemu/qemu_monitor_json.h |  3 ++-
 tests/qemumonitorjsontest.c  |  2 +-
 4 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ad5c572..dad1383 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2012,6 +2012,7 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
     int ret = -1;
     int rc;
     qemuMonitorCPUInfoPtr info = NULL;
+    bool fast;
 
     QEMU_CHECK_MONITOR(mon);
 
@@ -2028,8 +2029,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
         (qemuMonitorJSONGetHotpluggableCPUs(mon, &hotplugcpus, &nhotplugcpus)) < 0)
         goto cleanup;
 
+    fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps,
+                          QEMU_CAPS_QUERY_CPUS_FAST);
+
     if (mon->json)
-        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug);
+        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug,
+                                      fast);
     else
         rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);
 
@@ -2082,7 +2087,8 @@ qemuMonitorGetCpuHalted(qemuMonitorPtr mon,
     QEMU_CHECK_MONITOR_NULL(mon);
 
     if (mon->json)
-        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false);
+        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false,
+                                      false);
     else
         rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a09e93e..2ecdf0a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1466,7 +1466,8 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon)
 static int
 qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
                               struct qemuMonitorQueryCpusEntry **entries,
-                              size_t *nentries)
+                              size_t *nentries,
+                              bool fast)
 {
     struct qemuMonitorQueryCpusEntry *cpus = NULL;
     int ret = -1;
@@ -1491,11 +1492,13 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
         }
 
         /* Some older qemu versions don't report the thread_id so treat this as
-         * non-fatal, simply returning no data */
-        ignore_value(virJSONValueObjectGetNumberInt(entry, "CPU", &cpuid));
-        ignore_value(virJSONValueObjectGetNumberInt(entry, "thread_id", &thread));
+         * non-fatal, simply returning no data.
+         * The return data of query-cpus-fast has different field names
+         */
+        ignore_value(virJSONValueObjectGetNumberInt(entry, fast ? "cpu-index" : "CPU", &cpuid));
+        ignore_value(virJSONValueObjectGetNumberInt(entry, fast ? "thread-id" : "thread_id", &thread));
         ignore_value(virJSONValueObjectGetBoolean(entry, "halted", &halted));
-        qom_path = virJSONValueObjectGetString(entry, "qom_path");
+        qom_path = virJSONValueObjectGetString(entry, fast ? "qom-path" : "qom_path");
 
         cpus[i].qemu_id = cpuid;
         cpus[i].tid = thread;
@@ -1520,10 +1523,12 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
  * @mon: monitor object
  * @entries: filled with detected entries on success
  * @nentries: number of entries returned
+ * @force: force exit on error
+ * @fast: use query-cpus-fast
  *
  * Queries qemu for cpu-related information. Failure to execute the command or
  * extract results does not produce an error as libvirt can continue without
- * this information.
+ * this information, unless the caller has specified @force == true.
  *
  * Returns 0 on success, -1 on a fatal error (oom ...) and -2 if the
  * query failed gracefully.
@@ -1532,10 +1537,13 @@ int
 qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon,
                          struct qemuMonitorQueryCpusEntry **entries,
                          size_t *nentries,
-                         bool force)
+                         bool force,
+                         bool fast)
 {
     int ret = -1;
-    virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-cpus", NULL);
+    virJSONValuePtr cmd =
+        qemuMonitorJSONMakeCommand(fast ? "query-cpus-fast" : "query-cpus",
+                                   NULL);
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
 
@@ -1553,7 +1561,7 @@ qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon,
         goto cleanup;
     }
 
-    ret = qemuMonitorJSONExtractCPUInfo(data, entries, nentries);
+    ret = qemuMonitorJSONExtractCPUInfo(data, entries, nentries, fast);
 
  cleanup:
     virJSONValueFree(cmd);
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index ec243be..e03299a 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -60,7 +60,8 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon);
 int qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon,
                              struct qemuMonitorQueryCpusEntry **entries,
                              size_t *nentries,
-                             bool force);
+                             bool force,
+                             bool fast);
 int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon,
                                virDomainVirtType *virtType);
 int qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon,
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 908ec3a..6824623 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1420,7 +1420,7 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data)
         goto cleanup;
 
     if (qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test),
-                                 &cpudata, &ncpudata, true) < 0)
+                                 &cpudata, &ncpudata, true, false) < 0)
         goto cleanup;
 
     if (ncpudata != 4) {
-- 
1.9.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemu: use query-cpus-fast in JSON monitor
Posted by Peter Krempa 7 years, 2 months ago
On Fri, Mar 02, 2018 at 10:29:07 +0100, Viktor Mihajlovski wrote:
> Use query-cpus-fast instead of query-cpus if supported by QEMU.
> Based on the QEMU_CAPS_QUERY_CPUS_FAST capability.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
> ---
>  src/qemu/qemu_monitor.c      | 10 ++++++++--
>  src/qemu/qemu_monitor_json.c | 26 +++++++++++++++++---------
>  src/qemu/qemu_monitor_json.h |  3 ++-
>  tests/qemumonitorjsontest.c  |  2 +-
>  4 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index ad5c572..dad1383 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2012,6 +2012,7 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
>      int ret = -1;
>      int rc;
>      qemuMonitorCPUInfoPtr info = NULL;
> +    bool fast;
>  
>      QEMU_CHECK_MONITOR(mon);
>  
> @@ -2028,8 +2029,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
>          (qemuMonitorJSONGetHotpluggableCPUs(mon, &hotplugcpus, &nhotplugcpus)) < 0)
>          goto cleanup;
>  
> +    fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps,

Don't do this. You'll need to determine it sooner. The main reason being
that once you are in the monitor code, 'vm' is unlocked and should not
be accesssed. Also we don't access the VM object trhough the mon object
usually.

> +                          QEMU_CAPS_QUERY_CPUS_FAST);
> +
>      if (mon->json)
> -        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug);
> +        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug,
> +                                      fast);
>      else
>          rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);
>  
> @@ -2082,7 +2087,8 @@ qemuMonitorGetCpuHalted(qemuMonitorPtr mon,
>      QEMU_CHECK_MONITOR_NULL(mon);
>  
>      if (mon->json)
> -        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false);
> +        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false,
> +                                      false);
>      else
>          rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);
>  
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index a09e93e..2ecdf0a 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1466,7 +1466,8 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon)
>  static int
>  qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
>                                struct qemuMonitorQueryCpusEntry **entries,
> -                              size_t *nentries)
> +                              size_t *nentries,
> +                              bool fast)
>  {
>      struct qemuMonitorQueryCpusEntry *cpus = NULL;
>      int ret = -1;
> @@ -1491,11 +1492,13 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
>          }
>  
>          /* Some older qemu versions don't report the thread_id so treat this as
> -         * non-fatal, simply returning no data */
> -        ignore_value(virJSONValueObjectGetNumberInt(entry, "CPU", &cpuid));
> -        ignore_value(virJSONValueObjectGetNumberInt(entry, "thread_id", &thread));
> +         * non-fatal, simply returning no data.
> +         * The return data of query-cpus-fast has different field names
> +         */
> +        ignore_value(virJSONValueObjectGetNumberInt(entry, fast ? "cpu-index" : "CPU", &cpuid));
> +        ignore_value(virJSONValueObjectGetNumberInt(entry, fast ? "thread-id" : "thread_id", &thread));

No ternary operators please. Add an if-block and two code paths.

>          ignore_value(virJSONValueObjectGetBoolean(entry, "halted", &halted));
> -        qom_path = virJSONValueObjectGetString(entry, "qom_path");
> +        qom_path = virJSONValueObjectGetString(entry, fast ? "qom-path" : "qom_path");
>  
>          cpus[i].qemu_id = cpuid;
>          cpus[i].tid = thread;
> @@ -1520,10 +1523,12 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
>   * @mon: monitor object
>   * @entries: filled with detected entries on success
>   * @nentries: number of entries returned
> + * @force: force exit on error
> + * @fast: use query-cpus-fast
>   *
>   * Queries qemu for cpu-related information. Failure to execute the command or
>   * extract results does not produce an error as libvirt can continue without
> - * this information.
> + * this information, unless the caller has specified @force == true.
>   *
>   * Returns 0 on success, -1 on a fatal error (oom ...) and -2 if the
>   * query failed gracefully.
> @@ -1532,10 +1537,13 @@ int
>  qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon,
>                           struct qemuMonitorQueryCpusEntry **entries,
>                           size_t *nentries,
> -                         bool force)
> +                         bool force,
> +                         bool fast)
>  {
>      int ret = -1;
> -    virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-cpus", NULL);
> +    virJSONValuePtr cmd =
> +        qemuMonitorJSONMakeCommand(fast ? "query-cpus-fast" : "query-cpus",
> +                                   NULL);

Same here, no ternaries please.

>      virJSONValuePtr reply = NULL;
>      virJSONValuePtr data;
>  
> @@ -1553,7 +1561,7 @@ qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon,
>          goto cleanup;
>      }
>  
> -    ret = qemuMonitorJSONExtractCPUInfo(data, entries, nentries);
> +    ret = qemuMonitorJSONExtractCPUInfo(data, entries, nentries, fast);
>  
>   cleanup:
>      virJSONValueFree(cmd);
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index ec243be..e03299a 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -60,7 +60,8 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon);
>  int qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon,
>                               struct qemuMonitorQueryCpusEntry **entries,
>                               size_t *nentries,
> -                             bool force);
> +                             bool force,
> +                             bool fast);
>  int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon,
>                                 virDomainVirtType *virtType);
>  int qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon,
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index 908ec3a..6824623 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -1420,7 +1420,7 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data)
>          goto cleanup;
>  
>      if (qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test),
> -                                 &cpudata, &ncpudata, true) < 0)
> +                                 &cpudata, &ncpudata, true, false) < 0)
>          goto cleanup;
>  
>      if (ncpudata != 4) {
> -- 
> 1.9.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemu: use query-cpus-fast in JSON monitor
Posted by Viktor Mihajlovski 7 years, 2 months ago
On 02.03.2018 12:33, Peter Krempa wrote:
[...]
>> @@ -2028,8 +2029,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
>>          (qemuMonitorJSONGetHotpluggableCPUs(mon, &hotplugcpus, &nhotplugcpus)) < 0)
>>          goto cleanup;
>>  
>> +    fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps,
> 
> Don't do this. You'll need to determine it sooner. The main reason being
> that once you are in the monitor code, 'vm' is unlocked and should not
> be accesssed. Also we don't access the VM object trhough the mon object
> usually.
> OK. I've been struggling over this part since the availability of
query-cpus-fast is tied to the QEMU instance and I didn't want to
duplicate the capability in the qemuMonitor struct. Looks like I have to ...

I'll rework the series (including your other comments).
[...]
-- 
Regards,
 Viktor Mihajlovski

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemu: use query-cpus-fast in JSON monitor
Posted by Peter Krempa 7 years, 2 months ago
On Fri, Mar 02, 2018 at 13:32:19 +0100, Viktor Mihajlovski wrote:
> On 02.03.2018 12:33, Peter Krempa wrote:
> [...]
> >> @@ -2028,8 +2029,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
> >>          (qemuMonitorJSONGetHotpluggableCPUs(mon, &hotplugcpus, &nhotplugcpus)) < 0)
> >>          goto cleanup;
> >>  
> >> +    fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps,
> > 
> > Don't do this. You'll need to determine it sooner. The main reason being
> > that once you are in the monitor code, 'vm' is unlocked and should not
> > be accesssed. Also we don't access the VM object trhough the mon object
> > usually.
> > OK. I've been struggling over this part since the availability of
> query-cpus-fast is tied to the QEMU instance and I didn't want to
> duplicate the capability in the qemuMonitor struct. Looks like I have to ...

You can pass it from the caller as we do in more than one place.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list