On 08/21/2014 10:09 PM, Lin Ma wrote: > > >>>> "Lin Ma" <lma@xxxxxxxx> 8/20/2014 1:44 下午 >>> > > > >>>> Cole Robinson <crobinso@xxxxxxxxxx> 08/16/14 1:25 AM >>> >>...... >> I'd like to see this arranged in a bit different way. We already have similar >>> code to match a hostdev to its nodedev in details.py:lookup_nodedev. Please >>> move that code to a function like nodedev.py NodeDevice.compare_to_nodedev, >>> then you can even separate the logic and implement the function in PCIDevice, >>> USBDevice, etc. Bonus points for unit tests in tests/nodedev.py. That'll be >>> one patch >> >>I'll use NodeDevice.compare_to_nodedev instead of > VirtualHostDevice.is_conflict_hostdev function. >>In compare_to_nodedev function, I'll iterate all of hostdevs of all of vms, > call lookup_nodedev for every >>hostdev to get the matched nodedev, Compare this nodedev with the passed > nodedev which representing >>the pci/usb device that users picked up while adding hardware.(put the > comparson logic in PCIDevice >>and USBDevice) >> >> >>> You'll still need details.py:lookup_nodedev to iterate over all the >>> nodedevices and compare against the passed hostdev, but the comparison logic >>> will be removed. >> >>Do you mean I should use the code of lookup_nodedev function in > compare_to_nodedev function? >>If so, why ? If you want to make the comparson happens between nodedev and > nodedev, Just calling >>lookup_nodedev function to get the matched nodedev and comparing this nodedev > with passed nodedev, just like what >>I mentioned above. > Perhaps because of it's impossible to call lookup_nodedev function directly > in NodeDevice.compare_to_nodedev function, > So you suggest to use the code of lookup_nodedev in > NodeDevice.compare_to_nodedev, Am I right ? > Sorry, I might not be fully following the question here. But the reason I suggest separating the 'iterate over all VMs' part and 'compare nodedev and hostdev' part is a few reasons: - The comparison then becomes very easy and quick to unit test, hence my suggestion to look at tests/nodedev.py. - The comparison function will have two consumers: details.py and addhardware.py. But if you lump it in with the 'iterate over all VMs' behavior, it's only useful in addhardware.py. So your recent patches to the list still aren't optimal IMO. Thanks, Cole _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
