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

Re: [libvirt] [PATCH 1/2 v3] Fix virDomainChrDefParseTargetXML() to parse the target port if available



On 02/18/2011 07:11 AM, Michal Novotny wrote:
> Hi,
> this is the patch to fix the virDomainChrDefParseTargetXML() functionality
> to parse the target port from XML if available. This is necessary for
> multiple serial port support which is the second part of this patch.
> 
> Michal
> 
> Signed-off-by: Michal Novotny <minovotn redhat com>
> ---
>  src/conf/domain_conf.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)

> @@ -5566,7 +5568,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>          if (!chr)
>              goto error;
>  
> -        chr->target.port = i;
> +        if (chr->target.port < 0)
> +            chr->target.port = i;
>          def->serials[def->nserials++] = chr;

I think this fails to reject collisions, if two <serial> devices request
the same port number.

Also, I think it mis-handles the case where things are interleaved out
of order:

<devices>
  <serial type='dev'>
    <source .../>
    <target port='1'/>
  </serial>
  <serial type='dev'>
    <source .../>
    <target type='serial'/>
  <serial/>
</devices>

The second serial device should default to the first available port
number (0), but it looks like this patch will assign it to port 1 and
cause a duplicate.

Also, def->parallels shares virDomainDefParseXML, so it probably needs
the same treatment.  That is, I think this side of the patch still needs
a bit of work.  We've got other code in domain_conf.c that assigns ports
to the first available slot; for example, look near line 5628 at how
virtio-serial ports are assigned using maxport and traversal of all
previously assigned ports.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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