From nobody Thu May 15 10:21:15 2025 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