On 04/15/2015 01:29 PM, Huanle Han wrote: > Fix for such a case: > 1. Domain A and B xml contain the same SRIOV net hostdev(<interface > type='hostdev' /> with same pci address). > 2. virsh start A (Successfully, and configure the SRIOV net with > custom mac) > 3. virsh start B (Fail because of the hostdev used by domain A or other > reason.) > In step 3, 'virHostdevNetConfigRestore' is called for the hostdev > which is still used by domain A. It makes the mac/vlan of the SRIOV net > change. > > Code Change in this fix: > 1. As the pci used by other domain have been removed from > 'pcidevs' in previous loop, we only restore the nic config for > the hostdev still in 'pcidevs'(used by this domain) > 2. wrap a function 'virHostdevIsPCINetDevice', which detect whether the > hostdev is a pci net device or not. > 3. update the comments to make it more clear > > Signed-off-by: Huanle Han <hanxueluo@xxxxxxxxx> > --- > src/util/virhostdev.c | 70 ++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 47 insertions(+), 23 deletions(-) > This seems reasonable - although it should have been two patches - the first just to create virHostdevIsPCINetDevice and the second to fix the issue and update the comments... Something I can take care of for you before pushing... Hopefully Laine can also take a quick look at it and verify the actions since he's most familiar with the SRIOV code path... John > diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c > index f583e54..a2719d3 100644 > --- a/src/util/virhostdev.c > +++ b/src/util/virhostdev.c > @@ -350,6 +350,14 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev, > return ret; > } > > +static int > +virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev) > +{ > + return hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && > + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && > + hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && > + hostdev->parent.data.net; > +} > > static int > virHostdevNetConfigVirtPortProfile(const char *linkdev, int vf, > @@ -481,10 +489,7 @@ virHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev, > /* This is only needed for PCI devices that have been defined > * using <interface type='hostdev'>. For all others, it is a NOP. > */ > - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || > - hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || > - hostdev->parent.type != VIR_DOMAIN_DEVICE_NET || > - !hostdev->parent.data.net) > + if (!virHostdevIsPCINetDevice(hostdev)) > return 0; > > isvf = virHostdevIsVirtualFunction(hostdev); > @@ -604,16 +609,11 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr, > * the network device, set the netdev config */ > for (i = 0; i < nhostdevs; i++) { > virDomainHostdevDefPtr hostdev = hostdevs[i]; > - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) > + if (!virHostdevIsPCINetDevice(hostdev)) > continue; > - if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) > - continue; > - if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && > - hostdev->parent.data.net) { > - if (virHostdevNetConfigReplace(hostdev, uuid, > - hostdev_mgr->stateDir) < 0) { > - goto resetvfnetconfig; > - } > + if (virHostdevNetConfigReplace(hostdev, uuid, > + hostdev_mgr->stateDir) < 0) { > + goto resetvfnetconfig; > } > last_processed_hostdev_vf = i; > } > @@ -781,19 +781,20 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, > goto cleanup; > } > > - /* Again 4 loops; mark all devices as inactive before reset > + /* Here are 4 loops; mark all devices as inactive before reset > * them and reset all the devices before re-attach. > * Attach mac and port profile parameters to devices > */ > + > + /* Loop 1: delete the copy of the dev from pcidevs if it's used by > + * other domain. Or delete it from activePCIHostDevs if it had > + * been used by this domain. > + */ > i = 0; > while (i < virPCIDeviceListCount(pcidevs)) { > virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i); > virPCIDevicePtr activeDev = NULL; > > - /* delete the copy of the dev from pcidevs if it's used by > - * other domain. Or delete it from activePCIHostDevs if it had > - * been used by this domain. > - */ > activeDev = virPCIDeviceListFind(hostdev_mgr->activePCIHostdevs, dev); > if (activeDev) { > const char *usedby_drvname; > @@ -815,13 +816,33 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, > */ > > /* > - * For SRIOV net host devices, unset mac and port profile before > - * reset and reattach device > + * Loop 2: For SRIOV net host devices used by this domain, unset mac and port profile before > + * resetting and reattaching device > */ > - for (i = 0; i < nhostdevs; i++) > - virHostdevNetConfigRestore(hostdevs[i], hostdev_mgr->stateDir, > - oldStateDir); > + for (i = 0; i < nhostdevs; i++) { > + virDomainHostdevDefPtr hostdev = hostdevs[i]; > > + if (virHostdevIsPCINetDevice(hostdev)) { > + virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; > + virPCIDevicePtr dev = NULL; > + dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, > + pcisrc->addr.slot, pcisrc->addr.function); > + if (dev) { > + if (virPCIDeviceListFind(pcidevs, dev)) { > + virHostdevNetConfigRestore(hostdev, hostdev_mgr->stateDir, > + oldStateDir); > + } > + } else { > + virErrorPtr err = virGetLastError(); > + VIR_ERROR(_("Failed to new PCI device: %s"), > + err ? err->message : _("unknown error")); > + virResetError(err); > + } > + virPCIDeviceFree(dev); > + } > + } > + > + /* Loop 3: reset pci device used by this domain */ > for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { > virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i); > > @@ -834,6 +855,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, > } > } > > + /* Loop 4: reattach pci devices used by this domain > + * and steal all the devices from pcidevs > + */ > while (virPCIDeviceListCount(pcidevs) > 0) { > virPCIDevicePtr dev = virPCIDeviceListStealIndex(pcidevs, 0); > virHostdevReattachPCIDevice(dev, hostdev_mgr); > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list