Re: [PATCH RESEND 17/20] virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFind()

2021-02-21 Thread Laine Stump

On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:

Signed-off-by: Daniel Henrique Barboza 
---
  src/hypervisor/virhostdev.c | 12 
  src/util/virpci.c   | 16 
  src/util/virpci.h   |  2 +-
  3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index 402d7be42d..3bfb04c674 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -657,7 +657,8 @@ virHostdevReattachAllPCIDevices(virHostdevManagerPtr mgr,
  
  /* We need to look up the actual device because that's what

   * virPCIDeviceReattach() expects as its argument */
-if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)))
+if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs,
+virPCIDeviceGetAddress(pci
  continue;
  
  if (virPCIDeviceGetManaged(actual)) {

@@ -777,7 +778,8 @@ virHostdevPreparePCIDevicesImpl(virHostdevManagerPtr mgr,
  
  /* Unmanaged devices should already have been marked as

   * inactive: if that's the case, we can simply move on */
-if (virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)) {
+if (virPCIDeviceListFind(mgr->inactivePCIHostdevs,
+ virPCIDeviceGetAddress(pci))) {


Wondered at first why you were using a access function to get the 
pointer rather than just referencing it directly. I'd forgotten that 
virPCIDevice is one of those rare "private structures" in libvirt - 
defined only in a single .c file.


Anyway,

Reviewed-by: Laine Stump 



  VIR_DEBUG("Not detaching unmanaged PCI device %s",
virPCIDeviceGetName(pci));
  continue;
@@ -860,7 +862,8 @@ virHostdevPreparePCIDevicesImpl(virHostdevManagerPtr mgr,
   * there because 'pci' only contain address information and will
   * be released at the end of the function */
  pci = virPCIDeviceListGet(pcidevs, i);
-actual = virPCIDeviceListFind(mgr->activePCIHostdevs, pci);
+actual = virPCIDeviceListFind(mgr->activePCIHostdevs,
+  virPCIDeviceGetAddress(pci));
  
  VIR_DEBUG("Setting driver and domain information for PCI device %s",

virPCIDeviceGetName(pci));
@@ -992,7 +995,8 @@ virHostdevReAttachPCIDevicesImpl(virHostdevManagerPtr mgr,
   * information such as by which domain and driver it is used. As a
   * side effect, by looking it up we can also tell whether it was
   * really active in the first place */
-actual = virPCIDeviceListFind(mgr->activePCIHostdevs, pci);
+actual = virPCIDeviceListFind(mgr->activePCIHostdevs,
+  virPCIDeviceGetAddress(pci));
  if (actual) {
  const char *actual_drvname;
  const char *actual_domname;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 1554acffb6..9544275c31 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -705,7 +705,7 @@ virPCIDeviceSharesBusWithActive(virPCIDevicePtr dev, 
virPCIDevicePtr check, void
  return 0;
  
  /* same bus, but inactive, i.e. about to be assigned to guest */

-if (inactiveDevs && virPCIDeviceListFind(inactiveDevs, check))
+if (inactiveDevs && virPCIDeviceListFind(inactiveDevs, >address))
  return 0;
  
  return 1;

@@ -1022,7 +1022,7 @@ virPCIDeviceReset(virPCIDevicePtr dev,
  return -1;
  }
  
-if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) {

+if (activeDevs && virPCIDeviceListFind(activeDevs, >address)) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _("Not resetting active device %s"), dev->name);
  return -1;
@@ -1294,7 +1294,7 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
  if (virPCIProbeStubDriver(dev->stubDriver) < 0)
  return -1;
  
-if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) {

+if (activeDevs && virPCIDeviceListFind(activeDevs, >address)) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _("Not detaching active device %s"), dev->name);
  return -1;
@@ -1306,7 +1306,7 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
  /* Add *a copy of* the dev into list inactiveDevs, if
   * it's not already there.
   */
-if (inactiveDevs && !virPCIDeviceListFind(inactiveDevs, dev)) {
+if (inactiveDevs && !virPCIDeviceListFind(inactiveDevs, >address)) {
  VIR_DEBUG("Adding PCI device %s to inactive list", dev->name);
  if (virPCIDeviceListAddCopy(inactiveDevs, dev) < 0)
  return -1;
@@ -1324,7 +1324,7 @@ virPCIDeviceReattach(virPCIDevicePtr dev,
   virPCIDeviceListPtr activeDevs,
   virPCIDeviceListPtr inactiveDevs)
  {
-if (activeDevs && 

[PATCH RESEND 17/20] virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFind()

2021-01-18 Thread Daniel Henrique Barboza
Signed-off-by: Daniel Henrique Barboza 
---
 src/hypervisor/virhostdev.c | 12 
 src/util/virpci.c   | 16 
 src/util/virpci.h   |  2 +-
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index 402d7be42d..3bfb04c674 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -657,7 +657,8 @@ virHostdevReattachAllPCIDevices(virHostdevManagerPtr mgr,
 
 /* We need to look up the actual device because that's what
  * virPCIDeviceReattach() expects as its argument */
-if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)))
+if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs,
+virPCIDeviceGetAddress(pci
 continue;
 
 if (virPCIDeviceGetManaged(actual)) {
@@ -777,7 +778,8 @@ virHostdevPreparePCIDevicesImpl(virHostdevManagerPtr mgr,
 
 /* Unmanaged devices should already have been marked as
  * inactive: if that's the case, we can simply move on */
-if (virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci)) {
+if (virPCIDeviceListFind(mgr->inactivePCIHostdevs,
+ virPCIDeviceGetAddress(pci))) {
 VIR_DEBUG("Not detaching unmanaged PCI device %s",
   virPCIDeviceGetName(pci));
 continue;
@@ -860,7 +862,8 @@ virHostdevPreparePCIDevicesImpl(virHostdevManagerPtr mgr,
  * there because 'pci' only contain address information and will
  * be released at the end of the function */
 pci = virPCIDeviceListGet(pcidevs, i);
-actual = virPCIDeviceListFind(mgr->activePCIHostdevs, pci);
+actual = virPCIDeviceListFind(mgr->activePCIHostdevs,
+  virPCIDeviceGetAddress(pci));
 
 VIR_DEBUG("Setting driver and domain information for PCI device %s",
   virPCIDeviceGetName(pci));
@@ -992,7 +995,8 @@ virHostdevReAttachPCIDevicesImpl(virHostdevManagerPtr mgr,
  * information such as by which domain and driver it is used. As a
  * side effect, by looking it up we can also tell whether it was
  * really active in the first place */
-actual = virPCIDeviceListFind(mgr->activePCIHostdevs, pci);
+actual = virPCIDeviceListFind(mgr->activePCIHostdevs,
+  virPCIDeviceGetAddress(pci));
 if (actual) {
 const char *actual_drvname;
 const char *actual_domname;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 1554acffb6..9544275c31 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -705,7 +705,7 @@ virPCIDeviceSharesBusWithActive(virPCIDevicePtr dev, 
virPCIDevicePtr check, void
 return 0;
 
 /* same bus, but inactive, i.e. about to be assigned to guest */
-if (inactiveDevs && virPCIDeviceListFind(inactiveDevs, check))
+if (inactiveDevs && virPCIDeviceListFind(inactiveDevs, >address))
 return 0;
 
 return 1;
@@ -1022,7 +1022,7 @@ virPCIDeviceReset(virPCIDevicePtr dev,
 return -1;
 }
 
-if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) {
+if (activeDevs && virPCIDeviceListFind(activeDevs, >address)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Not resetting active device %s"), dev->name);
 return -1;
@@ -1294,7 +1294,7 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
 if (virPCIProbeStubDriver(dev->stubDriver) < 0)
 return -1;
 
-if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) {
+if (activeDevs && virPCIDeviceListFind(activeDevs, >address)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Not detaching active device %s"), dev->name);
 return -1;
@@ -1306,7 +1306,7 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
 /* Add *a copy of* the dev into list inactiveDevs, if
  * it's not already there.
  */
-if (inactiveDevs && !virPCIDeviceListFind(inactiveDevs, dev)) {
+if (inactiveDevs && !virPCIDeviceListFind(inactiveDevs, >address)) {
 VIR_DEBUG("Adding PCI device %s to inactive list", dev->name);
 if (virPCIDeviceListAddCopy(inactiveDevs, dev) < 0)
 return -1;
@@ -1324,7 +1324,7 @@ virPCIDeviceReattach(virPCIDevicePtr dev,
  virPCIDeviceListPtr activeDevs,
  virPCIDeviceListPtr inactiveDevs)
 {
-if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) {
+if (activeDevs && virPCIDeviceListFind(activeDevs, >address)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Not reattaching active device %s"), dev->name);
 return -1;
@@ -1684,7 +1684,7 @@ int
 virPCIDeviceListAdd(virPCIDeviceListPtr list,
 virPCIDevicePtr dev)
 {
-if