7:05 p.m.
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
Instead of receiving 4 uints in order and write
domain/bus/slot/function,
receive a virPCIDeviceAddressPtr instead and write into it.
This change will allow us to simplify the API for virPCIDeviceNew()
in the next patch.
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
src/hypervisor/domain_driver.c | 13 +++++--------
src/hypervisor/domain_driver.h | 5 +----
src/libxl/libxl_driver.c | 18 +++++++++---------
src/qemu/qemu_driver.c | 18 +++++++++---------
4 files changed, 24 insertions(+), 30 deletions(-)
diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index 68dbf10ac6..cdb53bcf3e 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -341,20 +341,17 @@ virDomainDriverSetupPersistentDefBlkioParams(virDomainDefPtr
persistentDef,
int
virDomainDriverNodeDeviceGetPCIInfo(virNodeDeviceDefPtr def,
- unsigned *domain,
- unsigned *bus,
- unsigned *slot,
- unsigned *function)
+ virPCIDeviceAddressPtr devAddr)
{
virNodeDevCapsDefPtr cap;
cap = def->caps;
while (cap) {
if (cap->data.type == VIR_NODE_DEV_CAP_PCI_DEV) {
- *domain = cap->data.pci_dev.domain;
- *bus = cap->data.pci_dev.bus;
- *slot = cap->data.pci_dev.slot;
- *function = cap->data.pci_dev.function;
+ devAddr->domain = cap->data.pci_dev.domain;
+ devAddr->bus = cap->data.pci_dev.bus;
+ devAddr->slot = cap->data.pci_dev.slot;
+ devAddr->function = cap->data.pci_dev.function;
Looking at this, my first thought was "surely cap->data.pci_dev should
have a single virPCIDeviceAddress rather than the individual members!
Then those 4 lines could just be "devAddr = cap->data.pci_dev.address".
Except that somewhere along the way someone decided that the
virZPCIDeviceAddress should be a subelement of a virPCIDeviceAddress, so
there is extra stuff in there that's unwanted in this case (can't place
the blame entirely there of course :-) - even before that, there was the
extra "multi" setting, which is really an attribute of a PCI slot, not
an attribute of a PCI address.)
At any rate, what you've done is much better than what was there before,
and simplifications/consolidations like this will often lead to
identification of other simplifications/consolidations that can be made
later.
(Oh, and we mustn't forget that passing a single virPCIDeviceAddressPtr
instead of 4 int* makes it much less likely that someone will typo the
items in the wrong order)
Reviewed-by: Laine Stump <laine(a)redhat.com>
I'm really liking these so far! :-)
break;
}
diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h
index 2bb053d559..86b92d0284 100644
--- a/src/hypervisor/domain_driver.h
+++ b/src/hypervisor/domain_driver.h
@@ -48,7 +48,4 @@ int virDomainDriverSetupPersistentDefBlkioParams(virDomainDefPtr
persistentDef,
int nparams);
int virDomainDriverNodeDeviceGetPCIInfo(virNodeDeviceDefPtr def,
- unsigned *domain,
- unsigned *bus,
- unsigned *slot,
- unsigned *function);
+ virPCIDeviceAddressPtr devAddr);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0821d39c9b..360d553a22 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5780,7 +5780,7 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev,
unsigned int flags)
{
virPCIDevicePtr pci = NULL;
- unsigned domain = 0, bus = 0, slot = 0, function = 0;
+ virPCIDeviceAddress devAddr;
int ret = -1;
virNodeDeviceDefPtr def = NULL;
char *xml = NULL;
@@ -5815,10 +5815,10 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev,
if (virNodeDeviceDetachFlagsEnsureACL(dev->conn, def) < 0)
goto cleanup;
- if (virDomainDriverNodeDeviceGetPCIInfo(def, &domain, &bus, &slot,
&function) < 0)
+ if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
- pci = virPCIDeviceNew(domain, bus, slot, function);
+ pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
if (!pci)
goto cleanup;
@@ -5853,7 +5853,7 @@ static int
libxlNodeDeviceReAttach(virNodeDevicePtr dev)
{
virPCIDevicePtr pci = NULL;
- unsigned domain = 0, bus = 0, slot = 0, function = 0;
+ virPCIDeviceAddress devAddr;
int ret = -1;
virNodeDeviceDefPtr def = NULL;
char *xml = NULL;
@@ -5886,10 +5886,10 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev)
if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0)
goto cleanup;
- if (virDomainDriverNodeDeviceGetPCIInfo(def, &domain, &bus, &slot,
&function) < 0)
+ if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
- pci = virPCIDeviceNew(domain, bus, slot, function);
+ pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
if (!pci)
goto cleanup;
@@ -5911,7 +5911,7 @@ static int
libxlNodeDeviceReset(virNodeDevicePtr dev)
{
virPCIDevicePtr pci = NULL;
- unsigned domain = 0, bus = 0, slot = 0, function = 0;
+ virPCIDeviceAddress devAddr;
int ret = -1;
virNodeDeviceDefPtr def = NULL;
char *xml = NULL;
@@ -5944,10 +5944,10 @@ libxlNodeDeviceReset(virNodeDevicePtr dev)
if (virNodeDeviceResetEnsureACL(dev->conn, def) < 0)
goto cleanup;
- if (virDomainDriverNodeDeviceGetPCIInfo(def, &domain, &bus, &slot,
&function) < 0)
+ if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
- pci = virPCIDeviceNew(domain, bus, slot, function);
+ pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
if (!pci)
goto cleanup;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0a732a241d..28781cc34b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11962,7 +11962,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
{
virQEMUDriverPtr driver = dev->conn->privateData;
virPCIDevicePtr pci = NULL;
- unsigned domain = 0, bus = 0, slot = 0, function = 0;
+ virPCIDeviceAddress devAddr;
int ret = -1;
virNodeDeviceDefPtr def = NULL;
g_autofree char *xml = NULL;
@@ -11997,10 +11997,10 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
if (virNodeDeviceDetachFlagsEnsureACL(dev->conn, def) < 0)
goto cleanup;
- if (virDomainDriverNodeDeviceGetPCIInfo(def, &domain, &bus, &slot,
&function) < 0)
+ if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
- pci = virPCIDeviceNew(domain, bus, slot, function);
+ pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
if (!pci)
goto cleanup;
@@ -12046,7 +12046,7 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev)
{
virQEMUDriverPtr driver = dev->conn->privateData;
virPCIDevicePtr pci = NULL;
- unsigned domain = 0, bus = 0, slot = 0, function = 0;
+ virPCIDeviceAddress devAddr;
int ret = -1;
virNodeDeviceDefPtr def = NULL;
g_autofree char *xml = NULL;
@@ -12078,10 +12078,10 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev)
if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0)
goto cleanup;
- if (virDomainDriverNodeDeviceGetPCIInfo(def, &domain, &bus, &slot,
&function) < 0)
+ if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
- pci = virPCIDeviceNew(domain, bus, slot, function);
+ pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
if (!pci)
goto cleanup;
@@ -12100,7 +12100,7 @@ qemuNodeDeviceReset(virNodeDevicePtr dev)
{
virQEMUDriverPtr driver = dev->conn->privateData;
virPCIDevicePtr pci;
- unsigned domain = 0, bus = 0, slot = 0, function = 0;
+ virPCIDeviceAddress devAddr;
int ret = -1;
virNodeDeviceDefPtr def = NULL;
g_autofree char *xml = NULL;
@@ -12132,10 +12132,10 @@ qemuNodeDeviceReset(virNodeDevicePtr dev)
if (virNodeDeviceResetEnsureACL(dev->conn, def) < 0)
goto cleanup;
- if (virDomainDriverNodeDeviceGetPCIInfo(def, &domain, &bus, &slot,
&function) < 0)
+ if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0)
goto cleanup;
- pci = virPCIDeviceNew(domain, bus, slot, function);
+ pci = virPCIDeviceNew(devAddr.domain, devAddr.bus, devAddr.slot, devAddr.function);
if (!pci)
goto cleanup;