On Mon, Jul 23, 2012 at 03:14:15PM -0400, Laine Stump wrote:
On 07/23/2012 10:26 AM, Hendrik Schwartke wrote:
I'm currently working on a feature that dumps the network traffic of a virtual interface over the streaming api of libvirt. The goal is to offer the possiblity to debug network related problems of guests without the need to have access to a shell on the host. E.g. "virsh iface-dumptraffic virbr0 icmp | tcpdump -n -r -" prints all icmp traffic on virbr0 to stdout. The code below is only a proof of concept, but it should be possible to recognize what I have in mind and how I want to implement it.
So I would appreciate any comments on that!
It's definitely interesting functionality, but putting it in netcf_driver.c doesn't seem like the right place, since it doesn't use any netcf functionality, and could just as easily be made to work without it. On the other hand, since (the way you've implemented it) it is related to host interfaces, it kind of makes logical sense for it to be named virInterface<something>, and the backend of all those commands is currently in netcf_driver.c.
When I first saw the patch, my initial reaction was that it may be better suited to making two separate APIs, virNetworkDumpTraffic(), which would have the same functionality you're proposing, but would determine the name of the device to dump by looking in the config for the named network, and virDomainDumpTraffic() which would dump the network traffic for a specific domain (conceptually this should relieve the programmer from learning the name of the physical device). However, I then realized that:
1) in the case of a network, not all networks even have a bridge device associated with them - some only have a pool of physical devices, and those devices can't even be tapped into anyway (macvtap devices don't support iptables, ebtables, or tapping in for tcpdump).
I don't think that's neccessarily a problem. It is perfectly OK to have only certain configurations supported.
2) in the case of a domain, there could be multiple <interface>s, and they have no permanent logical name, so the user of this new API would still end up needing to grab the XML for the domain and parse out the <target dev='xxx'/> for each interface, and that interface name would in the end be a name on the host not the guest (so the name of the domain would really be irrelevant to this new API), *AND* again any type='direct' (macvtap) or type='hostdev' (PCI passthrough) interface could not be tapped.
This is no different to the usage scenario for virDomainInterfaceStats, so I don't think that's an argument against virDomainInterfaceCapture() In addidition there is the "alias" identifier given to each interface which can also be used.
So, I have no good alternative - no solutions here, just maybe fodder for more discussion. (hmm, maybe this new functionality could be put in a separate .c file that is linked to netcf_driver.c (and would theoretically be linked to any other interface driver that needed the same implementation of the same functionality).
I say 90% of the functionality should go into a src/util/virnetdevcapture.{c,h} file. We can then expose this via the virNetwork, virDomain & virInterface APIs as desired. In fact it could even make sense to expose it via the virNodeDevice APIs, since I can imagine wanting to be able to capture traffic without actually configuring a NIC. You might say there is overlap by having the APIs in all these different levels, but it is useful from a access control POV. eg, if there is an API at the virDomainPtr level, we can apply access controls per-guest. 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 :|