1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH v2] 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/20180715164811.8555-1-jcfaracco@gmail.com
Test syntax-check passed
There is a newer version of this series
src/qemu/qemu_driver.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
[libvirt] [PATCH v2] 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 | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5de9aaefbb..a4df482c9e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20914,6 +20914,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);
 
@@ -20934,6 +20936,30 @@ 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;
+
+        if (unlink(old_dom_autostart_link) < 0 &&
+            errno != ENOENT &&
+            errno != ENOTDIR) {
+            virReportSystemError(errno,
+                                 _("Failed to delete symlink '%s'"),
+                                 old_dom_autostart_link);
+            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,
                                             VIR_DOMAIN_EVENT_UNDEFINED,
                                             VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
@@ -20960,6 +20986,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);
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: fix broken autostart symlink after renaming domain.
Posted by Erik Skultety 5 years, 9 months ago 
On Sun, Jul 15, 2018 at 01:48:11PM -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>
> ---

It would be nice to mention here what the change since v1 is.

>  src/qemu/qemu_driver.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5de9aaefbb..a4df482c9e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20914,6 +20914,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);
>
> @@ -20934,6 +20936,30 @@ 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;

What if we fail to define the new domain afterwards? We'll do a rollback,
however, we're stuck with the new symlink, so ^this tiny hunk above should
stay here (just like querying the config paths) and the reset below should come
after we successfully renamed a domain, however before we emit the
VIR_DOMAIN_EVENT_DEFINE event. Also, the rollback needs to be modified to
account for this link change.


> +
> +        if (unlink(old_dom_autostart_link) < 0 &&
> +            errno != ENOENT &&
> +            errno != ENOTDIR) {
> +            virReportSystemError(errno,
> +                                 _("Failed to delete symlink '%s'"),
> +                                 old_dom_autostart_link);
> +            goto cleanup;
> +        }

^I think you can save a line if you simply do virFileIsLink first and then
unlink it.

> +
> +        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;

indentation is still off ^here...

Erik

> +        }
> +    }
> +
>      event_old = virDomainEventLifecycleNewFromObj(vm,
>                                              VIR_DOMAIN_EVENT_UNDEFINED,
>                                              VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
> @@ -20960,6 +20986,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);

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.