Re: [PATCH v5 05/12] qapi: net: add stream and dgram netdevs

[Markus Armbruster]

Laurent Vivier <lvivier@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 <lvivier@redhat.com>
> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>

Suggest to add a summary of what we learned during review.  Let me try.

  The two new types need to parsed the modern way with -netdev, because
  <explanation goes here>.

  The previous commit paved the way for parsing the modern way, but
  omitted one detail: how to pick modern vs. traditional, in
  netdev_is_modern().

  We want to pick based on the value of parameter "type".  But how to
  extract it from the option argument?

  Parsing the option argument, either the modern or the traditional way,
  extracts it for us, but only if parsing succeeds.

  If parsing fails, there is no good option.  No matter which parser we
  pick, it'll be the wrong one for some arguments, and the error
  reporting will be confusing.

  Fortunately, the traditional parser accepts *anything* when called in
  a certain way.  This maximizes our chance to extract the value of
  "type", and in turn minimizes the risk of confusing error reporting.

How do you like it?