| Message ID | 20171030112112.6952-2-quintela@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 30, 2017 at 12:21:07PM +0100, Juan Quintela wrote: > For tcp sockets we read back what is the socket/address. So we know > what is the port that we are listening into. > > Looked all callers of the function, and they just create the addr, use > it, and drop it, so no problem that we always update the port in the > address. Can you explain more why you need this ? Nothing should be using the socket_listen() method directly any more IIRC, and for the QIOChannelSocket classes, you should not rely on the address that you pass in, being the same as the one that ultimately gets passed into the socket_listen() method. Patches that I have pending change things so that listening happens in two phases. First we take the SocketAddress and do DNS resolution to create mutliple new SocketAddress structs. These are then passed into the lowlevel socket_listen() method. So with that happening, even if you update the address in socket_listen() that info won't get back upto the caller. If you have a QIOChannelSocket instance, and you want to know what port it ended up listening on, you should call qio_channel_socket_get_local_address() method instead, which returns a dynamically popualted SocketAddress struct. This should mean socket_listen() never needs to update the address that it binds on. IOW, I think this patch is redundant. Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> wrote: > On Mon, Oct 30, 2017 at 12:21:07PM +0100, Juan Quintela wrote: >> For tcp sockets we read back what is the socket/address. So we know >> what is the port that we are listening into. >> >> Looked all callers of the function, and they just create the addr, use >> it, and drop it, so no problem that we always update the port in the >> address. > > Can you explain more why you need this ? I need to get back somewhere the port that bind gave me. I can do it later by hand. But that function has one parameter that says that it update the sockaddr that gets passed, and it don't do it. Auditing all the users of it, they don't care (basically all of them just free the addr after returning for this function). But if you preffer that I do this directly on the migration code, I have no problem with that. > Nothing should be using the socket_listen() method directly any more IIRC, > and for the QIOChannelSocket classes, you should not rely on the address that > you pass in, being the same as the one that ultimately gets passed into the > socket_listen() method. Ok, I will change to do this directly on the migration code. > Patches that I have pending change things so that listening happens in two > phases. First we take the SocketAddress and do DNS resolution to create > mutliple new SocketAddress structs. These are then passed into the lowlevel > socket_listen() method. So with that happening, even if you update the address > in socket_listen() that info won't get back upto the caller. > > If you have a QIOChannelSocket instance, and you want to know what port it > ended up listening on, you should call qio_channel_socket_get_local_address() > method instead, which returns a dynamically popualted SocketAddress struct. > This should mean socket_listen() never needs to update the address that it > binds on. Ah, yet another function O:-) Ok, I will use this one. Sorry for the disturbance. > IOW, I think this patch is redundant. Later, Juan.
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index b47fb45885..b099c88dd1 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -322,13 +322,16 @@ listen_failed: listen_ok: if (update_addr) { + SocketAddress *addr; + + addr = socket_local_address(slisten, errp); g_free(saddr->host); - saddr->host = g_strdup(uaddr); + saddr->host = g_strdup(addr->u.inet.host); g_free(saddr->port); - saddr->port = g_strdup_printf("%d", - inet_getport(e) - port_offset); + saddr->port = g_strdup(addr->u.inet.port); saddr->has_ipv6 = saddr->ipv6 = e->ai_family == PF_INET6; saddr->has_ipv4 = saddr->ipv4 = e->ai_family != PF_INET6; + qapi_free_SocketAddress(addr); } freeaddrinfo(res); return slisten; @@ -1047,7 +1050,7 @@ int socket_listen(SocketAddress *addr, Error **errp) switch (addr->type) { case SOCKET_ADDRESS_TYPE_INET: - fd = inet_listen_saddr(&addr->u.inet, 0, false, errp); + fd = inet_listen_saddr(&addr->u.inet, 0, true, errp); break; case SOCKET_ADDRESS_TYPE_UNIX:
For tcp sockets we read back what is the socket/address. So we know what is the port that we are listening into. Looked all callers of the function, and they just create the addr, use it, and drop it, so no problem that we always update the port in the address. Signed-off-by: Juan Quintela <quintela@redhat.com> CC: Daniel P. Berrange <berrange@redhat.com> --- util/qemu-sockets.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)