9:47 a.m.
danpb had asked for some documentation on the changes to the APIs of
internal functions in V1. I've added some standard function header
docs in patches 5 & 6. Everything else is completely unchanged (and
already reviewed by mprivozn).
Laine Stump (7):
qemu: use g_autoptr for a virPCIDevice
util: simplify calling of virPCIDeviceDetectFunctionLevelReset()
util: simplify call to virPCIDeviceDetectPowerManagementReset()
util: make read error of PCI config file more detailed
util: change call sequence for virPCIDeviceFindCapabilityOffset()
util: make virPCIDeviceIsPCIExpress() more intelligent
qemu: remove now-redundant check for file length when determining PCIe
vs. PCI
src/qemu/qemu_domain_address.c | 43 +--------
src/util/virpci.c | 166 +++++++++++++++++++++++++++------
2 files changed, 141 insertions(+), 68 deletions(-)
--
2.28.0
9:47 a.m.
New subject: [libvirt PATCH v2 1/7] qemu: use g_autoptr for a virPCIDevice
The one instance of a virPCIDevice in
qemuDomainDeviceCalculatePCIConnectFlags() needs to be converted to
use g_autoptr as a prerequisite for a bugfix. It's in this patch by
itself (rather than in a patch converting all virPCIDevice usages to
g_autoptr) to simplify any backport of said bugfix.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/qemu/qemu_domain_address.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index d872f75b38..b07672e2f4 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -803,7 +803,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
case VIR_DOMAIN_DEVICE_HOSTDEV: {
virDomainHostdevDefPtr hostdev = dev->data.hostdev;
bool isExpress = false;
- virPCIDevicePtr pciDev;
+ g_autoptr(virPCIDevice) pciDev = NULL;
virPCIDeviceAddressPtr hostAddr = &hostdev->source.subsys.u.pci.addr;
if (!virHostdevIsMdevDevice(hostdev) &&
@@ -891,8 +891,6 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
off_t configLen
= virFileLength(virPCIDeviceGetConfigPath(pciDev), -1);
- virPCIDeviceFree(pciDev);
-
if (configLen == 256)
return pciFlags;
@@ -904,7 +902,6 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
* a definitive answer.
*/
isExpress = virPCIDeviceIsPCIExpress(pciDev);
- virPCIDeviceFree(pciDev);
if (isExpress)
return pcieFlags;
--
2.28.0
9:47 a.m.
New subject: [libvirt PATCH v2 2/7] util: simplify calling of virPCIDeviceDetectFunctionLevelReset()
This function returned an int, and that int was being checked for < 0
in its solitary caller, but within the function it would only ever
return 0 or 1. Change the function itself to return a bool, and the
caller to just directly set the flag in the virPCIDevice.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/util/virpci.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index b63abac8cc..403ef1ec8e 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -549,7 +549,7 @@ virPCIDeviceFindExtendedCapabilityOffset(virPCIDevicePtr dev,
/* detects whether this device has FLR. Returns 0 if the device does
* not have FLR, 1 if it does, and -1 on error
*/
-static int
+static bool
virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd)
{
uint32_t caps;
@@ -567,7 +567,7 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd)
caps = virPCIDeviceRead32(dev, cfgfd, dev->pcie_cap_pos + PCI_EXP_DEVCAP);
if (caps & PCI_EXP_DEVCAP_FLR) {
VIR_DEBUG("%s %s: detected PCIe FLR capability", dev->id,
dev->name);
- return 1;
+ return true;
}
}
@@ -580,7 +580,7 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd)
caps = virPCIDeviceRead16(dev, cfgfd, pos + PCI_AF_CAP);
if (caps & PCI_AF_CAP_FLR) {
VIR_DEBUG("%s %s: detected PCI FLR capability", dev->id,
dev->name);
- return 1;
+ return true;
}
}
@@ -596,12 +596,12 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int
cfgfd)
if (found) {
VIR_DEBUG("%s %s: buggy device didn't advertise FLR, but is a VF;
forcing flr on",
dev->id, dev->name);
- return 1;
+ return true;
}
VIR_DEBUG("%s %s: no FLR capability found", dev->id, dev->name);
- return 0;
+ return false;
}
/* Require the device has the PCI Power Management capability
@@ -885,15 +885,10 @@ virPCIDeviceTryPowerManagementReset(virPCIDevicePtr dev, int cfgfd)
static int
virPCIDeviceInit(virPCIDevicePtr dev, int cfgfd)
{
- int flr;
-
dev->pcie_cap_pos = virPCIDeviceFindCapabilityOffset(dev, cfgfd,
PCI_CAP_ID_EXP);
dev->pci_pm_cap_pos = virPCIDeviceFindCapabilityOffset(dev, cfgfd,
PCI_CAP_ID_PM);
- flr = virPCIDeviceDetectFunctionLevelReset(dev, cfgfd);
- if (flr < 0)
- return flr;
- dev->has_flr = !!flr;
- dev->has_pm_reset = !!virPCIDeviceDetectPowerManagementReset(dev, cfgfd);
+ dev->has_flr = virPCIDeviceDetectFunctionLevelReset(dev, cfgfd);
+ dev->has_pm_reset = !!virPCIDeviceDetectPowerManagementReset(dev, cfgfd);
return 0;
}
--
2.28.0
9:47 a.m.
New subject: [libvirt PATCH v2 3/7] util: simplify call to virPCIDeviceDetectPowerManagementReset()
This function returned an int, but would only return 0 or 1, and the
one place it was called would just use !! to convert that value to a
bool. Change the function to directly return bool instead.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/util/virpci.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 403ef1ec8e..31622cddfa 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -608,7 +608,7 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd)
* and that a D3hot->D0 transition will results in a full
* internal reset, not just a soft reset.
*/
-static unsigned int
+static bool
virPCIDeviceDetectPowerManagementReset(virPCIDevicePtr dev, int cfgfd)
{
if (dev->pci_pm_cap_pos) {
@@ -618,13 +618,13 @@ virPCIDeviceDetectPowerManagementReset(virPCIDevicePtr dev, int
cfgfd)
ctl = virPCIDeviceRead32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL);
if (!(ctl & PCI_PM_CTRL_NO_SOFT_RESET)) {
VIR_DEBUG("%s %s: detected PM reset capability", dev->id,
dev->name);
- return 1;
+ return true;
}
}
VIR_DEBUG("%s %s: no PM reset capability found", dev->id,
dev->name);
- return 0;
+ return false;
}
/* Any active devices on the same domain/bus ? */
@@ -888,7 +888,7 @@ virPCIDeviceInit(virPCIDevicePtr dev, int cfgfd)
dev->pcie_cap_pos = virPCIDeviceFindCapabilityOffset(dev, cfgfd,
PCI_CAP_ID_EXP);
dev->pci_pm_cap_pos = virPCIDeviceFindCapabilityOffset(dev, cfgfd,
PCI_CAP_ID_PM);
dev->has_flr = virPCIDeviceDetectFunctionLevelReset(dev, cfgfd);
- dev->has_pm_reset = !!virPCIDeviceDetectPowerManagementReset(dev, cfgfd);
+ dev->has_pm_reset = virPCIDeviceDetectPowerManagementReset(dev, cfgfd);
return 0;
}
--
2.28.0
9:47 a.m.
New subject: [libvirt PATCH v2 4/7] util: make read error of PCI config file more detailed
The new message is more verbose/useful, but only logged at debug level
instead of as a warning (since it could easily happen in a non-error
situation).
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/util/virpci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 31622cddfa..9109fb4f3a 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -332,8 +332,8 @@ virPCIDeviceRead(virPCIDevicePtr dev,
if (lseek(cfgfd, pos, SEEK_SET) != pos ||
saferead(cfgfd, buf, buflen) != buflen) {
- VIR_WARN("Failed to read from '%s' : %s", dev->path,
- g_strerror(errno));
+ VIR_DEBUG("Failed to read %u bytes at %u from '%s' : %s",
+ buflen, pos, dev->path, g_strerror(errno));
return -1;
}
return 0;
--
2.28.0
9:47 a.m.
New subject: [libvirt PATCH v2 5/7] util: change call sequence for virPCIDeviceFindCapabilityOffset()
Previously there was no way to differentiate between this function 1)
encountering an error while reading the pci config, and 2) determining
that the device in question is a conventional PCI device, and so has
no Express Capabilities.
The difference between these two conditions is important, because an
unprivileged libvirtd will be unable to read all of the pci config (it
can only read the first 64 bytes, and will get ENOENT when it tries to
seek past that limit) even though the device is in fact a PCIe device.
This patch changes virPCIDeviceFindCapabilityOffset() to put the
determined offset into an argument of the function (rather than
sending it back as the return value), and to return the standard "0 on
success, -1 on failure". Failure is determined by checking the value
of errno after each attemptd read of the config file (which can only
work reliably if errno is reset to 0 before each read, and after
virPCIDeviceFindCapabilityOffset() has finished examining it).
(NB: if the config file is read successfully, but no Express
Capabilities are found, then the function returns success, but the
returned offset will be 0 (which is an impossible offset for Express
Capabilities, and so easily recognizeable).
An upcoming patch will take advantage of the change made here.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/util/virpci.c | 77 ++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 66 insertions(+), 11 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 9109fb4f3a..5ec559dec0 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -329,6 +329,7 @@ virPCIDeviceRead(virPCIDevicePtr dev,
unsigned int buflen)
{
memset(buf, 0, buflen);
+ errno = 0;
if (lseek(cfgfd, pos, SEEK_SET) != pos ||
saferead(cfgfd, buf, buflen) != buflen) {
@@ -339,6 +340,27 @@ virPCIDeviceRead(virPCIDevicePtr dev,
return 0;
}
+
+/**
+ * virPCIDeviceReadN:
+ * @dev: virPCIDevice object (used only to log name of config file)
+ * @cfgfd: open file descriptor for device config file in sysfs
+ * @pos: byte offset in the file to read from
+ *
+ * read "N" (where "N" is "8", "16", or
"32", and appears at the end
+ * of the function name) bytes from a PCI device's already-opened
+ * sysfs config file and return them as the return value from the
+ * function.
+ *
+ * Returns the value at @pos in the file, or 0 if there was an
+ * error. NB: since 0 could be a valid value, occurence of an error
+ * must be determined by examining errno. errno is always reset to 0
+ * before the seek/read is attempted (see virPCIDeviceRead()), so if
+ * errno != 0 on return from one of these functions, then either the
+ * seek or the read operation failed for some reason. If errno == 0
+ * and the return value is 0, then the config file really does contain
+ * the value 0 at @pos.
+ */
static uint8_t
virPCIDeviceRead8(virPCIDevicePtr dev, int cfgfd, unsigned int pos)
{
@@ -483,19 +505,38 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate,
return ret;
}
-static uint8_t
+
+/**
+ * virPCIDeviceFindCapabilityOffset:
+ * @dev: virPCIDevice object (used only to log name of config file)
+ * @cfgfd: open file descriptor for device config file in sysfs
+ * @capability: PCI_CAP_ID_* being requested
+ * @offset: used to return the offset of @capability in the file
+ *
+ * Find the offset of @capability within the PCI config file @cfgfd of
+ * the device @dev. if found, the offset is returned in @offset,
+ * otherwise @offset is set to 0.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int
virPCIDeviceFindCapabilityOffset(virPCIDevicePtr dev,
int cfgfd,
- unsigned int capability)
+ unsigned int capability,
+ unsigned int *offset)
{
uint16_t status;
uint8_t pos;
+ *offset = 0; /* assume failure (*nothing* can be at offset 0) */
+
status = virPCIDeviceRead16(dev, cfgfd, PCI_STATUS);
- if (!(status & PCI_STATUS_CAP_LIST))
- return 0;
+ if (errno != 0 || !(status & PCI_STATUS_CAP_LIST))
+ goto error;
pos = virPCIDeviceRead8(dev, cfgfd, PCI_CAPABILITY_LIST);
+ if (errno != 0)
+ goto error;
/* Zero indicates last capability, capabilities can't
* be in the config space header and 0xff is returned
@@ -506,18 +547,31 @@ virPCIDeviceFindCapabilityOffset(virPCIDevicePtr dev,
*/
while (pos >= PCI_CONF_HEADER_LEN && pos != 0xff) {
uint8_t capid = virPCIDeviceRead8(dev, cfgfd, pos);
+ if (errno != 0)
+ goto error;
+
if (capid == capability) {
VIR_DEBUG("%s %s: found cap 0x%.2x at 0x%.2x",
dev->id, dev->name, capability, pos);
- return pos;
+ *offset = pos;
+ return 0;
}
pos = virPCIDeviceRead8(dev, cfgfd, pos + 1);
+ if (errno != 0)
+ goto error;
}
- VIR_DEBUG("%s %s: failed to find cap 0x%.2x", dev->id, dev->name,
capability);
+ error:
+ VIR_DEBUG("%s %s: failed to find cap 0x%.2x (%s)",
+ dev->id, dev->name, capability, g_strerror(errno));
+
+ /* reset errno in case the failure was due to insufficient
+ * privileges to read the entire PCI config file
+ */
+ errno = 0;
- return 0;
+ return -1;
}
static unsigned int
@@ -553,7 +607,7 @@ static bool
virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd)
{
uint32_t caps;
- uint8_t pos;
+ unsigned int pos;
g_autofree char *path = NULL;
int found;
@@ -575,7 +629,8 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd)
* the same thing, except for conventional PCI
* devices. This is not common yet.
*/
- pos = virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_AF);
+ virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_AF, &pos);
+
if (pos) {
caps = virPCIDeviceRead16(dev, cfgfd, pos + PCI_AF_CAP);
if (caps & PCI_AF_CAP_FLR) {
@@ -885,8 +940,8 @@ virPCIDeviceTryPowerManagementReset(virPCIDevicePtr dev, int cfgfd)
static int
virPCIDeviceInit(virPCIDevicePtr dev, int cfgfd)
{
- dev->pcie_cap_pos = virPCIDeviceFindCapabilityOffset(dev, cfgfd,
PCI_CAP_ID_EXP);
- dev->pci_pm_cap_pos = virPCIDeviceFindCapabilityOffset(dev, cfgfd,
PCI_CAP_ID_PM);
+ virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_EXP,
&dev->pcie_cap_pos);
+ virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_PM,
&dev->pci_pm_cap_pos);
dev->has_flr = virPCIDeviceDetectFunctionLevelReset(dev, cfgfd);
dev->has_pm_reset = virPCIDeviceDetectPowerManagementReset(dev, cfgfd);
--
2.28.0
9:47 a.m.
New subject: [libvirt PATCH v2 6/7] util: make virPCIDeviceIsPCIExpress() more intelligent
Until now there has been an extra bit of code in
qemuDomainDeviceCalculatePCIConnectFlag() (one of the two callers of
virPCIDeviceIsPCIExpress()) that tries to determine if a device is
PCIe by looking at the *length* of its sysfs config file; it only does
this when libvirt is running as a non-root process.
This patch takes advantage of our newfound ability to tell the
difference between "I read a 0 from the device PCI config file" and "I
couldn't read the PCI Express Capabilities because I don't have
sufficient permission" to put the file length check down in
virPCIDeviceIsPCIExpress(), and do that check any time we fail while
reading the config file (not only when the process is non-root).
Fixes: https://bugzilla.redhat.com/1901685
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/util/virpci.c | 66 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 62 insertions(+), 4 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 5ec559dec0..9bfc743fbd 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -73,10 +73,18 @@ struct _virPCIDevice {
char *used_by_drvname;
char *used_by_domname;
+ /* The following 5 items are only valid after virPCIDeviceInit()
+ * has been called for the virPCIDevice object. This is *not* done
+ * in most cases (because it creates extra overhead, and parts of
+ * it can fail if libvirtd is running unprivileged)
+ */
unsigned int pcie_cap_pos;
unsigned int pci_pm_cap_pos;
bool has_flr;
bool has_pm_reset;
+ bool is_pcie;
+ /**/
+
bool managed;
virPCIStubDriver stubDriver;
@@ -629,7 +637,8 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd)
* the same thing, except for conventional PCI
* devices. This is not common yet.
*/
- virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_AF, &pos);
+ if (virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_AF, &pos) < 0)
+ goto error;
if (pos) {
caps = virPCIDeviceRead16(dev, cfgfd, pos + PCI_AF_CAP);
@@ -654,8 +663,8 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd)
return true;
}
+ error:
VIR_DEBUG("%s %s: no FLR capability found", dev->id, dev->name);
-
return false;
}
@@ -937,10 +946,59 @@ virPCIDeviceTryPowerManagementReset(virPCIDevicePtr dev, int cfgfd)
return 0;
}
+/**
+ * virPCIDeviceInit:
+ * @dev: virPCIDevice object needing its PCI capabilities info initialized
+ * @cfgfd: open file descriptor for device config file in sysfs
+ *
+ * Initialize the PCI capabilities attributes of a virPCIDevice object
+ * (i.e. pcie_cap_pos, pci_pm_cap_pos, has_flr, has_pm_reset, and
+ * is_pcie). This is done by walking the info in the (already-opened)
+ * device PCI config file in sysfs. This function can be called
+ * regardless of whether a process has sufficient privilege to read
+ * the entire file (unprivileged processes can only read the 1st 64
+ * bytes, while the Express Capabilities are all located beyond that
+ * boundary).
+ *
+ * In the case that we are unable to read a capability
+ * directly, we will attempt to infer its value by other means. In
+ * particular, we can determine that a device is (almost surely) PCIe
+ * by checking that the length of the config file is != 256 (since all
+ * conventional PCI config files are 256 bytes), and we know that any
+ * device that is an SR-IOV VF will have FLR available (since that is
+ * required by the SR-IOV spec.)
+ *
+ * Always returns success (0) (for now)
+ */
static int
virPCIDeviceInit(virPCIDevicePtr dev, int cfgfd)
{
- virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_EXP,
&dev->pcie_cap_pos);
+ dev->is_pcie = false;
+ if (virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_EXP,
&dev->pcie_cap_pos) < 0) {
+ /* an unprivileged process is unable to read *all* of a
+ * device's PCI config (it can only read the first 64
+ * bytes, which isn't enough for see the Express
+ * Capabilities data). If virPCIDeviceFindCapabilityOffset
+ * returns failure (and not just a pcie_cap_pos == 0,
+ * which is *success* at determining the device is *not*
+ * PCIe) we make an educated guess based on the length of
+ * the device's config file - if it is 256 bytes, then it
+ * is definitely a legacy PCI device. If it's larger than
+ * that, then it is *probably PCIe (although it could be
+ * PCI-x, but those are extremely rare). If the config
+ * file can't be found (in which case the "length" will be
+ * -1), then we blindly assume the most likely outcome -
+ * PCIe.
+ */
+ off_t configLen = virFileLength(virPCIDeviceGetConfigPath(dev), -1);
+
+ if (configLen != 256)
+ dev->is_pcie = true;
+
+ } else {
+ dev->is_pcie = (dev->pcie_cap_pos != 0);
+ }
+
virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_PM,
&dev->pci_pm_cap_pos);
dev->has_flr = virPCIDeviceDetectFunctionLevelReset(dev, cfgfd);
dev->has_pm_reset = virPCIDeviceDetectPowerManagementReset(dev, cfgfd);
@@ -2644,7 +2702,7 @@ virPCIDeviceIsPCIExpress(virPCIDevicePtr dev)
if (virPCIDeviceInit(dev, fd) < 0)
goto cleanup;
- ret = dev->pcie_cap_pos != 0;
+ ret = dev->is_pcie;
cleanup:
virPCIDeviceConfigClose(dev, fd);
--
2.28.0
9:47 a.m.
New subject: [libvirt PATCH v2 7/7] qemu: remove now-redundant check for file length when determining PCIe vs. PCI
Now that virPCIDeviceIsPCIExpress() checks the length of the file when
the process lacks sufficient privilege to read the entire PCI config
file in sysfs, we can remove the open-coding for that case from its
consumer.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/qemu/qemu_domain_address.c | 38 +++-------------------------------
1 file changed, 3 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index b07672e2f4..f0ba318cc8 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -579,7 +579,6 @@ qemuDomainDeviceCalculatePCIAddressExtensionFlags(virQEMUCapsPtr
qemuCaps,
*/
static virDomainPCIConnectFlags
qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
- virQEMUDriverPtr driver,
virDomainPCIConnectFlags pcieFlags,
virDomainPCIConnectFlags virtioFlags)
{
@@ -802,7 +801,6 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
case VIR_DOMAIN_DEVICE_HOSTDEV: {
virDomainHostdevDefPtr hostdev = dev->data.hostdev;
- bool isExpress = false;
g_autoptr(virPCIDevice) pciDev = NULL;
virPCIDeviceAddressPtr hostAddr = &hostdev->source.subsys.u.pci.addr;
@@ -873,37 +871,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
return pcieFlags;
}
- if (!driver->privileged) {
- /* unprivileged libvirtd is unable to read *all* of a
- * device's PCI config (it can only read the first 64
- * bytes, which isn't enough for the check that's done
- * in virPCIDeviceIsPCIExpress()), so instead of
- * trying and failing, we make an educated guess based
- * on the length of the device's config file - if it
- * is 256 bytes, then it is definitely a legacy PCI
- * device. If it's larger than that, then it is
- * *probably PCIe (although it could be PCI-x, but
- * those are extremely rare). If the config file can't
- * be found (in which case the "length" will be -1),
- * then we blindly assume the most likely outcome -
- * PCIe.
- */
- off_t configLen
- = virFileLength(virPCIDeviceGetConfigPath(pciDev), -1);
-
- if (configLen == 256)
- return pciFlags;
-
- return pcieFlags;
- }
-
- /* If we are running with privileges, we can examine the
- * PCI config contents with virPCIDeviceIsPCIExpress() for
- * a definitive answer.
- */
- isExpress = virPCIDeviceIsPCIExpress(pciDev);
-
- if (isExpress)
+ if (virPCIDeviceIsPCIExpress(pciDev))
return pcieFlags;
return pciFlags;
@@ -1124,7 +1092,7 @@ qemuDomainFillDevicePCIConnectFlagsIter(virDomainDefPtr def
G_GNUC_UNUSED,
qemuDomainFillDevicePCIConnectFlagsIterData *data = opaque;
info->pciConnectFlags
- = qemuDomainDeviceCalculatePCIConnectFlags(dev, data->driver,
+ = qemuDomainDeviceCalculatePCIConnectFlags(dev,
data->pcieFlags,
data->virtioFlags);
return 0;
@@ -1468,7 +1436,7 @@ qemuDomainFillDevicePCIConnectFlags(virDomainDefPtr def,
qemuDomainFillDevicePCIConnectFlagsIterInit(def, qemuCaps, driver, &data);
info->pciConnectFlags
- = qemuDomainDeviceCalculatePCIConnectFlags(dev, data.driver,
+ = qemuDomainDeviceCalculatePCIConnectFlags(dev,
data.pcieFlags,
data.virtioFlags);
}
--
2.28.0
10:41 a.m.
On 12/11/20 4:47 PM, Laine Stump wrote:
danpb had asked for some documentation on the changes to the APIs of
internal functions in V1. I've added some standard function header
docs in patches 5 & 6. Everything else is completely unchanged (and
already reviewed by mprivozn).
Laine Stump (7):
qemu: use g_autoptr for a virPCIDevice
util: simplify calling of virPCIDeviceDetectFunctionLevelReset()
util: simplify call to virPCIDeviceDetectPowerManagementReset()
util: make read error of PCI config file more detailed
util: change call sequence for virPCIDeviceFindCapabilityOffset()
util: make virPCIDeviceIsPCIExpress() more intelligent
qemu: remove now-redundant check for file length when determining PCIe
vs. PCI
src/qemu/qemu_domain_address.c | 43 +--------
src/util/virpci.c | 166 +++++++++++++++++++++++++++------
2 files changed, 141 insertions(+), 68 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1443
days inactive
1443
days old
8 comments
2 participants
participants (2)
-
Laine Stump -
Michal Privoznik