Re: [Qemu-devel] [PATCH v2] sockets: fix parsing of ipv4/ipv6 opts in parse_socket_addr

[Paolo Bonzini]

On 25/01/2018 18:14, Daniel P. Berrangé wrote:
> From: "Daniel P. Berrange" <address@hidden>
> 
> The inet_parse() function looks for 'ipv4' and 'ipv6' flags, but only
> treats them as bare bool flags. The normal QemuOpts parsing would allow
> on/off values to be set too.
> 
> This updates inet_parse() so that its handling of the 'ipv4' and 'ipv6'
> flags matches that done by QemuOpts.
> 
> This impacts the NBD block driver parsing the legacy filename syntax and
> the migration code parsing the socket scheme.
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  util/qemu-sockets.c | 44 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index d6a1e1759e..300ebce795 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -554,6 +554,33 @@ err:
>  }
>  
>  /* compatibility wrapper */
> +static int inet_parse_flag(const char *flagname, const char *optstr, bool 
> *val,
> +                           Error **errp)
> +{
> +    char *end;
> +    size_t len;
> +
> +    end = strstr(optstr, ",");
> +    if (end) {
> +        if (end[1] == ',') { /* Reject 'ipv6=on,,foo' */
> +            error_setg(errp, "error parsing '%s' flag '%s'", flagname, 
> optstr);
> +            return -1;
> +        }
> +        len = end - optstr;
> +    } else {
> +        len = strlen(optstr);
> +    }
> +    if (len == 0 || (len == 3 && strncmp(optstr, "=on", len) == 0)) {
> +        *val = true;
> +    } else if ((len == 4) && strncmp(optstr, "=off", len) == 0) {
> +        *val = false;
> +    } else {
> +        error_setg(errp, "error parsing '%s' flag '%s'", flagname, optstr);
> +        return -1;
> +    }
> +    return 0;
> +}
> +
>  int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
>  {
>      const char *optstr, *h;
> @@ -561,6 +588,7 @@ int inet_parse(InetSocketAddress *addr, const char *str, 
> Error **errp)
>      char port[33];
>      int to;
>      int pos;
> +    char *begin;
>  
>      memset(addr, 0, sizeof(*addr));
>  
> @@ -602,11 +630,19 @@ int inet_parse(InetSocketAddress *addr, const char 
> *str, Error **errp)
>          addr->has_to = true;
>          addr->to = to;
>      }
> -    if (strstr(optstr, ",ipv4")) {
> -        addr->ipv4 = addr->has_ipv4 = true;
> +    begin = strstr(optstr, ",ipv4");
> +    if (begin) {
> +        if (inet_parse_flag("ipv4", begin + 5, &addr->ipv4, errp) < 0) {
> +            return -1;
> +        }
> +        addr->has_ipv4 = true;
>      }
> -    if (strstr(optstr, ",ipv6")) {
> -        addr->ipv6 = addr->has_ipv6 = true;
> +    begin = strstr(optstr, ",ipv6");
> +    if (begin) {
> +        if (inet_parse_flag("ipv6", begin + 5, &addr->ipv6, errp) < 0) {
> +            return -1;
> +        }
> +        addr->has_ipv6 = true;
>      }
>      return 0;
>  }
> 

Applied with Eric's suggested change.

Paolo