8:27 p.m.
Changelog:
---
v4:
- Update commit messages
---
v3:
- Added Reviewed-By
- Included CI Results Link
---
v2:
- Split patch into two commits
- Added fixes tag
---
This libvirt patch does the following:
1. fixes an issue with virtio console device port assignment on vioserial buses
2. updates console port reservation comment and changes the allowZero variable to
allowPortZero for clarity
Currently in libvirt, a virtio console device cannot be assigned a port number greater
than zero on a vioserial bus. This leads to port collision errors when adding more than 1
virtio console device on a single vioserial bus.
After applying this patch, one can add multiple console ports under a single vioserial
bus.
Here is a link to CI results for this series:
https://gitlab.com/aaronbmalik/libvirt/-/pipelines/1832324065
Aaron M. Brown (2):
virDomainVirtioSerialAddrAssign: Fix virtio console port assignment on
vioserial bus
domain_addr.c: Update console port reservation comment and allowZero
variable for clarity
src/conf/domain_addr.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
--
2.39.5 (Apple Git-154)
8:27 p.m.
New subject: [PATCH v4 1/2] virDomainVirtioSerialAddrAssign: Fix virtio console port assignment on vioserial bus
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(a)linux.ibm.com>
Signed-off-by: Aaron M. Brown <aaronmbr(a)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)
1:25 a.m.
New subject: [PATCH v4 1/2] virDomainVirtioSerialAddrAssign: Fix virtio
console port assignment on vioserial bus
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(a)linux.ibm.com>
Signed-off-by: Aaron M. Brown <aaronmbr(a)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
10:29 a.m.
New subject: [PATCH v4 1/2] virDomainVirtioSerialAddrAssign: Fix virtio
console port assignment on vioserial bus
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(a)linux.ibm.com>
> Signed-off-by: Aaron M. Brown <aaronmbr(a)linux.ibm.com>
> ---
> src/conf/domain_addr.c | 6 ++++++
7:05 p.m.
New subject: [PATCH v4 1/2] virDomainVirtioSerialAddrAssign: Fix virtio
console port assignment on vioserial bus
Thank you Ján, I have fixed this in the next version
8:27 p.m.
New subject: [PATCH v4 2/2] domain_addr.c: Update console port reservation comment and allowZero variable for clarity
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Signed-off-by: Aaron M. Brown <aaronmbr(a)linux.ibm.com>
---
src/conf/domain_addr.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index bc2b0f50e8..86f5cfc337 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1638,14 +1638,14 @@ static int
virDomainVirtioSerialAddrNext(virDomainDef *def,
virDomainVirtioSerialAddrSet *addrs,
virDomainDeviceVirtioSerialAddress *addr,
- bool allowZero)
+ bool allowPortZero)
{
ssize_t port, startPort = 0;
ssize_t i;
unsigned int controller;
- /* port number 0 is reserved for virtconsoles */
- if (allowZero)
+ /* port number 0 is reserved for the first virtconsole */
+ if (allowPortZero)
startPort = -1;
if (addrs->ncontrollers == 0) {
@@ -1725,11 +1725,11 @@ static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
virDomainVirtioSerialAddrAssign(virDomainDef *def,
virDomainVirtioSerialAddrSet *addrs,
virDomainDeviceInfo *info,
- bool allowZero,
+ bool allowPortZero,
bool portOnly)
{
virDomainDeviceInfo nfo = { 0 };
- virDomainDeviceInfo *ptr = allowZero ? &nfo : info;
+ virDomainDeviceInfo *ptr = allowPortZero ? &nfo : info;
ptr->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL;
@@ -1745,7 +1745,7 @@ virDomainVirtioSerialAddrAssign(virDomainDef *def,
} else {
if (virDomainVirtioSerialAddrNext(def, addrs, &ptr->addr.vioserial,
- allowZero) < 0)
+ allowPortZero) < 0)
return -1;
}
@@ -1764,20 +1764,20 @@ int
virDomainVirtioSerialAddrAutoAssignFromCache(virDomainDef *def,
virDomainVirtioSerialAddrSet *addrs,
virDomainDeviceInfo *info,
- bool allowZero)
+ bool allowPortZero)
{
bool portOnly = info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL;
if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL &&
info->addr.vioserial.port)
return virDomainVirtioSerialAddrReserve(NULL, NULL, info, addrs);
else
- return virDomainVirtioSerialAddrAssign(def, addrs, info, allowZero, portOnly);
+ return virDomainVirtioSerialAddrAssign(def, addrs, info, allowPortZero,
portOnly);
}
int
virDomainVirtioSerialAddrAutoAssign(virDomainDef *def,
virDomainDeviceInfo *info,
- bool allowZero)
+ bool allowPortZero)
{
virDomainVirtioSerialAddrSet *addrs = NULL;
int ret = -1;
@@ -1785,7 +1785,7 @@ virDomainVirtioSerialAddrAutoAssign(virDomainDef *def,
if (!(addrs = virDomainVirtioSerialAddrSetCreateFromDomain(def)))
goto cleanup;
- if (virDomainVirtioSerialAddrAutoAssignFromCache(def, addrs, info, allowZero) <
0)
+ if (virDomainVirtioSerialAddrAutoAssignFromCache(def, addrs, info, allowPortZero)
< 0)
goto cleanup;
ret = 0;
--
2.39.5 (Apple Git-154)
1:16 a.m.
On Thu, May 22, 2025 at 21:27:33 -0400, Aaron M. Brown wrote:
Changelog:
---
v4:
- Update commit messages
---
v3:
- Added Reviewed-By
- Included CI Results Link
---
v2:
- Split patch into two commits
- Added fixes tag
---
Hi,
I didn't find any of the 3 previous versions in the archives.
7:04 p.m.
My apologies Peter, this is my first time upstream. The first two versions were internally
reviewed
18
days inactive
32
days old
7 comments
4 participants
participants (4)
-
Aaron Brown -
Aaron M. Brown
-
Ján Tomko -
Peter Krempa