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@xxxxxxxxxx>
> > ---
> > 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
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list