[libvirt] [PATCH] qemuDomainAttachDeviceMknodHelper: Remove symlink before creating it

[Andrea Bolognani]

On Thu, 2018-01-04 at 11:17 +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1528502
> 
> So imagine you have /dev/blah symlink which points to /dev/sda.
> You attach /dev/blah as disk to your domain. Libvirt correctly
> creates the /dev/blah -> /dev/sda symlink in the qemu namespace.
> However, then you detach the disk, change the symlink so that it
> points to /dev/sdb and tries to attach the disk again. This time,
> however, the attach fails (well, qemu attaches wrong disk)
> because the code assumes that symlinks don't change. Well they
> do.
> 
> This is inspired by test fix written by Eduardo Habkost.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_domain.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 70fb40650..5f29d1ad5 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -9737,13 +9737,23 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
>  
>      if (isLink) {
>          VIR_DEBUG("Creating symlink %s -> %s", data->file, data->target);
> +
> +        /* First, unlink the symlink target. Symlinks change and
> +         * therefore we have no guarantees that pre-existing
> +         * symlink is still valid. */
> +        if (unlink(data->file) < 0 &&

Here...

> +            errno != ENOENT) {
> +            virReportSystemError(errno,
> +                                 _("Unable to remove symlink %s"),
> +                                 data->file);

... and here, shouldn't you be using data->target instead
of data->file?

-- 
Andrea Bolognani / Red Hat / Virtualization