[libvirt] [PATCH v4] qemu: fix broken autostart symlink after renaming domain.

Julio Faracco posted 1 patch 5 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180719042148.10476-1-jcfaracco@gmail.com
Test syntax-check passed
src/qemu/qemu_driver.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
[libvirt] [PATCH v4] qemu: fix broken autostart symlink after renaming domain.
Posted by Julio Faracco 5 years, 9 months ago
If a domain is configured to start on boot, it has a symlink to the
domain definition inside the autostart directory. If you rename this
domain, the definition is renamed too. The symlink need to be pointed to
this renamed file. This commit recreates the symlink after renaming the
XML file.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1594985

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 src/qemu/qemu_driver.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 25170f6f26..09af231dfc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20917,6 +20917,8 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
     char *old_dom_name = NULL;
     char *new_dom_cfg_file = NULL;
     char *old_dom_cfg_file = NULL;
+    char *new_dom_autostart_link = NULL;
+    char *old_dom_autostart_link = NULL;
 
     virCheckFlags(0, ret);
 
@@ -20937,6 +20939,14 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
                                                  vm->def->name)))
         goto cleanup;
 
+    if (vm->autostart) {
+        if (!(new_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
+                                                          new_dom_name)) ||
+            !(old_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
+                                                          vm->def->name)))
+            goto cleanup;
+    }
+
     event_old = virDomainEventLifecycleNewFromObj(vm,
                                             VIR_DOMAIN_EVENT_UNDEFINED,
                                             VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
@@ -20949,6 +20959,23 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
     if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
         goto rollback;
 
+    if (vm->autostart) {
+        if (virFileIsLink(old_dom_autostart_link) &&
+            unlink(old_dom_autostart_link) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to delete symlink '%s'"),
+                                 old_dom_autostart_link);
+            goto rollback;
+        }
+
+        if (symlink(new_dom_cfg_file, new_dom_autostart_link) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to create symlink '%s to '%s'"),
+                                 new_dom_autostart_link, new_dom_cfg_file);
+            goto rollback;
+        }
+    }
+
     if (virFileExists(old_dom_cfg_file) &&
         unlink(old_dom_cfg_file) < 0) {
         virReportSystemError(errno,
@@ -20963,6 +20990,8 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
     ret = 0;
 
  cleanup:
+    VIR_FREE(old_dom_autostart_link);
+    VIR_FREE(new_dom_autostart_link);
     VIR_FREE(old_dom_cfg_file);
     VIR_FREE(new_dom_cfg_file);
     VIR_FREE(old_dom_name);
@@ -20982,6 +21011,11 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
     if (virFileExists(new_dom_cfg_file))
         unlink(new_dom_cfg_file);
 
+    if (vm->autostart) {
+        if (virFileExists(new_dom_autostart_link))
+            unlink(new_dom_autostart_link);
+    }
+
     goto cleanup;
 }
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] qemu: fix broken autostart symlink after renaming domain.
Posted by Erik Skultety 5 years, 9 months ago
On Thu, Jul 19, 2018 at 01:21:48AM -0300, Julio Faracco wrote:
> If a domain is configured to start on boot, it has a symlink to the
> domain definition inside the autostart directory. If you rename this
> domain, the definition is renamed too. The symlink need to be pointed to
> this renamed file. This commit recreates the symlink after renaming the
> XML file.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1594985
>
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>  src/qemu/qemu_driver.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 25170f6f26..09af231dfc 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20917,6 +20917,8 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>      char *old_dom_name = NULL;
>      char *new_dom_cfg_file = NULL;
>      char *old_dom_cfg_file = NULL;
> +    char *new_dom_autostart_link = NULL;
> +    char *old_dom_autostart_link = NULL;
>
>      virCheckFlags(0, ret);
>
> @@ -20937,6 +20939,14 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>                                                   vm->def->name)))
>          goto cleanup;
>
> +    if (vm->autostart) {
> +        if (!(new_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
> +                                                          new_dom_name)) ||
> +            !(old_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
> +                                                          vm->def->name)))
> +            goto cleanup;
> +    }
> +
>      event_old = virDomainEventLifecycleNewFromObj(vm,
>                                              VIR_DOMAIN_EVENT_UNDEFINED,
>                                              VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
> @@ -20949,6 +20959,23 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>      if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
>          goto rollback;
>
> +    if (vm->autostart) {
> +        if (virFileIsLink(old_dom_autostart_link) &&
> +            unlink(old_dom_autostart_link) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Failed to delete symlink '%s'"),
> +                                 old_dom_autostart_link);
> +            goto rollback;
> +        }
> +
> +        if (symlink(new_dom_cfg_file, new_dom_autostart_link) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Failed to create symlink '%s to '%s'"),
> +                                 new_dom_autostart_link, new_dom_cfg_file);
> +            goto rollback;

If ^this symlink fails, you don't restore the original. The only thing your
code really needs is to move some bits around, the logic is fine otherwise.
I've done that, you can see the diff of the changes I performed in the attached
diff and pushed the patch.

Thanks,
Erik

> +        }
> +    }
> +
>      if (virFileExists(old_dom_cfg_file) &&
>          unlink(old_dom_cfg_file) < 0) {
>          virReportSystemError(errno,
> @@ -20963,6 +20990,8 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>      ret = 0;
>
>   cleanup:
> +    VIR_FREE(old_dom_autostart_link);
> +    VIR_FREE(new_dom_autostart_link);
>      VIR_FREE(old_dom_cfg_file);
>      VIR_FREE(new_dom_cfg_file);
>      VIR_FREE(old_dom_name);
> @@ -20982,6 +21011,11 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>      if (virFileExists(new_dom_cfg_file))
>          unlink(new_dom_cfg_file);
>
> +    if (vm->autostart) {
> +        if (virFileExists(new_dom_autostart_link))
> +            unlink(new_dom_autostart_link);
> +    }
> +
>      goto cleanup;
>  }
>
> --
> 2.17.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 09af231dfc..b5e6fe8132 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20945,6 +20945,13 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
             !(old_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
                                                           vm->def->name)))
             goto cleanup;
+
+        if (symlink(new_dom_cfg_file, new_dom_autostart_link) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to create symlink '%s to '%s'"),
+                                 new_dom_autostart_link, new_dom_cfg_file);
+            goto cleanup;
+        }
     }
 
     event_old = virDomainEventLifecycleNewFromObj(vm,
@@ -20959,23 +20966,6 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
     if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
         goto rollback;
 
-    if (vm->autostart) {
-        if (virFileIsLink(old_dom_autostart_link) &&
-            unlink(old_dom_autostart_link) < 0) {
-            virReportSystemError(errno,
-                                 _("Failed to delete symlink '%s'"),
-                                 old_dom_autostart_link);
-            goto rollback;
-        }
-
-        if (symlink(new_dom_cfg_file, new_dom_autostart_link) < 0) {
-            virReportSystemError(errno,
-                                 _("Failed to create symlink '%s to '%s'"),
-                                 new_dom_autostart_link, new_dom_cfg_file);
-            goto rollback;
-        }
-    }
-
     if (virFileExists(old_dom_cfg_file) &&
         unlink(old_dom_cfg_file) < 0) {
         virReportSystemError(errno,
@@ -20984,6 +20974,16 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
         goto rollback;
     }
 
+    if (vm->autostart) {
+        if (virFileIsLink(old_dom_autostart_link) &&
+            unlink(old_dom_autostart_link) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to delete symlink '%s'"),
+                                 old_dom_autostart_link);
+            goto rollback;
+        }
+    }
+
     event_new = virDomainEventLifecycleNewFromObj(vm,
                                               VIR_DOMAIN_EVENT_DEFINED,
                                               VIR_DOMAIN_EVENT_DEFINED_RENAMED);
@@ -21011,10 +21011,9 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
     if (virFileExists(new_dom_cfg_file))
         unlink(new_dom_cfg_file);
 
-    if (vm->autostart) {
-        if (virFileExists(new_dom_autostart_link))
-            unlink(new_dom_autostart_link);
-    }
+    if (vm->autostart &&
+        virFileExists(new_dom_autostart_link))
+        unlink(new_dom_autostart_link);
 
     goto cleanup;
 }
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list