[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 00/33] Refactor all network device handling code



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(-)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]