When support for s390x was introduced in libvirt, it naturally
followed the conventions established at the time for x86, which
were to have a USB controller added by default.
Later, in 2013, commit 3a82f628a964 made the default USB
controller model for s390x VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE,
effectively overriding the architecture-independent default.
However, an exception was carved out at the time: if the USB
controller had an address assigned to it, then it would be left
alone.
A couple of years later, commit 09ab9dcc85ec changed things
again in two ways: for starters, libvirt would no longer
automatically attempt to add a USB controller to newly-defined
s390x guests; moreover, the command line generator was changed
so that the legacy USB controller (-usb) would never be used
on s390x.
In other words, unless a model name is explicitly provided for
the USB controller, which is something that only actually works
when using a recent QEMU version (see commit f9ed4d385ab8),
s390x guests will never have USB controllers attached to them.
Remove the exception carved out a decade ago and always
reflect this fact accurately in the guest XML.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
src/qemu/qemu_domain.c | 14 +++++++++-----
.../qemuhotplug-base-ccw-live+ccw-virtio.xml | 5 +----
...live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 5 +----
...qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 5 +----
...-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 5 +----
...-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 5 +----
.../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 5 +----
.../qemuhotplug-base-ccw-live.xml | 5 +----
.../s390-usb-address.s390x-latest.xml | 6 +-----
9 files changed, 17 insertions(+), 38 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 31d634b151..03ad590759 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5669,11 +5669,9 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont,
cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
if (ARCH_IS_S390(def->os.arch)) {
- if (cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
- /* set the default USB model to none for s390 unless an
- * address is found */
- cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
- }
+ /* No default model on s390x, one has to be provided
+ * explicitly by the user */
+ cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
} else if (ARCH_IS_PPC64(def->os.arch)) {
/* To not break migration we need to set default USB controller
* for ppc64 to pci-ohci if we cannot change ABI of the VM.
@@ -5698,6 +5696,12 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont,
cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
}
}
+
+ /* Make sure the 'none' USB controller doesn't have an address
+ * associated with it, as that would trip up later checks */
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE)
+ cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
+
/* forbid usb model 'qusb1' and 'qusb2' in this kind of hyperviosr */
if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1 ||
cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2) {
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml
index 6e879ded86..300dea1382 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml
@@ -29,11 +29,8 @@
<alias name='virtio-disk4'/>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
</disk>
- <controller type='usb' index='0'>
+ <controller type='usb' index='0' model='none'>
<alias name='usb'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
- <zpci uid='0x0001' fid='0x00000000'/>
- </address>
</controller>
<controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml
index 9b16951e46..882a509eeb 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml
@@ -39,11 +39,8 @@
<alias name='virtio-disk1'/>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
</disk>
- <controller type='usb' index='0'>
+ <controller type='usb' index='0' model='none'>
<alias name='usb'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
- <zpci uid='0x0001' fid='0x00000000'/>
- </address>
</controller>
<controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml
index b5292a7ed2..6167d54bd2 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml
@@ -29,11 +29,8 @@
<alias name='virtio-disk0'/>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/>
</disk>
- <controller type='usb' index='0'>
+ <controller type='usb' index='0' model='none'>
<alias name='usb'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
- <zpci uid='0x0001' fid='0x00000000'/>
- </address>
</controller>
<controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml
index f37868101c..67a5c84a6c 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml
@@ -38,11 +38,8 @@
<alias name='virtio-disk4'/>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
</disk>
- <controller type='usb' index='0'>
+ <controller type='usb' index='0' model='none'>
<alias name='usb'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
- <zpci uid='0x0001' fid='0x00000000'/>
- </address>
</controller>
<controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml
index f37868101c..67a5c84a6c 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml
@@ -38,11 +38,8 @@
<alias name='virtio-disk4'/>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
</disk>
- <controller type='usb' index='0'>
+ <controller type='usb' index='0' model='none'>
<alias name='usb'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
- <zpci uid='0x0001' fid='0x00000000'/>
- </address>
</controller>
<controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml
index 42f89a07a2..07bbfa24a2 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml
@@ -28,11 +28,8 @@
<alias name='virtio-disk4'/>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
</disk>
- <controller type='usb' index='0'>
+ <controller type='usb' index='0' model='none'>
<alias name='usb'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
- <zpci uid='0x0001' fid='0x00000000'/>
- </address>
</controller>
<controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml
index f0570b5cf4..4869103a06 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml
@@ -19,11 +19,8 @@
<on_crash>restart</on_crash>
<devices>
<emulator>/usr/bin/qemu-system-s390x</emulator>
- <controller type='usb' index='0'>
+ <controller type='usb' index='0' model='none'>
<alias name='usb'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
- <zpci uid='0x0001' fid='0x00000000'/>
- </address>
</controller>
<controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
diff --git a/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml b/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml
index 595d0b1a1e..e17098a3df 100644
--- a/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml
+++ b/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml
@@ -17,11 +17,7 @@
<on_crash>destroy</on_crash>
<devices>
<emulator>/usr/bin/qemu-system-s390x</emulator>
- <controller type='usb' index='0'>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
- <zpci uid='0x0001' fid='0x00000000'/>
- </address>
- </controller>
+ <controller type='usb' index='0' model='none'/>
<controller type='pci' index='0' model='pci-root'/>
<audio id='1' type='none'/>
<memballoon model='none'/>
--
2.43.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Wed, Feb 14, 2024 at 18:11:16 +0100, Andrea Bolognani wrote: > When support for s390x was introduced in libvirt, it naturally > followed the conventions established at the time for x86, which > were to have a USB controller added by default. > > Later, in 2013, commit 3a82f628a964 made the default USB > controller model for s390x VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE, > effectively overriding the architecture-independent default. > > However, an exception was carved out at the time: if the USB > controller had an address assigned to it, then it would be left > alone. > > A couple of years later, commit 09ab9dcc85ec changed things > again in two ways: for starters, libvirt would no longer > automatically attempt to add a USB controller to newly-defined > s390x guests; moreover, the command line generator was changed > so that the legacy USB controller (-usb) would never be used > on s390x. > > In other words, unless a model name is explicitly provided for > the USB controller, which is something that only actually works > when using a recent QEMU version (see commit f9ed4d385ab8), > s390x guests will never have USB controllers attached to them. > > Remove the exception carved out a decade ago and always > reflect this fact accurately in the guest XML. > > Signed-off-by: Andrea Bolognani <abologna@redhat.com> > --- > src/qemu/qemu_domain.c | 14 +++++++++----- > .../qemuhotplug-base-ccw-live+ccw-virtio.xml | 5 +---- > ...live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 5 +---- > ...qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 5 +---- > ...-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 5 +---- > ...-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 5 +---- > .../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 5 +---- > .../qemuhotplug-base-ccw-live.xml | 5 +---- > .../s390-usb-address.s390x-latest.xml | 6 +----- > 9 files changed, 17 insertions(+), 38 deletions(-) [...] > @@ -5698,6 +5696,12 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont, > cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; > } > } > + > + /* Make sure the 'none' USB controller doesn't have an address > + * associated with it, as that would trip up later checks */ > + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) > + cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; One thing I'm slightly unsure about is whether the removal of address won't have effect on generation of other addresses and thus in certain very weird situations could trip up the virDomainDefCheckABIStability check. Since the usb controller itself was never seen by the guest ABI that part will be okay, but not reserving the address for it could cause issues. Now for migration this shouldn't be a problem unless somebody is passing very weird migration XMLs. For new VMs it can theoretically cause re-ordering of devices on the PCI bus. _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
On Fri, Feb 16, 2024 at 04:09:12PM +0100, Peter Krempa wrote: > On Wed, Feb 14, 2024 at 18:11:16 +0100, Andrea Bolognani wrote: > > + /* Make sure the 'none' USB controller doesn't have an address > > + * associated with it, as that would trip up later checks */ > > + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) > > + cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; > > One thing I'm slightly unsure about is whether the removal of address > won't have effect on generation of other addresses and thus in certain > very weird situations could trip up the virDomainDefCheckABIStability check. > > Since the usb controller itself was never seen by the guest ABI that > part will be okay, but not reserving the address for it could cause > issues. > > Now for migration this shouldn't be a problem unless somebody is passing > very weird migration XMLs. > > For new VMs it can theoretically cause re-ordering of devices on the PCI > bus. For new VMs, the guest ABI has not been set in stone yet so even if devices and controllers were to be shuffled around it wouldn't matter. For existing VMs, all addressess will have been recorded in the domain XML and libvirt would never attempt to change them. The point about migration is potentially a good one though. The incoming XML will have the (default) model and address, but after parsing they will be gone. Will that trip the ABI stability check? I'm never sure at what point of the process that gets executed, and on which inputs. -- Andrea Bolognani / Red Hat / Virtualization _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
On Fri, Feb 16, 2024 at 07:46:01 -0800, Andrea Bolognani wrote: > On Fri, Feb 16, 2024 at 04:09:12PM +0100, Peter Krempa wrote: > > On Wed, Feb 14, 2024 at 18:11:16 +0100, Andrea Bolognani wrote: > > > + /* Make sure the 'none' USB controller doesn't have an address > > > + * associated with it, as that would trip up later checks */ > > > + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) > > > + cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; > > > > One thing I'm slightly unsure about is whether the removal of address > > won't have effect on generation of other addresses and thus in certain > > very weird situations could trip up the virDomainDefCheckABIStability check. > > > > Since the usb controller itself was never seen by the guest ABI that > > part will be okay, but not reserving the address for it could cause > > issues. > > > > Now for migration this shouldn't be a problem unless somebody is passing > > very weird migration XMLs. > > > > For new VMs it can theoretically cause re-ordering of devices on the PCI > > bus. > > For new VMs, the guest ABI has not been set in stone yet so even if > devices and controllers were to be shuffled around it wouldn't > matter. > > For existing VMs, all addressess will have been recorded in the > domain XML and libvirt would never attempt to change them. Note that historically we've considered that in most cases a partial XML to be freshly defined or used with virDomainCreateXML() should be ABI compatible with how we've treated it historically. It is in certain cases not practical/possible, but generally this should be kept as much as possible. Basically we can't decide to remove an auto-added device. For shuffling addresses around, while we shouldn't do just for the heck of it, it can be a reasonable change. If a user cares about the address they should put it into the XML. > The point about migration is potentially a good one though. The > incoming XML will have the (default) model and address, but after > parsing they will be gone. Will that trip the ABI stability check? No, unless you're providing a custom migration XML there's no ABI stability check happening. The XML formatted by libvirt is considered to be authoritative and well-enough defined. Additionally there's nothing to check the ABI against, as the source of the migration is simply declaring what's happening. > I'm never sure at what point of the process that gets executed, and > on which inputs. _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
On 2/14/24 18:11, Andrea Bolognani wrote:
> When support for s390x was introduced in libvirt, it naturally
> followed the conventions established at the time for x86, which
> were to have a USB controller added by default.
>
> Later, in 2013, commit 3a82f628a964 made the default USB
> controller model for s390x VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE,
> effectively overriding the architecture-independent default.
>
> However, an exception was carved out at the time: if the USB
> controller had an address assigned to it, then it would be left
> alone.
>
> A couple of years later, commit 09ab9dcc85ec changed things
> again in two ways: for starters, libvirt would no longer
> automatically attempt to add a USB controller to newly-defined
> s390x guests; moreover, the command line generator was changed
> so that the legacy USB controller (-usb) would never be used
> on s390x.
>
> In other words, unless a model name is explicitly provided for
> the USB controller, which is something that only actually works
> when using a recent QEMU version (see commit f9ed4d385ab8),
> s390x guests will never have USB controllers attached to them.
>
> Remove the exception carved out a decade ago and always
> reflect this fact accurately in the guest XML.
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> src/qemu/qemu_domain.c | 14 +++++++++-----
> .../qemuhotplug-base-ccw-live+ccw-virtio.xml | 5 +----
> ...live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 5 +----
> ...qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 5 +----
> ...-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 5 +----
> ...-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 5 +----
> .../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 5 +----
> .../qemuhotplug-base-ccw-live.xml | 5 +----
> .../s390-usb-address.s390x-latest.xml | 6 +-----
> 9 files changed, 17 insertions(+), 38 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 31d634b151..03ad590759 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5669,11 +5669,9 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont,
> cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
>
> if (ARCH_IS_S390(def->os.arch)) {
> - if (cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> - /* set the default USB model to none for s390 unless an
> - * address is found */
> - cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
> - }
> + /* No default model on s390x, one has to be provided
> + * explicitly by the user */
> + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
> } else if (ARCH_IS_PPC64(def->os.arch)) {
> /* To not break migration we need to set default USB controller
> * for ppc64 to pci-ohci if we cannot change ABI of the VM.
> @@ -5698,6 +5696,12 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont,
> cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
> }
> }
> +
> + /* Make sure the 'none' USB controller doesn't have an address
> + * associated with it, as that would trip up later checks */
> + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE)
> + cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
> +
> /* forbid usb model 'qusb1' and 'qusb2' in this kind of hyperviosr */
> if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1 ||
> cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2) {
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml
> index 6e879ded86..300dea1382 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml
> @@ -29,11 +29,8 @@
> <alias name='virtio-disk4'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml
> index 9b16951e46..882a509eeb 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml
> @@ -39,11 +39,8 @@
> <alias name='virtio-disk1'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml
> index b5292a7ed2..6167d54bd2 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml
> @@ -29,11 +29,8 @@
> <alias name='virtio-disk0'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml
> index f37868101c..67a5c84a6c 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml
> @@ -38,11 +38,8 @@
> <alias name='virtio-disk4'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml
> index f37868101c..67a5c84a6c 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml
> @@ -38,11 +38,8 @@
> <alias name='virtio-disk4'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml
> index 42f89a07a2..07bbfa24a2 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml
> @@ -28,11 +28,8 @@
> <alias name='virtio-disk4'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml
> index f0570b5cf4..4869103a06 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml
> @@ -19,11 +19,8 @@
> <on_crash>restart</on_crash>
> <devices>
> <emulator>/usr/bin/qemu-system-s390x</emulator>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml b/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml
> index 595d0b1a1e..e17098a3df 100644
> --- a/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml
> +++ b/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml
> @@ -17,11 +17,7 @@
> <on_crash>destroy</on_crash>
> <devices>
> <emulator>/usr/bin/qemu-system-s390x</emulator>
> - <controller type='usb' index='0'>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> - </controller>
> + <controller type='usb' index='0' model='none'/>
> <controller type='pci' index='0' model='pci-root'/>
> <audio id='1' type='none'/>
> <memballoon model='none'/>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On 14/02/2024 18.11, Andrea Bolognani wrote:
> When support for s390x was introduced in libvirt, it naturally
> followed the conventions established at the time for x86, which
> were to have a USB controller added by default.
>
> Later, in 2013, commit 3a82f628a964 made the default USB
> controller model for s390x VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE,
> effectively overriding the architecture-independent default.
>
> However, an exception was carved out at the time: if the USB
> controller had an address assigned to it, then it would be left
> alone.
>
> A couple of years later, commit 09ab9dcc85ec changed things
> again in two ways: for starters, libvirt would no longer
> automatically attempt to add a USB controller to newly-defined
> s390x guests; moreover, the command line generator was changed
> so that the legacy USB controller (-usb) would never be used
> on s390x.
>
> In other words, unless a model name is explicitly provided for
> the USB controller, which is something that only actually works
> when using a recent QEMU version (see commit f9ed4d385ab8),
> s390x guests will never have USB controllers attached to them.
>
> Remove the exception carved out a decade ago and always
> reflect this fact accurately in the guest XML.
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> src/qemu/qemu_domain.c | 14 +++++++++-----
> .../qemuhotplug-base-ccw-live+ccw-virtio.xml | 5 +----
> ...live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 5 +----
> ...qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 5 +----
> ...-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 5 +----
> ...-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 5 +----
> .../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 5 +----
> .../qemuhotplug-base-ccw-live.xml | 5 +----
> .../s390-usb-address.s390x-latest.xml | 6 +-----
> 9 files changed, 17 insertions(+), 38 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 31d634b151..03ad590759 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5669,11 +5669,9 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont,
> cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
>
> if (ARCH_IS_S390(def->os.arch)) {
> - if (cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> - /* set the default USB model to none for s390 unless an
> - * address is found */
> - cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
> - }
> + /* No default model on s390x, one has to be provided
> + * explicitly by the user */
> + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
> } else if (ARCH_IS_PPC64(def->os.arch)) {
> /* To not break migration we need to set default USB controller
> * for ppc64 to pci-ohci if we cannot change ABI of the VM.
> @@ -5698,6 +5696,12 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont,
> cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
> }
> }
> +
> + /* Make sure the 'none' USB controller doesn't have an address
> + * associated with it, as that would trip up later checks */
> + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE)
> + cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
> +
> /* forbid usb model 'qusb1' and 'qusb2' in this kind of hyperviosr */
> if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1 ||
> cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2) {
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml
> index 6e879ded86..300dea1382 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml
> @@ -29,11 +29,8 @@
> <alias name='virtio-disk4'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml
> index 9b16951e46..882a509eeb 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml
> @@ -39,11 +39,8 @@
> <alias name='virtio-disk1'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml
> index b5292a7ed2..6167d54bd2 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml
> @@ -29,11 +29,8 @@
> <alias name='virtio-disk0'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml
> index f37868101c..67a5c84a6c 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml
> @@ -38,11 +38,8 @@
> <alias name='virtio-disk4'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml
> index f37868101c..67a5c84a6c 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml
> @@ -38,11 +38,8 @@
> <alias name='virtio-disk4'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml
> index 42f89a07a2..07bbfa24a2 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-ccw-virtio.xml
> @@ -28,11 +28,8 @@
> <alias name='virtio-disk4'/>
> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
> </disk>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml
> index f0570b5cf4..4869103a06 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live.xml
> @@ -19,11 +19,8 @@
> <on_crash>restart</on_crash>
> <devices>
> <emulator>/usr/bin/qemu-system-s390x</emulator>
> - <controller type='usb' index='0'>
> + <controller type='usb' index='0' model='none'>
> <alias name='usb'/>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> </controller>
> <controller type='scsi' index='0' model='virtio-scsi'>
> <alias name='scsi0'/>
> diff --git a/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml b/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml
> index 595d0b1a1e..e17098a3df 100644
> --- a/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml
> +++ b/tests/qemuxmlconfdata/s390-usb-address.s390x-latest.xml
> @@ -17,11 +17,7 @@
> <on_crash>destroy</on_crash>
> <devices>
> <emulator>/usr/bin/qemu-system-s390x</emulator>
> - <controller type='usb' index='0'>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'>
> - <zpci uid='0x0001' fid='0x00000000'/>
> - </address>
> - </controller>
> + <controller type='usb' index='0' model='none'/>
> <controller type='pci' index='0' model='pci-root'/>
> <audio id='1' type='none'/>
> <memballoon model='none'/>
Reviewed-by: Thomas Huth <thuth@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Wed, Feb 14, 2024 at 18:11:16 +0100, Andrea Bolognani wrote: > When support for s390x was introduced in libvirt, it naturally > followed the conventions established at the time for x86, which > were to have a USB controller added by default. > > Later, in 2013, commit 3a82f628a964 made the default USB > controller model for s390x VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE, > effectively overriding the architecture-independent default. > > However, an exception was carved out at the time: if the USB > controller had an address assigned to it, then it would be left > alone. > > A couple of years later, commit 09ab9dcc85ec changed things > again in two ways: for starters, libvirt would no longer > automatically attempt to add a USB controller to newly-defined > s390x guests; moreover, the command line generator was changed > so that the legacy USB controller (-usb) would never be used > on s390x. > > In other words, unless a model name is explicitly provided for > the USB controller, which is something that only actually works > when using a recent QEMU version (see commit f9ed4d385ab8), > s390x guests will never have USB controllers attached to them. > > Remove the exception carved out a decade ago and always > reflect this fact accurately in the guest XML. > > Signed-off-by: Andrea Bolognani <abologna@redhat.com> > --- > src/qemu/qemu_domain.c | 14 +++++++++----- > .../qemuhotplug-base-ccw-live+ccw-virtio.xml | 5 +---- > ...live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 5 +---- > ...qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml | 5 +---- > ...-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 5 +---- > ...-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml | 5 +---- > .../qemuhotplug-base-ccw-live-with-ccw-virtio.xml | 5 +---- > .../qemuhotplug-base-ccw-live.xml | 5 +---- > .../s390-usb-address.s390x-latest.xml | 6 +----- > 9 files changed, 17 insertions(+), 38 deletions(-) Okay, so the change is that addresses no longer get reserved for these devices as they reallistically never existed in s390 VMs. Reviewed-by: Peter Krempa <pkrempa@redhat.com> _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
© 2016 - 2026 Red Hat, Inc.