On Tue, Jun 30, 2015 at 16:04:23 +0200, Michal Privoznik wrote: > 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@xxxxxxxxxx> > > --- > > 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? I think it shouldn't happen but the check for cfg was there before and it is also used in several other places so I didn't really feel like changing it within this patch. > > + 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 Thanks and pushed, Jirk -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list