| Message ID | 20220706064607.1397659-1-lvivier@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi: net: add unix socket type support to netdev backend | expand |
* Laurent Vivier (lvivier@redhat.com) wrote: > Remove 'tcp:' prefix for inet type (because inet can be 'tcp' or 'udp' > and socket_parse() doesn't recognize it), the format is 'host:port'. I don't think I understand why tests/qtest/migration-test.c test_precopy_common is happy with this; it does: if (!args->connect_uri) { g_autofree char *local_connect_uri = migrate_get_socket_address(to, "socket-address"); migrate_qmp(from, local_connect_uri, "{}"); which hmm, is the code you're changing what was in SocketAddress_to_str which is what migrate_get_socket_address uses; but then the migrate_qmp I don't think will take a migrate uri without the tcp: on the front. Dave > Use 'vsock:' prefix for vsock type rather than 'tcp:' because it makes > a vsock address look like an inet address with CID misinterpreted as host. > Goes back to commit 9aca82ba31 "migration: Create socket-address parameter" > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > util/qemu-sockets.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index 870a36eb0e93..4cd76b3ae3af 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -1102,7 +1102,7 @@ char *socket_uri(SocketAddress *addr) > { > switch (addr->type) { > case SOCKET_ADDRESS_TYPE_INET: > - return g_strdup_printf("tcp:%s:%s", > + return g_strdup_printf("%s:%s", > addr->u.inet.host, > addr->u.inet.port); > case SOCKET_ADDRESS_TYPE_UNIX: > @@ -1111,7 +1111,7 @@ char *socket_uri(SocketAddress *addr) > case SOCKET_ADDRESS_TYPE_FD: > return g_strdup_printf("fd:%s", addr->u.fd.str); > case SOCKET_ADDRESS_TYPE_VSOCK: > - return g_strdup_printf("tcp:%s:%s", > + return g_strdup_printf("vsock:%s:%s", > addr->u.vsock.cid, > addr->u.vsock.port); > default: > -- > 2.36.1 >
On 12/07/2022 14:05, Dr. David Alan Gilbert wrote: > * Laurent Vivier (lvivier@redhat.com) wrote: >> Remove 'tcp:' prefix for inet type (because inet can be 'tcp' or 'udp' >> and socket_parse() doesn't recognize it), the format is 'host:port'. > > I don't think I understand why tests/qtest/migration-test.c > test_precopy_common is happy with this; it does: > > if (!args->connect_uri) { > g_autofree char *local_connect_uri = > migrate_get_socket_address(to, "socket-address"); > migrate_qmp(from, local_connect_uri, "{}"); > > which hmm, is the code you're changing what was in SocketAddress_to_str > which is what migrate_get_socket_address uses; but then the migrate_qmp > I don't think will take a migrate uri without the tcp: on the front. It's a good point. I think socket_parse() should accept the 'tcp:' prefix, and thus socket_uri() should generate it, so it will be usable with the qmp migrate command. I'm going to add 'tcp:' case in socket_parse() and make socket_uri() to generate it. Thanks, Laurent
On Wed, Jul 13, 2022 at 06:46:17PM +0200, Laurent Vivier wrote: > On 12/07/2022 14:05, Dr. David Alan Gilbert wrote: > > * Laurent Vivier (lvivier@redhat.com) wrote: > > > Remove 'tcp:' prefix for inet type (because inet can be 'tcp' or 'udp' > > > and socket_parse() doesn't recognize it), the format is 'host:port'. > > > > I don't think I understand why tests/qtest/migration-test.c > > test_precopy_common is happy with this; it does: > > > > if (!args->connect_uri) { > > g_autofree char *local_connect_uri = > > migrate_get_socket_address(to, "socket-address"); > > migrate_qmp(from, local_connect_uri, "{}"); > > > > which hmm, is the code you're changing what was in SocketAddress_to_str > > which is what migrate_get_socket_address uses; but then the migrate_qmp > > I don't think will take a migrate uri without the tcp: on the front. > > It's a good point. > > I think socket_parse() should accept the 'tcp:' prefix, and thus > socket_uri() should generate it, so it will be usable with the qmp migrate > command. > > I'm going to add 'tcp:' case in socket_parse() and make socket_uri() to generate it. I'd say any code in util/qemu-sockets.c should only work in terms of generic concepts. If we're formattting/parsing a migration URI, the helper APIs should live in the migration/ subdir. With regards, Daniel
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 870a36eb0e93..4cd76b3ae3af 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -1102,7 +1102,7 @@ char *socket_uri(SocketAddress *addr) { switch (addr->type) { case SOCKET_ADDRESS_TYPE_INET: - return g_strdup_printf("tcp:%s:%s", + return g_strdup_printf("%s:%s", addr->u.inet.host, addr->u.inet.port); case SOCKET_ADDRESS_TYPE_UNIX: @@ -1111,7 +1111,7 @@ char *socket_uri(SocketAddress *addr) case SOCKET_ADDRESS_TYPE_FD: return g_strdup_printf("fd:%s", addr->u.fd.str); case SOCKET_ADDRESS_TYPE_VSOCK: - return g_strdup_printf("tcp:%s:%s", + return g_strdup_printf("vsock:%s:%s", addr->u.vsock.cid, addr->u.vsock.port); default:
Remove 'tcp:' prefix for inet type (because inet can be 'tcp' or 'udp' and socket_parse() doesn't recognize it), the format is 'host:port'. Use 'vsock:' prefix for vsock type rather than 'tcp:' because it makes a vsock address look like an inet address with CID misinterpreted as host. Goes back to commit 9aca82ba31 "migration: Create socket-address parameter" Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- util/qemu-sockets.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)