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@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@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;