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

[Erik Skultety]

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 at 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);