[libvirt] Regression in allocating ports for serial/parallel devs

[Michal Novotny]

On 04/05/2011 06:13 PM, Cole Robinson wrote:
> Hi Michal,
>
> The following commit introduced a regression:
>
> http://libvirt.org/git/?p=libvirt.git;a=commit;h=79c3fe4d1681cd94598d2bd42e38a98f51cb645d
>
> Now, defining a guest with XML like
>
> <serial type='pty'/>
> <serial type='null'/>
> <serial type='stdio'/>
>
> Will allocate <target port='0'/> to all 3. The reason is that
> target.port is never set to -1 unless the user specified some <target>
> XML. A simple fix is:
>
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3265,6 +3265,8 @@ virDomainChrDefParseXML(virCapsPtr caps,
>          return NULL;
>      }
>
> +    def->target.port = -1;
> +
>      type = virXMLPropString(node, "type");
>      if (type == NULL) {
>          def->source.type = VIR_DOMAIN_CHR_TYPE_PTY;
>
> But that doesn't solve the problem for users who are building ChrDef's
> by hand, like when converting between formats as xen and vmware drivers
> do. I didn't look at those users so they may be safe, but the interface
> should be improved. Maybe add a ChrDefNew function that sets the -1 default.
>
> Additionally we should add a qemuxml2xml test for this to prevent
> against future regressions.
>
> Thanks,
> Cole

Hi Cole,
do you think it's worth implementing the ChrDefNew besides the ChrDef
which we already have?

There's a regression testing but maybe it's not for qemuxml2xml test.

Adding DV to the CC list for his opinion whether he has some better idea
about this.

Thanks,
Michal

-- 
Michal Novotny <minovotn at redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat