Re: [PATCH 2/2] hotplug: fix char device detach

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

 



On 10/31/2014 06:16 PM, John Ferlan wrote:


On 10/31/2014 12:45 PM, Pavel Hrdina wrote:
Hotplugging and hotunplugging char devices is only supported through
'-device' and the check for device capability should be independently.

Coverity also complains about 'tmpChr->info.alias' could be NULL and we
are dereferencing it but it somehow only in this case don't recognize
that the value is set by 'qemuAssignDeviceChrAlias' so it's clearly
false positive. Add comment to skip this error in this case.

Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
---
  src/qemu/qemu_hotplug.c | 12 ++++++++----
  1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d3bf392..520c0db 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3713,18 +3713,22 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
          return ret;
      }

-    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
-        !tmpChr->info.alias) {
-        if (qemuAssignDeviceChrAlias(vmdef, tmpChr, -1) < 0)
-            return ret;
+    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
+        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                       _("qemu does not support -device"));
+        return ret;
      }

Doh!  I should have looked at the Attach code which has this check first
and done the same thing. Instead I was following the model of other
patches in my series.

You can choose to leave it there or hoist it as first check, too


+    if (!tmpChr->info.alias && qemuAssignDeviceChrAlias(vmdef, tmpChr, -1) < 0)
+        return ret;
+

Add 'sa_assert(tmpChr->info.alias);' see [1]

      if (qemuBuildChrDeviceStr(&devstr, vm->def, chr, priv->qemuCaps) < 0)
          return ret;

      qemuDomainMarkDeviceForRemoval(vm, &tmpChr->info);

      qemuDomainObjEnterMonitor(driver, vm);
+    /* coverity[var_deref_model] */

[1] This could be too generic...

By adding the sa_assert() above which only gets executed under Coverity
you avoid the need for this

ACK with those changes


Oh, that's better solution. I'll update the patch and push it tomorrow if we don't get any answer about the first patch.

Thanks,

Pavel

John

      if (devstr && qemuMonitorDelDevice(priv->mon, tmpChr->info.alias) < 0) {
          qemuDomainObjExitMonitor(driver, vm);
          goto cleanup;


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




[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]