* [PATCH v3 0/4] Introducing QMP query-netdevs command
@ 2020-09-01 18:23 Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 1/4] qapi: net: Add " Alexey Kirillov
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-01 18:23 UTC (permalink / raw)
To: Eric Blake, Markus Armbruster, Thomas Huth, Jason Wang
Cc: Laurent Vivier, Michael S. Tsirkin, Stefan Weil, qemu-devel,
Vincenzo Maffione, yc-core, Paolo Bonzini, Samuel Thibault,
Giuseppe Lettieri, Luigi Rizzo
This patch series introduces a new QMP command "query-netdevs" to get
information about currently attached backend network devices (netdevs).
Also, since the "info_str" field of "NetClientState" is now deprecated,
we no longer use it for netdevs, only for NIC/hubports.
The HMP command "info network" now also uses the new QMP command inside.
Usage example:
-> { "execute": "query-netdevs" }
<- { "return": [
{
"listen": "127.0.0.1:90",
"type": "socket",
"peer-id": "hub0port1",
"id": "__org.qemu.net1"
},
{
"script": "/etc/qemu-ifup",
"downscript": "/etc/qemu-ifdown",
"ifname": "tap0",
"type": "tap",
"peer-id": "net5",
"vnet_hdr": true,
"id": "tap0"
},
{
"ipv6": true,
"ipv4": true,
"host": "10.0.2.2",
"ipv6-dns": "fec0::3",
"ipv6-prefix": "fec0::",
"net": "10.0.2.0/255.255.255.0",
"ipv6-host": "fec0::2",
"type": "user",
"peer-id": "net0",
"dns": "10.0.2.3",
"hostfwd": [
{
"str": "tcp::20004-:22"
}
],
"ipv6-prefixlen": 64,
"id": "netdev0",
"restrict": false
}
]
}
v2->v3:
- Remove NIC and hubports from query-netdevs.
- Remove several fields from NetdevInfo since they are unnecessary.
- Rename field @peer to @peer-id.
- Add support of vhost-vdpa.
- Keep "info_str" for NIC/hubports, but remove it for netdevs.
v1->v2:
- Rewrite HMP "info network" to get information from results of QMP command.
- Remove obsolete field "info_str" from "NetClientState".
Alexey Kirillov (4):
qapi: net: Add query-netdevs command
tests: Add tests for query-netdevs command
hmp: Use QMP query-netdevs in hmp_info_network
net: Do not use legacy info_str for backends
include/net/net.h | 4 +-
net/clients.h | 1 +
net/hub.c | 4 +-
net/hub.h | 2 +-
net/l2tpv3.c | 21 ++-
net/net.c | 213 ++++++++++++++++++++++++++++++-
net/netmap.c | 13 ++
net/slirp.c | 128 ++++++++++++++++++-
net/socket.c | 91 ++++++++++---
net/tap-win32.c | 10 +-
net/tap.c | 107 ++++++++++++++--
net/vde.c | 39 +++++-
net/vhost-user.c | 20 ++-
net/vhost-vdpa.c | 15 ++-
qapi/net.json | 68 ++++++++++
tests/qtest/meson.build | 3 +
tests/qtest/test-query-netdevs.c | 117 +++++++++++++++++
17 files changed, 797 insertions(+), 59 deletions(-)
create mode 100644 tests/qtest/test-query-netdevs.c
--
2.25.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-01 18:23 [PATCH v3 0/4] Introducing QMP query-netdevs command Alexey Kirillov
@ 2020-09-01 18:23 ` Alexey Kirillov
2020-09-02 11:23 ` Markus Armbruster
2020-09-08 14:29 ` Markus Armbruster
2020-09-01 18:23 ` [PATCH v3 2/4] tests: Add tests for " Alexey Kirillov
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-01 18:23 UTC (permalink / raw)
To: Eric Blake, Markus Armbruster, Thomas Huth, Jason Wang
Cc: Laurent Vivier, Michael S. Tsirkin, Stefan Weil, qemu-devel,
Vincenzo Maffione, yc-core, Paolo Bonzini, Samuel Thibault,
Giuseppe Lettieri, Luigi Rizzo
Add a qmp command that provides information about currently attached
backend network devices and their configuration.
Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
---
include/net/net.h | 1 +
net/l2tpv3.c | 19 +++++++
net/net.c | 32 ++++++++++++
net/netmap.c | 13 +++++
net/slirp.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++
net/socket.c | 71 ++++++++++++++++++++++++++
net/tap-win32.c | 9 ++++
net/tap.c | 103 +++++++++++++++++++++++++++++++++++--
net/vde.c | 26 ++++++++++
net/vhost-user.c | 18 +++++--
net/vhost-vdpa.c | 14 ++++++
qapi/net.json | 68 +++++++++++++++++++++++++
12 files changed, 492 insertions(+), 8 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index e7ef42d62b..04d51ac215 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -92,6 +92,7 @@ struct NetClientState {
char *model;
char *name;
char info_str[256];
+ NetdevInfo *stored_config;
unsigned receive_disabled : 1;
NetClientDestructor *destructor;
unsigned int queue_index;
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 55fea17c0f..f4e45e7b28 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -535,6 +535,7 @@ int net_init_l2tpv3(const Netdev *netdev,
struct addrinfo hints;
struct addrinfo *result = NULL;
char *srcport, *dstport;
+ NetdevL2TPv3Options *stored;
nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name);
@@ -726,6 +727,24 @@ int net_init_l2tpv3(const Netdev *netdev,
l2tpv3_read_poll(s, true);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_CLIENT_DRIVER_L2TPV3;
+ stored = &nc->stored_config->u.l2tpv3;
+
+ memcpy(stored, l2tpv3, sizeof(NetdevL2TPv3Options));
+
+ stored->src = g_strdup(l2tpv3->src);
+ stored->dst = g_strdup(l2tpv3->dst);
+
+ if (l2tpv3->has_srcport) {
+ stored->srcport = g_strdup(l2tpv3->srcport);
+ }
+
+ if (l2tpv3->has_dstport) {
+ stored->dstport = g_strdup(l2tpv3->dstport);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"l2tpv3: connected");
return 0;
diff --git a/net/net.c b/net/net.c
index bbaedb3c7a..923e3d0bc6 100644
--- a/net/net.c
+++ b/net/net.c
@@ -54,6 +54,7 @@
#include "sysemu/sysemu.h"
#include "net/filter.h"
#include "qapi/string-output-visitor.h"
+#include "qapi/clone-visitor.h"
/* Net bridge is currently not supported for W32. */
#if !defined(_WIN32)
@@ -351,6 +352,7 @@ static void qemu_free_net_client(NetClientState *nc)
}
g_free(nc->name);
g_free(nc->model);
+ qapi_free_NetdevInfo(nc->stored_config);
if (nc->destructor) {
nc->destructor(nc);
}
@@ -1250,6 +1252,36 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
return filter_list;
}
+NetdevInfoList *qmp_query_netdevs(Error **errp)
+{
+ NetdevInfoList *list = NULL;
+ NetClientState *nc;
+
+ QTAILQ_FOREACH(nc, &net_clients, next) {
+ /*
+ * Only look at netdevs (backend network devices), not for each queue
+ * or NIC / hubport
+ */
+ if (nc->stored_config) {
+ NetdevInfoList *node = g_new0(NetdevInfoList, 1);
+
+ node->value = QAPI_CLONE(NetdevInfo, nc->stored_config);
+ g_free(node->value->id); /* Need to dealloc default empty id */
+ node->value->id = g_strdup(nc->name);
+
+ node->value->has_peer_id = nc->peer != NULL;
+ if (node->value->has_peer_id) {
+ node->value->peer_id = g_strdup(nc->peer->name);
+ }
+
+ node->next = list;
+ list = node;
+ }
+ }
+
+ return list;
+}
+
void hmp_info_network(Monitor *mon, const QDict *qdict)
{
NetClientState *nc, *peer;
diff --git a/net/netmap.c b/net/netmap.c
index 350f097f91..71feacb92c 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -411,6 +411,7 @@ int net_init_netmap(const Netdev *netdev,
NetClientState *nc;
Error *err = NULL;
NetmapState *s;
+ NetdevNetmapOptions *stored;
nmd = netmap_open(netmap_opts, &err);
if (err) {
@@ -427,6 +428,18 @@ int net_init_netmap(const Netdev *netdev,
pstrcpy(s->ifname, sizeof(s->ifname), netmap_opts->ifname);
netmap_read_poll(s, true); /* Initially only poll for reads. */
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_CLIENT_DRIVER_NETMAP;
+ stored = &nc->stored_config->u.netmap;
+
+ stored->ifname = g_strdup(netmap_opts->ifname);
+
+ if (netmap_opts->has_devname) {
+ stored->has_devname = true;
+ stored->devname = g_strdup(netmap_opts->devname);
+ }
+
return 0;
}
diff --git a/net/slirp.c b/net/slirp.c
index 77042e6df7..54c33d1173 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -346,6 +346,14 @@ static SaveVMHandlers savevm_slirp_state = {
.load_state = net_slirp_state_load,
};
+#define APPEND_STRINGLIST(tail, new_val) \
+ do { \
+ *(tail) = g_new0(StringList, 1); \
+ (*(tail))->value = g_new0(String, 1); \
+ (*(tail))->value->str = g_strdup((new_val)); \
+ (tail) = &((*(tail))->next); \
+ } while (0)
+
static int net_slirp_init(NetClientState *peer, const char *model,
const char *name, int restricted,
bool ipv4, const char *vnetwork, const char *vhost,
@@ -378,6 +386,9 @@ static int net_slirp_init(NetClientState *peer, const char *model,
int shift;
char *end;
struct slirp_config_str *config;
+ NetdevUserOptions *stored;
+ StringList **stored_hostfwd;
+ StringList **stored_guestfwd;
if (!ipv4 && (vnetwork || vhost || vnameserver)) {
error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
@@ -553,6 +564,112 @@ static int net_slirp_init(NetClientState *peer, const char *model,
nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_CLIENT_DRIVER_USER;
+ stored = &nc->stored_config->u.user;
+
+ if (vhostname) {
+ stored->has_hostname = true;
+ stored->hostname = g_strdup(vhostname);
+ }
+
+ stored->has_q_restrict = true;
+ stored->q_restrict = restricted;
+
+ stored->has_ipv4 = true;
+ stored->ipv4 = ipv4;
+
+ stored->has_ipv6 = true;
+ stored->ipv6 = ipv6;
+
+ if (ipv4) {
+ uint8_t *net_bytes = (uint8_t *)&net;
+ uint8_t *mask_bytes = (uint8_t *)&mask;
+
+ stored->has_net = true;
+ stored->net = g_strdup_printf("%d.%d.%d.%d/%d.%d.%d.%d",
+ net_bytes[0], net_bytes[1],
+ net_bytes[2], net_bytes[3],
+ mask_bytes[0], mask_bytes[1],
+ mask_bytes[2], mask_bytes[3]);
+
+ stored->has_host = true;
+ stored->host = g_strdup(inet_ntoa(host));
+ }
+
+ if (tftp_export) {
+ stored->has_tftp = true;
+ stored->tftp = g_strdup(tftp_export);
+ }
+
+ if (bootfile) {
+ stored->has_bootfile = true;
+ stored->bootfile = g_strdup(bootfile);
+ }
+
+ if (vdhcp_start) {
+ stored->has_dhcpstart = true;
+ stored->dhcpstart = g_strdup(vdhcp_start);
+ }
+
+ if (ipv4) {
+ stored->has_dns = true;
+ stored->dns = g_strdup(inet_ntoa(dns));
+ }
+
+ if (dnssearch) {
+ stored->has_dnssearch = true;
+ StringList **stored_list = &stored->dnssearch;
+
+ for (int i = 0; dnssearch[i]; i++) {
+ APPEND_STRINGLIST(stored_list, dnssearch[i]);
+ }
+ }
+
+ if (vdomainname) {
+ stored->has_domainname = true;
+ stored->domainname = g_strdup(vdomainname);
+ }
+
+ if (ipv6) {
+ char addrstr[INET6_ADDRSTRLEN];
+ const char *res;
+
+ stored->has_ipv6_prefix = true;
+ stored->ipv6_prefix = g_strdup(vprefix6);
+
+ stored->has_ipv6_prefixlen = true;
+ stored->ipv6_prefixlen = vprefix6_len;
+
+ res = inet_ntop(AF_INET6, &ip6_host,
+ addrstr, sizeof(addrstr));
+
+ stored->has_ipv6_host = true;
+ stored->ipv6_host = g_strdup(res);
+
+ res = inet_ntop(AF_INET6, &ip6_dns,
+ addrstr, sizeof(addrstr));
+
+ stored->has_ipv6_dns = true;
+ stored->ipv6_dns = g_strdup(res);
+ }
+
+ if (smb_export) {
+ stored->has_smb = true;
+ stored->smb = g_strdup(smb_export);
+ }
+
+ if (vsmbserver) {
+ stored->has_smbserver = true;
+ stored->smbserver = g_strdup(vsmbserver);
+ }
+
+ if (tftp_server_name) {
+ stored->has_tftp_server_name = true;
+ stored->tftp_server_name = g_strdup(tftp_server_name);
+ }
+
snprintf(nc->info_str, sizeof(nc->info_str),
"net=%s,restrict=%s", inet_ntoa(net),
restricted ? "on" : "off");
@@ -582,14 +699,23 @@ static int net_slirp_init(NetClientState *peer, const char *model,
s->poll_notifier.notify = net_slirp_poll_notify;
main_loop_poll_add_notifier(&s->poll_notifier);
+ stored_hostfwd = &stored->hostfwd;
+ stored_guestfwd = &stored->guestfwd;
+
for (config = slirp_configs; config; config = config->next) {
if (config->flags & SLIRP_CFG_HOSTFWD) {
if (slirp_hostfwd(s, config->str, errp) < 0) {
goto error;
+ } else {
+ stored->has_hostfwd = true;
+ APPEND_STRINGLIST(stored_hostfwd, config->str);
}
} else {
if (slirp_guestfwd(s, config->str, errp) < 0) {
goto error;
+ } else {
+ stored->has_guestfwd = true;
+ APPEND_STRINGLIST(stored_guestfwd, config->str);
}
}
}
diff --git a/net/socket.c b/net/socket.c
index 2d21fddd9c..4a60de08e3 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -342,6 +342,7 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
NetSocketState *s;
SocketAddress *sa;
SocketAddressType sa_type;
+ NetdevSocketOptions *stored;
sa = socket_local_address(fd, errp);
if (!sa) {
@@ -385,8 +386,19 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
net_socket_rs_init(&s->rs, net_socket_rs_finalize, false);
net_socket_read_poll(s, true);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_CLIENT_DRIVER_SOCKET;
+ stored = &nc->stored_config->u.socket;
+
+ stored->has_fd = true;
+ stored->fd = g_strdup_printf("%d", fd);
+
/* mcast: save bound address as dst */
if (is_connected && mcast != NULL) {
+ stored->has_mcast = true;
+ stored->mcast = g_strdup(mcast);
+
s->dgram_dst = saddr;
snprintf(nc->info_str, sizeof(nc->info_str),
"socket: fd=%d (cloned mcast=%s:%d)",
@@ -428,6 +440,7 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
{
NetClientState *nc;
NetSocketState *s;
+ NetdevSocketOptions *stored;
nc = qemu_new_net_client(&net_socket_info, peer, model, name);
@@ -447,6 +460,15 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
} else {
qemu_set_fd_handler(s->fd, NULL, net_socket_connect, s);
}
+
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_CLIENT_DRIVER_SOCKET;
+ stored = &nc->stored_config->u.socket;
+
+ stored->has_fd = true;
+ stored->fd = g_strdup_printf("%d", fd);
+
return s;
}
@@ -483,6 +505,7 @@ static void net_socket_accept(void *opaque)
struct sockaddr_in saddr;
socklen_t len;
int fd;
+ NetdevSocketOptions *stored;
for(;;) {
len = sizeof(saddr);
@@ -498,6 +521,13 @@ static void net_socket_accept(void *opaque)
s->fd = fd;
s->nc.link_down = false;
net_socket_connect(s);
+
+ /* Store additional startup parameters (extend net_socket_listen_init) */
+ stored = &s->nc.stored_config->u.socket;
+
+ stored->has_fd = true;
+ stored->fd = g_strdup_printf("%d", fd);
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"socket: connection from %s:%d",
inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
@@ -513,6 +543,7 @@ static int net_socket_listen_init(NetClientState *peer,
NetSocketState *s;
struct sockaddr_in saddr;
int fd, ret;
+ NetdevSocketOptions *stored;
if (parse_host_port(&saddr, host_str, errp) < 0) {
return -1;
@@ -549,6 +580,15 @@ static int net_socket_listen_init(NetClientState *peer,
net_socket_rs_init(&s->rs, net_socket_rs_finalize, false);
qemu_set_fd_handler(s->listen_fd, net_socket_accept, NULL, s);
+
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_CLIENT_DRIVER_SOCKET;
+ stored = &nc->stored_config->u.socket;
+
+ stored->has_listen = true;
+ stored->listen = g_strdup(host_str);
+
return 0;
}
@@ -561,6 +601,7 @@ static int net_socket_connect_init(NetClientState *peer,
NetSocketState *s;
int fd, connected, ret;
struct sockaddr_in saddr;
+ NetdevSocketOptions *stored;
if (parse_host_port(&saddr, host_str, errp) < 0) {
return -1;
@@ -598,6 +639,12 @@ static int net_socket_connect_init(NetClientState *peer,
return -1;
}
+ /* Store additional startup parameters (extend net_socket_fd_init) */
+ stored = &s->nc.stored_config->u.socket;
+
+ stored->has_connect = true;
+ stored->connect = g_strdup(host_str);
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"socket: connect to %s:%d",
inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
@@ -615,6 +662,7 @@ static int net_socket_mcast_init(NetClientState *peer,
int fd;
struct sockaddr_in saddr;
struct in_addr localaddr, *param_localaddr;
+ NetdevSocketOptions *stored;
if (parse_host_port(&saddr, host_str, errp) < 0) {
return -1;
@@ -643,6 +691,19 @@ static int net_socket_mcast_init(NetClientState *peer,
s->dgram_dst = saddr;
+ /* Store additional startup parameters (extend net_socket_fd_init) */
+ stored = &s->nc.stored_config->u.socket;
+
+ if (!stored->has_mcast) {
+ stored->has_mcast = true;
+ stored->mcast = g_strdup(host_str);
+ }
+
+ if (localaddr_str) {
+ stored->has_localaddr = true;
+ stored->localaddr = g_strdup(localaddr_str);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"socket: mcast=%s:%d",
inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
@@ -660,6 +721,7 @@ static int net_socket_udp_init(NetClientState *peer,
NetSocketState *s;
int fd, ret;
struct sockaddr_in laddr, raddr;
+ NetdevSocketOptions *stored;
if (parse_host_port(&laddr, lhost, errp) < 0) {
return -1;
@@ -698,6 +760,15 @@ static int net_socket_udp_init(NetClientState *peer,
s->dgram_dst = raddr;
+ /* Store additional startup parameters (extend net_socket_fd_init) */
+ stored = &s->nc.stored_config->u.socket;
+
+ stored->has_localaddr = true;
+ stored->localaddr = g_strdup(lhost);
+
+ stored->has_udp = true;
+ stored->udp = g_strdup(rhost);
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"socket: udp=%s:%d",
inet_ntoa(raddr.sin_addr), ntohs(raddr.sin_port));
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 2b5dcda36e..20ba0b1dc8 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -768,6 +768,7 @@ static int tap_win32_init(NetClientState *peer, const char *model,
NetClientState *nc;
TAPState *s;
tap_win32_overlapped_t *handle;
+ NetdevTapOptions *stored;
if (tap_win32_open(&handle, ifname) < 0) {
printf("tap: Could not open '%s'\n", ifname);
@@ -778,6 +779,14 @@ static int tap_win32_init(NetClientState *peer, const char *model,
s = DO_UPCAST(TAPState, nc, nc);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_CLIENT_DRIVER_TAP;
+ stored = &nc->stored_config->u.tap;
+
+ stored->has_ifname = true;
+ stored->ifname = g_strdup(ifname);
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"tap: ifname=%s", ifname);
diff --git a/net/tap.c b/net/tap.c
index 14dc904fca..7a7cf4caea 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -585,6 +585,7 @@ int net_init_bridge(const Netdev *netdev, const char *name,
const char *helper, *br;
TAPState *s;
int fd, vnet_hdr;
+ NetdevBridgeOptions *stored;
assert(netdev->type == NET_CLIENT_DRIVER_BRIDGE);
bridge = &netdev->u.bridge;
@@ -605,6 +606,21 @@ int net_init_bridge(const Netdev *netdev, const char *name,
}
s = net_tap_fd_init(peer, "bridge", name, fd, vnet_hdr);
+ /* Store startup parameters */
+ s->nc.stored_config = g_new0(NetdevInfo, 1);
+ s->nc.stored_config->type = NET_CLIENT_DRIVER_BRIDGE;
+ stored = &s->nc.stored_config->u.bridge;
+
+ if (br) {
+ stored->has_br = true;
+ stored->br = g_strdup(br);
+ }
+
+ if (helper) {
+ stored->has_helper = true;
+ stored->helper = g_strdup(helper);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s,br=%s", helper,
br);
@@ -652,11 +668,13 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
const char *model, const char *name,
const char *ifname, const char *script,
const char *downscript, const char *vhostfdname,
- int vnet_hdr, int fd, Error **errp)
+ int vnet_hdr, int fd, NetdevInfo **common_stored,
+ Error **errp)
{
Error *err = NULL;
TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
int vhostfd;
+ NetdevTapOptions *stored;
tap_set_sndbuf(s->fd, tap, &err);
if (err) {
@@ -664,12 +682,65 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
return;
}
+ /* Store startup parameters */
+ if (!*common_stored) {
+ *common_stored = g_new0(NetdevInfo, 1);
+ (*common_stored)->type = NET_CLIENT_DRIVER_TAP;
+ s->nc.stored_config = *common_stored;
+ }
+ stored = &(*common_stored)->u.tap;
+
+ if (tap->has_sndbuf && !stored->has_sndbuf) {
+ stored->has_sndbuf = true;
+ stored->sndbuf = tap->sndbuf;
+ }
+
+ if (vnet_hdr && !stored->has_vnet_hdr) {
+ stored->has_vnet_hdr = true;
+ stored->vnet_hdr = true;
+ }
+
if (tap->has_fd || tap->has_fds) {
+ if (!stored->has_fds) {
+ stored->has_fds = true;
+ stored->fds = g_strdup_printf("%d", fd);
+ } else {
+ char *tmp_s = stored->fds;
+ stored->fds = g_strdup_printf("%s:%d", stored->fds, fd);
+ g_free(tmp_s);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "fd=%d", fd);
} else if (tap->has_helper) {
+ if (!stored->has_helper) {
+ stored->has_helper = true;
+ stored->helper = g_strdup(tap->helper);
+ }
+
+ if (!stored->has_br) {
+ stored->has_br = true;
+ stored->br = tap->has_br ? g_strdup(tap->br) :
+ g_strdup(DEFAULT_BRIDGE_INTERFACE);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s",
tap->helper);
} else {
+ if (ifname && !stored->has_ifname) {
+ stored->has_ifname = true;
+ stored->ifname = g_strdup(ifname);
+ }
+
+ if (script && !stored->has_script) {
+ stored->has_script = true;
+ stored->script = g_strdup(script);
+ }
+
+ if (downscript && !stored->has_downscript) {
+ stored->has_downscript = true;
+ stored->downscript = g_strdup(downscript);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"ifname=%s,script=%s,downscript=%s", ifname, script,
downscript);
@@ -685,9 +756,20 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
VhostNetOptions options;
+ stored->has_vhost = true;
+ stored->vhost = true;
+
+ if (tap->has_vhostforce && tap->vhostforce) {
+ stored->has_vhostforce = true;
+ stored->vhostforce = true;
+ }
+
options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
options.net_backend = &s->nc;
if (tap->has_poll_us) {
+ stored->has_poll_us = true;
+ stored->poll_us = tap->poll_us;
+
options.busyloop_timeout = tap->poll_us;
} else {
options.busyloop_timeout = 0;
@@ -727,6 +809,15 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
}
options.opaque = (void *)(uintptr_t)vhostfd;
+ if (!stored->has_vhostfds) {
+ stored->has_vhostfds = true;
+ stored->vhostfds = g_strdup_printf("%d", vhostfd);
+ } else {
+ char *tmp_s = stored->vhostfds;
+ stored->vhostfds = g_strdup_printf("%s:%d", stored->fds, vhostfd);
+ g_free(tmp_s);
+ }
+
s->vhost_net = vhost_net_init(&options);
if (!s->vhost_net) {
if (tap->has_vhostforce && tap->vhostforce) {
@@ -779,6 +870,7 @@ int net_init_tap(const Netdev *netdev, const char *name,
const char *vhostfdname;
char ifname[128];
int ret = 0;
+ NetdevInfo *common_stored = NULL; /* will store configuration */
assert(netdev->type == NET_CLIENT_DRIVER_TAP);
tap = &netdev->u.tap;
@@ -822,7 +914,7 @@ int net_init_tap(const Netdev *netdev, const char *name,
net_init_tap_one(tap, peer, "tap", name, NULL,
script, downscript,
- vhostfdname, vnet_hdr, fd, &err);
+ vhostfdname, vnet_hdr, fd, &common_stored, &err);
if (err) {
error_propagate(errp, err);
return -1;
@@ -884,7 +976,7 @@ int net_init_tap(const Netdev *netdev, const char *name,
net_init_tap_one(tap, peer, "tap", name, ifname,
script, downscript,
tap->has_vhostfds ? vhost_fds[i] : NULL,
- vnet_hdr, fd, &err);
+ vnet_hdr, fd, &common_stored, &err);
if (err) {
error_propagate(errp, err);
ret = -1;
@@ -927,7 +1019,7 @@ free_fail:
net_init_tap_one(tap, peer, "bridge", name, ifname,
script, downscript, vhostfdname,
- vnet_hdr, fd, &err);
+ vnet_hdr, fd, &common_stored, &err);
if (err) {
error_propagate(errp, err);
close(fd);
@@ -966,7 +1058,8 @@ free_fail:
net_init_tap_one(tap, peer, "tap", name, ifname,
i >= 1 ? "no" : script,
i >= 1 ? "no" : downscript,
- vhostfdname, vnet_hdr, fd, &err);
+ vhostfdname, vnet_hdr, fd,
+ &common_stored, &err);
if (err) {
error_propagate(errp, err);
close(fd);
diff --git a/net/vde.c b/net/vde.c
index 99189cccb6..c0ab2bb65c 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -84,6 +84,7 @@ static int net_vde_init(NetClientState *peer, const char *model,
VDECONN *vde;
char *init_group = (char *)group;
char *init_sock = (char *)sock;
+ NetdevVdeOptions *stored;
struct vde_open_args args = {
.port = port,
@@ -108,6 +109,31 @@ static int net_vde_init(NetClientState *peer, const char *model,
qemu_set_fd_handler(vde_datafd(s->vde), vde_to_qemu, NULL, s);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_CLIENT_DRIVER_VDE;
+ stored = &nc->stored_config->u.vde;
+
+ if (sock) {
+ stored->has_sock = true;
+ stored->sock = g_strdup(sock);
+ }
+
+ if (port) {
+ stored->has_port = true;
+ stored->port = port;
+ }
+
+ if (group) {
+ stored->has_group = true;
+ stored->group = g_strdup(group);
+ }
+
+ if (mode) {
+ stored->has_mode = true;
+ stored->mode = mode;
+ }
+
return 0;
}
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 17532daaf3..aa2dc53179 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -307,14 +307,15 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
}
static int net_vhost_user_init(NetClientState *peer, const char *device,
- const char *name, Chardev *chr,
- int queues)
+ const char *name, const char *chardev,
+ Chardev *chr, int queues)
{
Error *err = NULL;
NetClientState *nc, *nc0 = NULL;
NetVhostUserState *s = NULL;
VhostUserState *user;
int i;
+ NetdevVhostUserOptions *stored;
assert(name);
assert(queues > 0);
@@ -351,6 +352,16 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
assert(s->vhost_net);
+ /* Store startup parameters */
+ nc0->stored_config = g_new0(NetdevInfo, 1);
+ nc0->stored_config->type = NET_CLIENT_DRIVER_VHOST_USER;
+ stored = &nc0->stored_config->u.vhost_user;
+
+ stored->chardev = g_strdup(chardev);
+
+ stored->has_queues = true;
+ stored->queues = queues;
+
return 0;
err:
@@ -442,5 +453,6 @@ int net_init_vhost_user(const Netdev *netdev, const char *name,
return -1;
}
- return net_vhost_user_init(peer, "vhost_user", name, chr, queues);
+ return net_vhost_user_init(peer, "vhost_user", name,
+ vhost_user_opts->chardev, chr, queues);
}
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index bc0e0d2d35..088033cfc9 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -179,8 +179,22 @@ static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
VhostVDPAState *s;
int vdpa_device_fd = -1;
int ret = 0;
+ NetdevVhostVDPAOptions *stored;
+
assert(name);
nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device, name);
+
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_CLIENT_DRIVER_VHOST_VDPA;
+ stored = &nc->stored_config->u.vhost_vdpa;
+
+ stored->has_vhostdev = true;
+ stored->vhostdev = g_strdup(vhostdev);
+
+ stored->has_queues = true;
+ stored->queues = 1; /* TODO: change when support multiqueue */
+
snprintf(nc->info_str, sizeof(nc->info_str), TYPE_VHOST_VDPA);
nc->queue_index = 0;
s = DO_UPCAST(VhostVDPAState, nc, nc);
diff --git a/qapi/net.json b/qapi/net.json
index ddb113e5e5..c09322bb42 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -714,3 +714,71 @@
##
{ 'event': 'FAILOVER_NEGOTIATED',
'data': {'device-id': 'str'} }
+
+##
+# @NetdevInfo:
+#
+# Configuration of a network backend device (netdev).
+#
+# @id: Device identifier.
+#
+# @type: Specify the driver used for interpreting remaining arguments.
+#
+# @peer-id: Connected frontend network device identifier.
+#
+# Since: 5.2
+##
+{ 'union': 'NetdevInfo',
+ 'base': { 'id': 'str',
+ 'type': 'NetClientDriver',
+ '*peer-id': 'str' },
+ 'discriminator': 'type',
+ 'data': {
+ 'user': 'NetdevUserOptions',
+ 'tap': 'NetdevTapOptions',
+ 'l2tpv3': 'NetdevL2TPv3Options',
+ 'socket': 'NetdevSocketOptions',
+ 'vde': 'NetdevVdeOptions',
+ 'bridge': 'NetdevBridgeOptions',
+ 'netmap': 'NetdevNetmapOptions',
+ 'vhost-user': 'NetdevVhostUserOptions',
+ 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
+
+##
+# @query-netdevs:
+#
+# Get a list of @NetdevInfo for all virtual network backend devices (netdevs).
+#
+# Returns: a list of @NetdevInfo describing each netdev.
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "query-netdevs" }
+# <- { "return": [
+# {
+# "ipv6": true,
+# "ipv4": true,
+# "host": "10.0.2.2",
+# "ipv6-dns": "fec0::3",
+# "ipv6-prefix": "fec0::",
+# "net": "10.0.2.0/255.255.255.0",
+# "ipv6-host": "fec0::2",
+# "type": "user",
+# "peer-id": "net0",
+# "dns": "10.0.2.3",
+# "hostfwd": [
+# {
+# "str": "tcp::20004-:22"
+# }
+# ],
+# "ipv6-prefixlen": 64,
+# "id": "netdev0",
+# "restrict": false
+# }
+# ]
+# }
+#
+##
+{ 'command': 'query-netdevs', 'returns': ['NetdevInfo'] }
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 2/4] tests: Add tests for query-netdevs command
2020-09-01 18:23 [PATCH v3 0/4] Introducing QMP query-netdevs command Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 1/4] qapi: net: Add " Alexey Kirillov
@ 2020-09-01 18:23 ` Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 3/4] hmp: Use QMP query-netdevs in hmp_info_network Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 4/4] net: Do not use legacy info_str for backends Alexey Kirillov
3 siblings, 0 replies; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-01 18:23 UTC (permalink / raw)
To: Eric Blake, Markus Armbruster, Thomas Huth, Jason Wang
Cc: Laurent Vivier, Michael S. Tsirkin, Stefan Weil, qemu-devel,
Vincenzo Maffione, yc-core, Paolo Bonzini, Samuel Thibault,
Giuseppe Lettieri, Luigi Rizzo
Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
---
tests/qtest/meson.build | 3 +
tests/qtest/test-query-netdevs.c | 117 +++++++++++++++++++++++++++++++
2 files changed, 120 insertions(+)
create mode 100644 tests/qtest/test-query-netdevs.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 8f8fdb1336..a6c4ffe886 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -17,6 +17,9 @@ qtests_generic = [
if config_host.has_key('CONFIG_MODULES')
qtests_generic += [ 'modules-test' ]
endif
+if config_host.has_key('CONFIG_SLIRP')
+ qtests_generic += [ 'test-query-netdevs' ]
+endif
qtests_pci = \
(config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : []) + \
diff --git a/tests/qtest/test-query-netdevs.c b/tests/qtest/test-query-netdevs.c
new file mode 100644
index 0000000000..e711136111
--- /dev/null
+++ b/tests/qtest/test-query-netdevs.c
@@ -0,0 +1,117 @@
+/*
+ * QTest testcase for the query-netdevs
+ *
+ * Copyright Yandex N.V., 2019
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "libqos/libqtest.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
+
+/*
+ * Events can get in the way of responses we are actually waiting for.
+ */
+GCC_FMT_ATTR(2, 3)
+static QObject *wait_command(QTestState *who, const char *command, ...)
+{
+ va_list ap;
+ QDict *response;
+ QObject *result;
+
+ va_start(ap, command);
+ qtest_qmp_vsend(who, command, ap);
+ va_end(ap);
+
+ response = qtest_qmp_receive(who);
+
+ result = qdict_get(response, "return");
+ g_assert(result);
+ qobject_ref(result);
+ qobject_unref(response);
+
+ return result;
+}
+
+static void qmp_query_netdevs_no_error(QTestState *qts,
+ size_t netdevs_count)
+{
+ QObject *resp;
+ QList *netdevs;
+
+ resp = wait_command(qts, "{'execute': 'query-netdevs'}");
+
+ netdevs = qobject_to(QList, resp);
+ g_assert(netdevs);
+ g_assert(qlist_size(netdevs) == netdevs_count);
+
+ qobject_unref(resp);
+}
+
+static void test_query_netdevs(void)
+{
+ const char *arch = qtest_get_arch();
+ QObject *resp;
+ QTestState *state;
+
+ /* Skip test for some MCU */
+ if (g_str_equal(arch, "avr") ||
+ g_str_equal(arch, "rx")) {
+ return;
+ }
+
+ if (g_str_equal(arch, "arm") ||
+ g_str_equal(arch, "aarch64")) {
+ state = qtest_init(
+ "-nodefaults "
+ "-M virt "
+ "-netdev user,id=slirp0");
+ } else if (g_str_equal(arch, "tricore")) {
+ state = qtest_init(
+ "-nodefaults "
+ "-M tricore_testboard "
+ "-netdev user,id=slirp0");
+ } else {
+ state = qtest_init(
+ "-nodefaults "
+ "-netdev user,id=slirp0");
+ }
+ g_assert(state);
+
+ qmp_query_netdevs_no_error(state, 1);
+
+ resp = wait_command(state,
+ "{'execute': 'netdev_add', 'arguments': {"
+ " 'id': 'slirp1',"
+ " 'type': 'user'}}");
+ qobject_unref(resp);
+
+ qmp_query_netdevs_no_error(state, 2);
+
+ resp = wait_command(state,
+ "{'execute': 'netdev_del', 'arguments': {"
+ " 'id': 'slirp1'}}");
+ qobject_unref(resp);
+
+ qmp_query_netdevs_no_error(state, 1);
+
+ qtest_quit(state);
+}
+
+int main(int argc, char **argv)
+{
+ int ret = 0;
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("/net/qapi/query_netdevs",
+ test_query_netdevs);
+
+ ret = g_test_run();
+
+ return ret;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 3/4] hmp: Use QMP query-netdevs in hmp_info_network
2020-09-01 18:23 [PATCH v3 0/4] Introducing QMP query-netdevs command Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 1/4] qapi: net: Add " Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 2/4] tests: Add tests for " Alexey Kirillov
@ 2020-09-01 18:23 ` Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 4/4] net: Do not use legacy info_str for backends Alexey Kirillov
3 siblings, 0 replies; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-01 18:23 UTC (permalink / raw)
To: Eric Blake, Markus Armbruster, Thomas Huth, Jason Wang
Cc: Laurent Vivier, Michael S. Tsirkin, Stefan Weil, qemu-devel,
Vincenzo Maffione, yc-core, Paolo Bonzini, Samuel Thibault,
Giuseppe Lettieri, Luigi Rizzo
Replace usage of legacy field info_str of NetClientState for
backend network devices with result of QMP command query-netdevs.
NIC and hubports still use legacy info_str field.
Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
---
include/net/net.h | 3 +-
net/clients.h | 1 +
net/hub.c | 4 +-
net/hub.h | 2 +-
net/net.c | 181 ++++++++++++++++++++++++++++++++++++++++++++--
net/vde.c | 10 +++
6 files changed, 192 insertions(+), 9 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 04d51ac215..d6d3168a0d 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -172,7 +172,8 @@ void qemu_check_nic_model(NICInfo *nd, const char *model);
int qemu_find_nic_model(NICInfo *nd, const char * const *models,
const char *default_model);
-void print_net_client(Monitor *mon, NetClientState *nc);
+void print_net_client(Monitor *mon, NetClientState *nc,
+ NetdevInfoList *ni_list);
void hmp_info_network(Monitor *mon, const QDict *qdict);
void net_socket_rs_init(SocketReadState *rs,
SocketReadStateFinalize *finalize,
diff --git a/net/clients.h b/net/clients.h
index 92f9b59aed..fdf257f641 100644
--- a/net/clients.h
+++ b/net/clients.h
@@ -51,6 +51,7 @@ int net_init_l2tpv3(const Netdev *netdev, const char *name,
#ifdef CONFIG_VDE
int net_init_vde(const Netdev *netdev, const char *name,
NetClientState *peer, Error **errp);
+int net_vde_get_fd(const NetClientState *nc);
#endif
#ifdef CONFIG_NETMAP
diff --git a/net/hub.c b/net/hub.c
index 1375738bf1..7815248650 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -221,7 +221,7 @@ NetClientState *net_hub_port_find(int hub_id)
/**
* Print hub configuration
*/
-void net_hub_info(Monitor *mon)
+void net_hub_info(Monitor *mon, NetdevInfoList *ni_list)
{
NetHub *hub;
NetHubPort *port;
@@ -232,7 +232,7 @@ void net_hub_info(Monitor *mon)
monitor_printf(mon, " \\ %s", port->nc.name);
if (port->nc.peer) {
monitor_printf(mon, ": ");
- print_net_client(mon, port->nc.peer);
+ print_net_client(mon, port->nc.peer, ni_list);
} else {
monitor_printf(mon, "\n");
}
diff --git a/net/hub.h b/net/hub.h
index ce45f7b399..2c9a384077 100644
--- a/net/hub.h
+++ b/net/hub.h
@@ -17,7 +17,7 @@
NetClientState *net_hub_add_port(int hub_id, const char *name,
NetClientState *hubpeer);
-void net_hub_info(Monitor *mon);
+void net_hub_info(Monitor *mon, NetdevInfoList *ninfo);
void net_hub_check_clients(void);
bool net_hub_flush(NetClientState *nc);
diff --git a/net/net.c b/net/net.c
index 923e3d0bc6..384b57da3b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1175,14 +1175,182 @@ static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
monitor_printf(mon, "\n");
}
-void print_net_client(Monitor *mon, NetClientState *nc)
+static NetdevInfo *get_netdev_info(NetdevInfoList *ni_list, char *name)
+{
+ NetdevInfo *ni;
+
+ while (ni_list) {
+ ni = ni_list->value;
+ if (g_str_equal(ni->id, name)) {
+ return ni;
+ }
+ ni_list = ni_list->next;
+ }
+
+ return NULL;
+}
+
+static char *generate_info_str(NetdevInfo *ni, NetClientState *nc)
+{
+ char *info_str;
+
+ /* Use legacy field info_str for NIC and hubports */
+ if ((nc->info->type == NET_CLIENT_DRIVER_NIC) ||
+ (nc->info->type == NET_CLIENT_DRIVER_HUBPORT)) {
+ return g_strdup(nc->info_str);
+ }
+
+ if (!ni) {
+ return g_malloc0(1);
+ }
+
+ switch (ni->type) {
+#ifdef CONFIG_SLIRP
+ case NET_CLIENT_DRIVER_USER: {
+ size_t len = strchr(ni->u.user.net, '/') - ni->u.user.net;
+ char *net = g_strndup(ni->u.user.net, len);
+
+ info_str = g_strdup_printf("net=%s,restrict=%s",
+ net,
+ ni->u.user.q_restrict ? "on" : "off");
+ g_free(net);
+ break;
+ }
+#endif /* CONFIG_SLIRP */
+ case NET_CLIENT_DRIVER_TAP: {
+#ifndef _WIN32
+ if (ni->u.tap.has_fds) {
+ char **fds = g_strsplit(ni->u.tap.fds, ":", -1);
+
+ info_str = g_strdup_printf("fd=%s", fds[nc->queue_index]);
+ g_strfreev(fds);
+ } else if (ni->u.tap.has_helper) {
+ info_str = g_strdup_printf("helper=%s", ni->u.tap.helper);
+ } else {
+ info_str = g_strdup_printf("ifname=%s,script=%s,downscript=%s",
+ ni->u.tap.ifname,
+ nc->queue_index == 0 ? ni->u.tap.script : "no",
+ nc->queue_index == 0 ? ni->u.tap.downscript : "no");
+ }
+#else
+ info_str = g_strdup_printf("tap: ifname=%s", ni->u.tap.ifname);
+#endif /* _WIN32 */
+ break;
+ }
+#ifdef CONFIG_L2TPV3
+ case NET_CLIENT_DRIVER_L2TPV3: {
+ info_str = g_strdup_printf("l2tpv3: connected");
+ break;
+ }
+#endif /* CONFIG_L2TPV3 */
+ case NET_CLIENT_DRIVER_SOCKET: {
+ if (ni->u.socket.has_listen) {
+ if (ni->u.socket.has_fd) {
+ info_str = g_strdup_printf("socket: connection from %s",
+ ni->u.socket.listen);
+ } else {
+ info_str = g_strdup_printf("socket: wait from %s",
+ ni->u.socket.listen);
+ }
+ } else if (ni->u.socket.has_connect && ni->u.socket.has_fd) {
+ info_str = g_strdup_printf("socket: connect to %s",
+ ni->u.socket.connect);
+ } else if (ni->u.socket.has_mcast && ni->u.socket.has_fd) {
+ info_str = g_strdup_printf("socket: mcast=%s",
+ ni->u.socket.mcast);
+ } else if (ni->u.socket.has_udp && ni->u.socket.has_fd) {
+ info_str = g_strdup_printf("socket: udp=%s", ni->u.socket.udp);
+ } else {
+ g_assert(ni->u.socket.has_fd);
+ int so_type = -1;
+ int optlen = sizeof(so_type);
+ int fd = atoi(ni->u.socket.fd);
+
+ getsockopt(fd, SOL_SOCKET, SO_TYPE, (char *)&so_type,
+ (socklen_t *)&optlen);
+ if (so_type == SOCK_STREAM) {
+ info_str = g_strdup_printf("socket: fd=%s",
+ ni->u.socket.fd);
+ } else {
+ if (ni->u.socket.has_mcast) {
+ /*
+ * This branch is unreachable, according to how it is in
+ * net/socket.c at this moment
+ */
+ info_str = g_strdup_printf("socket: fd=%s "
+ "(cloned mcast=%s)",
+ ni->u.socket.fd,
+ ni->u.socket.mcast);
+ } else {
+ SocketAddress *sa = socket_local_address(fd, NULL);
+
+ info_str = g_strdup_printf("socket: fd=%s %s",
+ ni->u.socket.fd,
+ SocketAddressType_str(sa->type));
+ qapi_free_SocketAddress(sa);
+ }
+ }
+ }
+ break;
+ }
+#ifdef CONFIG_VDE
+ case NET_CLIENT_DRIVER_VDE: {
+ info_str = g_strdup_printf("sock=%s,fd=%d",
+ ni->u.vde.sock,
+ net_vde_get_fd(nc));
+ break;
+ }
+#endif /* CONFIG_VDE */
+#ifdef CONFIG_NET_BRIDGE
+ case NET_CLIENT_DRIVER_BRIDGE: {
+ info_str = g_strdup_printf("helper=%s,br=%s",
+ ni->u.bridge.helper,
+ ni->u.bridge.br);
+ break;
+ }
+#endif /* CONFIG_NET_BRIDGE */
+#ifdef CONFIG_NETMAP
+ case NET_CLIENT_DRIVER_NETMAP: {
+ info_str = g_strdup_printf("netmap: ifname=%s",
+ ni->u.netmap.ifname);
+ break;
+ }
+#endif /* CONFIG_NETMAP */
+#ifdef CONFIG_VHOST_NET_USER
+ case NET_CLIENT_DRIVER_VHOST_USER: {
+ info_str = g_strdup_printf("vhost-user%d to %s",
+ nc->queue_index,
+ ni->u.vhost_user.chardev);
+ break;
+ }
+#endif /* CONFIG_VHOST_NET_USER */
+#ifdef CONFIG_VHOST_NET_VDPA
+ case NET_CLIENT_DRIVER_VHOST_VDPA: {
+ info_str = g_strdup("vhost-vdpa");
+ break;
+ }
+#endif /* CONFIG_VHOST_NET_VDPA */
+ default: {
+ info_str = g_malloc0(1);
+ break;
+ }
+ }
+
+ return info_str;
+}
+
+void print_net_client(Monitor *mon, NetClientState *nc, NetdevInfoList *ni_list)
{
NetFilterState *nf;
+ NetdevInfo *ni = get_netdev_info(ni_list, nc->name);
+ char *info_str = generate_info_str(ni, nc);
monitor_printf(mon, "%s: index=%d,type=%s,%s\n", nc->name,
nc->queue_index,
NetClientDriver_str(nc->info->type),
- nc->info_str);
+ info_str);
+ g_free(info_str);
+
if (!QTAILQ_EMPTY(&nc->filters)) {
monitor_printf(mon, "filters:\n");
}
@@ -1286,8 +1454,9 @@ void hmp_info_network(Monitor *mon, const QDict *qdict)
{
NetClientState *nc, *peer;
NetClientDriver type;
+ NetdevInfoList *ni_list = qmp_query_netdevs(NULL);
- net_hub_info(mon);
+ net_hub_info(mon, ni_list);
QTAILQ_FOREACH(nc, &net_clients, next) {
peer = nc->peer;
@@ -1299,13 +1468,15 @@ void hmp_info_network(Monitor *mon, const QDict *qdict)
}
if (!peer || type == NET_CLIENT_DRIVER_NIC) {
- print_net_client(mon, nc);
+ print_net_client(mon, nc, ni_list);
} /* else it's a netdev connected to a NIC, printed with the NIC */
if (peer && type == NET_CLIENT_DRIVER_NIC) {
monitor_printf(mon, " \\ ");
- print_net_client(mon, peer);
+ print_net_client(mon, peer, ni_list);
}
}
+
+ qapi_free_NetdevInfoList(ni_list);
}
void colo_notify_filters_event(int event, Error **errp)
diff --git a/net/vde.c b/net/vde.c
index c0ab2bb65c..c4edf5cba1 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -153,3 +153,13 @@ int net_init_vde(const Netdev *netdev, const char *name,
return 0;
}
+
+int net_vde_get_fd(const NetClientState *nc)
+{
+ VDEState *s;
+ assert(nc->info->type == NET_CLIENT_DRIVER_VDE);
+
+ s = DO_UPCAST(VDEState, nc, nc);
+
+ return vde_datafd(s->vde);
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 4/4] net: Do not use legacy info_str for backends
2020-09-01 18:23 [PATCH v3 0/4] Introducing QMP query-netdevs command Alexey Kirillov
` (2 preceding siblings ...)
2020-09-01 18:23 ` [PATCH v3 3/4] hmp: Use QMP query-netdevs in hmp_info_network Alexey Kirillov
@ 2020-09-01 18:23 ` Alexey Kirillov
3 siblings, 0 replies; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-01 18:23 UTC (permalink / raw)
To: Eric Blake, Markus Armbruster, Thomas Huth, Jason Wang
Cc: Laurent Vivier, Michael S. Tsirkin, Stefan Weil, qemu-devel,
Vincenzo Maffione, yc-core, Paolo Bonzini, Samuel Thibault,
Giuseppe Lettieri, Luigi Rizzo
As we use QMP query-netdevs to store and get information about
backend network devices, we can drop usage legacy field info_str
for them.
We still use this field for NIC and hubports, so we can not
completely remove it.
Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
---
net/l2tpv3.c | 2 --
net/slirp.c | 4 ----
net/socket.c | 22 ----------------------
net/tap-win32.c | 3 ---
net/tap.c | 12 ------------
net/vde.c | 3 ---
net/vhost-user.c | 2 --
net/vhost-vdpa.c | 1 -
8 files changed, 49 deletions(-)
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index f4e45e7b28..7473619712 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -745,8 +745,6 @@ int net_init_l2tpv3(const Netdev *netdev,
stored->dstport = g_strdup(l2tpv3->dstport);
}
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "l2tpv3: connected");
return 0;
outerr:
qemu_del_net_client(nc);
diff --git a/net/slirp.c b/net/slirp.c
index 54c33d1173..4032829a1e 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -670,10 +670,6 @@ static int net_slirp_init(NetClientState *peer, const char *model,
stored->tftp_server_name = g_strdup(tftp_server_name);
}
- snprintf(nc->info_str, sizeof(nc->info_str),
- "net=%s,restrict=%s", inet_ntoa(net),
- restricted ? "on" : "off");
-
s = DO_UPCAST(SlirpState, nc, nc);
s->slirp = slirp_init(restricted, ipv4, net, mask, host,
diff --git a/net/socket.c b/net/socket.c
index 4a60de08e3..118b96b3e1 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -180,7 +180,6 @@ static void net_socket_send(void *opaque)
s->fd = -1;
net_socket_rs_init(&s->rs, net_socket_rs_finalize, false);
s->nc.link_down = true;
- memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
return;
}
@@ -400,16 +399,10 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
stored->mcast = g_strdup(mcast);
s->dgram_dst = saddr;
- snprintf(nc->info_str, sizeof(nc->info_str),
- "socket: fd=%d (cloned mcast=%s:%d)",
- fd, inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
} else {
if (sa_type == SOCKET_ADDRESS_TYPE_UNIX) {
s->dgram_dst.sin_family = AF_UNIX;
}
-
- snprintf(nc->info_str, sizeof(nc->info_str),
- "socket: fd=%d %s", fd, SocketAddressType_str(sa_type));
}
return s;
@@ -444,8 +437,6 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
nc = qemu_new_net_client(&net_socket_info, peer, model, name);
- snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd);
-
s = DO_UPCAST(NetSocketState, nc, nc);
s->fd = fd;
@@ -527,10 +518,6 @@ static void net_socket_accept(void *opaque)
stored->has_fd = true;
stored->fd = g_strdup_printf("%d", fd);
-
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "socket: connection from %s:%d",
- inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
}
static int net_socket_listen_init(NetClientState *peer,
@@ -645,9 +632,6 @@ static int net_socket_connect_init(NetClientState *peer,
stored->has_connect = true;
stored->connect = g_strdup(host_str);
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "socket: connect to %s:%d",
- inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
return 0;
}
@@ -704,9 +688,6 @@ static int net_socket_mcast_init(NetClientState *peer,
stored->localaddr = g_strdup(localaddr_str);
}
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "socket: mcast=%s:%d",
- inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
return 0;
}
@@ -769,9 +750,6 @@ static int net_socket_udp_init(NetClientState *peer,
stored->has_udp = true;
stored->udp = g_strdup(rhost);
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "socket: udp=%s:%d",
- inet_ntoa(raddr.sin_addr), ntohs(raddr.sin_port));
return 0;
}
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 20ba0b1dc8..54d4b1e25e 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -787,9 +787,6 @@ static int tap_win32_init(NetClientState *peer, const char *model,
stored->has_ifname = true;
stored->ifname = g_strdup(ifname);
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "tap: ifname=%s", ifname);
-
s->handle = handle;
qemu_add_wait_object(s->handle->tap_semaphore, tap_win32_send, s);
diff --git a/net/tap.c b/net/tap.c
index 7a7cf4caea..e59d85cba9 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -621,9 +621,6 @@ int net_init_bridge(const Netdev *netdev, const char *name,
stored->helper = g_strdup(helper);
}
- snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s,br=%s", helper,
- br);
-
return 0;
}
@@ -709,8 +706,6 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
stored->fds = g_strdup_printf("%s:%d", stored->fds, fd);
g_free(tmp_s);
}
-
- snprintf(s->nc.info_str, sizeof(s->nc.info_str), "fd=%d", fd);
} else if (tap->has_helper) {
if (!stored->has_helper) {
stored->has_helper = true;
@@ -722,9 +717,6 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
stored->br = tap->has_br ? g_strdup(tap->br) :
g_strdup(DEFAULT_BRIDGE_INTERFACE);
}
-
- snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s",
- tap->helper);
} else {
if (ifname && !stored->has_ifname) {
stored->has_ifname = true;
@@ -741,10 +733,6 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
stored->downscript = g_strdup(downscript);
}
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "ifname=%s,script=%s,downscript=%s", ifname, script,
- downscript);
-
if (strcmp(downscript, "no") != 0) {
snprintf(s->down_script, sizeof(s->down_script), "%s", downscript);
snprintf(s->down_script_arg, sizeof(s->down_script_arg),
diff --git a/net/vde.c b/net/vde.c
index c4edf5cba1..125433a89b 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -100,9 +100,6 @@ static int net_vde_init(NetClientState *peer, const char *model,
nc = qemu_new_net_client(&net_vde_info, peer, model, name);
- snprintf(nc->info_str, sizeof(nc->info_str), "sock=%s,fd=%d",
- sock, vde_datafd(vde));
-
s = DO_UPCAST(VDEState, nc, nc);
s->vde = vde;
diff --git a/net/vhost-user.c b/net/vhost-user.c
index aa2dc53179..dcc60f9c34 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -323,8 +323,6 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
user = g_new0(struct VhostUserState, 1);
for (i = 0; i < queues; i++) {
nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name);
- snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user%d to %s",
- i, chr->label);
nc->queue_index = i;
if (!nc0) {
nc0 = nc;
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 088033cfc9..6143a5e9e5 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -195,7 +195,6 @@ static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
stored->has_queues = true;
stored->queues = 1; /* TODO: change when support multiqueue */
- snprintf(nc->info_str, sizeof(nc->info_str), TYPE_VHOST_VDPA);
nc->queue_index = 0;
s = DO_UPCAST(VhostVDPAState, nc, nc);
vdpa_device_fd = qemu_open(vhostdev, O_RDWR);
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-01 18:23 ` [PATCH v3 1/4] qapi: net: Add " Alexey Kirillov
@ 2020-09-02 11:23 ` Markus Armbruster
2020-09-07 10:37 ` Alexey Kirillov
2020-09-08 14:29 ` Markus Armbruster
1 sibling, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-09-02 11:23 UTC (permalink / raw)
To: Alexey Kirillov
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, Luigi Rizzo, yc-core,
Samuel Thibault, Paolo Bonzini, Giuseppe Lettieri
Alexey Kirillov <lekiravi@yandex-team.ru> writes:
> Add a qmp command that provides information about currently attached
> backend network devices and their configuration.
>
> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
[...]
> diff --git a/qapi/net.json b/qapi/net.json
> index ddb113e5e5..c09322bb42 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -714,3 +714,71 @@
> ##
> { 'event': 'FAILOVER_NEGOTIATED',
> 'data': {'device-id': 'str'} }
> +
> +##
> +# @NetdevInfo:
> +#
> +# Configuration of a network backend device (netdev).
> +#
> +# @id: Device identifier.
> +#
> +# @type: Specify the driver used for interpreting remaining arguments.
> +#
> +# @peer-id: Connected frontend network device identifier.
> +#
> +# Since: 5.2
> +##
> +{ 'union': 'NetdevInfo',
> + 'base': { 'id': 'str',
> + 'type': 'NetClientDriver',
> + '*peer-id': 'str' },
> + 'discriminator': 'type',
> + 'data': {
> + 'user': 'NetdevUserOptions',
> + 'tap': 'NetdevTapOptions',
> + 'l2tpv3': 'NetdevL2TPv3Options',
> + 'socket': 'NetdevSocketOptions',
> + 'vde': 'NetdevVdeOptions',
> + 'bridge': 'NetdevBridgeOptions',
> + 'netmap': 'NetdevNetmapOptions',
> + 'vhost-user': 'NetdevVhostUserOptions',
> + 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
This is union Netdev plus a common member @peer-id, less the variant
members for NetClientDriver values 'nic' and 'hubport'.
Can 'type: 'nic' and 'type': 'hubport' occur?
What's the use case for @peer-id?
Assuming we want @peer-id: its documentation is too vague. Cases:
* Not connected to a frontend: I guess @peer-id is absent then.
* Connected to a frontend
- that has a qdev ID (the thing you set with -device id=...): I guess
it's the qdev ID then.
- that doesn't have a qdev ID: anyone's guess.
> +
> +##
> +# @query-netdevs:
> +#
> +# Get a list of @NetdevInfo for all virtual network backend devices (netdevs).
> +#
> +# Returns: a list of @NetdevInfo describing each netdev.
> +#
> +# Since: 5.2
> +#
> +# Example:
> +#
> +# -> { "execute": "query-netdevs" }
> +# <- { "return": [
> +# {
> +# "ipv6": true,
> +# "ipv4": true,
> +# "host": "10.0.2.2",
> +# "ipv6-dns": "fec0::3",
> +# "ipv6-prefix": "fec0::",
> +# "net": "10.0.2.0/255.255.255.0",
> +# "ipv6-host": "fec0::2",
> +# "type": "user",
> +# "peer-id": "net0",
> +# "dns": "10.0.2.3",
> +# "hostfwd": [
> +# {
> +# "str": "tcp::20004-:22"
> +# }
> +# ],
> +# "ipv6-prefixlen": 64,
> +# "id": "netdev0",
> +# "restrict": false
> +# }
> +# ]
> +# }
> +#
> +##
> +{ 'command': 'query-netdevs', 'returns': ['NetdevInfo'] }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-02 11:23 ` Markus Armbruster
@ 2020-09-07 10:37 ` Alexey Kirillov
2020-09-07 12:39 ` Markus Armbruster
0 siblings, 1 reply; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-07 10:37 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, Luigi Rizzo, yc-core,
Samuel Thibault, Paolo Bonzini, Giuseppe Lettieri
Hi!
02.09.2020, 14:23, "Markus Armbruster" <armbru@redhat.com>:
> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>
>> Add a qmp command that provides information about currently attached
>> backend network devices and their configuration.
>>
>> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>
> [...]
>> diff --git a/qapi/net.json b/qapi/net.json
>> index ddb113e5e5..c09322bb42 100644
>> --- a/qapi/net.json
>> +++ b/qapi/net.json
>> @@ -714,3 +714,71 @@
>> ##
>> { 'event': 'FAILOVER_NEGOTIATED',
>> 'data': {'device-id': 'str'} }
>> +
>> +##
>> +# @NetdevInfo:
>> +#
>> +# Configuration of a network backend device (netdev).
>> +#
>> +# @id: Device identifier.
>> +#
>> +# @type: Specify the driver used for interpreting remaining arguments.
>> +#
>> +# @peer-id: Connected frontend network device identifier.
>> +#
>> +# Since: 5.2
>> +##
>> +{ 'union': 'NetdevInfo',
>> + 'base': { 'id': 'str',
>> + 'type': 'NetClientDriver',
>> + '*peer-id': 'str' },
>> + 'discriminator': 'type',
>> + 'data': {
>> + 'user': 'NetdevUserOptions',
>> + 'tap': 'NetdevTapOptions',
>> + 'l2tpv3': 'NetdevL2TPv3Options',
>> + 'socket': 'NetdevSocketOptions',
>> + 'vde': 'NetdevVdeOptions',
>> + 'bridge': 'NetdevBridgeOptions',
>> + 'netmap': 'NetdevNetmapOptions',
>> + 'vhost-user': 'NetdevVhostUserOptions',
>> + 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
>
> This is union Netdev plus a common member @peer-id, less the variant
> members for NetClientDriver values 'nic' and 'hubport'.
>
> Can 'type: 'nic' and 'type': 'hubport' occur?
No, it can't. We don't support NIC/hubport in query-netdevs, so we neither create this
structure for them, nor store config.
> What's the use case for @peer-id?
Main reason is to provide information "is this backend connected to frontend device".
User also may want to know which backend used for frontend device.
> Assuming we want @peer-id: its documentation is too vague. Cases:
>
> * Not connected to a frontend: I guess @peer-id is absent then.
Absolutely correct.
> * Connected to a frontend
>
> - that has a qdev ID (the thing you set with -device id=...): I guess
> it's the qdev ID then.
Correct.
> - that doesn't have a qdev ID: anyone's guess.
We use field "name" of structure NetClientState, so if there is no direct ID setting,
there must be generated name (in format "model.id").
>> +
>> +##
>> +# @query-netdevs:
>> +#
>> +# Get a list of @NetdevInfo for all virtual network backend devices (netdevs).
>> +#
>> +# Returns: a list of @NetdevInfo describing each netdev.
>> +#
>> +# Since: 5.2
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "query-netdevs" }
>> +# <- { "return": [
>> +# {
>> +# "ipv6": true,
>> +# "ipv4": true,
>> +# "host": "10.0.2.2",
>> +# "ipv6-dns": "fec0::3",
>> +# "ipv6-prefix": "fec0::",
>> +# "net": "10.0.2.0/255.255.255.0",
>> +# "ipv6-host": "fec0::2",
>> +# "type": "user",
>> +# "peer-id": "net0",
>> +# "dns": "10.0.2.3",
>> +# "hostfwd": [
>> +# {
>> +# "str": "tcp::20004-:22"
>> +# }
>> +# ],
>> +# "ipv6-prefixlen": 64,
>> +# "id": "netdev0",
>> +# "restrict": false
>> +# }
>> +# ]
>> +# }
>> +#
>> +##
>> +{ 'command': 'query-netdevs', 'returns': ['NetdevInfo'] }
--
Alexey Kirillov
Yandex.Cloud
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-07 10:37 ` Alexey Kirillov
@ 2020-09-07 12:39 ` Markus Armbruster
2020-09-08 12:36 ` Eric Blake
2020-09-08 15:52 ` Alexey Kirillov
0 siblings, 2 replies; 17+ messages in thread
From: Markus Armbruster @ 2020-09-07 12:39 UTC (permalink / raw)
To: Alexey Kirillov
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, yc-core,
Paolo Bonzini, Samuel Thibault, Giuseppe Lettieri, Luigi Rizzo
Alexey Kirillov <lekiravi@yandex-team.ru> writes:
> Hi!
>
> 02.09.2020, 14:23, "Markus Armbruster" <armbru@redhat.com>:
>> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>>
>>> Add a qmp command that provides information about currently attached
>>> backend network devices and their configuration.
>>>
>>> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>>
>> [...]
>>> diff --git a/qapi/net.json b/qapi/net.json
>>> index ddb113e5e5..c09322bb42 100644
>>> --- a/qapi/net.json
>>> +++ b/qapi/net.json
>>> @@ -714,3 +714,71 @@
>>> ##
>>> { 'event': 'FAILOVER_NEGOTIATED',
>>> 'data': {'device-id': 'str'} }
>>> +
>>> +##
>>> +# @NetdevInfo:
>>> +#
>>> +# Configuration of a network backend device (netdev).
>>> +#
>>> +# @id: Device identifier.
>>> +#
>>> +# @type: Specify the driver used for interpreting remaining arguments.
>>> +#
>>> +# @peer-id: Connected frontend network device identifier.
>>> +#
>>> +# Since: 5.2
>>> +##
>>> +{ 'union': 'NetdevInfo',
>>> + 'base': { 'id': 'str',
>>> + 'type': 'NetClientDriver',
>>> + '*peer-id': 'str' },
>>> + 'discriminator': 'type',
>>> + 'data': {
>>> + 'user': 'NetdevUserOptions',
>>> + 'tap': 'NetdevTapOptions',
>>> + 'l2tpv3': 'NetdevL2TPv3Options',
>>> + 'socket': 'NetdevSocketOptions',
>>> + 'vde': 'NetdevVdeOptions',
>>> + 'bridge': 'NetdevBridgeOptions',
>>> + 'netmap': 'NetdevNetmapOptions',
>>> + 'vhost-user': 'NetdevVhostUserOptions',
>>> + 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
>>
>> This is union Netdev plus a common member @peer-id, less the variant
>> members for NetClientDriver values 'nic' and 'hubport'.
>>
>> Can 'type: 'nic' and 'type': 'hubport' occur?
>
> No, it can't. We don't support NIC/hubport in query-netdevs, so we neither create this
> structure for them, nor store config.
Same for 'none', I guess.
As defined, NetdevInfo allows types 'none', 'nic', and 'hubport', it
just has no variant members for them. The fact that they can't occur is
not coded into the type, and therefore not visible in introspection.
To make introspection more precise, we'd have to define a new enum type.
How much that would complicate the C code is unclear.
Do we need it to be more precise? Eric, got an opinion?
Existing type Netdev has the same issue for 'none' and 'nic'. I guess
this is so we can reuse the type for -net. Not sure that's a good idea,
but not this patch's problem.
>> What's the use case for @peer-id?
>
> Main reason is to provide information "is this backend connected to frontend device".
> User also may want to know which backend used for frontend device.
>
>> Assuming we want @peer-id: its documentation is too vague. Cases:
>>
>> * Not connected to a frontend: I guess @peer-id is absent then.
>
> Absolutely correct.
>
>> * Connected to a frontend
>>
>> - that has a qdev ID (the thing you set with -device id=...): I guess
>> it's the qdev ID then.
>
> Correct.
>
>> - that doesn't have a qdev ID: anyone's guess.
>
> We use field "name" of structure NetClientState, so if there is no direct ID setting,
> there must be generated name (in format "model.id").
Perhaps:
# @peer-id: the connected network backend's name (absent if no
# backend is connected)
>>> +
>>> +##
>>> +# @query-netdevs:
>>> +#
>>> +# Get a list of @NetdevInfo for all virtual network backend devices (netdevs).
>>> +#
>>> +# Returns: a list of @NetdevInfo describing each netdev.
>>> +#
>>> +# Since: 5.2
>>> +#
>>> +# Example:
>>> +#
>>> +# -> { "execute": "query-netdevs" }
>>> +# <- { "return": [
>>> +# {
>>> +# "ipv6": true,
>>> +# "ipv4": true,
>>> +# "host": "10.0.2.2",
>>> +# "ipv6-dns": "fec0::3",
>>> +# "ipv6-prefix": "fec0::",
>>> +# "net": "10.0.2.0/255.255.255.0",
>>> +# "ipv6-host": "fec0::2",
>>> +# "type": "user",
>>> +# "peer-id": "net0",
>>> +# "dns": "10.0.2.3",
>>> +# "hostfwd": [
>>> +# {
>>> +# "str": "tcp::20004-:22"
>>> +# }
>>> +# ],
>>> +# "ipv6-prefixlen": 64,
>>> +# "id": "netdev0",
>>> +# "restrict": false
>>> +# }
>>> +# ]
>>> +# }
>>> +#
>>> +##
>>> +{ 'command': 'query-netdevs', 'returns': ['NetdevInfo'] }
>
>
> --
> Alexey Kirillov
> Yandex.Cloud
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-07 12:39 ` Markus Armbruster
@ 2020-09-08 12:36 ` Eric Blake
2020-09-08 14:31 ` Markus Armbruster
2020-09-08 15:52 ` Alexey Kirillov
1 sibling, 1 reply; 17+ messages in thread
From: Eric Blake @ 2020-09-08 12:36 UTC (permalink / raw)
To: Markus Armbruster, Alexey Kirillov
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, yc-core,
Paolo Bonzini, Samuel Thibault, Giuseppe Lettieri, Luigi Rizzo
On 9/7/20 7:39 AM, Markus Armbruster wrote:
>>>
>>> This is union Netdev plus a common member @peer-id, less the variant
>>> members for NetClientDriver values 'nic' and 'hubport'.
>>>
>>> Can 'type: 'nic' and 'type': 'hubport' occur?
>>
>> No, it can't. We don't support NIC/hubport in query-netdevs, so we neither create this
>> structure for them, nor store config.
>
> Same for 'none', I guess.
>
> As defined, NetdevInfo allows types 'none', 'nic', and 'hubport', it
> just has no variant members for them. The fact that they can't occur is
> not coded into the type, and therefore not visible in introspection.
>
> To make introspection more precise, we'd have to define a new enum type.
> How much that would complicate the C code is unclear.
>
> Do we need it to be more precise? Eric, got an opinion?
Is the problem that a new enum would be duplicating things? Is it worth
allowing one enum to have a 'base':'OtherEnum' in the schema to reduce
some of the duplication?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-01 18:23 ` [PATCH v3 1/4] qapi: net: Add " Alexey Kirillov
2020-09-02 11:23 ` Markus Armbruster
@ 2020-09-08 14:29 ` Markus Armbruster
2020-09-08 15:52 ` Alexey Kirillov
1 sibling, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-09-08 14:29 UTC (permalink / raw)
To: Alexey Kirillov
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, Luigi Rizzo, yc-core,
Samuel Thibault, Paolo Bonzini, Giuseppe Lettieri
Alexey Kirillov <lekiravi@yandex-team.ru> writes:
> Add a qmp command that provides information about currently attached
> backend network devices and their configuration.
>
> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
One more: name it query-netdev for consistency with query-chardev and
query-memdev.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-08 12:36 ` Eric Blake
@ 2020-09-08 14:31 ` Markus Armbruster
2020-09-16 9:37 ` Alexey Kirillov
0 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-09-08 14:31 UTC (permalink / raw)
To: Eric Blake
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, yc-core,
Samuel Thibault, Alexey Kirillov, Paolo Bonzini,
Giuseppe Lettieri, Luigi Rizzo
Eric Blake <eblake@redhat.com> writes:
> On 9/7/20 7:39 AM, Markus Armbruster wrote:
>
>>>>
>>>> This is union Netdev plus a common member @peer-id, less the variant
>>>> members for NetClientDriver values 'nic' and 'hubport'.
>>>>
>>>> Can 'type: 'nic' and 'type': 'hubport' occur?
>>>
>>> No, it can't. We don't support NIC/hubport in query-netdevs, so we neither create this
>>> structure for them, nor store config.
>> Same for 'none', I guess.
>> As defined, NetdevInfo allows types 'none', 'nic', and 'hubport', it
>> just has no variant members for them. The fact that they can't occur is
>> not coded into the type, and therefore not visible in introspection.
>> To make introspection more precise, we'd have to define a new enum
>> type.
>> How much that would complicate the C code is unclear.
>> Do we need it to be more precise? Eric, got an opinion?
>
> Is the problem that a new enum would be duplicating things?
Enumerating network drivers twice is mildly annoying. I worry more
about having to convert between the two enumerations in C.
My actual question: do we need query-qmp-schema report the precise set
of 'type' values? As is, it reports a few that can't actually occur.
> Is it
> worth allowing one enum to have a 'base':'OtherEnum' in the schema to
> reduce some of the duplication?
We could then generate functions (or macros) to convert from base enum
to extended enum, and back, where the latter can fail.
Worthwhile only if we have sufficient use for it.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-07 12:39 ` Markus Armbruster
2020-09-08 12:36 ` Eric Blake
@ 2020-09-08 15:52 ` Alexey Kirillov
2020-09-09 11:41 ` Markus Armbruster
1 sibling, 1 reply; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-08 15:52 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, yc-core,
Paolo Bonzini, Samuel Thibault, Giuseppe Lettieri, Luigi Rizzo
07.09.2020, 15:40, "Markus Armbruster" <armbru@redhat.com>:
> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>
>> Hi!
>>
>> 02.09.2020, 14:23, "Markus Armbruster" <armbru@redhat.com>:
>>> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>>>
>>>> Add a qmp command that provides information about currently attached
>>>> backend network devices and their configuration.
>>>>
>>>> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>>>
>>> [...]
>>>> diff --git a/qapi/net.json b/qapi/net.json
>>>> index ddb113e5e5..c09322bb42 100644
>>>> --- a/qapi/net.json
>>>> +++ b/qapi/net.json
>>>> @@ -714,3 +714,71 @@
>>>> ##
>>>> { 'event': 'FAILOVER_NEGOTIATED',
>>>> 'data': {'device-id': 'str'} }
>>>> +
>>>> +##
>>>> +# @NetdevInfo:
>>>> +#
>>>> +# Configuration of a network backend device (netdev).
>>>> +#
>>>> +# @id: Device identifier.
>>>> +#
>>>> +# @type: Specify the driver used for interpreting remaining arguments.
>>>> +#
>>>> +# @peer-id: Connected frontend network device identifier.
>>>> +#
>>>> +# Since: 5.2
>>>> +##
>>>> +{ 'union': 'NetdevInfo',
>>>> + 'base': { 'id': 'str',
>>>> + 'type': 'NetClientDriver',
>>>> + '*peer-id': 'str' },
>>>> + 'discriminator': 'type',
>>>> + 'data': {
>>>> + 'user': 'NetdevUserOptions',
>>>> + 'tap': 'NetdevTapOptions',
>>>> + 'l2tpv3': 'NetdevL2TPv3Options',
>>>> + 'socket': 'NetdevSocketOptions',
>>>> + 'vde': 'NetdevVdeOptions',
>>>> + 'bridge': 'NetdevBridgeOptions',
>>>> + 'netmap': 'NetdevNetmapOptions',
>>>> + 'vhost-user': 'NetdevVhostUserOptions',
>>>> + 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
>>>
>>> This is union Netdev plus a common member @peer-id, less the variant
>>> members for NetClientDriver values 'nic' and 'hubport'.
>>>
>>> Can 'type: 'nic' and 'type': 'hubport' occur?
>>
>> No, it can't. We don't support NIC/hubport in query-netdevs, so we neither create this
>> structure for them, nor store config.
>
> Same for 'none', I guess.
>
> As defined, NetdevInfo allows types 'none', 'nic', and 'hubport', it
> just has no variant members for them. The fact that they can't occur is
> not coded into the type, and therefore not visible in introspection.
>
> To make introspection more precise, we'd have to define a new enum type.
> How much that would complicate the C code is unclear.
>
> Do we need it to be more precise? Eric, got an opinion?
>
> Existing type Netdev has the same issue for 'none' and 'nic'. I guess
> this is so we can reuse the type for -net. Not sure that's a good idea,
> but not this patch's problem.
>
>>> What's the use case for @peer-id?
>>
>> Main reason is to provide information "is this backend connected to frontend device".
>> User also may want to know which backend used for frontend device.
>>
>>> Assuming we want @peer-id: its documentation is too vague. Cases:
>>>
>>> * Not connected to a frontend: I guess @peer-id is absent then.
>>
>> Absolutely correct.
>>
>>> * Connected to a frontend
>>>
>>> - that has a qdev ID (the thing you set with -device id=...): I guess
>>> it's the qdev ID then.
>>
>> Correct.
>>
>>> - that doesn't have a qdev ID: anyone's guess.
>>
>> We use field "name" of structure NetClientState, so if there is no direct ID setting,
>> there must be generated name (in format "model.id").
>
> Perhaps:
>
> # @peer-id: the connected network backend's name (absent if no
> # backend is connected)
Maybe you mean:
# @peer-id: The connected frontend network device name (absent if no frontend
# is connected).
In any case, thanks for pointing. I'll add this in the next patch version.
>>>> +
>>>> +##
>>>> +# @query-netdevs:
>>>> +#
>>>> +# Get a list of @NetdevInfo for all virtual network backend devices (netdevs).
>>>> +#
>>>> +# Returns: a list of @NetdevInfo describing each netdev.
>>>> +#
>>>> +# Since: 5.2
>>>> +#
>>>> +# Example:
>>>> +#
>>>> +# -> { "execute": "query-netdevs" }
>>>> +# <- { "return": [
>>>> +# {
>>>> +# "ipv6": true,
>>>> +# "ipv4": true,
>>>> +# "host": "10.0.2.2",
>>>> +# "ipv6-dns": "fec0::3",
>>>> +# "ipv6-prefix": "fec0::",
>>>> +# "net": "10.0.2.0/255.255.255.0",
>>>> +# "ipv6-host": "fec0::2",
>>>> +# "type": "user",
>>>> +# "peer-id": "net0",
>>>> +# "dns": "10.0.2.3",
>>>> +# "hostfwd": [
>>>> +# {
>>>> +# "str": "tcp::20004-:22"
>>>> +# }
>>>> +# ],
>>>> +# "ipv6-prefixlen": 64,
>>>> +# "id": "netdev0",
>>>> +# "restrict": false
>>>> +# }
>>>> +# ]
>>>> +# }
>>>> +#
>>>> +##
>>>> +{ 'command': 'query-netdevs', 'returns': ['NetdevInfo'] }
--
Alexey Kirillov
Yandex.Cloud
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-08 14:29 ` Markus Armbruster
@ 2020-09-08 15:52 ` Alexey Kirillov
0 siblings, 0 replies; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-08 15:52 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, Luigi Rizzo, yc-core,
Samuel Thibault, Paolo Bonzini, Giuseppe Lettieri
08.09.2020, 17:29, "Markus Armbruster" <armbru@redhat.com>:
> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>
>> Add a qmp command that provides information about currently attached
>> backend network devices and their configuration.
>>
>> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>
> One more: name it query-netdev for consistency with query-chardev and
> query-memdev.
Yes, it make sense. Thank you, I'll rename it.
--
Alexey Kirillov
Yandex.Cloud
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-08 15:52 ` Alexey Kirillov
@ 2020-09-09 11:41 ` Markus Armbruster
2020-09-09 12:45 ` Alexey Kirillov
0 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2020-09-09 11:41 UTC (permalink / raw)
To: Alexey Kirillov
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, yc-core,
Samuel Thibault, Paolo Bonzini, Giuseppe Lettieri, Luigi Rizzo
Alexey Kirillov <lekiravi@yandex-team.ru> writes:
> 07.09.2020, 15:40, "Markus Armbruster" <armbru@redhat.com>:
>> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>>
>>> Hi!
>>>
>>> 02.09.2020, 14:23, "Markus Armbruster" <armbru@redhat.com>:
>>>> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>>>>
>>>>> Add a qmp command that provides information about currently attached
>>>>> backend network devices and their configuration.
>>>>>
>>>>> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>>>>
>>>> [...]
>>>>> diff --git a/qapi/net.json b/qapi/net.json
>>>>> index ddb113e5e5..c09322bb42 100644
>>>>> --- a/qapi/net.json
>>>>> +++ b/qapi/net.json
>>>>> @@ -714,3 +714,71 @@
>>>>> ##
>>>>> { 'event': 'FAILOVER_NEGOTIATED',
>>>>> 'data': {'device-id': 'str'} }
>>>>> +
>>>>> +##
>>>>> +# @NetdevInfo:
>>>>> +#
>>>>> +# Configuration of a network backend device (netdev).
>>>>> +#
>>>>> +# @id: Device identifier.
>>>>> +#
>>>>> +# @type: Specify the driver used for interpreting remaining arguments.
>>>>> +#
>>>>> +# @peer-id: Connected frontend network device identifier.
>>>>> +#
>>>>> +# Since: 5.2
>>>>> +##
>>>>> +{ 'union': 'NetdevInfo',
>>>>> + 'base': { 'id': 'str',
>>>>> + 'type': 'NetClientDriver',
>>>>> + '*peer-id': 'str' },
>>>>> + 'discriminator': 'type',
>>>>> + 'data': {
>>>>> + 'user': 'NetdevUserOptions',
>>>>> + 'tap': 'NetdevTapOptions',
>>>>> + 'l2tpv3': 'NetdevL2TPv3Options',
>>>>> + 'socket': 'NetdevSocketOptions',
>>>>> + 'vde': 'NetdevVdeOptions',
>>>>> + 'bridge': 'NetdevBridgeOptions',
>>>>> + 'netmap': 'NetdevNetmapOptions',
>>>>> + 'vhost-user': 'NetdevVhostUserOptions',
>>>>> + 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
>>>>
>>>> This is union Netdev plus a common member @peer-id, less the variant
>>>> members for NetClientDriver values 'nic' and 'hubport'.
>>>>
>>>> Can 'type: 'nic' and 'type': 'hubport' occur?
>>>
>>> No, it can't. We don't support NIC/hubport in query-netdevs, so we neither create this
>>> structure for them, nor store config.
>>
>> Same for 'none', I guess.
>>
>> As defined, NetdevInfo allows types 'none', 'nic', and 'hubport', it
>> just has no variant members for them. The fact that they can't occur is
>> not coded into the type, and therefore not visible in introspection.
>>
>> To make introspection more precise, we'd have to define a new enum type.
>> How much that would complicate the C code is unclear.
>>
>> Do we need it to be more precise? Eric, got an opinion?
>>
>> Existing type Netdev has the same issue for 'none' and 'nic'. I guess
>> this is so we can reuse the type for -net. Not sure that's a good idea,
>> but not this patch's problem.
>>
>>>> What's the use case for @peer-id?
>>>
>>> Main reason is to provide information "is this backend connected to frontend device".
>>> User also may want to know which backend used for frontend device.
>>>
>>>> Assuming we want @peer-id: its documentation is too vague. Cases:
>>>>
>>>> * Not connected to a frontend: I guess @peer-id is absent then.
>>>
>>> Absolutely correct.
>>>
>>>> * Connected to a frontend
>>>>
>>>> - that has a qdev ID (the thing you set with -device id=...): I guess
>>>> it's the qdev ID then.
>>>
>>> Correct.
>>>
>>>> - that doesn't have a qdev ID: anyone's guess.
>>>
>>> We use field "name" of structure NetClientState, so if there is no direct ID setting,
>>> there must be generated name (in format "model.id").
>>
>> Perhaps:
>>
>> # @peer-id: the connected network backend's name (absent if no
>> # backend is connected)
>
> Maybe you mean:
>
> # @peer-id: The connected frontend network device name (absent if no frontend
> # is connected).
Yes, I wrote backend, but meant frontend.
Aside: our naming is undisciplined: we use both "id" and "name" in QMP
for NetClientState member name. Unfortunate.
> In any case, thanks for pointing. I'll add this in the next patch version.
[...]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-09 11:41 ` Markus Armbruster
@ 2020-09-09 12:45 ` Alexey Kirillov
0 siblings, 0 replies; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-09 12:45 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, yc-core,
Samuel Thibault, Paolo Bonzini, Giuseppe Lettieri, Luigi Rizzo
09.09.2020, 14:41, "Markus Armbruster" <armbru@redhat.com>:
> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>
>> 07.09.2020, 15:40, "Markus Armbruster" <armbru@redhat.com>:
>>> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>>>
>>>> Hi!
>>>>
>>>> 02.09.2020, 14:23, "Markus Armbruster" <armbru@redhat.com>:
>>>>> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>>>>>
>>>>>> Add a qmp command that provides information about currently attached
>>>>>> backend network devices and their configuration.
>>>>>>
>>>>>> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>>>>>
>>>>> [...]
>>>>>> diff --git a/qapi/net.json b/qapi/net.json
>>>>>> index ddb113e5e5..c09322bb42 100644
>>>>>> --- a/qapi/net.json
>>>>>> +++ b/qapi/net.json
>>>>>> @@ -714,3 +714,71 @@
>>>>>> ##
>>>>>> { 'event': 'FAILOVER_NEGOTIATED',
>>>>>> 'data': {'device-id': 'str'} }
>>>>>> +
>>>>>> +##
>>>>>> +# @NetdevInfo:
>>>>>> +#
>>>>>> +# Configuration of a network backend device (netdev).
>>>>>> +#
>>>>>> +# @id: Device identifier.
>>>>>> +#
>>>>>> +# @type: Specify the driver used for interpreting remaining arguments.
>>>>>> +#
>>>>>> +# @peer-id: Connected frontend network device identifier.
>>>>>> +#
>>>>>> +# Since: 5.2
>>>>>> +##
>>>>>> +{ 'union': 'NetdevInfo',
>>>>>> + 'base': { 'id': 'str',
>>>>>> + 'type': 'NetClientDriver',
>>>>>> + '*peer-id': 'str' },
>>>>>> + 'discriminator': 'type',
>>>>>> + 'data': {
>>>>>> + 'user': 'NetdevUserOptions',
>>>>>> + 'tap': 'NetdevTapOptions',
>>>>>> + 'l2tpv3': 'NetdevL2TPv3Options',
>>>>>> + 'socket': 'NetdevSocketOptions',
>>>>>> + 'vde': 'NetdevVdeOptions',
>>>>>> + 'bridge': 'NetdevBridgeOptions',
>>>>>> + 'netmap': 'NetdevNetmapOptions',
>>>>>> + 'vhost-user': 'NetdevVhostUserOptions',
>>>>>> + 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
>>>>>
>>>>> This is union Netdev plus a common member @peer-id, less the variant
>>>>> members for NetClientDriver values 'nic' and 'hubport'.
>>>>>
>>>>> Can 'type: 'nic' and 'type': 'hubport' occur?
>>>>
>>>> No, it can't. We don't support NIC/hubport in query-netdevs, so we neither create this
>>>> structure for them, nor store config.
>>>
>>> Same for 'none', I guess.
>>>
>>> As defined, NetdevInfo allows types 'none', 'nic', and 'hubport', it
>>> just has no variant members for them. The fact that they can't occur is
>>> not coded into the type, and therefore not visible in introspection.
>>>
>>> To make introspection more precise, we'd have to define a new enum type.
>>> How much that would complicate the C code is unclear.
>>>
>>> Do we need it to be more precise? Eric, got an opinion?
>>>
>>> Existing type Netdev has the same issue for 'none' and 'nic'. I guess
>>> this is so we can reuse the type for -net. Not sure that's a good idea,
>>> but not this patch's problem.
>>>
>>>>> What's the use case for @peer-id?
>>>>
>>>> Main reason is to provide information "is this backend connected to frontend device".
>>>> User also may want to know which backend used for frontend device.
>>>>
>>>>> Assuming we want @peer-id: its documentation is too vague. Cases:
>>>>>
>>>>> * Not connected to a frontend: I guess @peer-id is absent then.
>>>>
>>>> Absolutely correct.
>>>>
>>>>> * Connected to a frontend
>>>>>
>>>>> - that has a qdev ID (the thing you set with -device id=...): I guess
>>>>> it's the qdev ID then.
>>>>
>>>> Correct.
>>>>
>>>>> - that doesn't have a qdev ID: anyone's guess.
>>>>
>>>> We use field "name" of structure NetClientState, so if there is no direct ID setting,
>>>> there must be generated name (in format "model.id").
>>>
>>> Perhaps:
>>>
>>> # @peer-id: the connected network backend's name (absent if no
>>> # backend is connected)
>>
>> Maybe you mean:
>>
>> # @peer-id: The connected frontend network device name (absent if no frontend
>> # is connected).
>
> Yes, I wrote backend, but meant frontend.
>
> Aside: our naming is undisciplined: we use both "id" and "name" in QMP
> for NetClientState member name. Unfortunate.
Well, I was looking at netdev_add/netdev_del when choosing member's name.
Looks like "name" more often used for NICs when "id" is for common netdevs.
It seems to me that the "id" is more suitable for network devices, since
it is uniquely define them when calling QMP commands.
>> In any case, thanks for pointing. I'll add this in the next patch version.
>
> [...]
--
Alexey Kirillov
Yandex.Cloud
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-08 14:31 ` Markus Armbruster
@ 2020-09-16 9:37 ` Alexey Kirillov
2020-09-16 11:28 ` Markus Armbruster
0 siblings, 1 reply; 17+ messages in thread
From: Alexey Kirillov @ 2020-09-16 9:37 UTC (permalink / raw)
To: Markus Armbruster, Eric Blake
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, yc-core,
Samuel Thibault, Paolo Bonzini, Giuseppe Lettieri, Luigi Rizzo
08.09.2020, 17:32, "Markus Armbruster" <armbru@redhat.com>:
> Eric Blake <eblake@redhat.com> writes:
>
>> On 9/7/20 7:39 AM, Markus Armbruster wrote:
>>
>>>>> This is union Netdev plus a common member @peer-id, less the variant
>>>>> members for NetClientDriver values 'nic' and 'hubport'.
>>>>>
>>>>> Can 'type: 'nic' and 'type': 'hubport' occur?
>>>>
>>>> No, it can't. We don't support NIC/hubport in query-netdevs, so we neither create this
>>>> structure for them, nor store config.
>>> Same for 'none', I guess.
>>> As defined, NetdevInfo allows types 'none', 'nic', and 'hubport', it
>>> just has no variant members for them. The fact that they can't occur is
>>> not coded into the type, and therefore not visible in introspection.
>>> To make introspection more precise, we'd have to define a new enum
>>> type.
>>> How much that would complicate the C code is unclear.
>>> Do we need it to be more precise? Eric, got an opinion?
>>
>> Is the problem that a new enum would be duplicating things?
>
> Enumerating network drivers twice is mildly annoying. I worry more
> about having to convert between the two enumerations in C.
>
> My actual question: do we need query-qmp-schema report the precise set
> of 'type' values? As is, it reports a few that can't actually occur.
>
>> Is it
>> worth allowing one enum to have a 'base':'OtherEnum' in the schema to
>> reduce some of the duplication?
>
> We could then generate functions (or macros) to convert from base enum
> to extended enum, and back, where the latter can fail.
>
> Worthwhile only if we have sufficient use for it.
I'm sorry, did I understand correctly that at the moment I don't need any
additional changes in the patch yet?
If that is, I will continue using NetClientDriver as a discriminator.
--
Alexey Kirillov
Yandex.Cloud
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command
2020-09-16 9:37 ` Alexey Kirillov
@ 2020-09-16 11:28 ` Markus Armbruster
0 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2020-09-16 11:28 UTC (permalink / raw)
To: Alexey Kirillov
Cc: Laurent Vivier, Thomas Huth, Michael S. Tsirkin, Stefan Weil,
Jason Wang, qemu-devel, Vincenzo Maffione, Luigi Rizzo, yc-core,
Paolo Bonzini, Samuel Thibault, Giuseppe Lettieri
Alexey Kirillov <lekiravi@yandex-team.ru> writes:
> 08.09.2020, 17:32, "Markus Armbruster" <armbru@redhat.com>:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> On 9/7/20 7:39 AM, Markus Armbruster wrote:
>>>
>>>>>> This is union Netdev plus a common member @peer-id, less the variant
>>>>>> members for NetClientDriver values 'nic' and 'hubport'.
>>>>>>
>>>>>> Can 'type: 'nic' and 'type': 'hubport' occur?
>>>>>
>>>>> No, it can't. We don't support NIC/hubport in query-netdevs, so we neither create this
>>>>> structure for them, nor store config.
>>>> Same for 'none', I guess.
>>>> As defined, NetdevInfo allows types 'none', 'nic', and 'hubport', it
>>>> just has no variant members for them. The fact that they can't occur is
>>>> not coded into the type, and therefore not visible in introspection.
>>>> To make introspection more precise, we'd have to define a new enum
>>>> type.
>>>> How much that would complicate the C code is unclear.
>>>> Do we need it to be more precise? Eric, got an opinion?
>>>
>>> Is the problem that a new enum would be duplicating things?
>>
>> Enumerating network drivers twice is mildly annoying. I worry more
>> about having to convert between the two enumerations in C.
>>
>> My actual question: do we need query-qmp-schema report the precise set
>> of 'type' values? As is, it reports a few that can't actually occur.
>>
>>> Is it
>>> worth allowing one enum to have a 'base':'OtherEnum' in the schema to
>>> reduce some of the duplication?
>>
>> We could then generate functions (or macros) to convert from base enum
>> to extended enum, and back, where the latter can fail.
>>
>> Worthwhile only if we have sufficient use for it.
>
> I'm sorry, did I understand correctly that at the moment I don't need any
> additional changes in the patch yet?
Depends on how we answer my question: do we need query-qmp-schema report
the precise set of 'type' values? As is, it reports a few that can't
actually occur.
If we need it to be precise, you have to define a suitable enum.
Else, you may define such an enum, or reuse NetClientDriver.
If no clear answer emerges, the decision devolves to the maintainer in
charge (Jason, I think).
In my opinion, defining a suitable enum is the more prudent choice.
It partially duplicates NetClientDriver. No big deal for the schema, as
you already duplicate most of Netdev. Possibly awkward in C, but to
know for sure, we'd have to try.
> If that is, I will continue using NetClientDriver as a discriminator.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-09-16 11:29 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 18:23 [PATCH v3 0/4] Introducing QMP query-netdevs command Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 1/4] qapi: net: Add " Alexey Kirillov
2020-09-02 11:23 ` Markus Armbruster
2020-09-07 10:37 ` Alexey Kirillov
2020-09-07 12:39 ` Markus Armbruster
2020-09-08 12:36 ` Eric Blake
2020-09-08 14:31 ` Markus Armbruster
2020-09-16 9:37 ` Alexey Kirillov
2020-09-16 11:28 ` Markus Armbruster
2020-09-08 15:52 ` Alexey Kirillov
2020-09-09 11:41 ` Markus Armbruster
2020-09-09 12:45 ` Alexey Kirillov
2020-09-08 14:29 ` Markus Armbruster
2020-09-08 15:52 ` Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 2/4] tests: Add tests for " Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 3/4] hmp: Use QMP query-netdevs in hmp_info_network Alexey Kirillov
2020-09-01 18:23 ` [PATCH v3 4/4] net: Do not use legacy info_str for backends Alexey Kirillov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).