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

Re: [libvirt] [PATCH] spice: don't force user to specify spicevmc channel



On Mon, Feb 03, 2014 at 09:25:57AM +0100, Christophe Fergeau wrote:
> On Fri, Jan 31, 2014 at 05:08:33PM +0100, Martin Kletzander wrote:
> > We support only one spicevmc channel name anyway and the code is
> > prepared to use the default one, there's only one check missing.  I'm
> > not adding it to documentation in case there is another channel name
> > aded in the future, but this helps people using virsh for defining
>
> 'added'
>
> > domains with spice vdagent.
> >
> > Signed-off-by: Martin Kletzander <mkletzan redhat com>
> > ---
> > I extended the context to see what I meant by "the code is already
> > prepared to use the default one".
>
> ACK, this makes the code consistent with what the documentation says:
> "an optional attribute name controls how the guest will have access to the
> channel, and defaults to name='com.redhat.spice.0'." One small comment
> below.
>

Shame on me that I haven't even checked the documentation.  I added
the info to the commit as well.

> >
> >  src/qemu/qemu_command.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 2db745a..2124477 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -6127,30 +6127,31 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev,
> >                             "%s", _("virtio serial device has invalid address type"));
> >              goto error;
> >          }
> >
> >          virBufferAsprintf(&buf,
> >                            ",bus=" QEMU_VIRTIO_SERIAL_PREFIX "%d.%d",
> >                            dev->info.addr.vioserial.controller,
> >                            dev->info.addr.vioserial.bus);
> >          virBufferAsprintf(&buf,
> >                            ",nr=%d",
> >                            dev->info.addr.vioserial.port);
> >      }
> >
> >      if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
> >          dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC &&
> > +        dev->target.name &&
> >          STRNEQ_NULLABLE(dev->target.name, "com.redhat.spice.0")) {
>
> Should this be changed to STRENEQ() now that dev->target.name can't be
> NULL?
>

No need to, but it makes the code more readable and cleaner, so I
changed that too and pushed it.  Thanks for noticing it.

Martin

Attachment: signature.asc
Description: Digital signature


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