[PATCH 5/7] lxcDomainAttachDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy

Peter Krempa pkrempa at redhat.com
Wed Nov 23 12:36:39 UTC 2022


'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 at redhat.com>
---
 src/lxc/lxc_driver.c | 39 ++++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index d66c26221c..2801ed3d6d 100644
--- 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;
 }
-- 
2.37.3



More information about the libvir-list mailing list