From nobody Mon May 6 04:26:46 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1513611560009468.1481287130921; Mon, 18 Dec 2017 07:39:20 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5FB2DC070E0B; Mon, 18 Dec 2017 15:39:18 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E17C37010A; Mon, 18 Dec 2017 15:39:17 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id AEB5A180474A; Mon, 18 Dec 2017 15:39:16 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id vBIFa23u018487 for ; Mon, 18 Dec 2017 10:36:02 -0500 Received: by smtp.corp.redhat.com (Postfix) id CD71C60C57; Mon, 18 Dec 2017 15:36:02 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-93.phx2.redhat.com [10.3.117.93]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4EC0A61289; Mon, 18 Dec 2017 15:36:02 +0000 (UTC) From: Laine Stump To: libvir-list@redhat.com Date: Mon, 18 Dec 2017 10:35:52 -0500 Message-Id: <20171218153553.15903-2-laine@laine.org> In-Reply-To: <20171218153553.15903-1-laine@laine.org> References: <20171218153553.15903-1-laine@laine.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 1/2] qemu: assign correct type of PCI address for vhost-scsi when using pcie-root X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 18 Dec 2017 15:39:18 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Commit 10c73bf1 fixed a bug that I had introduced back in commit 70249927 - if a vhost-scsi device had no manually assigned PCI address, one wouldn't be assigned automatically. There was a slight problem with the logic of the fix though - in the case of domains with pcie-root (e.g. those with a q35 machinetype), qemuDomainDeviceCalculatePCIConnectFlags() will attempt to determine if the host-side PCI device is Express or legacy by examining sysfs based on the host-side PCI address stored in hostdev->source.subsys.u.pci.addr, but that part of the union is only valid for PCI hostdevs, *not* for SCSI hostdevs. So we end up trying to read sysfs for some probably-non-existent device, which fails, and the function virPCIDeviceIsPCIExpress() returns failure (-1). By coincidence, the return value is being examined as a boolean, and since -1 is true, we still end up assigning the vhost-scsi device to an Express slot, but that is just be chance (and could fail in the case that the gibberish in the "hostside PCI address" was the address of a real device that happened to be legacy PCI). Since (according to Paolo Bonzini at least) vhost-scsi devices appear just like virtio-scsi devices in the guest, they should follow the same rules as virtio devices when deciding whether they should be placed in an Express or a legacy slot. That's accomplished in this patch by returning early with virtioFlags, rather than erroneously using hostdev->source.subsys.u.pci.addr. It also adds a test case for PCIe to assure it doesn't get broken in the future. Reviewed-by: John Ferlan --- src/qemu/qemu_domain_address.c | 7 ++++ .../hostdev-scsi-vhost-scsi-pcie.args | 25 ++++++++++++++ .../hostdev-scsi-vhost-scsi-pcie.xml | 23 +++++++++++++ tests/qemuxml2argvtest.c | 7 ++++ .../hostdev-scsi-vhost-scsi-pcie.xml | 40 ++++++++++++++++++= ++++ tests/qemuxml2xmltest.c | 7 ++++ 6 files changed, 109 insertions(+) create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.args create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pcie.x= ml diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 6e7561d39..600de85f8 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -656,6 +656,13 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDevi= ceDefPtr dev, if (hostdev->source.subsys.type =3D=3D VIR_DOMAIN_HOSTDEV_SUBSYS_T= YPE_MDEV) return pcieFlags; =20 + /* according to pbonzini, from the guest PoV vhost-scsi devices + * are the same as virtio-scsi, so they should use virtioFlags + * (same as virtio-scsi) to determine Express vs. legacy placement + */ + if (hostdev->source.subsys.type =3D=3D VIR_DOMAIN_HOSTDEV_SUBSYS_T= YPE_SCSI_HOST) + return virtioFlags; + if (!(pciDev =3D virPCIDeviceNew(hostAddr->domain, hostAddr->bus, hostAddr->slot, diff --git a/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.args b/tes= ts/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.args new file mode 100644 index 000000000..296716f58 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.args @@ -0,0 +1,25 @@ +LC_ALL=3DC \ +PATH=3D/bin \ +HOME=3D/home/test \ +USER=3Dtest \ +LOGNAME=3Dtest \ +QEMU_AUDIO_DRV=3Dnone \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest2 \ +-S \ +-M pc-q35-2.7 \ +-m 214 \ +-smp 1,sockets=3D1,cores=3D1,threads=3D1 \ +-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=3Dcharmonitor,path=3D/tmp/lib/domain--1-QEMUGuest2/moni= tor.sock,\ +server,nowait \ +-mon chardev=3Dcharmonitor,id=3Dmonitor,mode=3Dreadline \ +-no-acpi \ +-boot c \ +-device pcie-root-port,port=3D0x10,chassis=3D1,id=3Dpci.1,bus=3Dpcie.0,\ +multifunction=3Don,addr=3D0x2 \ +-device pcie-root-port,port=3D0x11,chassis=3D2,id=3Dpci.2,bus=3Dpcie.0,add= r=3D0x2.0x1 \ +-device vhost-scsi-pci,wwpn=3Dnaa.5123456789abcde0,vhostfd=3D3,id=3Dhostde= v0,\ +bus=3Dpci.1,addr=3D0x0 diff --git a/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.xml b/test= s/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.xml new file mode 100644 index 000000000..c4e8c33a9 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.xml @@ -0,0 +1,23 @@ + + QEMUGuest2 + c7a5fdbd-edaf-9466-926a-d65c16db1809 + 219100 + 219100 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ca24e0bbb..d5b2881e7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2465,6 +2465,13 @@ mymain(void) DO_TEST("hostdev-scsi-vhost-scsi-pci", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI, QEMU_CAPS_DEVICE_SCSI_GENERIC); + DO_TEST("hostdev-scsi-vhost-scsi-pcie", + QEMU_CAPS_KVM, + QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI, + QEMU_CAPS_DEVICE_SCSI_GENERIC, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY); =20 DO_TEST("mlock-on", QEMU_CAPS_REALTIME_MLOCK); DO_TEST_FAILURE("mlock-on", NONE); diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pcie.xml b/te= sts/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pcie.xml new file mode 100644 index 000000000..ef48d8123 --- /dev/null +++ b/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pcie.xml @@ -0,0 +1,40 @@ + + QEMUGuest2 + c7a5fdbd-edaf-9466-926a-d65c16db1809 + 219100 + 219100 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + + +
+ + + + + +
+ + + + +
+ + + + + +
+ + + + diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2be8eb2c1..78519f509 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1109,6 +1109,13 @@ mymain(void) DO_TEST("hostdev-scsi-vhost-scsi-pci", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI, QEMU_CAPS_DEVICE_SCSI_GENERIC); + DO_TEST("hostdev-scsi-vhost-scsi-pcie", + QEMU_CAPS_KVM, + QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI, + QEMU_CAPS_DEVICE_SCSI_GENERIC, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY); DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_DEVICE_SCSI_GENERIC); --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Mon May 6 04:26:46 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1513611570362975.1575348994209; Mon, 18 Dec 2017 07:39:30 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CE04FC057F93; Mon, 18 Dec 2017 15:39:28 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A050280946; Mon, 18 Dec 2017 15:39:28 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 440EA1801217; Mon, 18 Dec 2017 15:39:28 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id vBIFa31G018500 for ; Mon, 18 Dec 2017 10:36:04 -0500 Received: by smtp.corp.redhat.com (Postfix) id D2BB261289; Mon, 18 Dec 2017 15:36:03 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-93.phx2.redhat.com [10.3.117.93]) by smtp.corp.redhat.com (Postfix) with ESMTP id 111D36C20C; Mon, 18 Dec 2017 15:36:02 +0000 (UTC) From: Laine Stump To: libvir-list@redhat.com Date: Mon, 18 Dec 2017 10:35:53 -0500 Message-Id: <20171218153553.15903-3-laine@laine.org> In-Reply-To: <20171218153553.15903-1-laine@laine.org> References: <20171218153553.15903-1-laine@laine.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com Cc: phrdina@redhat.com Subject: [libvirt] [PATCH 2/2] qemu: move qemuDomainDefValidateVideo into qemuDomainDeviceDefValidateVideo X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 18 Dec 2017 15:39:29 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" qemuDomainDefValidateVideo() is just a loop performing various checks on each video device. Rather than maintaining this outlyer function called from qemuDomainDefValidateVideo(), just fold the validations into qemuDomainDeviceDefValidateVideo(), which is called once for each video device (my guess is that ...DeviceDefValidateVideo() didn't exist yet when ...DomainDefValidateVideo() was added, but I haven't verified this). Reviewed-by: John Ferlan --- I randomly noticed this when looking up something else in the validation co= de... src/qemu/qemu_domain.c | 151 +++++++++++++++++++++------------------------= ---- 1 file changed, 66 insertions(+), 85 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 74b82450b..2ca45fde2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3216,79 +3216,6 @@ qemuDomainDefPostParse(virDomainDefPtr def, } =20 =20 -static int -qemuDomainDefValidateVideo(const virDomainDef *def) -{ - size_t i; - virDomainVideoDefPtr video; - - for (i =3D 0; i < def->nvideos; i++) { - video =3D def->videos[i]; - - switch (video->type) { - case VIR_DOMAIN_VIDEO_TYPE_XEN: - case VIR_DOMAIN_VIDEO_TYPE_VBOX: - case VIR_DOMAIN_VIDEO_TYPE_PARALLELS: - case VIR_DOMAIN_VIDEO_TYPE_DEFAULT: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("video type '%s' is not supported with QEMU"), - virDomainVideoTypeToString(video->type)); - return -1; - case VIR_DOMAIN_VIDEO_TYPE_VGA: - case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: - case VIR_DOMAIN_VIDEO_TYPE_VMVGA: - case VIR_DOMAIN_VIDEO_TYPE_QXL: - case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: - case VIR_DOMAIN_VIDEO_TYPE_LAST: - break; - } - - if (!video->primary && - video->type !=3D VIR_DOMAIN_VIDEO_TYPE_QXL && - video->type !=3D VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("video type '%s' is only valid as primary " - "video device"), - virDomainVideoTypeToString(video->type)); - return -1; - } - - if (video->accel && video->accel->accel2d =3D=3D VIR_TRISTATE_SWIT= CH_ON) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("qemu does not support the accel2d setting")); - return -1; - } - - if (video->type =3D=3D VIR_DOMAIN_VIDEO_TYPE_QXL) { - if (video->vram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'vram' must be less than '%u'"= ), - UINT_MAX / 1024); - return -1; - } - if (video->ram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'ram' must be less than '%u'"), - UINT_MAX / 1024); - return -1; - } - } - - if (video->type =3D=3D VIR_DOMAIN_VIDEO_TYPE_VGA || - video->type =3D=3D VIR_DOMAIN_VIDEO_TYPE_VMVGA) { - if (video->vram && video->vram < 1024) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("value for 'vram' must be at least " - "1 MiB (1024 KiB)")); - return -1; - } - } - } - - return 0; -} - - #define QEMU_MAX_VCPUS_WITHOUT_EIM 255 =20 =20 @@ -3388,9 +3315,6 @@ qemuDomainDefValidate(const virDomainDef *def, } } =20 - if (qemuDomainDefValidateVideo(def) < 0) - goto cleanup; - ret =3D 0; =20 cleanup: @@ -3851,18 +3775,75 @@ qemuDomainDeviceDefValidateHostdev(const virDomainH= ostdevDef *hostdev, static int qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) { - if (video->type =3D=3D VIR_DOMAIN_VIDEO_TYPE_QXL && - video->vgamem) { - if (video->vgamem < 1024) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("value for 'vgamem' must be at least 1 MiB " - "(1024 KiB)")); + switch (video->type) { + case VIR_DOMAIN_VIDEO_TYPE_XEN: + case VIR_DOMAIN_VIDEO_TYPE_VBOX: + case VIR_DOMAIN_VIDEO_TYPE_PARALLELS: + case VIR_DOMAIN_VIDEO_TYPE_DEFAULT: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type '%s' is not supported with QEMU"), + virDomainVideoTypeToString(video->type)); + return -1; + case VIR_DOMAIN_VIDEO_TYPE_VGA: + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + case VIR_DOMAIN_VIDEO_TYPE_VMVGA: + case VIR_DOMAIN_VIDEO_TYPE_QXL: + case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: + case VIR_DOMAIN_VIDEO_TYPE_LAST: + break; + } + + if (!video->primary && + video->type !=3D VIR_DOMAIN_VIDEO_TYPE_QXL && + video->type !=3D VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type '%s' is only valid as primary " + "video device"), + virDomainVideoTypeToString(video->type)); + return -1; + } + + if (video->accel && video->accel->accel2d =3D=3D VIR_TRISTATE_SWITCH_O= N) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu does not support the accel2d setting")); + return -1; + } + + if (video->type =3D=3D VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + return -1; + } + if (video->ram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'ram' must be less than '%u'"), + UINT_MAX / 1024); return -1; } + if (video->vgamem) { + if (video->vgamem < 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("value for 'vgamem' must be at least 1 Mi= B " + "(1024 KiB)")); + return -1; + } =20 - if (video->vgamem !=3D VIR_ROUND_UP_POWER_OF_TWO(video->vgamem)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("value for 'vgamem' must be power of two")); + if (video->vgamem !=3D VIR_ROUND_UP_POWER_OF_TWO(video->vgamem= )) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("value for 'vgamem' must be power of two"= )); + return -1; + } + } + } + + if (video->type =3D=3D VIR_DOMAIN_VIDEO_TYPE_VGA || + video->type =3D=3D VIR_DOMAIN_VIDEO_TYPE_VMVGA) { + if (video->vram && video->vram < 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("value for 'vram' must be at least " + "1 MiB (1024 KiB)")); return -1; } } --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list