On Tue, Aug 19, 2025 at 18:22:24 +0200, Andrea Bolognani via Devel 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 <[email protected]>
> ---
>  src/qemu/qemu_postparse.c                         | 15 ++++++++++-----
>  .../qemuhotplug-base-ccw-live+ccw-virtio.xml      |  5 +----
>  ...ive-with-2-ccw-virtio+ccw-virtio-1-reverse.xml |  5 +----
>  ...emuhotplug-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, 18 insertions(+), 38 deletions(-)
> 
> diff --git a/src/qemu/qemu_postparse.c b/src/qemu/qemu_postparse.c
> index e2004993f3..cbef101104 100644
> --- a/src/qemu/qemu_postparse.c
> +++ b/src/qemu/qemu_postparse.c
> @@ -380,11 +380,9 @@ qemuDomainControllerDefPostParse(virDomainControllerDef 
> *cont,
>                  cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
>  
>              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.
> @@ -412,6 +410,13 @@ qemuDomainControllerDefPostParse(virDomainControllerDef 
> *cont,
>                      cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
>              }
>          }
> +
> +        /* Make sure the 'none' USB controller doesn't have an address
> +         * associated with it, as that would trip up later checks and
> +         * it doesn't make sense anyway */
> +        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'/>

Having alias for a 'none' controller is nonsense.

> -      <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'/>

ditto

> -      <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.50.1
> 

With the above addressed:

Reviewed-by: Peter Krempa <[email protected]>

Reply via email to