Hi all!

As preparation for further development of TAP live local migration
(passing open fds through unix socket), here is a refactoring
of initialization code, to improve its readability and get rid
of duplication.

Below are the initialization flow diagrams showing the changes.

BEFORE REFACTORING:
==================

```
net_init_tap()
    |
    +-- if (tap->fd)
    |   +-- duplicated logic*
    |   +-- net_init_tap_one()
    |
    +-- else if (tap->fds)
    |   +-- for each fd:
    |       +-- duplicated logic*
    |       +-- net_init_tap_one()
    |
    +-- else if (tap->helper)
    |   +-- duplicated logic*
    |   +-- net_init_bridge()
    |
    +-- else (normal case)
        +-- for each queue:
            +-- net_tap_init()
            +-- net_init_tap_one()

net_init_bridge()
    |
    +-- duplicated logic*
    +-- net_tap_fd_init()

net_init_tap_one()
    |
    +-- net_tap_fd_init()

net_tap_init()
    |
    +-- tap_open()

net_tap_fd_init()
    |
    +-- qemu_new_net_client()
    +-- Initialize TAPState

* duplicated logic: set fd nonblocking + probe vnet_hdr
```

AFTER REFACTORING:
=================

```
net_init_tap()
    |
    +-- if (tap->fd)
    |   +-- net_tap_from_monitor_fd()
    |
    +-- else if (tap->fds)
    |   +-- for each fd:
    |       +-- net_tap_from_monitor_fd()
    |
    +-- else if (tap->helper)
    |   +-- net_init_bridge()
    |
    +-- else (normal case)
        +-- net_tap_open()

net_tap_open()
    |
    +-- for each queue:
        +-- net_tap_open_one()

net_tap_open_one()
    |
    +-- tap_open()
    +-- net_tap_fd_init_common()

net_tap_from_monitor_fd()
    |
    +-- net_tap_fd_init_external()

net_tap_fd_init_external()
    |
    +-- net_tap_fd_init_common()

net_init_bridge()
    |
    +-- net_tap_fd_init_external()

net_tap_fd_init_common()
    |
    +-- qemu_new_net_client()
    +-- Initialize TAPState
```

Solved problems:

- duplicated logic to handle external
  file descriptors (set nonblocking, probe vnet_hdr)

- duplication between tap/helper case in
  net_init_tap() and net_init_bridge()

- confusing naming and functionality spread between functions (we had
  net_init_tap(), together with net_tap_init(); also main central
  function was net_init_tap_one(), and part of its logic (not clear
  why) moved to separate net_tap_fd_init()),


Vladimir Sementsov-Ogievskiy (19):
  net/tap: net_init_tap_one(): add return value
  net/tap: add set_fd_nonblocking() helper
  net/tap: tap_set_sndbuf(): add return value
  net/tap: net_init_tap_one(): drop extra error propagation
  net/tap: net_init_tap_one(): move parameter checking earlier
  net/tap: net_init_tap(): refactor parameter checking
  net/tap: net_init_tap(): drop extra variable vhostfdname
  net/tap: move local variables related to the latter case to else
    branch
  net/tap: use glib strings vector and g_strsplit for fds case
  net/tap: drop extra tap_fd_get_ifname() call
  net/tap: net_init_tap_one(): refactor to use netdev as first arg
  net/tap: net_init_tap_one(): support bridge
  net/tap: net_init_bridge(): support tap
  net/tap: refactor net_tap_init() into net_tap_open_one()
  net/tap: introduce net_tap_open()
  net/tap: introduce net_tap_fd_init_external()
  net/tap: introduce net_tap_from_monitor_fd() helper
  net/tap: split net_tap_setup_vhost() separate function
  net/tap: drop net_tap_fd_init()

 net/tap-linux.c |   5 +-
 net/tap.c       | 548 ++++++++++++++++++++++--------------------------
 net/tap_int.h   |   2 +-
 3 files changed, 256 insertions(+), 299 deletions(-)

-- 
2.48.1


Reply via email to