[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



[snip]
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.

Is the code correct there?

The code is:

if (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL &&
            chr->info.addr.vioserial.port == 0) {
            int maxport = 0;
            int j;
            for (j = 0 ; j < i ; j++) {
                virDomainChrDefPtr thischr = def->channels[j];
if (thischr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL && thischr->info.addr.vioserial.controller == chr->info.addr.vioserial.controller && thischr->info.addr.vioserial.bus == chr->info.addr.vioserial.bus &&
                    (int)thischr->info.addr.vioserial.port > maxport)
                    maxport = thischr->info.addr.vioserial.port;
            }
            chr->info.addr.vioserial.port = maxport + 1;
        }


That is if it's found you're having maxport set to maximum value + 1, that's fine but if it's not found it doesn't start from 0 but it starts from number 1 since "chr->info.addr.vioserial.port = maxport + 1;" line. Maxport is being set to 0 and when nothing is found then the code is "chr->info.addr.vioserial.port = 0 + 1" therefore resulting into "chr->info.addr.vioserial.port = 1;". Based on this it doesn't start zero-based since this position (port = 0) is unassigned but only next position (port = 1) is defined.

Is that behavior correct?

Thanks,
Michal

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


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