External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-03-03 07:33:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

virpci.c: simplify virPCIDeviceNew() signature

Browse Source 
The current virPCIDeviceNew() signature, receiving 4 uints in sequence
(domain, bus, slot, function), is not neat.

We already have a way to represent a PCI address in virPCIDeviceAddress
that is used in the code. Aside from the test files, most of
virPCIDeviceNew() callers have access to a virPCIDeviceAddress reference,
but then we need to retrieve the 4 required uints (addr.domain, addr.bus,
addr.slot, addr.function) to satisfy virPCIDeviceNew(). The result is
that we have extra verbosity/boilerplate to retrieve an information that
is already available in virPCIDeviceAddress.

A better way is presented by virNVMEDeviceNew(), where the caller just
supplies a virPCIDeviceAddress pointer and the function handles the
details internally.

This patch changes virPCIDeviceNew() to receive a virPCIDeviceAddress
pointer instead of 4 uints.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This commit is contained in:
Daniel Henrique Barboza 2021-01-04 09:54:28 -03:00
parent 0e0c974b20
commit b0264e9404
15 changed files with 64 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/hypervisor/virhostdev.c
View File

@ -235,8 +235,7 @@ virHostdevGetPCIHostDevice(const virDomainHostdevDef *hostdev,
hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
return 0;
actual = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
pcisrc->addr.slot, pcisrc->addr.function);
actual = virPCIDeviceNew(&pcisrc->addr);
if (!actual)
return -1;

6
src/libxl/libxl_driver.c
View File

@ -5818,7 +5818,7 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev,
if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
pci = virPCIDeviceNew(&devAddr);
if (!pci)
goto cleanup;
@ -5889,7 +5889,7 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev)
if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
pci = virPCIDeviceNew(&devAddr);
if (!pci)
goto cleanup;
@ -5947,7 +5947,7 @@ libxlNodeDeviceReset(virNodeDevicePtr dev)
if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
pci = virPCIDeviceNew(&devAddr);
if (!pci)
goto cleanup;

11
src/node_device/node_device_udev.c
View File

@ -367,6 +367,7 @@ udevProcessPCI(struct udev_device *device,
virNodeDevCapPCIDevPtr pci_dev = &def->caps->data.pci_dev;
virPCIEDeviceInfoPtr pci_express = NULL;
virPCIDevicePtr pciDev = NULL;
virPCIDeviceAddress devAddr;
int ret = -1;
char *p;
bool privileged;
@ -416,10 +417,12 @@ udevProcessPCI(struct udev_device *device,
if (virNodeDeviceGetPCIDynamicCaps(def->sysfs_path, pci_dev) < 0)
goto cleanup;
if (!(pciDev = virPCIDeviceNew(pci_dev->domain,
pci_dev->bus,
pci_dev->slot,
pci_dev->function)))
devAddr.domain = pci_dev->domain;
devAddr.bus = pci_dev->bus;
devAddr.slot = pci_dev->slot;
devAddr.function = pci_dev->function;
if (!(pciDev = virPCIDeviceNew(&devAddr)))
goto cleanup;
/* We need to be root to read PCI device configs */

5
src/qemu/qemu_domain_address.c
View File

@ -863,10 +863,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
return 0;
}
if (!(pciDev = virPCIDeviceNew(hostAddr->domain,
hostAddr->bus,
hostAddr->slot,
hostAddr->function))) {
if (!(pciDev = virPCIDeviceNew(hostAddr))) {
/* libvirt should be able to perform all the
* operations in virPCIDeviceNew() even if it's
* running unprivileged, so if this fails, the device

6
src/qemu/qemu_driver.c
View File

@ -12009,7 +12009,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
pci = virPCIDeviceNew(&devAddr);
if (!pci)
goto cleanup;
@ -12090,7 +12090,7 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev)
if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
pci = virPCIDeviceNew(&devAddr);
if (!pci)
goto cleanup;
@ -12144,7 +12144,7 @@ qemuNodeDeviceReset(virNodeDevicePtr dev)
if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
pci = virPCIDeviceNew(&devAddr);
if (!pci)
goto cleanup;

3
src/security/security_apparmor.c
View File

@ -892,8 +892,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
virPCIDevicePtr pci =
virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
pcisrc->addr.slot, pcisrc->addr.function);
virPCIDeviceNew(&pcisrc->addr);
if (!pci)
goto done;

6
src/security/security_dac.c
View File

@ -1268,8 +1268,7 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
virPCIDevicePtr pci =
virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
pcisrc->addr.slot, pcisrc->addr.function);
virPCIDeviceNew(&pcisrc->addr);
if (!pci)
return -1;
@ -1437,8 +1436,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
virPCIDevicePtr pci =
virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
pcisrc->addr.slot, pcisrc->addr.function);
virPCIDeviceNew(&pcisrc->addr);
if (!pci)
return -1;

6
src/security/security_selinux.c
View File

@ -2105,8 +2105,7 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
virPCIDevicePtr pci =
virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
pcisrc->addr.slot, pcisrc->addr.function);
virPCIDeviceNew(&pcisrc->addr);
if (!pci)
return -1;
@ -2345,8 +2344,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
virPCIDevicePtr pci =
virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
pcisrc->addr.slot, pcisrc->addr.function);
virPCIDeviceNew(&pcisrc->addr);
if (!pci)
return -1;

6
src/security/virt-aa-helper.c
View File

@ -1105,11 +1105,7 @@ get_files(vahControl * ctl)
}
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
virPCIDevicePtr pci = virPCIDeviceNew(
dev->source.subsys.u.pci.addr.domain,
dev->source.subsys.u.pci.addr.bus,
dev->source.subsys.u.pci.addr.slot,
dev->source.subsys.u.pci.addr.function);
virPCIDevicePtr pci = virPCIDeviceNew(&dev->source.subsys.u.pci.addr);
virDomainHostdevSubsysPCIBackendType backend = dev->source.subsys.u.pci.backend;
if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||

3
src/util/virnetdev.c
View File

@ -1141,8 +1141,7 @@ virNetDevGetPCIDevice(const char *devName)
if (!vfPCIAddr)
return NULL;
return virPCIDeviceNew(vfPCIAddr->domain, vfPCIAddr->bus,
vfPCIAddr->slot, vfPCIAddr->function);
return virPCIDeviceNew(vfPCIAddr);
}
# endif

5
src/util/virnvme.c
View File

@ -290,10 +290,7 @@ virNVMeDeviceCreatePCIDevice(const virNVMeDevice *nvme)
{
g_autoptr(virPCIDevice) pci = NULL;
if (!(pci = virPCIDeviceNew(nvme->address.domain,
nvme->address.bus,
nvme->address.slot,
nvme->address.function)))
if (!(pci = virPCIDeviceNew(&nvme->address)))
return NULL;
/* NVMe devices must be bound to vfio */

40
src/util/virpci.c
View File

@ -475,24 +475,24 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate,
return -1;
while ((ret = virDirRead(dir, &entry, PCI_SYSFS "devices")) > 0) {
unsigned int domain, bus, slot, function;
g_autoptr(virPCIDevice) check = NULL;
virPCIDeviceAddress devAddr;
char *tmp;
/* expected format: <domain>:<bus>:<slot>.<function> */
if (/* domain */
virStrToLong_ui(entry->d_name, &tmp, 16, &domain) < 0 || *tmp != ':' ||
virStrToLong_ui(entry->d_name, &tmp, 16, &devAddr.domain) < 0 || *tmp != ':' ||
/* bus */
virStrToLong_ui(tmp + 1, &tmp, 16, &bus) < 0 || *tmp != ':' ||
virStrToLong_ui(tmp + 1, &tmp, 16, &devAddr.bus) < 0 || *tmp != ':' ||
/* slot */
virStrToLong_ui(tmp + 1, &tmp, 16, &slot) < 0 || *tmp != '.' ||
virStrToLong_ui(tmp + 1, &tmp, 16, &devAddr.slot) < 0 || *tmp != '.' ||
/* function */
virStrToLong_ui(tmp + 1, NULL, 16, &function) < 0) {
virStrToLong_ui(tmp + 1, NULL, 16, &devAddr.function) < 0) {
VIR_WARN("Unusual entry in " PCI_SYSFS "devices: %s", entry->d_name);
continue;
}
check = virPCIDeviceNew(domain, bus, slot, function);
check = virPCIDeviceNew(&devAddr);
if (!check) {
ret = -1;
break;
@ -767,10 +767,7 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void *data)
*/
if (dev->address.bus > secondary && dev->address.bus <= subordinate) {
if (*best == NULL) {
*best = virPCIDeviceNew(check->address.domain,
check->address.bus,
check->address.slot,
check->address.function);
*best = virPCIDeviceNew(&check->address);
if (*best == NULL) {
ret = -1;
goto cleanup;
@ -790,10 +787,7 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void *data)
if (secondary > best_secondary) {
virPCIDeviceFree(*best);
*best = virPCIDeviceNew(check->address.domain,
check->address.bus,
check->address.slot,
check->address.function);
*best = virPCIDeviceNew(&check->address);
if (*best == NULL) {
ret = -1;
goto cleanup;
@ -1455,10 +1449,7 @@ virPCIDeviceAddressAsString(const virPCIDeviceAddress *addr)
}
virPCIDevicePtr
virPCIDeviceNew(unsigned int domain,
unsigned int bus,
unsigned int slot,
unsigned int function)
virPCIDeviceNew(const virPCIDeviceAddress *address)
{
g_autoptr(virPCIDevice) dev = NULL;
g_autofree char *vendor = NULL;
@ -1466,10 +1457,7 @@ virPCIDeviceNew(unsigned int domain,
dev = g_new0(virPCIDevice, 1);
dev->address.domain = domain;
dev->address.bus = bus;
dev->address.slot = slot;
dev->address.function = function;
virPCIDeviceAddressCopy(&dev->address, address);
dev->name = virPCIDeviceAddressAsString(&dev->address);
@ -1896,8 +1884,7 @@ virPCIDeviceGetIOMMUGroupAddOne(virPCIDeviceAddressPtr newDevAddr, void *opaque)
virPCIDeviceListPtr groupList = opaque;
g_autoptr(virPCIDevice) newDev = NULL;
if (!(newDev = virPCIDeviceNew(newDevAddr->domain, newDevAddr->bus,
newDevAddr->slot, newDevAddr->function)))
if (!(newDev = virPCIDeviceNew(newDevAddr)))
return -1;
if (virPCIDeviceListAdd(groupList, newDev) < 0)
@ -2028,10 +2015,7 @@ virPCIDeviceAddressGetIOMMUGroupDev(const virPCIDeviceAddress *devAddr)
{
g_autoptr(virPCIDevice) pci = NULL;
if (!(pci = virPCIDeviceNew(devAddr->domain,
devAddr->bus,
devAddr->slot,
devAddr->function)))
if (!(pci = virPCIDeviceNew(devAddr)))
return NULL;
return virPCIDeviceGetIOMMUGroupDev(pci);

5
src/util/virpci.h
View File

@ -118,10 +118,7 @@ struct _virPCIEDeviceInfo {
virPCIELink *link_sta; /* Actually negotiated capabilities */
};
virPCIDevicePtr virPCIDeviceNew(unsigned int domain,
unsigned int bus,
unsigned int slot,
unsigned int function);
virPCIDevicePtr virPCIDeviceNew(const virPCIDeviceAddress *address);
virPCIDevicePtr virPCIDeviceCopy(virPCIDevicePtr dev);
void virPCIDeviceFree(virPCIDevicePtr dev);
const char *virPCIDeviceGetName(virPCIDevicePtr dev);

3
tests/virhostdevtest.c
View File

@ -138,7 +138,8 @@ myInit(void)
}
for (i = 0; i < nhostdevs; i++) {
if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
virDomainHostdevSubsys subsys = hostdevs[i]->source.subsys;
if (!(dev[i] = virPCIDeviceNew(&subsys.u.pci.addr)))
goto cleanup;
virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO);

35
tests/virpcitest.c
View File

@ -60,8 +60,9 @@ testVirPCIDeviceNew(const void *opaque G_GNUC_UNUSED)
int ret = -1;
virPCIDevicePtr dev;
const char *devName;
virPCIDeviceAddress devAddr = {.domain = 0, .bus = 0, .slot = 0, .function = 0};
if (!(dev = virPCIDeviceNew(0, 0, 0, 0)))
if (!(dev = virPCIDeviceNew(&devAddr)))
goto cleanup;
devName = virPCIDeviceGetName(dev);
@ -103,7 +104,9 @@ testVirPCIDeviceDetach(const void *opaque G_GNUC_UNUSED)
CHECK_LIST_COUNT(inactiveDevs, 0);
for (i = 0; i < nDev; i++) {
if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
virPCIDeviceAddress devAddr = {.domain = 0, .bus = 0,
.slot = i + 1, .function = 0};
if (!(dev[i] = virPCIDeviceNew(&devAddr)))
goto cleanup;
virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO);
@ -144,7 +147,9 @@ testVirPCIDeviceReset(const void *opaque G_GNUC_UNUSED)
CHECK_LIST_COUNT(inactiveDevs, 0);
for (i = 0; i < nDev; i++) {
if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
virPCIDeviceAddress devAddr = {.domain = 0, .bus = 0,
.slot = i + 1, .function = 0};
if (!(dev[i] = virPCIDeviceNew(&devAddr)))
goto cleanup;
virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO);
@ -176,7 +181,9 @@ testVirPCIDeviceReattach(const void *opaque G_GNUC_UNUSED)
goto cleanup;
for (i = 0; i < nDev; i++) {
if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
virPCIDeviceAddress devAddr = {.domain = 0, .bus = 0,
.slot = i + 1, .function = 0};
if (!(dev[i] = virPCIDeviceNew(&devAddr)))
goto cleanup;
if (virPCIDeviceListAdd(inactiveDevs, dev[i]) < 0) {
@ -222,8 +229,10 @@ testVirPCIDeviceIsAssignable(const void *opaque)
const struct testPCIDevData *data = opaque;
int ret = -1;
virPCIDevicePtr dev;
virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus,
.slot = data->slot, .function = data->function};
if (!(dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function)))
if (!(dev = virPCIDeviceNew(&devAddr)))
return -1;
if (virPCIDeviceIsAssignable(dev, true))
@ -239,8 +248,10 @@ testVirPCIDeviceDetachSingle(const void *opaque)
const struct testPCIDevData *data = opaque;
int ret = -1;
virPCIDevicePtr dev;
virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus,
.slot = data->slot, .function = data->function};
dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function);
dev = virPCIDeviceNew(&devAddr);
if (!dev)
goto cleanup;
@ -261,8 +272,10 @@ testVirPCIDeviceReattachSingle(const void *opaque)
const struct testPCIDevData *data = opaque;
int ret = -1;
virPCIDevicePtr dev;
virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus,
.slot = data->slot, .function = data->function};
dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function);
dev = virPCIDeviceNew(&devAddr);
if (!dev)
goto cleanup;
@ -285,8 +298,10 @@ testVirPCIDeviceCheckDriverTest(const void *opaque)
const struct testPCIDevData *data = opaque;
int ret = -1;
virPCIDevicePtr dev;
virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus,
.slot = data->slot, .function = data->function};
dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function);
dev = virPCIDeviceNew(&devAddr);
if (!dev)
goto cleanup;
@ -305,8 +320,10 @@ testVirPCIDeviceUnbind(const void *opaque)
const struct testPCIDevData *data = opaque;
int ret = -1;
virPCIDevicePtr dev;
virPCIDeviceAddress devAddr = {.domain = data->domain, .bus = data->bus,
.slot = data->slot, .function = data->function};
dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function);
dev = virPCIDeviceNew(&devAddr);
if (!dev)
goto cleanup;