[libvirt] [PATCH v2 1/3] virhostdev: introduce virHostdevResetAllPCIDevices

[Michal Privoznik]

On 7/23/19 7:35 PM, Daniel Henrique Barboza wrote:
> This code that executes virPCIDeviceReset in all virPCIDevicePtr
> objects of a given virPCIDeviceListPtr list is replicated twice
> in the code. Putting it in a helper function helps with
> readability.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> ---
>   src/util/virhostdev.c | 54 +++++++++++++++++++++++--------------------
>   1 file changed, 29 insertions(+), 25 deletions(-)
> 
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index a3647a6cf4..4dd24a8f65 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -613,6 +613,32 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev,
>       }
>   }
>   
> +static int
> +virHostdevResetAllPCIDevices(virHostdevManagerPtr mgr,
> +                             virPCIDeviceListPtr pcidevs)
> +{
> +    int ret = 0;
> +    size_t i;
> +
> +    for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
> +        virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
> +
> +        /* We can avoid looking up the actual device here, because performing
> +         * a PCI reset on a device doesn't require any information other than
> +         * the address, which 'pci' already contains */
> +        VIR_DEBUG("Resetting PCI device %s", virPCIDeviceGetName(pci));
> +        if (virPCIDeviceReset(pci, mgr->activePCIHostdevs,
> +                              mgr->inactivePCIHostdevs) < 0) {
> +            VIR_ERROR(_("Failed to reset PCI device: %s"),
> +                      virGetLastErrorMessage());
> +            virResetLastError();

I don't think this is a good idea. VIR_ERROR() logs the error message, 
but then virResetLastError() clears the thread local error object and 
thus if a PCI fails to reset the caller is left with 'uknown error' and 
sysadmin has to go to logs to find out what's happening. In fact, I 
think all those virResetLastError() calls should be removed from 
virHostdevReAttachPCIDevices(). I'll fix it here and in 3/3 too.

> +            ret = -1;
> +        }
> +    }
> +
> +    return ret;
> +}
> +

Michal