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

On 03/10/2016 06:02 PM, John Ferlan wrote: So I went through this one again (new day, fresh start, partially clear mind). First off - perhaps something for the previous documentation patch - PrepareDevices is called during the qemuProcessLaunch processing (eg domain startup) for all known host devices. It's also called during qemuDomainAttachHostPCIDevice or libxlDomainAttachHostPCIDevice (hotplug) for the *one* new device to be attached. The purpose of the function is to take the passed hostdev list, move the devices to a managed active host device list for the guest. Devices that do not have the managed attribute are not processed. So after step 6, all the pcidevs are on the activePCIHostdevs list. Theoretically at least ;-). None are on the inactivePCIHostdevs list. Why in step 7 do we run through pcidevs, to find the device on the activePCIHostdev list in order to set the UsedBy of the actual device on the active list. Why not run the 'activePCIHostdev' list here too? Since we really don't need pcidevs any more - can we delete it now? E.G. Step 9 it... Steps 7 & 8 don't really need pcidevs, right? There is no error path for either. I'm not asking for a patch - just making sure I'm reading things right! I'll answer my own question - because this is the Prepare function and we don't have to worry about it since everything is on the active list. So, I believe this part is correct albeit a bit confusing... More documentation thoughts... Because what exists now for the function doesn't really make sense - it's only describing one variable. Anyway, the purpose of this function is to take the device(s) passed on the hostdev list and return them to the host (?if they are managed?). This function is called either from hotplug device remove, hotplug device attach failure path, or from qemuProcessStop during process shutdown. When called from the host device remove or attach failure paths, there is only 1 hostdev in the list. ... Prior to patch 15, the pcidev's was a copy of the "active list", now it's a list of all pcidevs found. Hopefully on either one of the two lists. That is I hope a found hostdev couldn't be on neither list. Prior to patch 15, step1 would either remove the device from pcidevs or from the activeList depending on the matching driver name. When done, the pcidevs would contain the list of devices just removed from the activeList. Note that virPCIDeviceListDel will repeat the active list virPCIDeviceListFind (essentially)... After patch 15, the pcidevs gets reduced for devices not on the activeList or without a matching drv/dom name. Whether or not that is on the inactiveList isn't checked, but assumed. Step 2 is I believe the antecedent of step 4 during the Prepare path. Step 4 would be called after Reset and before placing devices on active list replacing the net config for any PCINet device... Prior to patch 15, step2 would walk the hostdev's list looking for devices on the pcidevs list (eg. the ones removed from the activeList). It would then take any PCINet device and restore it's config. After patch 15 and with these changes, walk the inactiveList and perform the same call. This is not the devices we just removed from the activeList, so I don't think using the inactiveList in this case is right. Before patch 15, step 3 would take any of the devices we removed from the activeList and perform a reset on them. After patch 15, step 3 would do a similar function assuming that no device could not be on neither list. Hopefully this all makes sense - I keep reading it and going back and fort between old and new code. The comment left in the code after step 1 is most helpful... John