On Fri, Nov 06, 2015 at 11:39:26AM -0500, John Ferlan wrote:
It's partially a reposting, partially some new stuff, and perhaps a v2 of something else.
Patch 1 is Dan's revert patch - just because it's easier to have it done first.
Patches 2 & 3 just adjust comments - no functional changes
Patch 4 adds a check for root in virNetDevGetFeatures (rather than both of the callers). If not root, an empty bitmap would be returned (somewhat similar to what a platform without the features does). I'm not against moving the check to before the virBitmapNew; however, I was considering the case of xml parsing which does a virBitmapFree, then just calls virNetDevGetFeatures. Not filling in the *out with either NULL or an empty bitmap would result in that caller referencing a net.features which no longer exists. If the check should be done before the virBitmapNew, then an "*out = NULL" would need to be added before returning 0 just to be 'safe'...
As an aside, it just felt better to add one check rather than two.
Patch 5 restore the virNetDevSetupControl from the reverted patch back into virNetDevSendEthtoolIoctl. Since we cannot be called unless we're privileged and the ioctl shouldn't fail any with EPERM, that was also removed from the list of filtered failure errno values.
ACK to all 5. THanks for picking this work up. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|