[PATCH 7/8] qemu: refactor qemuDomainDefineXMLFlags

Nikolay Shirokovskiy posted 8 patches 5 years, 11 months ago
There is a newer version of this series
[PATCH 7/8] qemu: refactor qemuDomainDefineXMLFlags
Posted by Nikolay Shirokovskiy 5 years, 11 months ago
Let's move objlist restoring to cleanup section so that we can
handle failure of actions between virDomainObjListAdd and
virDomainDefSave.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
---
 src/qemu/qemu_driver.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4553ebfb80..e1b9240893 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7630,7 +7630,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
 
     if (!(def = virDomainDefParseString(xml, driver->xmlopt,
                                         NULL, parse_flags)))
-        goto cleanup;
+        return NULL;
 
     if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
         goto cleanup;
@@ -7644,10 +7644,23 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
         goto cleanup;
     def = NULL;
 
+    if (virDomainDefSave(vm->newDef ? vm->newDef : vm->def,
+                         driver->xmlopt, cfg->configDir) < 0)
+        goto cleanup;
+
     vm->persistent = 1;
 
-    if (virDomainDefSave(vm->newDef ? vm->newDef : vm->def,
-                         driver->xmlopt, cfg->configDir) < 0) {
+    event = virDomainEventLifecycleNewFromObj(vm,
+                                     VIR_DOMAIN_EVENT_DEFINED,
+                                     !oldDef ?
+                                     VIR_DOMAIN_EVENT_DEFINED_ADDED :
+                                     VIR_DOMAIN_EVENT_DEFINED_UPDATED);
+
+    VIR_INFO("Creating domain '%s'", vm->def->name);
+    dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
+
+ cleanup:
+    if (!def) {
         if (oldDef) {
             /* There is backup so this VM was defined before.
              * Just restore the backup. */
@@ -7660,22 +7673,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
         } else {
             /* Brand new domain. Remove it */
             VIR_INFO("Deleting domain '%s'", vm->def->name);
-            vm->persistent = 0;
             qemuDomainRemoveInactiveJob(driver, vm);
         }
-        goto cleanup;
     }
-
-    event = virDomainEventLifecycleNewFromObj(vm,
-                                     VIR_DOMAIN_EVENT_DEFINED,
-                                     !oldDef ?
-                                     VIR_DOMAIN_EVENT_DEFINED_ADDED :
-                                     VIR_DOMAIN_EVENT_DEFINED_UPDATED);
-
-    VIR_INFO("Creating domain '%s'", vm->def->name);
-    dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
-
- cleanup:
     virDomainDefFree(oldDef);
     virDomainDefFree(def);
     virDomainObjEndAPI(&vm);
-- 
2.23.0


Re: [PATCH 7/8] qemu: refactor qemuDomainDefineXMLFlags
Posted by Nikolay Shirokovskiy 5 years, 11 months ago

On 03.03.2020 11:19, Nikolay Shirokovskiy wrote:
> Let's move objlist restoring to cleanup section so that we can
> handle failure of actions between virDomainObjListAdd and
> virDomainDefSave.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> ---
>  src/qemu/qemu_driver.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 4553ebfb80..e1b9240893 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7630,7 +7630,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
>  
>      if (!(def = virDomainDefParseString(xml, driver->xmlopt,
>                                          NULL, parse_flags)))
> -        goto cleanup;
> +        return NULL;
>  
>      if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
>          goto cleanup;
> @@ -7644,10 +7644,23 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
>          goto cleanup;
>      def = NULL;
>  
> +    if (virDomainDefSave(vm->newDef ? vm->newDef : vm->def,
> +                         driver->xmlopt, cfg->configDir) < 0)
> +        goto cleanup;
> +
>      vm->persistent = 1;
>  
> -    if (virDomainDefSave(vm->newDef ? vm->newDef : vm->def,
> -                         driver->xmlopt, cfg->configDir) < 0) {
> +    event = virDomainEventLifecycleNewFromObj(vm,
> +                                     VIR_DOMAIN_EVENT_DEFINED,
> +                                     !oldDef ?
> +                                     VIR_DOMAIN_EVENT_DEFINED_ADDED :
> +                                     VIR_DOMAIN_EVENT_DEFINED_UPDATED);
> +
> +    VIR_INFO("Creating domain '%s'", vm->def->name);
> +    dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
> +
> + cleanup:
> +    if (!def) {

if (!dom && !def)

Nikolay

>          if (oldDef) {
>              /* There is backup so this VM was defined before.
>               * Just restore the backup. */
> @@ -7660,22 +7673,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
>          } else {
>              /* Brand new domain. Remove it */
>              VIR_INFO("Deleting domain '%s'", vm->def->name);
> -            vm->persistent = 0;
>              qemuDomainRemoveInactiveJob(driver, vm);
>          }
> -        goto cleanup;
>      }
> -
> -    event = virDomainEventLifecycleNewFromObj(vm,
> -                                     VIR_DOMAIN_EVENT_DEFINED,
> -                                     !oldDef ?
> -                                     VIR_DOMAIN_EVENT_DEFINED_ADDED :
> -                                     VIR_DOMAIN_EVENT_DEFINED_UPDATED);
> -
> -    VIR_INFO("Creating domain '%s'", vm->def->name);
> -    dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
> -
> - cleanup:
>      virDomainDefFree(oldDef);
>      virDomainDefFree(def);
>      virDomainObjEndAPI(&vm);
>