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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]