Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2840 bytes --]

On 10/14/2016 04:00 AM, Stefan Hajnoczi wrote:
> Add the AF_VSOCK address family so that qemu-ga will be able to use
> virtio-vsock.
> 
> The AF_VSOCK address family uses <cid, port> address tuples.  The cid is
> the unique identifier comparable to an IP address.  AF_VSOCK does not
> use name resolution so it's easy to convert between struct sockaddr_vm
> and strings.
> 
> This patch defines a VsockSocketAddress instead of trying to piggy-back
> on InetSocketAddress.  This is cleaner in the long run since it avoids
> lots of IPv4 vs IPv6 vs vsock special casing.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---

>  ##
> +# @VsockSocketAddress
> +#
> +# Captures a socket address in the vsock namespace.
> +#
> +# @cid: unique host identifier
> +# @port: port
> +#
> +# Note that string types are used to allow for possible future hostname or
> +# service resolution support.
> +#
> +# Since 2.8
> +##
> +{ 'struct': 'VsockSocketAddress',
> +  'data': {
> +    'cid': 'str',
> +    'port': 'str' } }

It would also be possible to do this now:

{ 'struct': 'VsockSocketAddress',
  'data': { 'cid': 'int', 'port': 'int' } }

then down the road expand to:

{ 'alternate': 'StrOrInt',
  'data': { 's': 'str', 'i': 'int' } }
{ 'struct': 'VsockSocketAddress',
  'data': { 'cid': 'StrOrInt', 'port': 'StrOrInt' } }

although the C code to do type-safe access vsock->cid.u.i or
vsock->cid.u.s based on vsock->cid.type is a bit more verbose than just
accessing vsock->cid as a string and converting every time around.
Where it gets really interesting is that using the alternate would allow
all of these QMP forms:

{ "socket": { "cid": 1, "port": 2 } }
{ "socket": { "cid": "1", "port": "2" } }
{ "socket": { "cid": "host", "port": "service" } }

It MIGHT be worth going the type-safe route, especially if we WANT to
convert the existing SocketAddress uses to use the alternate rather than
always managing things as a string.  But it doesn't have to be in this
patch.

> +static VsockSocketAddress *vsock_parse(const char *str, Error **errp)
> +{
> +    VsockSocketAddress *addr = NULL;
> +    char cid[33];
> +    char port[33];
> +    int n;
> +
> +    if (sscanf(str, "%32[^:]:%32[^,]%n", cid, port, &n) != 2) {

This says stop at the first comma after the colon...

> +        error_setg(errp, "error parsing address '%s'", str);
> +        return NULL;
> +    }
> +    if (str[n] != '\0') {
> +        error_setg(errp, "trailing characters in address '%s'", str);

...but this rejects a trailing comma.  Is a trailing comma possible base
on how QemuOpts work? If so, do you need to handle it here?

Otherwise looking okay to me.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]