* [Qemu-devel] [PATCH v2 0/4] qga: add vsock-listen
@ 2016-10-14 9:00 Stefan Hajnoczi
2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 1/4] qga: drop unused sockaddr in accept(2) call Stefan Hajnoczi
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Stefan Hajnoczi @ 2016-10-14 9:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth, Stefan Hajnoczi
v2:
* s/seasy/easy/ typo fix in commit description [Eric]
* Use %n to check for trailing characters in addresses [Eric]
* Added Mike's R-b
This patch series adds virtio-vsock support to the QEMU guest agent.
$ qemu-system-x86_64 -device vhost-vsock-pci,guest-cid=3 ...
(guest)# qemu-ga -m vsock-listen -p 3:1234
You can interact with the qga monitor using the nc-vsock utility:
https://raw.githubusercontent.com/stefanha/linux/dd0d6a2aa62c0fd6cdc9dbd4b3dc4bfd0828c329/nc-vsock.c
$ nc-vsock 3 1234
{'execute': 'guest-info'}
...
For more information about virtio-vsock, see
http://qemu-project.org/Features/VirtioVsock.
Stefan Hajnoczi (4):
qga: drop unused sockaddr in accept(2) call
qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks
sockets: add AF_VSOCK support
qga: add vsock-listen method
qapi-schema.json | 23 +++++-
qga/channel-posix.c | 36 +++++++--
qga/channel.h | 1 +
qga/main.c | 6 +-
util/qemu-sockets.c | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 282 insertions(+), 11 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v2 1/4] qga: drop unused sockaddr in accept(2) call 2016-10-14 9:00 [Qemu-devel] [PATCH v2 0/4] qga: add vsock-listen Stefan Hajnoczi @ 2016-10-14 9:00 ` Stefan Hajnoczi 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks Stefan Hajnoczi ` (2 subsequent siblings) 3 siblings, 0 replies; 13+ messages in thread From: Stefan Hajnoczi @ 2016-10-14 9:00 UTC (permalink / raw) To: qemu-devel; +Cc: Michael Roth, Stefan Hajnoczi ga_channel_listen_accept() is currently hard-coded to support only AF_UNIX because the struct sockaddr_un type is used. This function should work with any address family. Drop the sockaddr since the client address is unused and is an optional argument to accept(2). Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- qga/channel-posix.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/qga/channel-posix.c b/qga/channel-posix.c index bb65d8b..bf32158 100644 --- a/qga/channel-posix.c +++ b/qga/channel-posix.c @@ -26,13 +26,10 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel, GAChannel *c = data; int ret, client_fd; bool accepted = false; - struct sockaddr_un addr; - socklen_t addrlen = sizeof(addr); g_assert(channel != NULL); - client_fd = qemu_accept(g_io_channel_unix_get_fd(channel), - (struct sockaddr *)&addr, &addrlen); + client_fd = qemu_accept(g_io_channel_unix_get_fd(channel), NULL, NULL); if (client_fd == -1) { g_warning("error converting fd to gsocket: %s", strerror(errno)); goto out; -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks 2016-10-14 9:00 [Qemu-devel] [PATCH v2 0/4] qga: add vsock-listen Stefan Hajnoczi 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 1/4] qga: drop unused sockaddr in accept(2) call Stefan Hajnoczi @ 2016-10-14 9:00 ` Stefan Hajnoczi 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 4/4] qga: add vsock-listen method Stefan Hajnoczi 3 siblings, 0 replies; 13+ messages in thread From: Stefan Hajnoczi @ 2016-10-14 9:00 UTC (permalink / raw) To: qemu-devel; +Cc: Michael Roth, Stefan Hajnoczi Throughout the code there are c->listen_channel checks which manage the listen socket file descriptor (waiting for accept(2), closing the file descriptor, etc). These checks are currently preceded by explicit c->method == GA_CHANNEL_UNIX_LISTEN checks. Explicit GA_CHANNEL_UNIX_LISTEN checks are not necessary since serial channel types do not create the listen channel (c->listen_channel). As more listen channel types are added, explicitly checking all of them becomes messy. Rely on c->listen_channel to determine whether or not a listen socket file descriptor is used. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- qga/channel-posix.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/qga/channel-posix.c b/qga/channel-posix.c index bf32158..579891d 100644 --- a/qga/channel-posix.c +++ b/qga/channel-posix.c @@ -61,7 +61,6 @@ static void ga_channel_listen_add(GAChannel *c, int listen_fd, bool create) static void ga_channel_listen_close(GAChannel *c) { - g_assert(c->method == GA_CHANNEL_UNIX_LISTEN); g_assert(c->listen_channel); g_io_channel_shutdown(c->listen_channel, true, NULL); g_io_channel_unref(c->listen_channel); @@ -77,7 +76,7 @@ static void ga_channel_client_close(GAChannel *c) g_io_channel_shutdown(c->client_channel, true, NULL); g_io_channel_unref(c->client_channel); c->client_channel = NULL; - if (c->method == GA_CHANNEL_UNIX_LISTEN && c->listen_channel) { + if (c->listen_channel) { ga_channel_listen_add(c, 0, false); } } @@ -255,8 +254,7 @@ GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path, void ga_channel_free(GAChannel *c) { - if (c->method == GA_CHANNEL_UNIX_LISTEN - && c->listen_channel) { + if (c->listen_channel) { ga_channel_listen_close(c); } if (c->client_channel) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support 2016-10-14 9:00 [Qemu-devel] [PATCH v2 0/4] qga: add vsock-listen Stefan Hajnoczi 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 1/4] qga: drop unused sockaddr in accept(2) call Stefan Hajnoczi 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks Stefan Hajnoczi @ 2016-10-14 9:00 ` Stefan Hajnoczi 2016-10-14 14:40 ` Eric Blake 2016-10-25 23:51 ` Michael Roth 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 4/4] qga: add vsock-listen method Stefan Hajnoczi 3 siblings, 2 replies; 13+ messages in thread From: Stefan Hajnoczi @ 2016-10-14 9:00 UTC (permalink / raw) To: qemu-devel; +Cc: Michael Roth, Stefan Hajnoczi 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> --- v2: * s/seasy/easy/ typo fix in commit description [Eric] * Use %n to check for trailing characters in addresses [Eric] --- qapi-schema.json | 23 +++++- util/qemu-sockets.c | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 249 insertions(+), 1 deletion(-) diff --git a/qapi-schema.json b/qapi-schema.json index 9e47b47..12aea99 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -988,12 +988,14 @@ # # @unix: unix socket # +# @vsock: vsock family (since 2.8) +# # @unknown: otherwise # # Since: 2.1 ## { 'enum': 'NetworkAddressFamily', - 'data': [ 'ipv4', 'ipv6', 'unix', 'unknown' ] } + 'data': [ 'ipv4', 'ipv6', 'unix', 'vsock', 'unknown' ] } ## # @VncBasicInfo @@ -3018,6 +3020,24 @@ 'path': 'str' } } ## +# @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' } } + +## # @SocketAddress # # Captures the address of a socket, which could also be a named file descriptor @@ -3028,6 +3048,7 @@ 'data': { 'inet': 'InetSocketAddress', 'unix': 'UnixSocketAddress', + 'vsock': 'VsockSocketAddress', 'fd': 'String' } } ## diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 6db48b3..6ef3cc5 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -17,6 +17,10 @@ */ #include "qemu/osdep.h" +#ifdef AF_VSOCK +#include <linux/vm_sockets.h> +#endif /* AF_VSOCK */ + #include "monitor/monitor.h" #include "qapi/error.h" #include "qemu/sockets.h" @@ -75,6 +79,9 @@ NetworkAddressFamily inet_netfamily(int family) case PF_INET6: return NETWORK_ADDRESS_FAMILY_IPV6; case PF_INET: return NETWORK_ADDRESS_FAMILY_IPV4; case PF_UNIX: return NETWORK_ADDRESS_FAMILY_UNIX; +#ifdef AF_VSOCK + case PF_VSOCK: return NETWORK_ADDRESS_FAMILY_VSOCK; +#endif /* AF_VSOCK */ } return NETWORK_ADDRESS_FAMILY_UNKNOWN; } @@ -650,6 +657,181 @@ int inet_connect(const char *str, Error **errp) return sock; } +#ifdef AF_VSOCK +static bool vsock_parse_vaddr_to_sockaddr(const VsockSocketAddress *vaddr, + struct sockaddr_vm *svm, + Error **errp) +{ + unsigned long long val; + + memset(svm, 0, sizeof(*svm)); + svm->svm_family = AF_VSOCK; + + if (parse_uint_full(vaddr->cid, &val, 10) < 0 || + val > UINT32_MAX) { + error_setg(errp, "Failed to parse cid '%s'", vaddr->cid); + return false; + } + svm->svm_cid = val; + + if (parse_uint_full(vaddr->port, &val, 10) < 0 || + val > UINT32_MAX) { + error_setg(errp, "Failed to parse port '%s'", vaddr->port); + return false; + } + svm->svm_port = val; + + return true; +} + +static int vsock_connect_addr(const struct sockaddr_vm *svm, bool *in_progress, + ConnectState *connect_state, Error **errp) +{ + int sock, rc; + + *in_progress = false; + + sock = qemu_socket(AF_VSOCK, SOCK_STREAM, 0); + if (sock < 0) { + error_setg_errno(errp, errno, "Failed to create socket"); + return -1; + } + if (connect_state != NULL) { + qemu_set_nonblock(sock); + } + /* connect to peer */ + do { + rc = 0; + if (connect(sock, (const struct sockaddr *)svm, sizeof(*svm)) < 0) { + rc = -errno; + } + } while (rc == -EINTR); + + if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) { + connect_state->fd = sock; + qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state); + *in_progress = true; + } else if (rc < 0) { + error_setg_errno(errp, errno, "Failed to connect socket"); + closesocket(sock); + return -1; + } + return sock; +} + +static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp, + NonBlockingConnectHandler *callback, + void *opaque) +{ + struct sockaddr_vm svm; + int sock = -1; + bool in_progress; + ConnectState *connect_state = NULL; + + if (!vsock_parse_vaddr_to_sockaddr(vaddr, &svm, errp)) { + return -1; + } + + if (callback != NULL) { + connect_state = g_malloc0(sizeof(*connect_state)); + connect_state->callback = callback; + connect_state->opaque = opaque; + } + + sock = vsock_connect_addr(&svm, &in_progress, connect_state, errp); + if (sock < 0) { + /* do nothing */ + } else if (in_progress) { + /* wait_for_connect() will do the rest */ + return sock; + } else { + if (callback) { + callback(sock, NULL, opaque); + } + } + g_free(connect_state); + return sock; +} + +static int vsock_listen_saddr(VsockSocketAddress *vaddr, + Error **errp) +{ + struct sockaddr_vm svm; + int slisten; + + if (!vsock_parse_vaddr_to_sockaddr(vaddr, &svm, errp)) { + return -1; + } + + slisten = qemu_socket(AF_VSOCK, SOCK_STREAM, 0); + if (slisten < 0) { + error_setg_errno(errp, errno, "Failed to create socket"); + return -1; + } + + if (bind(slisten, (const struct sockaddr *)&svm, sizeof(svm)) != 0) { + error_setg_errno(errp, errno, "Failed to bind socket"); + closesocket(slisten); + return -1; + } + + if (listen(slisten, 1) != 0) { + error_setg_errno(errp, errno, "Failed to listen on socket"); + closesocket(slisten); + return -1; + } + return slisten; +} + +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) { + error_setg(errp, "error parsing address '%s'", str); + return NULL; + } + if (str[n] != '\0') { + error_setg(errp, "trailing characters in address '%s'", str); + return NULL; + } + + addr = g_new0(VsockSocketAddress, 1); + addr->cid = g_strdup(cid); + addr->port = g_strdup(port); + return addr; +} +#else +static void vsock_unsupported(Error **errp) +{ + error_setg(errp, "socket family AF_VSOCK unsupported"); +} + +static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp, + NonBlockingConnectHandler *callback, + void *opaque) +{ + vsock_unsupported(errp); + return -1; +} + +static int vsock_listen_saddr(VsockSocketAddress *vaddr, + Error **errp) +{ + vsock_unsupported(errp); + return -1; +} + +static VsockSocketAddress *vsock_parse(const char *str, Error **errp) +{ + vsock_unsupported(errp); + return NULL; +} +#endif /* AF_VSOCK */ + #ifndef _WIN32 static int unix_listen_saddr(UnixSocketAddress *saddr, @@ -864,6 +1046,12 @@ SocketAddress *socket_parse(const char *str, Error **errp) addr->u.fd.data = g_new(String, 1); addr->u.fd.data->str = g_strdup(str + 3); } + } else if (strstart(str, "vsock:", NULL)) { + addr->type = SOCKET_ADDRESS_KIND_VSOCK; + addr->u.vsock.data = vsock_parse(str + strlen("vsock:"), errp); + if (addr->u.vsock.data == NULL) { + goto fail; + } } else { addr->type = SOCKET_ADDRESS_KIND_INET; addr->u.inet.data = inet_parse(str, errp); @@ -900,6 +1088,10 @@ int socket_connect(SocketAddress *addr, Error **errp, } break; + case SOCKET_ADDRESS_KIND_VSOCK: + fd = vsock_connect_saddr(addr->u.vsock.data, errp, callback, opaque); + break; + default: abort(); } @@ -923,6 +1115,10 @@ int socket_listen(SocketAddress *addr, Error **errp) fd = monitor_get_fd(cur_mon, addr->u.fd.data->str, errp); break; + case SOCKET_ADDRESS_KIND_VSOCK: + fd = vsock_listen_saddr(addr->u.vsock.data, errp); + break; + default: abort(); } @@ -1022,6 +1218,26 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa, } #endif /* WIN32 */ +#ifdef AF_VSOCK +static SocketAddress * +socket_sockaddr_to_address_vsock(struct sockaddr_storage *sa, + socklen_t salen, + Error **errp) +{ + SocketAddress *addr; + VsockSocketAddress *vaddr; + struct sockaddr_vm *svm = (struct sockaddr_vm *)sa; + + addr = g_new0(SocketAddress, 1); + addr->type = SOCKET_ADDRESS_KIND_VSOCK; + addr->u.vsock.data = vaddr = g_new0(VsockSocketAddress, 1); + vaddr->cid = g_strdup_printf("%u", svm->svm_cid); + vaddr->port = g_strdup_printf("%u", svm->svm_port); + + return addr; +} +#endif /* AF_VSOCK */ + SocketAddress * socket_sockaddr_to_address(struct sockaddr_storage *sa, socklen_t salen, @@ -1037,6 +1253,11 @@ socket_sockaddr_to_address(struct sockaddr_storage *sa, return socket_sockaddr_to_address_unix(sa, salen, errp); #endif /* WIN32 */ +#ifdef AF_VSOCK + case AF_VSOCK: + return socket_sockaddr_to_address_vsock(sa, salen, errp); +#endif + default: error_setg(errp, "socket family %d unsupported", sa->ss_family); @@ -1103,6 +1324,12 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp) buf = g_strdup(addr->u.fd.data->str); break; + case SOCKET_ADDRESS_KIND_VSOCK: + buf = g_strdup_printf("%s:%s", + addr->u.vsock.data->cid, + addr->u.vsock.data->port); + break; + default: error_setg(errp, "socket family %d unsupported", addr->type); -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi @ 2016-10-14 14:40 ` Eric Blake 2016-10-16 13:35 ` Stefan Hajnoczi 2016-10-25 23:51 ` Michael Roth 1 sibling, 1 reply; 13+ messages in thread From: Eric Blake @ 2016-10-14 14:40 UTC (permalink / raw) To: Stefan Hajnoczi, qemu-devel; +Cc: Michael Roth [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support 2016-10-14 14:40 ` Eric Blake @ 2016-10-16 13:35 ` Stefan Hajnoczi 2016-10-17 14:25 ` Eric Blake 0 siblings, 1 reply; 13+ messages in thread From: Stefan Hajnoczi @ 2016-10-16 13:35 UTC (permalink / raw) To: Eric Blake; +Cc: qemu-devel, Michael Roth [-- Attachment #1: Type: text/plain, Size: 3196 bytes --] On Fri, Oct 14, 2016 at 09:40:40AM -0500, Eric Blake wrote: > 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? Actually I just wanted to grab characters up until the end of string. It wasn't clear from the sscanf(3) man page what the best way to do that was, so I kept the comma which is also used in tcp addresses (because they support additional comma-separated options). Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support 2016-10-16 13:35 ` Stefan Hajnoczi @ 2016-10-17 14:25 ` Eric Blake 2016-10-18 10:21 ` Stefan Hajnoczi 0 siblings, 1 reply; 13+ messages in thread From: Eric Blake @ 2016-10-17 14:25 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: qemu-devel, Michael Roth [-- Attachment #1: Type: text/plain, Size: 1175 bytes --] On 10/16/2016 08:35 AM, Stefan Hajnoczi wrote: >>> + >>> + 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? > > Actually I just wanted to grab characters up until the end of string. > It wasn't clear from the sscanf(3) man page what the best way to do that > was, so I kept the comma which is also used in tcp addresses (because > they support additional comma-separated options). %32s instead of %32[^,] should grab up to all 32 remaining characters in the string; your %n trick then ensures there is no garbage. I guess it's still a question of whether we want to always treat a comma as trailing garbage. -- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support 2016-10-17 14:25 ` Eric Blake @ 2016-10-18 10:21 ` Stefan Hajnoczi 2017-11-05 18:59 ` Kashyap Chamarthy 0 siblings, 1 reply; 13+ messages in thread From: Stefan Hajnoczi @ 2016-10-18 10:21 UTC (permalink / raw) To: Eric Blake; +Cc: qemu-devel, Michael Roth [-- Attachment #1: Type: text/plain, Size: 1331 bytes --] On Mon, Oct 17, 2016 at 09:25:46AM -0500, Eric Blake wrote: > On 10/16/2016 08:35 AM, Stefan Hajnoczi wrote: > > >>> + > >>> + 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? > > > > Actually I just wanted to grab characters up until the end of string. > > It wasn't clear from the sscanf(3) man page what the best way to do that > > was, so I kept the comma which is also used in tcp addresses (because > > they support additional comma-separated options). > > %32s instead of %32[^,] should grab up to all 32 remaining characters in > the string; your %n trick then ensures there is no garbage. I guess > it's still a question of whether we want to always treat a comma as > trailing garbage. No comma options are currently accepted. Treating comma as garbage seems okay. Mike: Please change %32[^,] to %32s when merging. Thanks! Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support 2016-10-18 10:21 ` Stefan Hajnoczi @ 2017-11-05 18:59 ` Kashyap Chamarthy 2017-11-06 17:12 ` Stefan Hajnoczi 0 siblings, 1 reply; 13+ messages in thread From: Kashyap Chamarthy @ 2017-11-05 18:59 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Eric Blake, qemu-devel, Michael Roth [Reviving a more than 1-year old thread. I randomly noticed this (refer below) while browsing through AF_VSOCK changes in my 'qemu-devel' maildir.] On Tue, Oct 18, 2016 at 11:21:23AM +0100, Stefan Hajnoczi wrote: > On Mon, Oct 17, 2016 at 09:25:46AM -0500, Eric Blake wrote: > > On 10/16/2016 08:35 AM, Stefan Hajnoczi wrote: > > > > >>> + > > >>> + 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? > > > > > > Actually I just wanted to grab characters up until the end of string. > > > It wasn't clear from the sscanf(3) man page what the best way to do that > > > was, so I kept the comma which is also used in tcp addresses (because > > > they support additional comma-separated options). > > > > %32s instead of %32[^,] should grab up to all 32 remaining characters in > > the string; your %n trick then ensures there is no garbage. I guess > > it's still a question of whether we want to always treat a comma as > > trailing garbage. > > No comma options are currently accepted. Treating comma as garbage > seems okay. > > Mike: Please change %32[^,] to %32s when merging. Thanks! The above change didn't take place while merging at that time as part of v2.7.0-1720-g6a02c80. I still see %32[^,] in all the sscanf(3) calls in util/util/qemu-sockets.c Is it still important to change? -- /kashyap ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support 2017-11-05 18:59 ` Kashyap Chamarthy @ 2017-11-06 17:12 ` Stefan Hajnoczi 0 siblings, 0 replies; 13+ messages in thread From: Stefan Hajnoczi @ 2017-11-06 17:12 UTC (permalink / raw) To: Kashyap Chamarthy; +Cc: Eric Blake, qemu-devel, Michael Roth [-- Attachment #1: Type: text/plain, Size: 2063 bytes --] On Sun, Nov 05, 2017 at 07:59:59PM +0100, Kashyap Chamarthy wrote: > [Reviving a more than 1-year old thread. I randomly noticed this (refer > below) while browsing through AF_VSOCK changes in my 'qemu-devel' > maildir.] > > On Tue, Oct 18, 2016 at 11:21:23AM +0100, Stefan Hajnoczi wrote: > > On Mon, Oct 17, 2016 at 09:25:46AM -0500, Eric Blake wrote: > > > On 10/16/2016 08:35 AM, Stefan Hajnoczi wrote: > > > > > > >>> + > > > >>> + 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? > > > > > > > > Actually I just wanted to grab characters up until the end of string. > > > > It wasn't clear from the sscanf(3) man page what the best way to do that > > > > was, so I kept the comma which is also used in tcp addresses (because > > > > they support additional comma-separated options). > > > > > > %32s instead of %32[^,] should grab up to all 32 remaining characters in > > > the string; your %n trick then ensures there is no garbage. I guess > > > it's still a question of whether we want to always treat a comma as > > > trailing garbage. > > > > No comma options are currently accepted. Treating comma as garbage > > seems okay. > > > > Mike: Please change %32[^,] to %32s when merging. Thanks! > > The above change didn't take place while merging at that time as part of > v2.7.0-1720-g6a02c80. > > I still see %32[^,] in all the sscanf(3) calls in > util/util/qemu-sockets.c > > Is it still important to change? Thanks for noticing. I don't think it's necessary to make this change now. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi 2016-10-14 14:40 ` Eric Blake @ 2016-10-25 23:51 ` Michael Roth 2016-11-01 0:58 ` Michael Roth 1 sibling, 1 reply; 13+ messages in thread From: Michael Roth @ 2016-10-25 23:51 UTC (permalink / raw) To: Stefan Hajnoczi, qemu-devel Quoting Stefan Hajnoczi (2016-10-14 04:00:55) > 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> > --- > v2: > * s/seasy/easy/ typo fix in commit description [Eric] > * Use %n to check for trailing characters in addresses [Eric] > --- > qapi-schema.json | 23 +++++- > util/qemu-sockets.c | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 249 insertions(+), 1 deletion(-) > > diff --git a/qapi-schema.json b/qapi-schema.json > index 9e47b47..12aea99 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -988,12 +988,14 @@ > # > # @unix: unix socket > # > +# @vsock: vsock family (since 2.8) > +# > # @unknown: otherwise > # > # Since: 2.1 > ## > { 'enum': 'NetworkAddressFamily', > - 'data': [ 'ipv4', 'ipv6', 'unix', 'unknown' ] } > + 'data': [ 'ipv4', 'ipv6', 'unix', 'vsock', 'unknown' ] } > > ## > # @VncBasicInfo > @@ -3018,6 +3020,24 @@ > 'path': 'str' } } > > ## > +# @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' } } > + > +## > # @SocketAddress > # > # Captures the address of a socket, which could also be a named file descriptor > @@ -3028,6 +3048,7 @@ > 'data': { > 'inet': 'InetSocketAddress', > 'unix': 'UnixSocketAddress', > + 'vsock': 'VsockSocketAddress', > 'fd': 'String' } } > > ## > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index 6db48b3..6ef3cc5 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -17,6 +17,10 @@ > */ > #include "qemu/osdep.h" > > +#ifdef AF_VSOCK > +#include <linux/vm_sockets.h> > +#endif /* AF_VSOCK */ I have this series applied locally but I hit some build issues on Ubuntu 14.04 due to linux/vm_sockets.h not being provided by Ubuntu 14.04's linux-libc-dev package. It is however included with linux-libc-dev in 16.04. linux-headers package includes it in both cases, but installs to /usr/src/linux-headers*, which are not part of the default include path. Do you think we need a configure check and CONFIG_AF_VSOCK flag instead? > + > #include "monitor/monitor.h" > #include "qapi/error.h" > #include "qemu/sockets.h" > @@ -75,6 +79,9 @@ NetworkAddressFamily inet_netfamily(int family) > case PF_INET6: return NETWORK_ADDRESS_FAMILY_IPV6; > case PF_INET: return NETWORK_ADDRESS_FAMILY_IPV4; > case PF_UNIX: return NETWORK_ADDRESS_FAMILY_UNIX; > +#ifdef AF_VSOCK > + case PF_VSOCK: return NETWORK_ADDRESS_FAMILY_VSOCK; > +#endif /* AF_VSOCK */ > } > return NETWORK_ADDRESS_FAMILY_UNKNOWN; > } > @@ -650,6 +657,181 @@ int inet_connect(const char *str, Error **errp) > return sock; > } > > +#ifdef AF_VSOCK > +static bool vsock_parse_vaddr_to_sockaddr(const VsockSocketAddress *vaddr, > + struct sockaddr_vm *svm, > + Error **errp) > +{ > + unsigned long long val; > + > + memset(svm, 0, sizeof(*svm)); > + svm->svm_family = AF_VSOCK; > + > + if (parse_uint_full(vaddr->cid, &val, 10) < 0 || > + val > UINT32_MAX) { > + error_setg(errp, "Failed to parse cid '%s'", vaddr->cid); > + return false; > + } > + svm->svm_cid = val; > + > + if (parse_uint_full(vaddr->port, &val, 10) < 0 || > + val > UINT32_MAX) { > + error_setg(errp, "Failed to parse port '%s'", vaddr->port); > + return false; > + } > + svm->svm_port = val; > + > + return true; > +} > + > +static int vsock_connect_addr(const struct sockaddr_vm *svm, bool *in_progress, > + ConnectState *connect_state, Error **errp) > +{ > + int sock, rc; > + > + *in_progress = false; > + > + sock = qemu_socket(AF_VSOCK, SOCK_STREAM, 0); > + if (sock < 0) { > + error_setg_errno(errp, errno, "Failed to create socket"); > + return -1; > + } > + if (connect_state != NULL) { > + qemu_set_nonblock(sock); > + } > + /* connect to peer */ > + do { > + rc = 0; > + if (connect(sock, (const struct sockaddr *)svm, sizeof(*svm)) < 0) { > + rc = -errno; > + } > + } while (rc == -EINTR); > + > + if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) { > + connect_state->fd = sock; > + qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state); > + *in_progress = true; > + } else if (rc < 0) { > + error_setg_errno(errp, errno, "Failed to connect socket"); > + closesocket(sock); > + return -1; > + } > + return sock; > +} > + > +static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp, > + NonBlockingConnectHandler *callback, > + void *opaque) > +{ > + struct sockaddr_vm svm; > + int sock = -1; > + bool in_progress; > + ConnectState *connect_state = NULL; > + > + if (!vsock_parse_vaddr_to_sockaddr(vaddr, &svm, errp)) { > + return -1; > + } > + > + if (callback != NULL) { > + connect_state = g_malloc0(sizeof(*connect_state)); > + connect_state->callback = callback; > + connect_state->opaque = opaque; > + } > + > + sock = vsock_connect_addr(&svm, &in_progress, connect_state, errp); > + if (sock < 0) { > + /* do nothing */ > + } else if (in_progress) { > + /* wait_for_connect() will do the rest */ > + return sock; > + } else { > + if (callback) { > + callback(sock, NULL, opaque); > + } > + } > + g_free(connect_state); > + return sock; > +} > + > +static int vsock_listen_saddr(VsockSocketAddress *vaddr, > + Error **errp) > +{ > + struct sockaddr_vm svm; > + int slisten; > + > + if (!vsock_parse_vaddr_to_sockaddr(vaddr, &svm, errp)) { > + return -1; > + } > + > + slisten = qemu_socket(AF_VSOCK, SOCK_STREAM, 0); > + if (slisten < 0) { > + error_setg_errno(errp, errno, "Failed to create socket"); > + return -1; > + } > + > + if (bind(slisten, (const struct sockaddr *)&svm, sizeof(svm)) != 0) { > + error_setg_errno(errp, errno, "Failed to bind socket"); > + closesocket(slisten); > + return -1; > + } > + > + if (listen(slisten, 1) != 0) { > + error_setg_errno(errp, errno, "Failed to listen on socket"); > + closesocket(slisten); > + return -1; > + } > + return slisten; > +} > + > +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) { > + error_setg(errp, "error parsing address '%s'", str); > + return NULL; > + } > + if (str[n] != '\0') { > + error_setg(errp, "trailing characters in address '%s'", str); > + return NULL; > + } > + > + addr = g_new0(VsockSocketAddress, 1); > + addr->cid = g_strdup(cid); > + addr->port = g_strdup(port); > + return addr; > +} > +#else > +static void vsock_unsupported(Error **errp) > +{ > + error_setg(errp, "socket family AF_VSOCK unsupported"); > +} > + > +static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp, > + NonBlockingConnectHandler *callback, > + void *opaque) > +{ > + vsock_unsupported(errp); > + return -1; > +} > + > +static int vsock_listen_saddr(VsockSocketAddress *vaddr, > + Error **errp) > +{ > + vsock_unsupported(errp); > + return -1; > +} > + > +static VsockSocketAddress *vsock_parse(const char *str, Error **errp) > +{ > + vsock_unsupported(errp); > + return NULL; > +} > +#endif /* AF_VSOCK */ > + > #ifndef _WIN32 > > static int unix_listen_saddr(UnixSocketAddress *saddr, > @@ -864,6 +1046,12 @@ SocketAddress *socket_parse(const char *str, Error **errp) > addr->u.fd.data = g_new(String, 1); > addr->u.fd.data->str = g_strdup(str + 3); > } > + } else if (strstart(str, "vsock:", NULL)) { > + addr->type = SOCKET_ADDRESS_KIND_VSOCK; > + addr->u.vsock.data = vsock_parse(str + strlen("vsock:"), errp); > + if (addr->u.vsock.data == NULL) { > + goto fail; > + } > } else { > addr->type = SOCKET_ADDRESS_KIND_INET; > addr->u.inet.data = inet_parse(str, errp); > @@ -900,6 +1088,10 @@ int socket_connect(SocketAddress *addr, Error **errp, > } > break; > > + case SOCKET_ADDRESS_KIND_VSOCK: > + fd = vsock_connect_saddr(addr->u.vsock.data, errp, callback, opaque); > + break; > + > default: > abort(); > } > @@ -923,6 +1115,10 @@ int socket_listen(SocketAddress *addr, Error **errp) > fd = monitor_get_fd(cur_mon, addr->u.fd.data->str, errp); > break; > > + case SOCKET_ADDRESS_KIND_VSOCK: > + fd = vsock_listen_saddr(addr->u.vsock.data, errp); > + break; > + > default: > abort(); > } > @@ -1022,6 +1218,26 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa, > } > #endif /* WIN32 */ > > +#ifdef AF_VSOCK > +static SocketAddress * > +socket_sockaddr_to_address_vsock(struct sockaddr_storage *sa, > + socklen_t salen, > + Error **errp) > +{ > + SocketAddress *addr; > + VsockSocketAddress *vaddr; > + struct sockaddr_vm *svm = (struct sockaddr_vm *)sa; > + > + addr = g_new0(SocketAddress, 1); > + addr->type = SOCKET_ADDRESS_KIND_VSOCK; > + addr->u.vsock.data = vaddr = g_new0(VsockSocketAddress, 1); > + vaddr->cid = g_strdup_printf("%u", svm->svm_cid); > + vaddr->port = g_strdup_printf("%u", svm->svm_port); > + > + return addr; > +} > +#endif /* AF_VSOCK */ > + > SocketAddress * > socket_sockaddr_to_address(struct sockaddr_storage *sa, > socklen_t salen, > @@ -1037,6 +1253,11 @@ socket_sockaddr_to_address(struct sockaddr_storage *sa, > return socket_sockaddr_to_address_unix(sa, salen, errp); > #endif /* WIN32 */ > > +#ifdef AF_VSOCK > + case AF_VSOCK: > + return socket_sockaddr_to_address_vsock(sa, salen, errp); > +#endif > + > default: > error_setg(errp, "socket family %d unsupported", > sa->ss_family); > @@ -1103,6 +1324,12 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp) > buf = g_strdup(addr->u.fd.data->str); > break; > > + case SOCKET_ADDRESS_KIND_VSOCK: > + buf = g_strdup_printf("%s:%s", > + addr->u.vsock.data->cid, > + addr->u.vsock.data->port); > + break; > + > default: > error_setg(errp, "socket family %d unsupported", > addr->type); > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support 2016-10-25 23:51 ` Michael Roth @ 2016-11-01 0:58 ` Michael Roth 0 siblings, 0 replies; 13+ messages in thread From: Michael Roth @ 2016-11-01 0:58 UTC (permalink / raw) To: Stefan Hajnoczi, qemu-devel Quoting Michael Roth (2016-10-25 18:51:19) > Quoting Stefan Hajnoczi (2016-10-14 04:00:55) > > 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> > > --- > > v2: > > * s/seasy/easy/ typo fix in commit description [Eric] > > * Use %n to check for trailing characters in addresses [Eric] > > --- > > qapi-schema.json | 23 +++++- > > util/qemu-sockets.c | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 249 insertions(+), 1 deletion(-) > > > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 9e47b47..12aea99 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -988,12 +988,14 @@ > > # > > # @unix: unix socket > > # > > +# @vsock: vsock family (since 2.8) > > +# > > # @unknown: otherwise > > # > > # Since: 2.1 > > ## > > { 'enum': 'NetworkAddressFamily', > > - 'data': [ 'ipv4', 'ipv6', 'unix', 'unknown' ] } > > + 'data': [ 'ipv4', 'ipv6', 'unix', 'vsock', 'unknown' ] } > > > > ## > > # @VncBasicInfo > > @@ -3018,6 +3020,24 @@ > > 'path': 'str' } } > > > > ## > > +# @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' } } > > + > > +## > > # @SocketAddress > > # > > # Captures the address of a socket, which could also be a named file descriptor > > @@ -3028,6 +3048,7 @@ > > 'data': { > > 'inet': 'InetSocketAddress', > > 'unix': 'UnixSocketAddress', > > + 'vsock': 'VsockSocketAddress', > > 'fd': 'String' } } > > > > ## > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > > index 6db48b3..6ef3cc5 100644 > > --- a/util/qemu-sockets.c > > +++ b/util/qemu-sockets.c > > @@ -17,6 +17,10 @@ > > */ > > #include "qemu/osdep.h" > > > > +#ifdef AF_VSOCK > > +#include <linux/vm_sockets.h> > > +#endif /* AF_VSOCK */ > > I have this series applied locally but I hit some build issues on Ubuntu > 14.04 due to linux/vm_sockets.h not being provided by Ubuntu 14.04's > linux-libc-dev package. It is however included with linux-libc-dev in > 16.04. linux-headers package includes it in both cases, but installs > to /usr/src/linux-headers*, which are not part of the default include > path. > > Do you think we need a configure check and CONFIG_AF_VSOCK flag instead? I've applied this series with the above-mentioned configure check squashed into this patch. Here's the diff: diff --git a/configure b/configure index d3dafcb..60f1060 100755 --- a/configure +++ b/configure @@ -4605,6 +4605,33 @@ if compile_prog "" "" ; then have_rtnetlink=yes fi +########################################## +# check for usable AF_VSOCK environment +have_af_vsock=no +cat > $TMPC << EOF +#include <errno.h> +#include <sys/types.h> +#include <sys/socket.h> +#if !defined(AF_VSOCK) +# error missing AF_VSOCK flag +#endif +#include <linux/vm_sockets.h> +int main(void) { + int sock, ret; + struct sockaddr_vm svm; + socklen_t len = sizeof(svm); + sock = socket(AF_VSOCK, SOCK_STREAM, 0); + ret = getpeername(sock, (struct sockaddr *)&svm, &len); + if ((ret == -1) && (errno == ENOTCONN)) { + return 0; + } + return -1; +} +EOF +if compile_prog "" "" ; then + have_af_vsock=yes +fi + ################################################# # Sparc implicitly links with --relax, which is # incompatible with -r, so --no-relax should be @@ -5580,6 +5607,10 @@ if test "$replication" = "yes" ; then echo "CONFIG_REPLICATION=y" >> $config_host_mak fi +if test "$have_af_vsock" = "yes" ; then + echo "CONFIG_AF_VSOCK=y" >> $config_host_mak +fi + # Hold two types of flag: # CONFIG_THREAD_SETNAME_BYTHREAD - we've got a way of setting the name on # a thread we have a handle to diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 559f452..e616f48 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -17,9 +17,9 @@ */ #include "qemu/osdep.h" -#ifdef AF_VSOCK +#ifdef CONFIG_AF_VSOCK #include <linux/vm_sockets.h> -#endif /* AF_VSOCK */ +#endif /* CONFIG_AF_VSOCK */ #include "monitor/monitor.h" #include "qapi/error.h" @@ -79,9 +79,9 @@ NetworkAddressFamily inet_netfamily(int family) case PF_INET6: return NETWORK_ADDRESS_FAMILY_IPV6; case PF_INET: return NETWORK_ADDRESS_FAMILY_IPV4; case PF_UNIX: return NETWORK_ADDRESS_FAMILY_UNIX; -#ifdef AF_VSOCK +#ifdef CONFIG_AF_VSOCK case PF_VSOCK: return NETWORK_ADDRESS_FAMILY_VSOCK; -#endif /* AF_VSOCK */ +#endif /* CONFIG_AF_VSOCK */ } return NETWORK_ADDRESS_FAMILY_UNKNOWN; } @@ -657,7 +657,7 @@ int inet_connect(const char *str, Error **errp) return sock; } -#ifdef AF_VSOCK +#ifdef CONFIG_AF_VSOCK static bool vsock_parse_vaddr_to_sockaddr(const VsockSocketAddress *vaddr, struct sockaddr_vm *svm, Error **errp) @@ -830,7 +830,7 @@ static VsockSocketAddress *vsock_parse(const char *str, Error **errp) vsock_unsupported(errp); return NULL; } -#endif /* AF_VSOCK */ +#endif /* CONFIG_AF_VSOCK */ #ifndef _WIN32 @@ -1218,7 +1218,7 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa, } #endif /* WIN32 */ -#ifdef AF_VSOCK +#ifdef CONFIG_AF_VSOCK static SocketAddress * socket_sockaddr_to_address_vsock(struct sockaddr_storage *sa, socklen_t salen, @@ -1236,7 +1236,7 @@ socket_sockaddr_to_address_vsock(struct sockaddr_storage *sa, return addr; } -#endif /* AF_VSOCK */ +#endif /* CONFIG_AF_VSOCK */ SocketAddress * socket_sockaddr_to_address(struct sockaddr_storage *sa, @@ -1253,7 +1253,7 @@ socket_sockaddr_to_address(struct sockaddr_storage *sa, return socket_sockaddr_to_address_unix(sa, salen, errp); #endif /* WIN32 */ -#ifdef AF_VSOCK +#ifdef CONFIG_AF_VSOCK case AF_VSOCK: return socket_sockaddr_to_address_vsock(sa, salen, errp); #endif > > > + > > #include "monitor/monitor.h" > > #include "qapi/error.h" > > #include "qemu/sockets.h" > > @@ -75,6 +79,9 @@ NetworkAddressFamily inet_netfamily(int family) > > case PF_INET6: return NETWORK_ADDRESS_FAMILY_IPV6; > > case PF_INET: return NETWORK_ADDRESS_FAMILY_IPV4; > > case PF_UNIX: return NETWORK_ADDRESS_FAMILY_UNIX; > > +#ifdef AF_VSOCK > > + case PF_VSOCK: return NETWORK_ADDRESS_FAMILY_VSOCK; > > +#endif /* AF_VSOCK */ > > } > > return NETWORK_ADDRESS_FAMILY_UNKNOWN; > > } > > @@ -650,6 +657,181 @@ int inet_connect(const char *str, Error **errp) > > return sock; > > } > > > > +#ifdef AF_VSOCK > > +static bool vsock_parse_vaddr_to_sockaddr(const VsockSocketAddress *vaddr, > > + struct sockaddr_vm *svm, > > + Error **errp) > > +{ > > + unsigned long long val; > > + > > + memset(svm, 0, sizeof(*svm)); > > + svm->svm_family = AF_VSOCK; > > + > > + if (parse_uint_full(vaddr->cid, &val, 10) < 0 || > > + val > UINT32_MAX) { > > + error_setg(errp, "Failed to parse cid '%s'", vaddr->cid); > > + return false; > > + } > > + svm->svm_cid = val; > > + > > + if (parse_uint_full(vaddr->port, &val, 10) < 0 || > > + val > UINT32_MAX) { > > + error_setg(errp, "Failed to parse port '%s'", vaddr->port); > > + return false; > > + } > > + svm->svm_port = val; > > + > > + return true; > > +} > > + > > +static int vsock_connect_addr(const struct sockaddr_vm *svm, bool *in_progress, > > + ConnectState *connect_state, Error **errp) > > +{ > > + int sock, rc; > > + > > + *in_progress = false; > > + > > + sock = qemu_socket(AF_VSOCK, SOCK_STREAM, 0); > > + if (sock < 0) { > > + error_setg_errno(errp, errno, "Failed to create socket"); > > + return -1; > > + } > > + if (connect_state != NULL) { > > + qemu_set_nonblock(sock); > > + } > > + /* connect to peer */ > > + do { > > + rc = 0; > > + if (connect(sock, (const struct sockaddr *)svm, sizeof(*svm)) < 0) { > > + rc = -errno; > > + } > > + } while (rc == -EINTR); > > + > > + if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) { > > + connect_state->fd = sock; > > + qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state); > > + *in_progress = true; > > + } else if (rc < 0) { > > + error_setg_errno(errp, errno, "Failed to connect socket"); > > + closesocket(sock); > > + return -1; > > + } > > + return sock; > > +} > > + > > +static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp, > > + NonBlockingConnectHandler *callback, > > + void *opaque) > > +{ > > + struct sockaddr_vm svm; > > + int sock = -1; > > + bool in_progress; > > + ConnectState *connect_state = NULL; > > + > > + if (!vsock_parse_vaddr_to_sockaddr(vaddr, &svm, errp)) { > > + return -1; > > + } > > + > > + if (callback != NULL) { > > + connect_state = g_malloc0(sizeof(*connect_state)); > > + connect_state->callback = callback; > > + connect_state->opaque = opaque; > > + } > > + > > + sock = vsock_connect_addr(&svm, &in_progress, connect_state, errp); > > + if (sock < 0) { > > + /* do nothing */ > > + } else if (in_progress) { > > + /* wait_for_connect() will do the rest */ > > + return sock; > > + } else { > > + if (callback) { > > + callback(sock, NULL, opaque); > > + } > > + } > > + g_free(connect_state); > > + return sock; > > +} > > + > > +static int vsock_listen_saddr(VsockSocketAddress *vaddr, > > + Error **errp) > > +{ > > + struct sockaddr_vm svm; > > + int slisten; > > + > > + if (!vsock_parse_vaddr_to_sockaddr(vaddr, &svm, errp)) { > > + return -1; > > + } > > + > > + slisten = qemu_socket(AF_VSOCK, SOCK_STREAM, 0); > > + if (slisten < 0) { > > + error_setg_errno(errp, errno, "Failed to create socket"); > > + return -1; > > + } > > + > > + if (bind(slisten, (const struct sockaddr *)&svm, sizeof(svm)) != 0) { > > + error_setg_errno(errp, errno, "Failed to bind socket"); > > + closesocket(slisten); > > + return -1; > > + } > > + > > + if (listen(slisten, 1) != 0) { > > + error_setg_errno(errp, errno, "Failed to listen on socket"); > > + closesocket(slisten); > > + return -1; > > + } > > + return slisten; > > +} > > + > > +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) { > > + error_setg(errp, "error parsing address '%s'", str); > > + return NULL; > > + } > > + if (str[n] != '\0') { > > + error_setg(errp, "trailing characters in address '%s'", str); > > + return NULL; > > + } > > + > > + addr = g_new0(VsockSocketAddress, 1); > > + addr->cid = g_strdup(cid); > > + addr->port = g_strdup(port); > > + return addr; > > +} > > +#else > > +static void vsock_unsupported(Error **errp) > > +{ > > + error_setg(errp, "socket family AF_VSOCK unsupported"); > > +} > > + > > +static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp, > > + NonBlockingConnectHandler *callback, > > + void *opaque) > > +{ > > + vsock_unsupported(errp); > > + return -1; > > +} > > + > > +static int vsock_listen_saddr(VsockSocketAddress *vaddr, > > + Error **errp) > > +{ > > + vsock_unsupported(errp); > > + return -1; > > +} > > + > > +static VsockSocketAddress *vsock_parse(const char *str, Error **errp) > > +{ > > + vsock_unsupported(errp); > > + return NULL; > > +} > > +#endif /* AF_VSOCK */ > > + > > #ifndef _WIN32 > > > > static int unix_listen_saddr(UnixSocketAddress *saddr, > > @@ -864,6 +1046,12 @@ SocketAddress *socket_parse(const char *str, Error **errp) > > addr->u.fd.data = g_new(String, 1); > > addr->u.fd.data->str = g_strdup(str + 3); > > } > > + } else if (strstart(str, "vsock:", NULL)) { > > + addr->type = SOCKET_ADDRESS_KIND_VSOCK; > > + addr->u.vsock.data = vsock_parse(str + strlen("vsock:"), errp); > > + if (addr->u.vsock.data == NULL) { > > + goto fail; > > + } > > } else { > > addr->type = SOCKET_ADDRESS_KIND_INET; > > addr->u.inet.data = inet_parse(str, errp); > > @@ -900,6 +1088,10 @@ int socket_connect(SocketAddress *addr, Error **errp, > > } > > break; > > > > + case SOCKET_ADDRESS_KIND_VSOCK: > > + fd = vsock_connect_saddr(addr->u.vsock.data, errp, callback, opaque); > > + break; > > + > > default: > > abort(); > > } > > @@ -923,6 +1115,10 @@ int socket_listen(SocketAddress *addr, Error **errp) > > fd = monitor_get_fd(cur_mon, addr->u.fd.data->str, errp); > > break; > > > > + case SOCKET_ADDRESS_KIND_VSOCK: > > + fd = vsock_listen_saddr(addr->u.vsock.data, errp); > > + break; > > + > > default: > > abort(); > > } > > @@ -1022,6 +1218,26 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa, > > } > > #endif /* WIN32 */ > > > > +#ifdef AF_VSOCK > > +static SocketAddress * > > +socket_sockaddr_to_address_vsock(struct sockaddr_storage *sa, > > + socklen_t salen, > > + Error **errp) > > +{ > > + SocketAddress *addr; > > + VsockSocketAddress *vaddr; > > + struct sockaddr_vm *svm = (struct sockaddr_vm *)sa; > > + > > + addr = g_new0(SocketAddress, 1); > > + addr->type = SOCKET_ADDRESS_KIND_VSOCK; > > + addr->u.vsock.data = vaddr = g_new0(VsockSocketAddress, 1); > > + vaddr->cid = g_strdup_printf("%u", svm->svm_cid); > > + vaddr->port = g_strdup_printf("%u", svm->svm_port); > > + > > + return addr; > > +} > > +#endif /* AF_VSOCK */ > > + > > SocketAddress * > > socket_sockaddr_to_address(struct sockaddr_storage *sa, > > socklen_t salen, > > @@ -1037,6 +1253,11 @@ socket_sockaddr_to_address(struct sockaddr_storage *sa, > > return socket_sockaddr_to_address_unix(sa, salen, errp); > > #endif /* WIN32 */ > > > > +#ifdef AF_VSOCK > > + case AF_VSOCK: > > + return socket_sockaddr_to_address_vsock(sa, salen, errp); > > +#endif > > + > > default: > > error_setg(errp, "socket family %d unsupported", > > sa->ss_family); > > @@ -1103,6 +1324,12 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp) > > buf = g_strdup(addr->u.fd.data->str); > > break; > > > > + case SOCKET_ADDRESS_KIND_VSOCK: > > + buf = g_strdup_printf("%s:%s", > > + addr->u.vsock.data->cid, > > + addr->u.vsock.data->port); > > + break; > > + > > default: > > error_setg(errp, "socket family %d unsupported", > > addr->type); > > -- > > 2.7.4 > > > > ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 4/4] qga: add vsock-listen method 2016-10-14 9:00 [Qemu-devel] [PATCH v2 0/4] qga: add vsock-listen Stefan Hajnoczi ` (2 preceding siblings ...) 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi @ 2016-10-14 9:00 ` Stefan Hajnoczi 3 siblings, 0 replies; 13+ messages in thread From: Stefan Hajnoczi @ 2016-10-14 9:00 UTC (permalink / raw) To: qemu-devel; +Cc: Michael Roth, Stefan Hajnoczi Add AF_VSOCK (virtio-vsock) support as an alternative to virtio-serial. $ qemu-system-x86_64 -device vhost-vsock-pci,guest-cid=3 ... (guest)# qemu-ga -m vsock-listen -p 3:1234 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- qga/channel-posix.c | 25 +++++++++++++++++++++++++ qga/channel.h | 1 + qga/main.c | 6 ++++-- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/qga/channel-posix.c b/qga/channel-posix.c index 579891d..71582e0 100644 --- a/qga/channel-posix.c +++ b/qga/channel-posix.c @@ -193,6 +193,31 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod ga_channel_listen_add(c, fd, true); break; } + case GA_CHANNEL_VSOCK_LISTEN: { + Error *local_err = NULL; + SocketAddress *addr; + char *addr_str; + int fd; + + addr_str = g_strdup_printf("vsock:%s", path); + addr = socket_parse(addr_str, &local_err); + g_free(addr_str); + if (local_err != NULL) { + g_critical("%s", error_get_pretty(local_err)); + error_free(local_err); + return false; + } + + fd = socket_listen(addr, &local_err); + qapi_free_SocketAddress(addr); + if (local_err != NULL) { + g_critical("%s", error_get_pretty(local_err)); + error_free(local_err); + return false; + } + ga_channel_listen_add(c, fd, true); + break; + } default: g_critical("error binding/listening to specified socket"); return false; diff --git a/qga/channel.h b/qga/channel.h index ae8cf0f..8fd0c8f 100644 --- a/qga/channel.h +++ b/qga/channel.h @@ -19,6 +19,7 @@ typedef enum { GA_CHANNEL_VIRTIO_SERIAL, GA_CHANNEL_ISA_SERIAL, GA_CHANNEL_UNIX_LISTEN, + GA_CHANNEL_VSOCK_LISTEN, } GAChannelMethod; typedef gboolean (*GAChannelCallback)(GIOCondition condition, gpointer opaque); diff --git a/qga/main.c b/qga/main.c index 0b9d04e..6caf215 100644 --- a/qga/main.c +++ b/qga/main.c @@ -190,8 +190,8 @@ static void usage(const char *cmd) "Usage: %s [-m <method> -p <path>] [<options>]\n" "QEMU Guest Agent %s\n" "\n" -" -m, --method transport method: one of unix-listen, virtio-serial, or\n" -" isa-serial (virtio-serial is the default)\n" +" -m, --method transport method: one of unix-listen, virtio-serial,\n" +" isa-serial, or vsock-listen (virtio-serial is the default)\n" " -p, --path device/socket path (the default for virtio-serial is:\n" " %s,\n" " the default for isa-serial is:\n" @@ -659,6 +659,8 @@ static gboolean channel_init(GAState *s, const gchar *method, const gchar *path) channel_method = GA_CHANNEL_ISA_SERIAL; } else if (strcmp(method, "unix-listen") == 0) { channel_method = GA_CHANNEL_UNIX_LISTEN; + } else if (strcmp(method, "vsock-listen") == 0) { + channel_method = GA_CHANNEL_VSOCK_LISTEN; } else { g_critical("unsupported channel method/type: %s", method); return false; -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-11-06 17:12 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-14 9:00 [Qemu-devel] [PATCH v2 0/4] qga: add vsock-listen Stefan Hajnoczi 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 1/4] qga: drop unused sockaddr in accept(2) call Stefan Hajnoczi 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks Stefan Hajnoczi 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi 2016-10-14 14:40 ` Eric Blake 2016-10-16 13:35 ` Stefan Hajnoczi 2016-10-17 14:25 ` Eric Blake 2016-10-18 10:21 ` Stefan Hajnoczi 2017-11-05 18:59 ` Kashyap Chamarthy 2017-11-06 17:12 ` Stefan Hajnoczi 2016-10-25 23:51 ` Michael Roth 2016-11-01 0:58 ` Michael Roth 2016-10-14 9:00 ` [Qemu-devel] [PATCH v2 4/4] qga: add vsock-listen method Stefan Hajnoczi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.