lxcDomainAttachDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy

'virDomainDeviceDefCopy' formats the definition and parses it back.
Since we already are parsing the XML here, we're better off parsing it
twice and save the formatting step.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index d66c26221c959899f08abf449cd56590ef06a088..2801ed3d6deb981371fc9d2a7095b790163fb5fd 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4255,8 +4255,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
     virLXCDriver *driver = dom->conn->privateData;
     virDomainObj *vm = NULL;
     g_autoptr(virDomainDef) vmdef = NULL;
-    virDomainDeviceDef *dev = NULL;
-    virDomainDeviceDef *dev_copy = NULL;
+    g_autoptr(virDomainDeviceDef) dev_config = NULL;
+    g_autoptr(virDomainDeviceDef) dev_live = NULL;
     int ret = -1;
     g_autoptr(virLXCDriverConfig) cfg = virLXCDriverGetConfig(driver);
 
@@ -4275,21 +4275,17 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
     if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
         goto endjob;
 
-    dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
-                                             driver->xmlopt, NULL,
-                                             VIR_DOMAIN_DEF_PARSE_INACTIVE);
-    if (dev == NULL)
-        goto endjob;
+    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+        if (!(dev_config = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
+                                                   NULL,
+                                                   VIR_DOMAIN_DEF_PARSE_INACTIVE)))
+            goto endjob;
+    }
 
-    if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
-        flags & VIR_DOMAIN_AFFECT_LIVE) {
-        /* If we are affecting both CONFIG and LIVE
-         * create a deep copy of device as adding
-         * to CONFIG takes one instance.
-         */
-        dev_copy = virDomainDeviceDefCopy(dev, vm->def,
-                                          driver->xmlopt, NULL);
-        if (!dev_copy)
+    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+        if (!(dev_live = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
+                                                 NULL,
+                                                 VIR_DOMAIN_DEF_PARSE_INACTIVE)))
             goto endjob;
     }
 
@@ -4299,22 +4295,22 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
         if (!vmdef)
             goto endjob;
 
-        if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
+        if (virDomainDefCompatibleDevice(vmdef, dev_config, NULL,
                                          VIR_DOMAIN_DEVICE_ACTION_ATTACH,
                                          false) < 0)
             goto endjob;
 
-        if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev_copy)) < 0)
+        if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev_config)) < 0)
             goto endjob;
     }
 
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
-        if (virDomainDefCompatibleDevice(vm->def, dev, NULL,
+        if (virDomainDefCompatibleDevice(vm->def, dev_live, NULL,
                                          VIR_DOMAIN_DEVICE_ACTION_ATTACH,
                                          true) < 0)
             goto endjob;
 
-        if ((ret = lxcDomainAttachDeviceLive(driver, vm, dev)) < 0)
+        if ((ret = lxcDomainAttachDeviceLive(driver, vm, dev_live)) < 0)
             goto endjob;
         /*
          * update domain status forcibly because the domain status may be
@@ -4338,9 +4334,6 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
     virDomainObjEndJob(vm);
 
  cleanup:
-    if (dev != dev_copy)
-        virDomainDeviceDefFree(dev_copy);
-    virDomainDeviceDefFree(dev);
     virDomainObjEndAPI(&vm);
     return ret;
 }