On Tue, Jul 22, 2025 at 13:50:19 -0400, Aaron M. Brown wrote:
> This change fixes an issue with virtio console port assignment on virtio-serial buses.
> Currently, when trying to autoassign a virtio console device, the device cannot be
> assigned to a port greater than 0 on virtio-serial buses.
> You will receive the following error:
>
> `virtio-serial-bus: A port already exists at id 0`
>
> Therefore, the data needs to be passed back into info when allowZero is true.
> We should also preserve the controller data when allowZero is true, and
> propagate allowZero into virDomainVirtioSerialAddrNextFromController
> to get an appropriate startPort.
>
> Fixes: 16db8d2e ("Add functions to track virtio-serial addresses")
> Signed-off-by: Aaron M. Brown <aaronmbr@xxxxxxxxxxxxx>
> ---
> src/conf/domain_addr.c | 30 ++++++++++++++++---
> ...rial-autoassign-address.x86_64-latest.args | 4 +--
> ...erial-autoassign-address.x86_64-latest.xml | 4 +--
> 3 files changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 8dfa8feca0..5448d3d078 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -1692,12 +1692,16 @@ virDomainVirtioSerialAddrNext(virDomainDef *def,
>
> static int
> virDomainVirtioSerialAddrNextFromController(virDomainVirtioSerialAddrSet *addrs,
> - virDomainDeviceVirtioSerialAddress *addr)
> + virDomainDeviceVirtioSerialAddress *addr,
> + bool allowZero)
> {
> - ssize_t port;
> + ssize_t port, startPort = 0;
One definition per line.
> ssize_t i;
> virBitmap *map;
>
> + if (allowZero)
> + startPort = -1;
> +
> i = virDomainVirtioSerialAddrFindController(addrs, addr->controller);
> if (i < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -1707,7 +1711,7 @@ virDomainVirtioSerialAddrNextFromController(virDomainVirtioSerialAddrSet *addrs,
> }
>
> map = addrs->controllers[i]->ports;
> - if ((port = virBitmapNextClearBit(map, 0)) <= 0) {
> + if ((port = virBitmapNextClearBit(map, startPort)) < 0) {
> virReportError(VIR_ERR_XML_ERROR,
> _("Unable to find a free port on virtio-serial controller %1$u"),
> addr->controller);
> @@ -1718,6 +1722,15 @@ virDomainVirtioSerialAddrNextFromController(virDomainVirtioSerialAddrSet *addrs,
> addr->port = port;
> VIR_DEBUG("Found free virtio serial controller %u port %u", addr->controller,
> addr->port);
> +
> + /* if this is the first virtconsole, reserve port 0 */
> + if (allowZero && port == 0) {
> + ignore_value(virBitmapSetBit(map, 0));
> + VIR_DEBUG(
> + "Port 0 reserved for the first virtconsole on vioserial controller %1$u",
> + addr->controller);
> + }
This is suspicious because this function until now just returned the
next eligible port, but now it's also reserving it which doesn't seem to
fit well here.
Can you please explain this?
Also the spacing is weird, but I can fix that.
