[PATCH v2] qemu: Pass / fill niothreads for qemuMonitorGetIOThreads

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Let's pass along / fill @niothreads rather than trying to make dual
use as a return value and thread count.

This resolves a Coverity issue detected in qemuDomainGetIOThreadsMon
where if qemuDomainObjExitMonitor failed, then a -1 was returned and
overwrite @niothreads causing a memory leak.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---

 Since v1, updated the logic to pass @niothreads around rather than
 rely on the dual meaning. Took the full plunge.

 src/qemu/qemu_driver.c       | 23 +++++++++++------------
 src/qemu/qemu_monitor.c      |  8 +++++---
 src/qemu/qemu_monitor.h      |  3 ++-
 src/qemu/qemu_monitor_json.c |  6 ++++--
 src/qemu/qemu_monitor_json.h |  3 ++-
 src/qemu/qemu_process.c      |  4 ++--
 tests/qemumonitorjsontest.c  |  4 ++--
 7 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bca1c84630..65725b2ef2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4972,17 +4972,18 @@ qemuDomainGetMaxVcpus(virDomainPtr dom)
 static int
 qemuDomainGetIOThreadsMon(virQEMUDriverPtr driver,
                           virDomainObjPtr vm,
-                          qemuMonitorIOThreadInfoPtr **iothreads)
+                          qemuMonitorIOThreadInfoPtr **iothreads,
+                          int *niothreads)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
-    int niothreads = 0;
+    int ret = -1;
 
     qemuDomainObjEnterMonitor(driver, vm);
-    niothreads = qemuMonitorGetIOThreads(priv->mon, iothreads);
-    if (qemuDomainObjExitMonitor(driver, vm) < 0 || niothreads < 0)
+    ret = qemuMonitorGetIOThreads(priv->mon, iothreads, niothreads);
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
         return -1;
 
-    return niothreads;
+    return ret;
 }
 
 
@@ -5014,7 +5015,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
         goto endjob;
     }
 
-    if ((niothreads = qemuDomainGetIOThreadsMon(driver, vm, &iothreads)) < 0)
+    if ((ret = qemuDomainGetIOThreadsMon(driver, vm, &iothreads, &niothreads)) < 0)
         goto endjob;
 
     /* Nothing to do */
@@ -5314,8 +5315,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
      * IOThreads thread_id's, adjust the cgroups, thread affinity,
      * and add the thread_id to the vm->def->iothreadids list.
      */
-    if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
-                                                  &new_iothreads)) < 0)
+    if (qemuMonitorGetIOThreads(priv->mon, &new_iothreads, &new_niothreads) < 0)
         goto exit_monitor;
 
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
@@ -5425,8 +5425,7 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
     if (rc < 0)
         goto exit_monitor;
 
-    if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
-                                                  &new_iothreads)) < 0)
+    if (qemuMonitorGetIOThreads(priv->mon, &new_iothreads, &new_niothreads) < 0)
         goto exit_monitor;
 
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
@@ -18507,7 +18506,7 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver,
     qemuDomainObjPrivatePtr priv = dom->privateData;
     size_t i;
     qemuMonitorIOThreadInfoPtr *iothreads = NULL;
-    int niothreads;
+    int niothreads = 0;
     int ret = -1;
 
     if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom))
@@ -18516,7 +18515,7 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver,
     if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD))
         return 0;
 
-    if ((niothreads = qemuDomainGetIOThreadsMon(driver, dom, &iothreads)) < 0)
+    if (qemuDomainGetIOThreadsMon(driver, dom, &iothreads, &niothreads) < 0)
         return -1;
 
     /* qemuDomainGetIOThreadsMon returns a NULL-terminated list, so we must free
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ce1a06c4c8..551b65e778 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4211,22 +4211,24 @@ qemuMonitorRTCResetReinjection(qemuMonitorPtr mon)
  * qemuMonitorGetIOThreads:
  * @mon: Pointer to the monitor
  * @iothreads: Location to return array of IOThreadInfo data
+ * @niothreads: Count of the number of IOThreads in the array
  *
  * Issue query-iothreads command.
  * Retrieve the list of iothreads defined/running for the machine
  *
- * Returns count of IOThreadInfo structures on success
+ * Returns 0 on success
  *        -1 on error.
  */
 int
 qemuMonitorGetIOThreads(qemuMonitorPtr mon,
-                        qemuMonitorIOThreadInfoPtr **iothreads)
+                        qemuMonitorIOThreadInfoPtr **iothreads,
+                        int *niothreads)
 {
     VIR_DEBUG("iothreads=%p", iothreads);
 
     QEMU_CHECK_MONITOR(mon);
 
-    return qemuMonitorJSONGetIOThreads(mon, iothreads);
+    return qemuMonitorJSONGetIOThreads(mon, iothreads, niothreads);
 }
 
 
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8bc092870b..49be2d5412 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1365,7 +1365,8 @@ struct _qemuMonitorIOThreadInfo {
     bool set_poll_shrink;
 };
 int qemuMonitorGetIOThreads(qemuMonitorPtr mon,
-                            qemuMonitorIOThreadInfoPtr **iothreads);
+                            qemuMonitorIOThreadInfoPtr **iothreads,
+                            int *niothreads);
 int qemuMonitorSetIOThread(qemuMonitorPtr mon,
                            qemuMonitorIOThreadInfoPtr iothreadInfo);
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 5acc1a10aa..f70490d9b0 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8072,7 +8072,8 @@ qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon)
  */
 int
 qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
-                            qemuMonitorIOThreadInfoPtr **iothreads)
+                            qemuMonitorIOThreadInfoPtr **iothreads,
+                            int *niothreads)
 {
     int ret = -1;
     virJSONValuePtr cmd;
@@ -8149,9 +8150,10 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
             info->poll_valid = true;
     }
 
-    ret = n;
+    *niothreads = n;
     *iothreads = infolist;
     infolist = NULL;
+    ret = 0;
 
  cleanup:
     if (infolist) {
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index d2928b0ffc..4eb0f667a2 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -550,7 +550,8 @@ int qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon,
 int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon);
 
 int qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
-                                qemuMonitorIOThreadInfoPtr **iothreads)
+                                qemuMonitorIOThreadInfoPtr **iothreads,
+                                int *niothreads)
     ATTRIBUTE_NONNULL(2);
 
 int qemuMonitorJSONSetIOThread(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 20e90026e1..01afe66ec9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2498,10 +2498,10 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver,
     /* Get the list of IOThreads from qemu */
     if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
         goto cleanup;
-    niothreads = qemuMonitorGetIOThreads(priv->mon, &iothreads);
+    ret = qemuMonitorGetIOThreads(priv->mon, &iothreads, &niothreads);
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;
-    if (niothreads < 0)
+    if (ret < 0)
         goto cleanup;
 
     if (niothreads != vm->def->niothreadids) {
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 79ef2a545e..d0c37967d5 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2377,8 +2377,8 @@ testQemuMonitorJSONGetIOThreads(const void *opaque)
                                "}") < 0)
         goto cleanup;
 
-    if ((ninfo = qemuMonitorGetIOThreads(qemuMonitorTestGetMonitor(test),
-                                         &info)) < 0)
+    if (qemuMonitorGetIOThreads(qemuMonitorTestGetMonitor(test),
+                                &info, &ninfo) < 0)
         goto cleanup;
 
     if (ninfo != 2) {
-- 
2.28.0




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux