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

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

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 <mprivozn@xxxxxxxxxx>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
Tested-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>

>   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 = &hostdev->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 = &hostdev->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, &pci) < 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 = &hostdev->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, &actual) < 0)
> +            goto cleanup;
>   
>           if (!actual)
> +            continue;
> +
> +        if (virPCIDeviceSetUsedBy(actual, drv_name, dom_name) < 0)
>               goto cleanup;
>   
> -        virPCIDeviceSetManaged(actual, hostdev->managed);
> -        virPCIDeviceSetUsedBy(actual, drv_name, dom_name);
> -
> -        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);
> -
>           /* Setup the original states for the PCI device */
>           virPCIDeviceSetUnbindFromStub(actual, hostdev->origstates.states.pci.unbind_from_stub);
>           virPCIDeviceSetRemoveSlot(actual, hostdev->origstates.states.pci.remove_slot);

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list