Re: [Qemu-devel] [PATCH v1 5/8] net: Remove the deprecated way of dumping network packets

[Paolo Bonzini]

On 19/02/2018 10:15, Thomas Huth wrote:
> "-net dump" has been marked as deprecated since QEMU v2.10, since it
> only works with the deprecated 'vlan' parameter.

Or with hubs. :)  But still it's good to deprecate it.

> Network dumping should
> be done with "-object filter-dump" nowadays instead. Since nobody
> complained so far about the deprecation message, let's finally get rid
> of "-net dump" now.

Reviewed-by: Paolo Bonzini <address@hidden>

Paolo

> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  net/dump.c      | 102 
> ++------------------------------------------------------
>  net/net.c       |   9 +----
>  qapi/net.json   |  27 +++------------
>  qemu-doc.texi   |   6 ----
>  qemu-options.hx |   8 -----
>  5 files changed, 7 insertions(+), 145 deletions(-)
> 
> diff --git a/net/dump.c b/net/dump.c
> index 15df9a4..f16c354 100644
> --- a/net/dump.c
> +++ b/net/dump.c
> @@ -109,7 +109,7 @@ static int net_dump_state_init(DumpState *s, const char 
> *filename,
>  
>      fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY | O_BINARY, 0644);
>      if (fd < 0) {
> -        error_setg_errno(errp, errno, "-net dump: can't open %s", filename);
> +        error_setg_errno(errp, errno, "net dump: can't open %s", filename);
>          return -1;
>      }
>  
> @@ -122,7 +122,7 @@ static int net_dump_state_init(DumpState *s, const char 
> *filename,
>      hdr.linktype = 1;
>  
>      if (write(fd, &hdr, sizeof(hdr)) < sizeof(hdr)) {
> -        error_setg_errno(errp, errno, "-net dump write error");
> +        error_setg_errno(errp, errno, "net dump write error");
>          close(fd);
>          return -1;
>      }
> @@ -136,104 +136,6 @@ static int net_dump_state_init(DumpState *s, const char 
> *filename,
>      return 0;
>  }
>  
> -/* Dumping via VLAN netclient */
> -
> -struct DumpNetClient {
> -    NetClientState nc;
> -    DumpState ds;
> -};
> -typedef struct DumpNetClient DumpNetClient;
> -
> -static ssize_t dumpclient_receive(NetClientState *nc, const uint8_t *buf,
> -                                  size_t size)
> -{
> -    DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
> -    struct iovec iov = {
> -        .iov_base = (void *)buf,
> -        .iov_len = size
> -    };
> -
> -    return dump_receive_iov(&dc->ds, &iov, 1);
> -}
> -
> -static ssize_t dumpclient_receive_iov(NetClientState *nc,
> -                                      const struct iovec *iov, int cnt)
> -{
> -    DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
> -
> -    return dump_receive_iov(&dc->ds, iov, cnt);
> -}
> -
> -static void dumpclient_cleanup(NetClientState *nc)
> -{
> -    DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
> -
> -    dump_cleanup(&dc->ds);
> -}
> -
> -static NetClientInfo net_dump_info = {
> -    .type = NET_CLIENT_DRIVER_DUMP,
> -    .size = sizeof(DumpNetClient),
> -    .receive = dumpclient_receive,
> -    .receive_iov = dumpclient_receive_iov,
> -    .cleanup = dumpclient_cleanup,
> -};
> -
> -int net_init_dump(const Netdev *netdev, const char *name,
> -                  NetClientState *peer, Error **errp)
> -{
> -    int len, rc;
> -    const char *file;
> -    char def_file[128];
> -    const NetdevDumpOptions *dump;
> -    NetClientState *nc;
> -    DumpNetClient *dnc;
> -
> -    assert(netdev->type == NET_CLIENT_DRIVER_DUMP);
> -    dump = &netdev->u.dump;
> -
> -    assert(peer);
> -
> -    error_report("'-net dump' is deprecated. "
> -                 "Please use '-object filter-dump' instead.");
> -
> -    if (dump->has_file) {
> -        file = dump->file;
> -    } else {
> -        int id;
> -        int ret;
> -
> -        ret = net_hub_id_for_client(peer, &id);
> -        assert(ret == 0); /* peer must be on a hub */
> -
> -        snprintf(def_file, sizeof(def_file), "qemu-vlan%d.pcap", id);
> -        file = def_file;
> -    }
> -
> -    if (dump->has_len) {
> -        if (dump->len > INT_MAX) {
> -            error_setg(errp, "invalid length: %"PRIu64, dump->len);
> -            return -1;
> -        }
> -        len = dump->len;
> -    } else {
> -        len = 65536;
> -    }
> -
> -    nc = qemu_new_net_client(&net_dump_info, peer, "dump", name);
> -    snprintf(nc->info_str, sizeof(nc->info_str),
> -             "dump to %s (len=%d)", file, len);
> -
> -    dnc = DO_UPCAST(DumpNetClient, nc, nc);
> -    rc = net_dump_state_init(&dnc->ds, file, len, errp);
> -    if (rc) {
> -        qemu_del_net_client(nc);
> -    }
> -    return rc;
> -}
> -
> -/* Dumping via filter */
> -
>  #define TYPE_FILTER_DUMP "filter-dump"
>  
>  #define FILTER_DUMP(obj) \
> diff --git a/net/net.c b/net/net.c
> index bb63d82..af0b3e7 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -63,7 +63,6 @@ static QTAILQ_HEAD(, NetClientState) net_clients;
>  const char *host_net_devices[] = {
>      "tap",
>      "socket",
> -    "dump",
>  #ifdef CONFIG_NET_BRIDGE
>      "bridge",
>  #endif
> @@ -967,7 +966,6 @@ static int (* const 
> net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
>  #ifdef CONFIG_NETMAP
>          [NET_CLIENT_DRIVER_NETMAP]    = net_init_netmap,
>  #endif
> -        [NET_CLIENT_DRIVER_DUMP]      = net_init_dump,
>  #ifdef CONFIG_NET_BRIDGE
>          [NET_CLIENT_DRIVER_BRIDGE]    = net_init_bridge,
>  #endif
> @@ -993,8 +991,7 @@ static int net_client_init1(const void *object, bool 
> is_netdev, Error **errp)
>          netdev = object;
>          name = netdev->id;
>  
> -        if (netdev->type == NET_CLIENT_DRIVER_DUMP ||
> -            netdev->type == NET_CLIENT_DRIVER_NIC ||
> +        if (netdev->type == NET_CLIENT_DRIVER_NIC ||
>              !net_client_init_fun[netdev->type]) {
>              error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
>                         "a netdev backend type");
> @@ -1036,10 +1033,6 @@ static int net_client_init1(const void *object, bool 
> is_netdev, Error **errp)
>              legacy.type = NET_CLIENT_DRIVER_VDE;
>              legacy.u.vde = opts->u.vde;
>              break;
> -        case NET_LEGACY_OPTIONS_TYPE_DUMP:
> -            legacy.type = NET_CLIENT_DRIVER_DUMP;
> -            legacy.u.dump = opts->u.dump;
> -            break;
>          case NET_LEGACY_OPTIONS_TYPE_BRIDGE:
>              legacy.type = NET_CLIENT_DRIVER_BRIDGE;
>              legacy.u.bridge = opts->u.bridge;
> diff --git a/qapi/net.json b/qapi/net.json
> index 1238ba5..15ca2ed 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -39,8 +39,8 @@
>  #
>  # Add a network backend.
>  #
> -# @type: the type of network backend.  Current valid values are 'user', 
> 'tap',
> -#        'vde', 'socket', 'dump' and 'bridge'
> +# @type: the type of network backend.  Current valid values are for example
> +#        'user', 'tap', 'vde', 'socket', 'hubport' and 'bridge'
>  #
>  # @id: the name of the new network backend
>  #
> @@ -372,23 +372,6 @@
>      '*mode':  'uint16' } }
>  
>  ##
> -# @NetdevDumpOptions:
> -#
> -# Dump VLAN network traffic to a file.
> -#
> -# @len: per-packet size limit (64k default). Understands [TGMKkb]
> -# suffixes.
> -#
> -# @file: dump file path (default is qemu-vlan0.pcap)
> -#
> -# Since: 1.2
> -##
> -{ 'struct': 'NetdevDumpOptions',
> -  'data': {
> -    '*len':  'size',
> -    '*file': 'str' } }
> -
> -##
>  # @NetdevBridgeOptions:
>  #
>  # Connect a host TAP network interface to a host bridge device.
> @@ -468,7 +451,7 @@
>  # Since: 2.7
>  ##
>  { 'enum': 'NetClientDriver',
> -  'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde', 'dump',
> +  'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
>              'bridge', 'hubport', 'netmap', 'vhost-user' ] }
>  
>  ##
> @@ -495,7 +478,6 @@
>      'l2tpv3':   'NetdevL2TPv3Options',
>      'socket':   'NetdevSocketOptions',
>      'vde':      'NetdevVdeOptions',
> -    'dump':     'NetdevDumpOptions',
>      'bridge':   'NetdevBridgeOptions',
>      'hubport':  'NetdevHubPortOptions',
>      'netmap':   'NetdevNetmapOptions',
> @@ -530,7 +512,7 @@
>  ##
>  { 'enum': 'NetLegacyOptionsType',
>    'data': ['none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
> -           'dump', 'bridge', 'netmap', 'vhost-user'] }
> +           'bridge', 'netmap', 'vhost-user'] }
>  
>  ##
>  # @NetLegacyOptions:
> @@ -550,7 +532,6 @@
>      'l2tpv3':   'NetdevL2TPv3Options',
>      'socket':   'NetdevSocketOptions',
>      'vde':      'NetdevVdeOptions',
> -    'dump':     'NetdevDumpOptions',
>      'bridge':   'NetdevBridgeOptions',
>      'netmap':   'NetdevNetmapOptions',
>      'vhost-user': 'NetdevVhostUserOptions' } }
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 137f581..55954b0 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2716,12 +2716,6 @@ that can be specified with the ``-device'' parameter.
>  The drive addr argument is replaced by the the addr argument
>  that can be specified with the ``-device'' parameter.
>  
> address@hidden -net dump (since 2.10.0)
> -
> -The ``--net dump'' argument is now replaced with the
> -``-object filter-dump'' argument which works in combination
> -with the modern ``-netdev`` backends instead.
> -
>  @subsection -usbdevice (since 2.10.0)
>  
>  The ``-usbdevice DEV'' argument is now a synonym for setting
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b81b53b..f6172e5 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2009,8 +2009,6 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>      "                configure or create an on-board (or machine default) 
> NIC and\n"
>      "                connect it either to VLAN 'n' or the netdev 'nd' (for 
> pluggable\n"
>      "                NICs please use '-device devtype,netdev=nd' instead)\n"
> -    "-net dump[,vlan=n][,file=f][,len=n]\n"
> -    "                dump traffic on vlan 'n' to file 'f' (max n bytes per 
> packet)\n"
>      "-net none       use it alone to have zero network devices. If no -net 
> option\n"
>      "                is provided, the default is '-net nic -net user'\n"
>      "-net ["
> @@ -2458,12 +2456,6 @@ qemu -m 512 -object 
> memory-backend-file,id=mem,size=512M,mem-path=/hugetlbfs,sha
>       -device virtio-net-pci,netdev=net0
>  @end example
>  
> address@hidden -net dump[,address@hidden,address@hidden,address@hidden
> -Dump network traffic on VLAN @var{n} to file @var{file} 
> (@file{qemu-vlan0.pcap} by default).
> -At most @var{len} bytes (64k by default) per packet are stored. The file 
> format is
> -libpcap, so it can be analyzed with tools such as tcpdump or Wireshark.
> -Note: For devices created with '-netdev', use '-object filter-dump,...' 
> instead.
> -
>  @item -net none
>  Indicate that no network devices should be configured. It is used to
>  override the default configuration (@option{-net nic -net user}) which
>