From nobody Wed Jan 15 14:44:34 2025 Delivered-To: importer@patchew.org Received-SPF: none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; spf=none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1707932481473659.5533577345384; Wed, 14 Feb 2024 09:41:21 -0800 (PST) Received: by lists.libvirt.org (Postfix, from userid 996) id 682F61C2C; Wed, 14 Feb 2024 12:41:20 -0500 (EST) Received: from lists.libvirt.org.85.43.8.in-addr.arpa (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 201E119FE; Wed, 14 Feb 2024 12:14:05 -0500 (EST) Received: by lists.libvirt.org (Postfix, from userid 996) id 87C8219FD; Wed, 14 Feb 2024 12:11:58 -0500 (EST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id 165B11A1B for ; Wed, 14 Feb 2024 12:11:38 -0500 (EST) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-67-dWNhlvBbNDqj-X-MpxHg9w-1; Wed, 14 Feb 2024 12:11:35 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 838D1106CFF3 for ; Wed, 14 Feb 2024 17:11:34 +0000 (UTC) Received: from harajuku.usersys.redhat.com (unknown [10.45.226.93]) by smtp.corp.redhat.com (Postfix) with ESMTPS id ED22D10BC282 for ; Wed, 14 Feb 2024 17:11:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.4 X-MC-Unique: dWNhlvBbNDqj-X-MpxHg9w-1 From: Andrea Bolognani To: devel@lists.libvirt.org Subject: [PATCH v2 11/17] qemu: Validate USB controllers earlier Date: Wed, 14 Feb 2024 18:11:18 +0100 Message-ID: <20240214171124.508000-12-abologna@redhat.com> In-Reply-To: <20240214171124.508000-1-abologna@redhat.com> References: <20240214171124.508000-1-abologna@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Message-ID-Hash: CBOW4JSJSXK7E7YGMTWIAX42MDHKD3S3 X-Message-ID-Hash: CBOW4JSJSXK7E7YGMTWIAX42MDHKD3S3 X-MailFrom: abologna@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="utf-8"; x-default="true" Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1707932483645100001 Right now we call qemuValidateDomainDeviceDefControllerUSB() quite late, just as we're generating the QEMU command line. The intention here is to prevent configurations from being rejected, even though a default USB controller model could not be found, because using -usb could work as a last resort. As it turns out, this premise is flawed, as can easily be demonstrated by using a build of QEMU which has the default USB controller compiled out: $ qemu-system-x86_64 -M pc -device piix3-usb-uhci 'piix3-usb-uhci' is not a valid device model name $ qemu-system-x86_64 -M pc -usb missing object type 'piix3-usb-uhci' In other words, if the device that needs to be built into QEMU in order for -usb to work was available, we would have detected it and used it via -device instead; if we didn't, using -usb won't change anything. Unsurprisingly, a number of test cases start failing, or fail in a different way, because of this change. That's okay: even in the unlikely event that there actually are any existing guests with such problematic configurations out there, they will not disappear but merely fail to start, and the user will have the opportunity to fix them. Signed-off-by: Andrea Bolognani Reviewed-by: Peter Krempa --- src/qemu/qemu_command.c | 68 +------------------ src/qemu/qemu_validate.c | 68 ++++++++++++++++++- ...ault-unavailable-i440fx.x86_64-latest.args | 33 --------- ...fault-unavailable-i440fx.x86_64-latest.err | 1 + ...fault-unavailable-i440fx.x86_64-latest.xml | 31 --------- ...-default-unavailable-q35.x86_64-latest.xml | 33 --------- ...ler-nec-xhci-unavailable.x86_64-latest.xml | 33 --------- .../usb-legacy-device.x86_64-latest.args | 33 --------- .../usb-legacy-device.x86_64-latest.err | 1 + .../usb-legacy-device.x86_64-latest.xml | 30 -------- .../usb-legacy-multiple.x86_64-latest.err | 2 +- .../usb-legacy-multiple.x86_64-latest.xml | 32 --------- tests/qemuxmlconftest.c | 12 ++-- 13 files changed, 79 insertions(+), 298 deletions(-) delete mode 100644 tests/qemuxmlconfdata/usb-controller-default-unavailabl= e-i440fx.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/usb-controller-default-unavailabl= e-i440fx.x86_64-latest.err delete mode 100644 tests/qemuxmlconfdata/usb-controller-default-unavailabl= e-i440fx.x86_64-latest.xml delete mode 100644 tests/qemuxmlconfdata/usb-controller-default-unavailabl= e-q35.x86_64-latest.xml delete mode 100644 tests/qemuxmlconfdata/usb-controller-nec-xhci-unavailab= le.x86_64-latest.xml delete mode 100644 tests/qemuxmlconfdata/usb-legacy-device.x86_64-latest.a= rgs create mode 100644 tests/qemuxmlconfdata/usb-legacy-device.x86_64-latest.e= rr delete mode 100644 tests/qemuxmlconfdata/usb-legacy-device.x86_64-latest.x= ml delete mode 100644 tests/qemuxmlconfdata/usb-legacy-multiple.x86_64-latest= .xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7824c31bde..a544a3ccdc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2505,66 +2505,6 @@ qemuBuildFilesystemCommandLine(virCommand *cmd, } =20 =20 -static int -qemuControllerModelUSBToCaps(int model) -{ - switch (model) { - case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI: - return QEMU_CAPS_PIIX3_USB_UHCI; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI: - return QEMU_CAPS_PIIX4_USB_UHCI; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI: - return QEMU_CAPS_USB_EHCI; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: - return QEMU_CAPS_ICH9_USB_EHCI1; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI: - return QEMU_CAPS_VT82C686B_USB_UHCI; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI: - return QEMU_CAPS_PCI_OHCI; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI: - return QEMU_CAPS_NEC_USB_XHCI; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI: - return QEMU_CAPS_DEVICE_QEMU_XHCI; - default: - return -1; - } -} - - -static int -qemuValidateDomainDeviceDefControllerUSB(const virDomainControllerDef *def, - virQEMUCaps *qemuCaps) -{ - if (def->model =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("no model provided for USB controller")); - return -1; - } - - if (!virQEMUCapsGet(qemuCaps, qemuControllerModelUSBToCaps(def->model)= )) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("USB controller model '%1$s' not supported in thi= s QEMU binary"), - virDomainControllerModelUSBTypeToString(def->model)= ); - return -1; - } - - if (def->opts.usbopts.ports !=3D -1) { - if (def->model !=3D VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI && - def->model !=3D VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("usb controller type '%1$s' doesn't support '= ports' with this QEMU binary"), - virDomainControllerModelUSBTypeToString(def->mo= del)); - return -1; - } - } - - return 0; -} - - static const char * qemuBuildUSBControllerFindMasterAlias(const virDomainDef *domainDef, const virDomainControllerDef *def) @@ -2592,14 +2532,10 @@ qemuBuildUSBControllerFindMasterAlias(const virDoma= inDef *domainDef, =20 static virJSONValue * qemuBuildUSBControllerDevProps(const virDomainDef *domainDef, - virDomainControllerDef *def, - virQEMUCaps *qemuCaps) + virDomainControllerDef *def) { g_autoptr(virJSONValue) props =3D NULL; =20 - if (qemuValidateDomainDeviceDefControllerUSB(def, qemuCaps) < 0) - return NULL; - if (virJSONValueObjectAdd(&props, "s:driver", qemuControllerModelUSBTypeToStri= ng(def->model), "k:p2", def->opts.usbopts.ports, @@ -2886,7 +2822,7 @@ qemuBuildControllerDevProps(const virDomainDef *domai= nDef, break; =20 case VIR_DOMAIN_CONTROLLER_TYPE_USB: - if (!(props =3D qemuBuildUSBControllerDevProps(domainDef, def, qem= uCaps))) + if (!(props =3D qemuBuildUSBControllerDevProps(domainDef, def))) return -1; =20 break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 73afd094a9..ad1621a120 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3523,6 +3523,69 @@ qemuValidateDomainDeviceDefControllerSCSI(const virD= omainControllerDef *controll } =20 =20 +static int +qemuControllerModelUSBToCaps(int model) +{ + switch (model) { + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI: + return QEMU_CAPS_PIIX3_USB_UHCI; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI: + return QEMU_CAPS_PIIX4_USB_UHCI; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI: + return QEMU_CAPS_USB_EHCI; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: + return QEMU_CAPS_ICH9_USB_EHCI1; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI: + return QEMU_CAPS_VT82C686B_USB_UHCI; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI: + return QEMU_CAPS_PCI_OHCI; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI: + return QEMU_CAPS_NEC_USB_XHCI; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI: + return QEMU_CAPS_DEVICE_QEMU_XHCI; + default: + return -1; + } +} + + +static int +qemuValidateDomainDeviceDefControllerUSB(const virDomainControllerDef *def, + virQEMUCaps *qemuCaps) +{ + if (def->model =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) + return 0; + + if (def->model =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("no model provided for USB controller")); + return -1; + } + + if (!virQEMUCapsGet(qemuCaps, qemuControllerModelUSBToCaps(def->model)= )) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("USB controller model '%1$s' not supported in thi= s QEMU binary"), + virDomainControllerModelUSBTypeToString(def->model)= ); + return -1; + } + + if (def->opts.usbopts.ports !=3D -1) { + if (def->model !=3D VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI && + def->model !=3D VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("usb controller type '%1$s' doesn't support '= ports' with this QEMU binary"), + virDomainControllerModelUSBTypeToString(def->mo= del)); + return -1; + } + } + + return 0; +} + + /** * virValidateControllerPCIModelNameToQEMUCaps: * @modelName: model name @@ -4150,10 +4213,13 @@ qemuValidateDomainDeviceDefController(const virDoma= inControllerDef *controller, qemuCaps); break; =20 + case VIR_DOMAIN_CONTROLLER_TYPE_USB: + ret =3D qemuValidateDomainDeviceDefControllerUSB(controller, qemuC= aps); + break; + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: - case VIR_DOMAIN_CONTROLLER_TYPE_USB: case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS: case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: diff --git a/tests/qemuxmlconfdata/usb-controller-default-unavailable-i440f= x.x86_64-latest.args b/tests/qemuxmlconfdata/usb-controller-default-unavail= able-i440fx.x86_64-latest.args deleted file mode 100644 index c8de26e4ee..0000000000 --- a/tests/qemuxmlconfdata/usb-controller-default-unavailable-i440fx.x86_6= 4-latest.args +++ /dev/null @@ -1,33 +0,0 @@ -LC_ALL=3DC \ -PATH=3D/bin \ -HOME=3D/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ -USER=3Dtest \ -LOGNAME=3Dtest \ -XDG_DATA_HOME=3D/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ -XDG_CACHE_HOME=3D/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ -XDG_CONFIG_HOME=3D/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -/usr/bin/qemu-system-x86_64 \ --name guest=3DQEMUGuest1,debug-threads=3Don \ --S \ --object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/va= r/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ --machine pc,usb=3Doff,dump-guest-core=3Doff,memory-backend=3Dpc.ram,acpi= =3Doff \ --accel tcg \ --cpu qemu64 \ --m size=3D219136k \ --object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}'= \ --overcommit mem-lock=3Doff \ --smp 1,sockets=3D1,cores=3D1,threads=3D1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --display none \ --no-user-config \ --nodefaults \ --chardev socket,id=3Dcharmonitor,fd=3D1729,server=3Don,wait=3Doff \ --mon chardev=3Dcharmonitor,id=3Dmonitor,mode=3Dcontrol \ --rtc base=3Dutc \ --no-shutdown \ --boot strict=3Don \ --usb \ --audiodev '{"id":"audio1","driver":"none"}' \ --device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","add= r":"0x2"}' \ --sandbox on,obsolete=3Ddeny,elevateprivileges=3Ddeny,spawn=3Ddeny,resource= control=3Ddeny \ --msg timestamp=3Don diff --git a/tests/qemuxmlconfdata/usb-controller-default-unavailable-i440f= x.x86_64-latest.err b/tests/qemuxmlconfdata/usb-controller-default-unavaila= ble-i440fx.x86_64-latest.err new file mode 100644 index 0000000000..7a71aa107d --- /dev/null +++ b/tests/qemuxmlconfdata/usb-controller-default-unavailable-i440fx.x86_6= 4-latest.err @@ -0,0 +1 @@ +unsupported configuration: no model provided for USB controller diff --git a/tests/qemuxmlconfdata/usb-controller-default-unavailable-i440f= x.x86_64-latest.xml b/tests/qemuxmlconfdata/usb-controller-default-unavaila= ble-i440fx.x86_64-latest.xml deleted file mode 100644 index 183cfe3b9a..0000000000 --- a/tests/qemuxmlconfdata/usb-controller-default-unavailable-i440fx.x86_6= 4-latest.xml +++ /dev/null @@ -1,31 +0,0 @@ - - QEMUGuest1 - c7a5fdbd-edaf-9455-926a-d65c16db1809 - 219136 - 219136 - 1 - - hvm - - - - qemu64 - - - destroy - restart - destroy - - /usr/bin/qemu-system-x86_64 - -
- - - - -