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

[Thomas Huth]

On 20.02.2018 23:35, Eric Blake wrote:
> On 02/20/2018 11:40 AM, Thomas Huth wrote:
>> "-net dump" has been marked as deprecated since QEMU v2.10, since it
>> only works with the deprecated 'vlan' parameter (or hubs). 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 
>> Reviewed-by: Eric Blake 
>> Signed-off-by: Thomas Huth 
>> ---
> 
>> +++ b/qapi/net.json
>> @@ -39,8 +39,10 @@
>>   #
>>   # 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. Possible values in version
>> +#    2.11: 'user', 'tap', 'vde', 'socket', 'hubport', 'bridge',
>> +#  'dump', 'l2tpv3', 'netmap', 'vhost-user'
>> +#    2.12: 'dump' dropped
>>   #
> 
> That's a bit fuzzy, especially since the command has been around since
> 0.14.  It might be easier to word it as:
> 
> @type: the type of network backend. Possible values are listed in
> NetClientDriver.

That's a really good idea to avoid that we have to maintain the list
multiple times...

> Then defer the actual listing by version...
> 
>> @@ -468,7 +453,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' ] }
> 
> ...to here
... but 'none' and 'nic' are not valid for netdev_add. Well, I guess I
can simply write something like "Possible values are listed in
NetClientDriver (excluding 'none' and 'nic')". I'll send a v3 with that.

 Thomas


PS: By the way, now that you've mentioned this, I think "dump" was also
not valid for netdev_add, so the comment was even wrong there.

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

[Eric Blake]

On 02/20/2018 11:40 AM, Thomas Huth wrote:

"-net dump" has been marked as deprecated since QEMU v2.10, since it
only works with the deprecated 'vlan' parameter (or hubs). 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 
Reviewed-by: Eric Blake 
Signed-off-by: Thomas Huth 
---



+++ b/qapi/net.json
@@ -39,8 +39,10 @@
  #
  # 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. Possible values in version
+#2.11: 'user', 'tap', 'vde', 'socket', 'hubport', 'bridge',
+#  'dump', 'l2tpv3', 'netmap', 'vhost-user'
+#2.12: 'dump' dropped
  #


That's a bit fuzzy, especially since the command has been around since 
0.14.  It might be easier to word it as:


@type: the type of network backend. Possible values are listed in 
NetClientDriver.


Then defer the actual listing by version...


@@ -468,7 +453,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' ] }


...to here (although this type only mentions 2.7 as its starting point). 
 (Hmm, I should see if it is worth reviving my attempted patches to 
properly QAPI-fy netdev_add into using a proper schema description - 
when I last proposed it, we ditched it at the last minute because of 
minor incompatibilities in parsing between a QAPI parse and the manual 
parse).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

[Thomas Huth]

"-net dump" has been marked as deprecated since QEMU v2.10, since it
only works with the deprecated 'vlan' parameter (or hubs). 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 
Reviewed-by: Eric Blake 
Signed-off-by: Thomas Huth 
---
 net/dump.c  | 102 ++--
 net/net.c   |   9 +
 qapi/net.json   |  29 
 qemu-doc.texi   |   6 
 qemu-options.hx |   8 -
 5 files changed, 9 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, , 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(>ds, , 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(>ds, iov, cnt);
-}
-
-static void dumpclient_cleanup(NetClientState *nc)
-{
-DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
-
-dump_cleanup(>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 = >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, );
-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(_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(>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 dd80f1b..cbd553d 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