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 :|