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

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

 



[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@xxxxxxxxxx>, RHCE
Virtualization Team (xen userspace), Red Hat

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]