Laurent Vivier <lviv...@redhat.com> writes:
> On 15/06/2022 13:46, Markus Armbruster wrote:
>> Laurent Vivier <lviv...@redhat.com> writes:
>>
>>> On 13/05/2022 13:44, Markus Armbruster wrote:
>>>> 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/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".
>>>
>>> I think "type" is always passed because of the '.implied_opt_name =
>>> "type"'. Am I wrong?
>>
>> .implied_opt_name = "type" lets you shorten "type=T,..." to "T,...". It
>> doesn't make key "type" mandatory. "-netdev id=foo" is still permitted.
>> Even "-netdev ''" is.
>
>
> In fact type is checked before by QAPI definition:
>
> { 'union': 'Netdev',
> 'base': { 'id': 'str', 'type': 'NetClientDriver' },
> 'discriminator': 'type',
> ...
>
> As it's the discriminator it must be there.
>
> $ qemu-system-x86_64 -netdev id=foo
> qemu-system-x86_64: -netdev id=foo: Parameter 'type' is missing
It does crash for me:
(gdb) bt
#0 0x00007ffff4d25dcb in __strcmp_avx2 () at /lib64/libc.so.6
#1 0x0000555555b4574b in netdev_is_modern (optarg=0x7fffffffe2ae "id=foo")
at ../net/net.c:1626
#2 0x0000555555b457ad in net_client_parse
(opts_list=0x555556563780 <qemu_netdev_opts>, optarg=0x7fffffffe2ae
"id=foo") at ../net/net.c:1636
#3 0x0000555555ad98de in qemu_init (argc=3, argv=0x7fffffffdf08, envp=0x0)
at ../softmmu/vl.c:2901
#4 0x0000555555842c01 in qemu_main (argc=3, argv=0x7fffffffdf08, envp=0x0)
at ../softmmu/main.c:35
#5 0x0000555555842c37 in main (argc=3, argv=0x7fffffffdf08)
at ../softmmu/main.c:45
(gdb) up
#1 0x0000555555b4574b in netdev_is_modern (optarg=0x7fffffffe2ae "id=foo")
at ../net/net.c:1626
1626 is_modern = strcmp(netdev, "stream") == 0 ||
(gdb) p netdev
$1 = 0x0
This is
https://github.com/patchew-project/qemu
tags/patchew/20220512080932.735962-1-lviv...@redhat.com
I suspect you tested with your v3, which doesn't crash for me, either.
[...]
