[PATCH v2] qemu: Fix a regression of attaching device

[Osier Yang <jyang@xxxxxxxxxx>]

The regression is introduced by Commit da1eba6b, the new
codes with this commit doesn't reset "ret" to "-1" when
it fails on parsing the device XML (live device attachment)

This patch changes the codes to reset the "ret" and "-1",
and also changes the codes so that it don't modify "ret"
for condition checking.

How to reproduce:

<disk type='oops' device='disk'>
  <driver name='qemu' type='raw'/>
  <source file='/var/lib/libvirt/images/test.img'/>
  <target dev='vda' bus='virtio'/>
</disk>

Device attached successfully
---
 src/qemu/qemu_driver.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3d4207e..73118fd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4903,16 +4903,20 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
                             _("unknown domain modify action %d"), action);
             break;
         }
-    } else
-        ret = 0;
 
-    if (!ret && (flags & VIR_DOMAIN_AFFECT_LIVE)) {
+        if (ret == -1)
+            goto endjob;
+    }
+
+    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
         /* If dev exists it was created to modify the domain config. Free it. */
         virDomainDeviceDefFree(dev);
         dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
                                       VIR_DOMAIN_XML_INACTIVE);
-        if (dev == NULL)
+        if (dev == NULL) {
+            ret = -1;
             goto endjob;
+        }
 
         switch (action) {
         case QEMU_DEVICE_ATTACH:
@@ -4927,18 +4931,25 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
         default:
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
                             _("unknown domain modify action %d"), action);
+            ret = -1;
             break;
         }
+
+        if (ret == -1)
+            goto endjob;
         /*
          * update domain status forcibly because the domain status may be
          * changed even if we attach the device failed. For example, a
          * For example, a new controller may be created.
          */
-        if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
+        if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) {
             ret = -1;
+            goto endjob;
+        }
     }
+
     /* Finally, if no error until here, we can save config. */
-    if (!ret && (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
+    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
         ret = virDomainSaveConfig(driver->configDir, vmdef);
         if (!ret) {
             virDomainObjAssignDef(vm, vmdef, false);
-- 
1.7.6

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