[libvirt] [PATCH 00/33] Refactor all network device handling code
Daniel P. Berrange
berrange at redhat.com
Thu Nov 3 17:29:56 UTC 2011
This series is a major re-arrangement and de-duplication of
internal code for dealing with physical network interfaces.
Currently code for physical network interfaces is split
across the following files
- bridge.c: bridge management, TAP device management and
interface IPv4 address management
- network.c: socket address management, network bandwidth
and virtual port profile data management
- interface.c: some generic interface management and
macvtap/macvlan management
- macvtap.c: more macvtap/macvlan management and virtual
port profile interface setup code
- lxc/veth.c: veth device management and generic interface
management
Furthermore across these files
- Argument ordering is inconsistent - sometimes ifname is
the first parameter, sometimes it isn't
- Error handing is terribly inconsistent
1. Return errno values
2. Raise errors
3. Raise errors and return errno values
4. Raise errors, except when a parameter tells
it not to
- API naming is inconsistent. Some APIs have a vir prefix,
others have different prefixes, other don't even follow
a standard prefix, eg virSocketFormatAddr vs virSocketAddrMaks
vs virSocketGetPort
- The bridge.c APIs have an annoying 'brControl *' struct
that must be passed around just to avoid opening & closing
an unbound socket.
- Some APIs are implemented natively, while others call
out to external commands
- Duplication of functionality across APIs
- XML parsing & formatting code is in src/util instead
of src/conf
- Some of the API declarations are hidden behind #ifdefs
forcing yet more #ifdefs in the callers of the APIs
After applying this series, we get to a stage where the source
file split is:
- src/util/virnetdev.c: APIs relating to any type of interface
- src/util/virnetdevbridge.c: APIs relating to bridge interfaces
- src/util/virnetdevmacvlan.c: APIs relating to macvlan/macvtap interfaces
- src/util/virnetdevveth.c: APIs relating to veth interfaces
- src/util/virnetdevbandwidth.c: APIs for network bandwidth controls
- src/util/virnetdevvportprofile.c: APIs for 802.11Qb{g,h} port profiles
- src/util/virsocketaddr.c: the socket address management APIs
- src/conf/netdev_bandwidth_conf.c: Parsing/formatting bandwidth XML
- src/conf/netdev_vport_profile_conf.c: parsing/formatting 801.11Qb{g,h} profile XML
The following style is followed
- All APIs return -1 on error and raise a libvirt error. No exceptions
or flags to turn off errors
- Callers which don't want the raised error call virResetLastError
- All APIs are annotated with ATTRIBUTE_NONNULL and ATTRIBUTE_RETURN_CHECK
where appropriate
- The first parameter of all APIs operating on a network interface is
the interface name
- All APIs have a fixed name prefix which matches the source file. No
exceptions.
- All XML handling code is under src/conf/
- All APIs are unconditionally declared in header files, and stubbed
out with virReportSystemError(ENOSYS...) where unsupported.
- No functionality is duplicated across files
- External commands are avoided except in case of setting IPv4
addresses and network bandwidth controls
The diffstat is a little bit misleading, showing a slight growth in the
number of lines. This is primarily due to the extra GPL copyright header
lines in the new files, being much larger than those removed from old
files. Overall we have a decrease in actual real code, through removal
of duplicated APIs
b/configure.ac | 5
b/daemon/libvirtd.h | 1
b/daemon/remote.c | 1
b/libvirt.spec.in | 2
b/po/POTFILES.in | 12
b/src/Makefile.am | 26
b/src/conf/domain_conf.c | 59 -
b/src/conf/domain_conf.h | 20
b/src/conf/netdev_bandwidth_conf.c | 230 ++++
b/src/conf/netdev_bandwidth_conf.h | 37
b/src/conf/netdev_vport_profile_conf.c | 236 ++++
b/src/conf/netdev_vport_profile_conf.h | 39
b/src/conf/network_conf.c | 101 +-
b/src/conf/network_conf.h | 12
b/src/conf/nwfilter_conf.c | 16
b/src/conf/nwfilter_conf.h | 2
b/src/esx/esx_util.h | 2
b/src/libvirt_private.syms | 71 -
b/src/lxc/lxc_container.c | 9
b/src/lxc/lxc_controller.c | 7
b/src/lxc/lxc_driver.c | 42
b/src/network/bridge_driver.c | 192 +--
b/src/nwfilter/nwfilter_ebiptables_driver.c | 4
b/src/nwfilter/nwfilter_gentech_driver.c | 25
b/src/nwfilter/nwfilter_learnipaddr.c | 32
b/src/openvz/openvz_driver.c | 1
b/src/qemu/qemu_command.c | 86 -
b/src/qemu/qemu_command.h | 4
b/src/qemu/qemu_conf.c | 2
b/src/qemu/qemu_conf.h | 3
b/src/qemu/qemu_driver.c | 16
b/src/qemu/qemu_hotplug.c | 19
b/src/qemu/qemu_migration.c | 29
b/src/qemu/qemu_process.c | 15
b/src/qemu/qemu_process.h | 2
b/src/rpc/virnetsocket.c | 7
b/src/rpc/virnetsocket.h | 2
b/src/uml/uml_conf.c | 41
b/src/uml/uml_conf.h | 2
b/src/uml/uml_driver.c | 22
b/src/util/dnsmasq.c | 4
b/src/util/dnsmasq.h | 2
b/src/util/iptables.c | 20
b/src/util/iptables.h | 2
b/src/util/virnetdev.c | 1083 ++++++++++++++++++++++
b/src/util/virnetdev.h | 102 ++
b/src/util/virnetdevbandwidth.c | 265 +++++
b/src/util/virnetdevbandwidth.h | 53 +
b/src/util/virnetdevbridge.c | 527 ++++++++++
b/src/util/virnetdevbridge.h | 54 +
b/src/util/virnetdevmacvlan.c | 674 +++++++++++++
b/src/util/virnetdevmacvlan.h | 77 +
b/src/util/virnetdevtap.c | 301 ++++++
b/src/util/virnetdevtap.h | 45
b/src/util/virnetdevveth.c | 189 +++
b/src/util/virnetdevveth.h | 35
b/src/util/virnetdevvportprofile.c | 1073 +++++++++++++++++++++
b/src/util/virnetdevvportprofile.h | 95 +
b/src/util/virsocketaddr.c | 687 ++++++++++++++
b/src/util/virsocketaddr.h | 103 ++
b/src/vbox/vbox_tmpl.c | 10
b/tests/qemuxml2argvtest.c | 2
b/tests/qemuxmlnstest.c | 2
b/tests/sockettest.c | 20
b/tests/virnetsockettest.c | 1
b/tests/virnettlscontexttest.c | 6
b/tools/virsh.c | 6
src/lxc/veth.c | 319 ------
src/lxc/veth.h | 33
src/util/bridge.c | 845 -----------------
src/util/bridge.h | 120 --
src/util/interface.c | 1348 ---------------------------
src/util/interface.h | 91 -
src/util/macvtap.c | 1203 ------------------------
src/util/macvtap.h | 93 -
src/util/network.c | 1370 ----------------------------
src/util/network.h | 167 ---
77 files changed, 6302 insertions(+), 6159 deletions(-)
More information about the libvir-list
mailing list