[libvirt] [PATCH] qemu: Avoid using ".(null)" in UNIX socket path

[Michal Privoznik]

On 30.06.2015 13:49, Jiri Denemark wrote:
> The code which generates paths for UNIX socket blindly used target name
> without checking if it was set. Thus for the following device XML
> 
>     <channel type='unix'>
>       <source mode='bind'/>
>       <target type='virtio'/>
>     </channel>
> 
> we would generate "/var/lib/libvirt/qemu/channel/target/NAME.(null)"
> path which works but is not really correct. Let's not use the
> ".target_name" suffix at all if target name is not set.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1226854
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/qemu/qemu_domain.c                             | 20 +++++++++---
>  .../qemuxml2argv-channel-virtio-unix.args          | 19 +++++++++++
>  .../qemuxml2argv-channel-virtio-unix.xml           | 37 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  5 +++
>  4 files changed, 77 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.xml
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 6213fd9..404489c 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1249,11 +1249,23 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>          dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
>          dev->data.chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
>          dev->data.chr->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
> -        !dev->data.chr->source.data.nix.path && cfg) {
> -        if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s.%s",
> -                        cfg->channelTargetDir,
> -                        def->name, dev->data.chr->target.name) < 0)
> +        !dev->data.chr->source.data.nix.path) {
> +        if (!cfg) {

Can this ever happen or you just want to be super-trouper future proof?

> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("cannot generate UNIX socket path"));
>              goto cleanup;
> +        }
> +
> +        if (dev->data.chr->target.name) {
> +            if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s.%s",
> +                            cfg->channelTargetDir,
> +                            def->name, dev->data.chr->target.name) < 0)
> +                goto cleanup;
> +        } else {
> +            if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s",
> +                            cfg->channelTargetDir, def->name) < 0)
> +                goto cleanup;
> +        }
>  
>          dev->data.chr->source.data.nix.listen = true;
>      }

ACK and safe for the freeze.

Micha