* [Qemu-devel] [PATCH 0/4] qga: add vsock-listen
@ 2016-10-06 16:40 Stefan Hajnoczi
2016-10-06 16:40 ` [Qemu-devel] [PATCH 1/4] qga: drop unused sockaddr in accept(2) call Stefan Hajnoczi
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2016-10-06 16:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth, Stefan Hajnoczi
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 | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 277 insertions(+), 11 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 1/4] qga: drop unused sockaddr in accept(2) call
2016-10-06 16:40 [Qemu-devel] [PATCH 0/4] qga: add vsock-listen Stefan Hajnoczi
@ 2016-10-06 16:40 ` Stefan Hajnoczi
2016-10-07 16:01 ` Michael Roth
2016-10-06 16:40 ` [Qemu-devel] [PATCH 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks Stefan Hajnoczi
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Stefan Hajnoczi @ 2016-10-06 16:40 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>
---
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] 15+ messages in thread
* [Qemu-devel] [PATCH 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks
2016-10-06 16:40 [Qemu-devel] [PATCH 0/4] qga: add vsock-listen Stefan Hajnoczi
2016-10-06 16:40 ` [Qemu-devel] [PATCH 1/4] qga: drop unused sockaddr in accept(2) call Stefan Hajnoczi
@ 2016-10-06 16:40 ` Stefan Hajnoczi
2016-10-07 16:04 ` Michael Roth
2016-10-06 16:40 ` [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi
2016-10-06 16:40 ` [Qemu-devel] [PATCH 4/4] qga: add vsock-listen method Stefan Hajnoczi
3 siblings, 1 reply; 15+ messages in thread
From: Stefan Hajnoczi @ 2016-10-06 16:40 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>
---
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] 15+ messages in thread
* [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support
2016-10-06 16:40 [Qemu-devel] [PATCH 0/4] qga: add vsock-listen Stefan Hajnoczi
2016-10-06 16:40 ` [Qemu-devel] [PATCH 1/4] qga: drop unused sockaddr in accept(2) call Stefan Hajnoczi
2016-10-06 16:40 ` [Qemu-devel] [PATCH 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks Stefan Hajnoczi
@ 2016-10-06 16:40 ` Stefan Hajnoczi
2016-10-06 18:14 ` Eric Blake
2016-10-07 16:42 ` Michael Roth
2016-10-06 16:40 ` [Qemu-devel] [PATCH 4/4] qga: add vsock-listen method Stefan Hajnoczi
3 siblings, 2 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2016-10-06 16:40 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 seasy 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>
---
qapi-schema.json | 23 +++++-
util/qemu-sockets.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 244 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index c3dcf11..8864a96 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -987,12 +987,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
@@ -3017,6 +3019,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
@@ -3027,6 +3047,7 @@
'data': {
'inet': 'InetSocketAddress',
'unix': 'UnixSocketAddress',
+ 'vsock': 'VsockSocketAddress',
'fd': 'String' } }
##
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 6db48b3..46cd1ba 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,176 @@ 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];
+
+ if (sscanf(str, "%32[^:]:%32[^,]", cid, port) != 2) {
+ error_setg(errp, "error parsing 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 +1041,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 +1083,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 +1110,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 +1213,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 +1248,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 +1319,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] 15+ messages in thread
* [Qemu-devel] [PATCH 4/4] qga: add vsock-listen method
2016-10-06 16:40 [Qemu-devel] [PATCH 0/4] qga: add vsock-listen Stefan Hajnoczi
` (2 preceding siblings ...)
2016-10-06 16:40 ` [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi
@ 2016-10-06 16:40 ` Stefan Hajnoczi
2016-10-07 17:07 ` Michael Roth
3 siblings, 1 reply; 15+ messages in thread
From: Stefan Hajnoczi @ 2016-10-06 16:40 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>
---
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] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support
2016-10-06 16:40 ` [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi
@ 2016-10-06 18:14 ` Eric Blake
2016-10-07 12:21 ` Stefan Hajnoczi
2016-10-07 16:42 ` Michael Roth
1 sibling, 1 reply; 15+ messages in thread
From: Eric Blake @ 2016-10-06 18:14 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel; +Cc: Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]
On 10/06/2016 11:40 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 seasy to convert between struct sockaddr_vm
s/seasy/easy/
> 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.
At any rate, it seems like SocketAddress would be a better fit for a
tri-state union between InetSocketAddress, UnixSocketAddress, and
VnetSocketAddress.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> qapi-schema.json | 23 +++++-
> util/qemu-sockets.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 244 insertions(+), 1 deletion(-)
>
> +##
> # @SocketAddress
> #
> # Captures the address of a socket, which could also be a named file descriptor
> @@ -3027,6 +3047,7 @@
> 'data': {
> 'inet': 'InetSocketAddress',
> 'unix': 'UnixSocketAddress',
> + 'vsock': 'VsockSocketAddress',
> 'fd': 'String' } }
Which is in fact what you did.
> +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);
Isn't the presence of vsock support sufficient to prove that we have
SOCK_NONBLOCK support as part of our socket() call? In which case,
wouldn't it be better to pass that option up front to atomically get a
non-blocking socket, rather than having to change its state after the fact?
> +static VsockSocketAddress *vsock_parse(const char *str, Error **errp)
> +{
> + VsockSocketAddress *addr = NULL;
> + char cid[33];
> + char port[33];
> +
> + if (sscanf(str, "%32[^:]:%32[^,]", cid, port) != 2) {
Would it be a wise idea to also use %n to ensure that you aren't
ignoring 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] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support
2016-10-06 18:14 ` Eric Blake
@ 2016-10-07 12:21 ` Stefan Hajnoczi
0 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2016-10-07 12:21 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]
On Thu, Oct 06, 2016 at 01:14:08PM -0500, Eric Blake wrote:
> On 10/06/2016 11:40 AM, Stefan Hajnoczi wrote:
> > +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);
>
> Isn't the presence of vsock support sufficient to prove that we have
> SOCK_NONBLOCK support as part of our socket() call? In which case,
> wouldn't it be better to pass that option up front to atomically get a
> non-blocking socket, rather than having to change its state after the fact?
I'm sending a separate patch series to introduce a QemuSockFlags
argument for qemu_socket() and qemu_accept(). It will convert existing
qemu_set_nonblock() callers too.
> > +static VsockSocketAddress *vsock_parse(const char *str, Error **errp)
> > +{
> > + VsockSocketAddress *addr = NULL;
> > + char cid[33];
> > + char port[33];
> > +
> > + if (sscanf(str, "%32[^:]:%32[^,]", cid, port) != 2) {
>
> Would it be a wise idea to also use %n to ensure that you aren't
> ignoring trailing garbage?
Okay, will fix in the next version.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] qga: drop unused sockaddr in accept(2) call
2016-10-06 16:40 ` [Qemu-devel] [PATCH 1/4] qga: drop unused sockaddr in accept(2) call Stefan Hajnoczi
@ 2016-10-07 16:01 ` Michael Roth
0 siblings, 0 replies; 15+ messages in thread
From: Michael Roth @ 2016-10-07 16:01 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Quoting Stefan Hajnoczi (2016-10-06 11:40:15)
> 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 [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks
2016-10-06 16:40 ` [Qemu-devel] [PATCH 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks Stefan Hajnoczi
@ 2016-10-07 16:04 ` Michael Roth
0 siblings, 0 replies; 15+ messages in thread
From: Michael Roth @ 2016-10-07 16:04 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Quoting Stefan Hajnoczi (2016-10-06 11:40:16)
> 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 [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support
2016-10-06 16:40 ` [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi
2016-10-06 18:14 ` Eric Blake
@ 2016-10-07 16:42 ` Michael Roth
2016-10-12 15:06 ` Stefan Hajnoczi
1 sibling, 1 reply; 15+ messages in thread
From: Michael Roth @ 2016-10-07 16:42 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Quoting Stefan Hajnoczi (2016-10-06 11:40:17)
> 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 seasy 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>
> ---
> qapi-schema.json | 23 +++++-
> util/qemu-sockets.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 244 insertions(+), 1 deletion(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index c3dcf11..8864a96 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -987,12 +987,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
> @@ -3017,6 +3019,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' } }
Is there any reason to not define these as uint32_t? Not sure if there
are other reasons for this, but if it's just for consistency with how
Inet is handled, the code seems to do straight atoi()<->printf("%d") to
covert between numerical and string representation so it doesn't seem
like we need to account for any differences between command-line and
internal representation in sockaddr_vm.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qga: add vsock-listen method
2016-10-06 16:40 ` [Qemu-devel] [PATCH 4/4] qga: add vsock-listen method Stefan Hajnoczi
@ 2016-10-07 17:07 ` Michael Roth
2016-10-12 15:07 ` Stefan Hajnoczi
0 siblings, 1 reply; 15+ messages in thread
From: Michael Roth @ 2016-10-07 17:07 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Quoting Stefan Hajnoczi (2016-10-06 11:40:18)
> 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>
I still need to get a vsock environment set up to test with, but looks
good other than minor comments in patch 3.
> ---
> 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 [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support
2016-10-07 16:42 ` Michael Roth
@ 2016-10-12 15:06 ` Stefan Hajnoczi
2016-10-13 20:20 ` Michael Roth
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Hajnoczi @ 2016-10-12 15:06 UTC (permalink / raw)
To: Michael Roth; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2574 bytes --]
On Fri, Oct 07, 2016 at 11:42:35AM -0500, Michael Roth wrote:
> Quoting Stefan Hajnoczi (2016-10-06 11:40:17)
> > 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 seasy 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>
> > ---
> > qapi-schema.json | 23 +++++-
> > util/qemu-sockets.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 244 insertions(+), 1 deletion(-)
> >
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index c3dcf11..8864a96 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -987,12 +987,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
> > @@ -3017,6 +3019,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' } }
>
> Is there any reason to not define these as uint32_t? Not sure if there
> are other reasons for this, but if it's just for consistency with how
> Inet is handled, the code seems to do straight atoi()<->printf("%d") to
> covert between numerical and string representation so it doesn't seem
> like we need to account for any differences between command-line and
> internal representation in sockaddr_vm.
Just in case AF_VSOCK ever supports name and service resolution like
TCP/IP. In that case cid could be a host name and port could be a
service name.
(I mentioned this in the doc comment.)
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qga: add vsock-listen method
2016-10-07 17:07 ` Michael Roth
@ 2016-10-12 15:07 ` Stefan Hajnoczi
2016-10-13 20:29 ` Michael Roth
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Hajnoczi @ 2016-10-12 15:07 UTC (permalink / raw)
To: Michael Roth; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 720 bytes --]
On Fri, Oct 07, 2016 at 12:07:41PM -0500, Michael Roth wrote:
> Quoting Stefan Hajnoczi (2016-10-06 11:40:18)
> > 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>
>
> I still need to get a vsock environment set up to test with, but looks
> good other than minor comments in patch 3.
Linux 4.8 has the guest and vhost drivers:
CONFIG_VSOCKETS=m
CONFIG_VIRTIO_VSOCKETS=m
CONFIG_VIRTIO_VSOCKETS_COMMON=m
CONFIG_VHOST_VSOCK=m
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support
2016-10-12 15:06 ` Stefan Hajnoczi
@ 2016-10-13 20:20 ` Michael Roth
0 siblings, 0 replies; 15+ messages in thread
From: Michael Roth @ 2016-10-13 20:20 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel
Quoting Stefan Hajnoczi (2016-10-12 10:06:10)
> On Fri, Oct 07, 2016 at 11:42:35AM -0500, Michael Roth wrote:
> > Quoting Stefan Hajnoczi (2016-10-06 11:40:17)
> > > 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 seasy 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>
> > > ---
> > > qapi-schema.json | 23 +++++-
> > > util/qemu-sockets.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 244 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index c3dcf11..8864a96 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -987,12 +987,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
> > > @@ -3017,6 +3019,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' } }
> >
> > Is there any reason to not define these as uint32_t? Not sure if there
> > are other reasons for this, but if it's just for consistency with how
> > Inet is handled, the code seems to do straight atoi()<->printf("%d") to
> > covert between numerical and string representation so it doesn't seem
> > like we need to account for any differences between command-line and
> > internal representation in sockaddr_vm.
>
> Just in case AF_VSOCK ever supports name and service resolution like
> TCP/IP. In that case cid could be a host name and port could be a
> service name.
>
> (I mentioned this in the doc comment.)
Ahh, sorry I missed that completely. Makes perfect sense then.
>
> Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qga: add vsock-listen method
2016-10-12 15:07 ` Stefan Hajnoczi
@ 2016-10-13 20:29 ` Michael Roth
0 siblings, 0 replies; 15+ messages in thread
From: Michael Roth @ 2016-10-13 20:29 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel
Quoting Stefan Hajnoczi (2016-10-12 10:07:33)
> On Fri, Oct 07, 2016 at 12:07:41PM -0500, Michael Roth wrote:
> > Quoting Stefan Hajnoczi (2016-10-06 11:40:18)
> > > 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>
> >
> > I still need to get a vsock environment set up to test with, but looks
> > good other than minor comments in patch 3.
>
> Linux 4.8 has the guest and vhost drivers:
>
> CONFIG_VSOCKETS=m
> CONFIG_VIRTIO_VSOCKETS=m
> CONFIG_VIRTIO_VSOCKETS_COMMON=m
> CONFIG_VHOST_VSOCK=m
I still need to do some work to get this fully integrated into my
test set up, but I ran these patches through some basic testing
with a 4.8.0 host and 4.9 guest and everything seems to be in
working order. I think Eric had some comments related to parameter
parsing in patch 3, but otherwise looks good.
>
> Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-10-13 20:29 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-06 16:40 [Qemu-devel] [PATCH 0/4] qga: add vsock-listen Stefan Hajnoczi
2016-10-06 16:40 ` [Qemu-devel] [PATCH 1/4] qga: drop unused sockaddr in accept(2) call Stefan Hajnoczi
2016-10-07 16:01 ` Michael Roth
2016-10-06 16:40 ` [Qemu-devel] [PATCH 2/4] qga: drop unnecessary GA_CHANNEL_UNIX_LISTEN checks Stefan Hajnoczi
2016-10-07 16:04 ` Michael Roth
2016-10-06 16:40 ` [Qemu-devel] [PATCH 3/4] sockets: add AF_VSOCK support Stefan Hajnoczi
2016-10-06 18:14 ` Eric Blake
2016-10-07 12:21 ` Stefan Hajnoczi
2016-10-07 16:42 ` Michael Roth
2016-10-12 15:06 ` Stefan Hajnoczi
2016-10-13 20:20 ` Michael Roth
2016-10-06 16:40 ` [Qemu-devel] [PATCH 4/4] qga: add vsock-listen method Stefan Hajnoczi
2016-10-07 17:07 ` Michael Roth
2016-10-12 15:07 ` Stefan Hajnoczi
2016-10-13 20:29 ` Michael Roth
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.