Laurent Vivier <lviv...@redhat.com> writes:
> Copied from socket netdev file and modified to use SocketAddress
> to be able to introduce new features like unix socket.
>
> "udp" and "mcast" are squashed into dgram netdev, multicast is detected
> according to the IP address type.
> "listen" and "connect" modes are managed by stream netdev. An optional
> parameter "server" defines the mode (server by default)
>
> Signed-off-by: Laurent Vivier <lviv...@redhat.com>
> ---
> hmp-commands.hx | 2 +-
> net/clients.h | 6 +
> net/dgram.c | 630 ++++++++++++++++++++++++++++++++++++++++++++++++
> net/hub.c | 2 +
> net/meson.build | 2 +
> net/net.c | 24 +-
> net/stream.c | 425 ++++++++++++++++++++++++++++++++
> qapi/net.json | 38 ++-
> 8 files changed, 1125 insertions(+), 4 deletions(-)
> create mode 100644 net/dgram.c
> create mode 100644 net/stream.c
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 03e6a73d1f55..172dbab1dfed 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1269,7 +1269,7 @@ ERST
> {
> .name = "netdev_add",
> .args_type = "netdev:O",
> - .params =
> "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
> + .params =
> "[user|tap|socket|stream|dgram|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
> .help = "add host network device",
> .cmd = hmp_netdev_add,
> .command_completion = netdev_add_completion,
Does qemu-options.hx need an update, too?
> diff --git a/net/clients.h b/net/clients.h
> index 92f9b59aedce..c1b51d79b147 100644
> --- a/net/clients.h
> +++ b/net/clients.h
> @@ -40,6 +40,12 @@ int net_init_hubport(const Netdev *netdev, const char
> *name,
> int net_init_socket(const Netdev *netdev, const char *name,
> NetClientState *peer, Error **errp);
>
> +int net_init_stream(const Netdev *netdev, const char *name,
> + NetClientState *peer, Error **errp);
> +
> +int net_init_dgram(const Netdev *netdev, const char *name,
> + NetClientState *peer, Error **errp);
> +
> int net_init_tap(const Netdev *netdev, const char *name,
> NetClientState *peer, Error **errp);
>
> diff --git a/net/dgram.c b/net/dgram.c
> new file mode 100644
> index 000000000000..aa4240501ed0
> --- /dev/null
> +++ b/net/dgram.c
> @@ -0,0 +1,630 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
Blank line here, please.
Why not GPLv2+?
> +#include "qemu/osdep.h"
[...]
> diff --git a/net/net.c b/net/net.c
> index 2aab7167316c..fd6b30a10c57 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1015,6 +1015,8 @@ static int (* const
> net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
> #endif
> [NET_CLIENT_DRIVER_TAP] = net_init_tap,
> [NET_CLIENT_DRIVER_SOCKET] = net_init_socket,
> + [NET_CLIENT_DRIVER_STREAM] = net_init_stream,
> + [NET_CLIENT_DRIVER_DGRAM] = net_init_dgram,
> #ifdef CONFIG_VDE
> [NET_CLIENT_DRIVER_VDE] = net_init_vde,
> #endif
> @@ -1097,6 +1099,8 @@ void show_netdevs(void)
> int idx;
> const char *available_netdevs[] = {
> "socket",
> + "stream",
> + "dgram",
> "hubport",
> "tap",
> #ifdef CONFIG_SLIRP
> @@ -1606,7 +1610,25 @@ int net_init_clients(Error **errp)
> */
> static bool netdev_is_modern(const char *optarg)
> {
> - return false;
> + static QemuOptsList dummy_opts = {
> + .name = "netdev",
> + .implied_opt_name = "type",
> + .head = QTAILQ_HEAD_INITIALIZER(dummy_opts.head),
> + .desc = { { } },
> + };
> + const char *netdev;
> + QemuOpts *opts;
> + bool is_modern;
> +
> + opts = qemu_opts_parse(&dummy_opts, optarg, true, &error_fatal);
> + netdev = qemu_opt_get(opts, "type");
> +
> + is_modern = strcmp(netdev, "stream") == 0 ||
> + strcmp(netdev, "dgram") == 0;
Crashes when user neglects to pass "type".
> +
> + qemu_opts_reset(&dummy_opts);
> +
> + return is_modern;
> }
Reminder: netdev_is_modern() governs whether to use QemuOpts + opts
visitor or qobject_input_visitor_new_str().
To decide, it uses QemuOpts with a dummy QemuOptsList.
Since we parse errors are fatal here, new syntax must also parse fine as
QemuOpts. Undesirable, I think.
Cleaner, I think:
args = keyval_parse(optarg, "type", NULL, NULL);
if (!args) {
return false;
}
type = qdict_get_try_str(args, "type");
return !g_strcmp0(type, "dgram") || !g_strcmp0(type, "stream");
Not even compile-tested.
Still probematic: syntax error reporting. When keyval_parse() fails
here, we use QemuOpts, and therefore get QemuOpts syntax errors. I fear
that could be quite confusing.
>
> int net_client_parse(QemuOptsList *opts_list, const char *optarg)
> diff --git a/net/stream.c b/net/stream.c
> new file mode 100644
> index 000000000000..fdc97ee43a56
> --- /dev/null
> +++ b/net/stream.c
> @@ -0,0 +1,425 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
Blank line here, please.
Why not GPLv2+?
> +#include "qemu/osdep.h"
[...]
> diff --git a/qapi/net.json b/qapi/net.json
> index b92f3f5fb444..eef288886e1b 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -7,6 +7,7 @@
> ##
>
> { 'include': 'common.json' }
> +{ 'include': 'sockets.json' }
>
> ##
> # @set_link:
> @@ -452,6 +453,37 @@
> '*vhostdev': 'str',
> '*queues': 'int' } }
>
> +##
> +# @NetdevStreamOptions:
> +#
> +# Configuration info for stream socket netdev
> +#
> +# @addr: socket address to listen on (server=true)
> +# or connect to (server=false)
> +# @server: create server socket (default: true)
> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'NetdevStreamOptions',
> + 'data': {
> + 'addr': 'SocketAddress',
> + '*server': 'bool' } }
> +
> +##
> +# @NetdevDgramOptions:
> +#
> +# Configuration info for datagram socket netdev.
> +#
> +# @remote: remote address
> +# @local: local address
Defaults?
> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'NetdevDgramOptions',
> + 'data': {
> + '*local': 'SocketAddress',
> + '*remote': 'SocketAddress' } }
> +
> ##
> # @NetClientDriver:
> #
> @@ -462,8 +494,8 @@
> # @vhost-vdpa since 5.1
> ##
> { 'enum': 'NetClientDriver',
> - 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
> - 'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa' ] }
> + 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'stream',
> 'dgram',
> + 'vde', 'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa'
> ] }
Long lines.
>
> ##
> # @Netdev:
> @@ -487,6 +519,8 @@
> 'tap': 'NetdevTapOptions',
> 'l2tpv3': 'NetdevL2TPv3Options',
> 'socket': 'NetdevSocketOptions',
> + 'stream': 'NetdevStreamOptions',
> + 'dgram': 'NetdevDgramOptions',
> 'vde': 'NetdevVdeOptions',
> 'bridge': 'NetdevBridgeOptions',
> 'hubport': 'NetdevHubPortOptions',
