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

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(a)redhat.com&gt; > --- > 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