Re: [libvirt] [PATCH 03/12] virhostdev: Unify virDomainHostdevDef to virPCIDevice translation

2019-08-21 Thread Ján Tomko

On Tue, Aug 20, 2019 at 04:30:23PM +0200, Michal Privoznik wrote:

There are two places where we need to create virPCIDevice from
given virDomainHostdevDef. In both places the code is duplicated.
Move them into a single function and call it from those two
places.

Signed-off-by: Michal Privoznik 
---
src/util/virhostdev.c | 93 +--
1 file changed, 54 insertions(+), 39 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 6861b8a84e..e3f48a9a2e 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -212,6 +212,51 @@ virHostdevManagerGetDefault(void)
return virObjectRef(manager);
}

+/**
+ * virHostdevGetPCIHostDevice:
+ * @hostdev: domain hostdev definition
+ * @pci: returned PCI device
+ *
+ * For given @hostdev which represents a PCI device construct its
+ * virPCIDevice representation and returned it in @pci. If


s/returned/return/


+ * @hostdev does not represent a PCI device then @pci is set to
+ * NULL and 0 is returned.
+ *
+ * Returns: 0 on success (@pci might be NULL though),
+ * -1 otherwise (with error reported).
+ */


Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 03/12] virhostdev: Unify virDomainHostdevDef to virPCIDevice translation

2019-08-20 Thread Daniel Henrique Barboza




On 8/20/19 11:30 AM, Michal Privoznik wrote:

There are two places where we need to create virPCIDevice from
given virDomainHostdevDef. In both places the code is duplicated.
Move them into a single function and call it from those two
places.

Signed-off-by: Michal Privoznik 
---


Reviewed-by: Daniel Henrique Barboza 
Tested-by: Daniel Henrique Barboza 


  src/util/virhostdev.c | 93 +--
  1 file changed, 54 insertions(+), 39 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 6861b8a84e..e3f48a9a2e 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -212,6 +212,51 @@ virHostdevManagerGetDefault(void)
  return virObjectRef(manager);
  }
  
+/**

+ * virHostdevGetPCIHostDevice:
+ * @hostdev: domain hostdev definition
+ * @pci: returned PCI device
+ *
+ * For given @hostdev which represents a PCI device construct its
+ * virPCIDevice representation and returned it in @pci. If
+ * @hostdev does not represent a PCI device then @pci is set to
+ * NULL and 0 is returned.
+ *
+ * Returns: 0 on success (@pci might be NULL though),
+ * -1 otherwise (with error reported).
+ */
+static int
+virHostdevGetPCIHostDevice(const virDomainHostdevDef *hostdev,
+   virPCIDevicePtr *pci)
+{
+VIR_AUTOPTR(virPCIDevice) actual = NULL;
+const virDomainHostdevSubsysPCI *pcisrc = >source.subsys.u.pci;
+
+*pci = NULL;
+
+if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
+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);
+
+if (!actual)
+return -1;
+
+virPCIDeviceSetManaged(actual, hostdev->managed);
+
+if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
+virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_VFIO);
+else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN)
+virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_XEN);
+else
+virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_KVM);
+
+VIR_STEAL_PTR(*pci, actual);
+return 0;
+}
+
  static virPCIDeviceListPtr
  virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int 
nhostdevs)
  {
@@ -223,27 +268,13 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr 
*hostdevs, int nhostdevs)
  
  for (i = 0; i < nhostdevs; i++) {

  virDomainHostdevDefPtr hostdev = hostdevs[i];
-virDomainHostdevSubsysPCIPtr pcisrc = >source.subsys.u.pci;
  VIR_AUTOPTR(virPCIDevice) pci = NULL;
  
-if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)

-continue;
-if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
-continue;
+if (virHostdevGetPCIHostDevice(hostdev, ) < 0)
+return NULL;
  
-pci = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,

-  pcisrc->addr.slot, pcisrc->addr.function);
  if (!pci)
-return NULL;
-
-virPCIDeviceSetManaged(pci, hostdev->managed);
-
-if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
-virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO);
-else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN)
-virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN);
-else
-virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_KVM);
+continue;
  
  if (virPCIDeviceListAdd(pcidevs, pci) < 0)

  return NULL;
@@ -253,7 +284,6 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr 
*hostdevs, int nhostdevs)
  VIR_RETURN_PTR(pcidevs);
  }
  
-

  static int
  virHostdevPCISysfsPath(virDomainHostdevDefPtr hostdev,
 char **sysfs_path)
@@ -1067,7 +1097,6 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr,
   const char *drv_name,
   const char *dom_name)
  {
-virDomainHostdevDefPtr hostdev = NULL;
  size_t i;
  int ret = -1;
  
@@ -1078,32 +1107,18 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr,

  virObjectLock(mgr->inactivePCIHostdevs);
  
  for (i = 0; i < nhostdevs; i++) {

+const virDomainHostdevDef *hostdev = hostdevs[i];
  VIR_AUTOPTR(virPCIDevice) actual = NULL;
-virDomainHostdevSubsysPCIPtr pcisrc;
-hostdev = hostdevs[i];
-pcisrc = >source.subsys.u.pci;
  
-if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)

-continue;
-if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
-continue;
-
-actual = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
- pcisrc->addr.slot, pcisrc->addr.function);
+if 

[libvirt] [PATCH 03/12] virhostdev: Unify virDomainHostdevDef to virPCIDevice translation

2019-08-20 Thread Michal Privoznik
There are two places where we need to create virPCIDevice from
given virDomainHostdevDef. In both places the code is duplicated.
Move them into a single function and call it from those two
places.

Signed-off-by: Michal Privoznik 
---
 src/util/virhostdev.c | 93 +--
 1 file changed, 54 insertions(+), 39 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 6861b8a84e..e3f48a9a2e 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -212,6 +212,51 @@ virHostdevManagerGetDefault(void)
 return virObjectRef(manager);
 }
 
+/**
+ * virHostdevGetPCIHostDevice:
+ * @hostdev: domain hostdev definition
+ * @pci: returned PCI device
+ *
+ * For given @hostdev which represents a PCI device construct its
+ * virPCIDevice representation and returned it in @pci. If
+ * @hostdev does not represent a PCI device then @pci is set to
+ * NULL and 0 is returned.
+ *
+ * Returns: 0 on success (@pci might be NULL though),
+ * -1 otherwise (with error reported).
+ */
+static int
+virHostdevGetPCIHostDevice(const virDomainHostdevDef *hostdev,
+   virPCIDevicePtr *pci)
+{
+VIR_AUTOPTR(virPCIDevice) actual = NULL;
+const virDomainHostdevSubsysPCI *pcisrc = >source.subsys.u.pci;
+
+*pci = NULL;
+
+if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
+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);
+
+if (!actual)
+return -1;
+
+virPCIDeviceSetManaged(actual, hostdev->managed);
+
+if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
+virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_VFIO);
+else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN)
+virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_XEN);
+else
+virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_KVM);
+
+VIR_STEAL_PTR(*pci, actual);
+return 0;
+}
+
 static virPCIDeviceListPtr
 virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
 {
@@ -223,27 +268,13 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr 
*hostdevs, int nhostdevs)
 
 for (i = 0; i < nhostdevs; i++) {
 virDomainHostdevDefPtr hostdev = hostdevs[i];
-virDomainHostdevSubsysPCIPtr pcisrc = >source.subsys.u.pci;
 VIR_AUTOPTR(virPCIDevice) pci = NULL;
 
-if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
-continue;
-if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
-continue;
+if (virHostdevGetPCIHostDevice(hostdev, ) < 0)
+return NULL;
 
-pci = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
-  pcisrc->addr.slot, pcisrc->addr.function);
 if (!pci)
-return NULL;
-
-virPCIDeviceSetManaged(pci, hostdev->managed);
-
-if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
-virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO);
-else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN)
-virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN);
-else
-virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_KVM);
+continue;
 
 if (virPCIDeviceListAdd(pcidevs, pci) < 0)
 return NULL;
@@ -253,7 +284,6 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr 
*hostdevs, int nhostdevs)
 VIR_RETURN_PTR(pcidevs);
 }
 
-
 static int
 virHostdevPCISysfsPath(virDomainHostdevDefPtr hostdev,
char **sysfs_path)
@@ -1067,7 +1097,6 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr,
  const char *drv_name,
  const char *dom_name)
 {
-virDomainHostdevDefPtr hostdev = NULL;
 size_t i;
 int ret = -1;
 
@@ -1078,32 +1107,18 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr 
mgr,
 virObjectLock(mgr->inactivePCIHostdevs);
 
 for (i = 0; i < nhostdevs; i++) {
+const virDomainHostdevDef *hostdev = hostdevs[i];
 VIR_AUTOPTR(virPCIDevice) actual = NULL;
-virDomainHostdevSubsysPCIPtr pcisrc;
-hostdev = hostdevs[i];
-pcisrc = >source.subsys.u.pci;
 
-if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
-continue;
-if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
-continue;
-
-actual = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
- pcisrc->addr.slot, pcisrc->addr.function);
+if (virHostdevGetPCIHostDevice(hostdev, ) < 0)
+goto cleanup;
 
 if (!actual)
+continue;
+
+if (virPCIDeviceSetUsedBy(actual, drv_name,