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(a)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