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>
---
 src/qemu/qemu_domain.c       | 14 +++++++++++---
 src/qemu/qemu_monitor.c      | 30 ++++++++++++++++++------------
 src/qemu/qemu_monitor.h      |  7 +++++--
 src/qemu/qemu_monitor_json.c | 37 +++++++++++++++++++++++++++----------
 src/qemu/qemu_monitor_json.h |  3 ++-
 tests/qemumonitorjsontest.c  |  4 ++--
 6 files changed, 65 insertions(+), 30 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9d1c33b..662937b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9006,7 +9006,12 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
     if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
         return -1;
 
-    rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus, hotplug);
+    rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm),
+                               &info,
+                               maxvcpus,
+                               hotplug,
+                               virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
+                                              QEMU_CAPS_QUERY_CPUS_FAST));
 
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;
@@ -9025,7 +9030,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
          * thread, but it runs every vCPU in that same thread. So it
          * is impossible to setup different affinity per thread.
          *
-         * What's more the 'query-cpus' command returns bizarre
+         * What's more the 'query-cpus[-fast]' command returns bizarre
          * data for the threads. It gives the TCG thread for the
          * vCPU 0, but for vCPUs 1-> N, it actually replies with
          * the main process thread ID.
@@ -9126,7 +9131,10 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver,
     if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
         return -1;
 
-    haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus);
+    haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm),
+                                        maxvcpus,
+                                        virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
+                                                       QEMU_CAPS_QUERY_CPUS_FAST));
 
     if (qemuDomainObjExitMonitor(driver, vm) < 0 || !haltedmap)
         goto cleanup;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7b64752..2b4b7c7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1852,15 +1852,16 @@ qemuMonitorGetCPUInfoLegacy(struct qemuMonitorQueryCpusEntry *cpuentries,
  *
  * This function stitches together data retrieved via query-hotpluggable-cpus
  * which returns entities on the hotpluggable level (which may describe more
- * than one guest logical vcpu) with the output of query-cpus, having an entry
- * per enabled guest logical vcpu.
+ * than one guest logical vcpu) with the output of query-cpus (or
+ * query-cpus-fast), having an entry per enabled guest logical vcpu.
  *
  * query-hotpluggable-cpus conveys following information:
  * - topology information and number of logical vcpus this entry creates
  * - device type name of the entry that needs to be used when hotplugging
- * - qom path in qemu which can be used to map the entry against query-cpus
+ * - qom path in qemu which can be used to map the entry against
+ *   query-cpus[-fast]
  *
- * query-cpus conveys following information:
+ * query-cpus[-fast] conveys following information:
  * - thread id of a given guest logical vcpu
  * - order in which the vcpus were inserted
  * - qom path to allow mapping the two together
@@ -1895,7 +1896,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl
     for (i = 0; i < nhotplugvcpus; i++)
         totalvcpus += hotplugvcpus[i].vcpus;
 
-    /* trim '/thread...' suffix from the data returned by query-cpus */
+    /* trim '/thread...' suffix from the data returned by query-cpus[-fast] */
     for (i = 0; i < ncpuentries; i++) {
         if (cpuentries[i].qom_path &&
             (tmp = strstr(cpuentries[i].qom_path, "/thread")))
@@ -1908,7 +1909,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl
     }
 
     /* Note the order in which the hotpluggable entities are inserted by
-     * matching them to the query-cpus entries */
+     * matching them to the query-cpus[-fast] entries */
     for (i = 0; i < ncpuentries; i++) {
         for (j = 0; j < nhotplugvcpus; j++) {
             if (!cpuentries[i].qom_path ||
@@ -1963,7 +1964,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl
         }
 
         if (anyvcpu == maxvcpus) {
-            VIR_DEBUG("too many query-cpus entries for a given "
+            VIR_DEBUG("too many query-cpus[-fast] entries for a given "
                       "query-hotpluggable-cpus entry");
             return -1;
         }
@@ -1991,6 +1992,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl
  * @vcpus: pointer filled by array of qemuMonitorCPUInfo structures
  * @maxvcpus: total possible number of vcpus
  * @hotplug: query data relevant for hotplug support
+ * @fast: use QMP query-cpus-fast if supported
  *
  * Detects VCPU information. If qemu doesn't support or fails reporting
  * information this function will return success as other parts of libvirt
@@ -2003,7 +2005,8 @@ int
 qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
                       qemuMonitorCPUInfoPtr *vcpus,
                       size_t maxvcpus,
-                      bool hotplug)
+                      bool hotplug,
+                      bool fast)
 {
     struct qemuMonitorQueryHotpluggableCpusEntry *hotplugcpus = NULL;
     size_t nhotplugcpus = 0;
@@ -2029,7 +2032,8 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
         goto cleanup;
 
     if (mon->json)
-        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug);
+        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug,
+                                      fast);
     else
         rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);
 
@@ -2067,11 +2071,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
  * qemuMonitorGetCpuHalted:
  *
  * Returns a bitmap of vcpu id's that are halted. The id's correspond to the
- * 'CPU' field as reported by query-cpus'.
+ * 'CPU' field as reported by query-cpus[-fast]'.
  */
 virBitmapPtr
 qemuMonitorGetCpuHalted(qemuMonitorPtr mon,
-                        size_t maxvcpus)
+                        size_t maxvcpus,
+                        bool fast ATTRIBUTE_UNUSED)
 {
     struct qemuMonitorQueryCpusEntry *cpuentries = NULL;
     size_t ncpuentries = 0;
@@ -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.h b/src/qemu/qemu_monitor.h
index d04148e..8b4353c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -542,8 +542,11 @@ void qemuMonitorCPUInfoFree(qemuMonitorCPUInfoPtr list,
 int qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
                           qemuMonitorCPUInfoPtr *vcpus,
                           size_t maxvcpus,
-                          bool hotplug);
-virBitmapPtr qemuMonitorGetCpuHalted(qemuMonitorPtr mon, size_t maxvcpus);
+                          bool hotplug,
+                          bool fast);
+virBitmapPtr qemuMonitorGetCpuHalted(qemuMonitorPtr mon,
+                                     size_t maxvcpus,
+                                     bool fast);
 
 int qemuMonitorGetVirtType(qemuMonitorPtr mon,
                            virDomainVirtType *virtType);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 57c2c4d..4f2018d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1532,7 +1532,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;
@@ -1557,11 +1558,19 @@ 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));
-        ignore_value(virJSONValueObjectGetBoolean(entry, "halted", &halted));
-        qom_path = virJSONValueObjectGetString(entry, "qom_path");
+         * non-fatal, simply returning no data.
+         * The return data of query-cpus-fast has different field names
+         */
+        if (fast) {
+            ignore_value(virJSONValueObjectGetNumberInt(entry, "cpu-index", &cpuid));
+            ignore_value(virJSONValueObjectGetNumberInt(entry, "thread-id", &thread));
+            qom_path = virJSONValueObjectGetString(entry, "qom-path");
+        } else {
+            ignore_value(virJSONValueObjectGetNumberInt(entry, "CPU", &cpuid));
+            ignore_value(virJSONValueObjectGetNumberInt(entry, "thread_id", &thread));
+            ignore_value(virJSONValueObjectGetBoolean(entry, "halted", &halted));
+            qom_path = virJSONValueObjectGetString(entry, "qom_path");
+        }
 
         cpus[i].qemu_id = cpuid;
         cpus[i].tid = thread;
@@ -1586,10 +1595,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.
@@ -1598,13 +1609,19 @@ 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;
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
 
+    if (fast)
+        cmd = qemuMonitorJSONMakeCommand("query-cpus-fast", NULL);
+    else
+        cmd = qemuMonitorJSONMakeCommand("query-cpus", NULL);
+
     if (!cmd)
         return -1;
 
@@ -1619,7 +1636,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 045df49..c9fc727 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 0afdc80..0add50a 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1423,7 +1423,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) {
@@ -2719,7 +2719,7 @@ testQemuMonitorCPUInfo(const void *opaque)
         goto cleanup;
 
     rc = qemuMonitorGetCPUInfo(qemuMonitorTestGetMonitor(test),
-                               &vcpus, data->maxvcpus, true);
+                               &vcpus, data->maxvcpus, true, false);
 
     if (rc < 0)
         goto cleanup;
-- 
1.9.1

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

On 04/04/2018 10:45 AM, 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>
> ---
>  src/qemu/qemu_domain.c       | 14 +++++++++++---
>  src/qemu/qemu_monitor.c      | 30 ++++++++++++++++++------------
>  src/qemu/qemu_monitor.h      |  7 +++++--
>  src/qemu/qemu_monitor_json.c | 37 +++++++++++++++++++++++++++----------
>  src/qemu/qemu_monitor_json.h |  3 ++-
>  tests/qemumonitorjsontest.c  |  4 ++--
>  6 files changed, 65 insertions(+), 30 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9d1c33b..662937b 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -9006,7 +9006,12 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
>      if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
>          return -1;
>  

Count me as one of those that would prefer to see:

    bool fast;
...
    fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
                          QEMU_CAPS_QUERY_CPUS_FAST);
...

> -    rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus, hotplug);
> +    rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm),
> +                               &info,
> +                               maxvcpus,
> +                               hotplug,
> +                               virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
> +                                              QEMU_CAPS_QUERY_CPUS_FAST));

   rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus,
                              hotplug, fast);

>  
>      if (qemuDomainObjExitMonitor(driver, vm) < 0)
>          goto cleanup;
> @@ -9025,7 +9030,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
>           * thread, but it runs every vCPU in that same thread. So it
>           * is impossible to setup different affinity per thread.
>           *
> -         * What's more the 'query-cpus' command returns bizarre
> +         * What's more the 'query-cpus[-fast]' command returns bizarre
>           * data for the threads. It gives the TCG thread for the
>           * vCPU 0, but for vCPUs 1-> N, it actually replies with
>           * the main process thread ID.
> @@ -9126,7 +9131,10 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver,
>      if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
>          return -1;
>  
> -    haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus);


> +    haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm),
> +                                        maxvcpus,
> +                                        virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
> +                                                       QEMU_CAPS_QUERY_CPUS_FAST));

Likewise,

 bool fast;
...

 fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
                       QEMU_CAPS_QUERY_CPUS_FAST);
 haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus,
                                     fast);

>  
>      if (qemuDomainObjExitMonitor(driver, vm) < 0 || !haltedmap)
>          goto cleanup;

[...]

John

If you're OK with this I can make the alterations...  What you have
works - it's just one of those preferential things related to seeing
and/or relying on function calls within function calls.

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

On 12.04.2018 17:52, John Ferlan wrote:
> 
> 
> On 04/04/2018 10:45 AM, 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>
>> ---
>>  src/qemu/qemu_domain.c       | 14 +++++++++++---
>>  src/qemu/qemu_monitor.c      | 30 ++++++++++++++++++------------
>>  src/qemu/qemu_monitor.h      |  7 +++++--
>>  src/qemu/qemu_monitor_json.c | 37 +++++++++++++++++++++++++++----------
>>  src/qemu/qemu_monitor_json.h |  3 ++-
>>  tests/qemumonitorjsontest.c  |  4 ++--
>>  6 files changed, 65 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 9d1c33b..662937b 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -9006,7 +9006,12 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
>>      if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
>>          return -1;
>>  
> 
> Count me as one of those that would prefer to see:
> 
>     bool fast;
> ...
>     fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
>                           QEMU_CAPS_QUERY_CPUS_FAST);
> ...
> 
>> -    rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus, hotplug);
>> +    rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm),
>> +                               &info,
>> +                               maxvcpus,
>> +                               hotplug,
>> +                               virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
>> +                                              QEMU_CAPS_QUERY_CPUS_FAST));
> 
>    rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus,
>                               hotplug, fast);
> >>
>>      if (qemuDomainObjExitMonitor(driver, vm) < 0)
>>          goto cleanup;
>> @@ -9025,7 +9030,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
>>           * thread, but it runs every vCPU in that same thread. So it
>>           * is impossible to setup different affinity per thread.
>>           *
>> -         * What's more the 'query-cpus' command returns bizarre
>> +         * What's more the 'query-cpus[-fast]' command returns bizarre
>>           * data for the threads. It gives the TCG thread for the
>>           * vCPU 0, but for vCPUs 1-> N, it actually replies with
>>           * the main process thread ID.
>> @@ -9126,7 +9131,10 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver,
>>      if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
>>          return -1;
>>  
>> -    haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus);
> 
> 
>> +    haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm),
>> +                                        maxvcpus,
>> +                                        virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
>> +                                                       QEMU_CAPS_QUERY_CPUS_FAST));
> 
> Likewise,
> 
>  bool fast;
> ...
> 
>  fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
>                        QEMU_CAPS_QUERY_CPUS_FAST);
>  haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus,
>                                      fast);
> 
>>  
>>      if (qemuDomainObjExitMonitor(driver, vm) < 0 || !haltedmap)
>>          goto cleanup;
> 
> [...]
> 
> John
> 
> If you're OK with this I can make the alterations...  What you have
> works - it's just one of those preferential things related to seeing
> and/or relying on function calls within function calls.
> 

Sure. On another day I might have used a local variable as well.
Appreciating and accepting your offer to squash in this changes.

-- 
Regards,
  Viktor Mihajlovski

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