[PATCH v4 1/2] virDomainVirtioSerialAddrAssign: Fix virtio console port assignment on vioserial bus

Aaron M. Brown posted 2 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v4 1/2] virDomainVirtioSerialAddrAssign: Fix virtio console port assignment on vioserial bus
Posted by Aaron M. Brown 3 months, 2 weeks ago
This change fixes an issue with virito console port assignment on vioserial buses.
Currently, a virtio console device cannot be assigned to a port greater than 0 on
vioserial buses. When trying to add more than one virtio console device on a single
vioserial bus, you will get a port already exists with id 0 error.
Therefore, the data needs to be passed back into info when allowZero is true

Fixes: 16db8d2ec540 ("Add functions to track virtio-serial addresses")
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Aaron M. Brown <aaronmbr@linux.ibm.com>
---
 src/conf/domain_addr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 8dfa8feca0..bc2b0f50e8 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1737,6 +1737,12 @@ virDomainVirtioSerialAddrAssign(virDomainDef *def,
         if (virDomainVirtioSerialAddrNextFromController(addrs,
                                                         &ptr->addr.vioserial) < 0)
             return -1;
+
+        if (ptr == &nfo) {
+            /* pass the vioserial data back into info */
+            info->addr.vioserial = ptr->addr.vioserial;
+        }
+
     } else {
         if (virDomainVirtioSerialAddrNext(def, addrs, &ptr->addr.vioserial,
                                           allowZero) < 0)
-- 
2.39.5 (Apple Git-154)
Re: [PATCH v4 1/2] virDomainVirtioSerialAddrAssign: Fix virtio console port assignment on vioserial bus
Posted by Peter Krempa via Devel 3 months, 2 weeks ago
On Thu, May 22, 2025 at 21:27:34 -0400, Aaron M. Brown wrote:
> This change fixes an issue with virito console port assignment on vioserial buses.
> Currently, a virtio console device cannot be assigned to a port greater than 0 on
> vioserial buses. When trying to add more than one virtio console device on a single
> vioserial bus, you will get a port already exists with id 0 error.

Ideally paste the error verbatim here rather than paraphrasing it, so
that it can be looked up.

> Therefore, the data needs to be passed back into info when allowZero is true
> 
> Fixes: 16db8d2ec540 ("Add functions to track virtio-serial addresses")
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> Signed-off-by: Aaron M. Brown <aaronmbr@linux.ibm.com>
> ---
>  src/conf/domain_addr.c | 6 ++++++

A change in address allocation really requires a test case showing
what's happening and more importantly prevents regressions in the
future.

Ideally add the test before this change showing old behaviour and this
patch will then modify it to show how it was fixed.

qemuxmlconftest should be able to demonstrate what's going on [1]

>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 8dfa8feca0..bc2b0f50e8 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -1737,6 +1737,12 @@ virDomainVirtioSerialAddrAssign(virDomainDef *def,
>          if (virDomainVirtioSerialAddrNextFromController(addrs,
>                                                          &ptr->addr.vioserial) < 0)
>              return -1;
> +
> +        if (ptr == &nfo) {
> +            /* pass the vioserial data back into info */

The comment should also say why you're passing it back

> +            info->addr.vioserial = ptr->addr.vioserial;
> +        }
> +
>      } else {
>          if (virDomainVirtioSerialAddrNext(def, addrs, &ptr->addr.vioserial,
>                                            allowZero) < 0)

[1]:
 - add xml to tests/qemuxmlconfdata/ with configuration that shows the
   problem
 - invoke it from tests/qemuxmlconftest.c
 - use VIR_TEST_REGENERATE_OUTPUT=1
   PATH_TO_BUILDDIR/tests/qemuxmlconftest to generate required output
   files
Re: [PATCH v4 1/2] virDomainVirtioSerialAddrAssign: Fix virtio console port assignment on vioserial bus
Posted by Ján Tomko via Devel 3 months, 2 weeks ago
On a Monday in 2025, Peter Krempa via Devel wrote:
>On Thu, May 22, 2025 at 21:27:34 -0400, Aaron M. Brown wrote:
>> This change fixes an issue with virito console port assignment on vioserial buses.

s/viritio/virtio/

Jano

>> Currently, a virtio console device cannot be assigned to a port greater than 0 on
>> vioserial buses. When trying to add more than one virtio console device on a single
>> vioserial bus, you will get a port already exists with id 0 error.
>
>Ideally paste the error verbatim here rather than paraphrasing it, so
>that it can be looked up.
>
>> Therefore, the data needs to be passed back into info when allowZero is true
>>
>> Fixes: 16db8d2ec540 ("Add functions to track virtio-serial addresses")
>> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
>> Signed-off-by: Aaron M. Brown <aaronmbr@linux.ibm.com>
>> ---
>>  src/conf/domain_addr.c | 6 ++++++
>
Re: [PATCH v4 1/2] virDomainVirtioSerialAddrAssign: Fix virtio console port assignment on vioserial bus
Posted by Aaron Brown 3 months ago
Thank you Ján, I have fixed this in the next version