[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 20/24] hostdev: Save netdev configuration of actual device




On 03/07/2016 12:24 PM, Andrea Bolognani wrote:
> We might be just fine looking up the information in pcidevs, but
> it wouldn't save us any trouble and it's better to be consistent.
> ---
>  src/util/virhostdev.c | 41 ++++++++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index a431f0a..03c3445 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -650,7 +650,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
>  
>      /* Step 8: Now set the original states for hostdev def */
>      for (i = 0; i < nhostdevs; i++) {
> -        virPCIDevicePtr pci;
> +        virPCIDevicePtr actual;
>          virDomainHostdevDefPtr hostdev = hostdevs[i];
>          virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci;
>  
> @@ -659,24 +659,27 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
>          if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
>              continue;
>  
> -        pci = virPCIDeviceListFindByIDs(pcidevs,
> -                                        pcisrc->addr.domain,
> -                                        pcisrc->addr.bus,
> -                                        pcisrc->addr.slot,
> -                                        pcisrc->addr.function);
> +        /* We need to look up the actual device because it's the one
> +         * that contains the information we care about (unbind_from_stub,
> +         * remove_slot, reprobe) */

When a device goes from the inactivePCIHostdevs list to the
activePCIHostdevs list "at some point in time" in the future - does it
do a similar save?

That is this change only grabs devices from the active list for the
save; whereas, prior to this change it would pull from all pcidevs

> +        actual = virPCIDeviceListFindByIDs(mgr->activePCIHostdevs,
> +                                           pcisrc->addr.domain,
> +                                           pcisrc->addr.bus,
> +                                           pcisrc->addr.slot,
> +                                           pcisrc->addr.function);
>  
>          /* Appropriate values for the unbind_from_stub, remove_slot
>           * and reprobe properties of the device were set earlier
>           * by virPCIDeviceDetach() */
> -        if (pci) {
> +        if (actual) {
>              VIR_DEBUG("Saving network configuration of PCI device %s",
> -                      virPCIDeviceGetName(pci));
> +                      virPCIDeviceGetName(actual));
>              hostdev->origstates.states.pci.unbind_from_stub =
> -                virPCIDeviceGetUnbindFromStub(pci);
> +                virPCIDeviceGetUnbindFromStub(actual);
>              hostdev->origstates.states.pci.remove_slot =
> -                virPCIDeviceGetRemoveSlot(pci);
> +                virPCIDeviceGetRemoveSlot(actual);
>              hostdev->origstates.states.pci.reprobe =
> -                virPCIDeviceGetReprobe(pci);
> +                virPCIDeviceGetReprobe(actual);
>          }
>      }
>  
> @@ -844,17 +847,17 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr,
>  
>          if (virHostdevIsPCINetDevice(hostdev)) {
>              virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci;
> -            virPCIDevicePtr pci;
> +            virPCIDevicePtr actual;
>  
> -            pci = virPCIDeviceListFindByIDs(pcidevs,
> -                                            pcisrc->addr.domain,
> -                                            pcisrc->addr.bus,
> -                                            pcisrc->addr.slot,
> -                                            pcisrc->addr.function);

So the previous loop took care of the activePCIHostdevs, correct?  And
this loop takes care of the inactivePCIHostdevs for reattachement?  I
think this part is correct - although is it worthy of splitting into
it's own separate patch?

John

Going to stop here for now (we have company).

> +            actual = virPCIDeviceListFindByIDs(mgr->inactivePCIHostdevs,
> +                                               pcisrc->addr.domain,
> +                                               pcisrc->addr.bus,
> +                                               pcisrc->addr.slot,
> +                                               pcisrc->addr.function);
>  
> -            if (pci) {
> +            if (actual) {
>                  VIR_DEBUG("Restoring network configuration of PCI device %s",
> -                          virPCIDeviceGetName(pci));
> +                          virPCIDeviceGetName(actual));
>                  virHostdevNetConfigRestore(hostdev, mgr->stateDir,
>                                             oldStateDir);
>              }
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]