On 2020/11/10 18:12, Daniel P. Berrangé wrote:
> On Fri, Nov 06, 2020 at 06:54:54PM +0800, Chuan Zheng wrote:
>> Since we now support tls multifd, when we cancel migration, the TLS
>> sockets will be left as CLOSE-WAIT On Src which results in socket
>> leak.
>> Fix it by closing TLS channel before socket finalize.
>>
>> Signed-off-by: Chuan Zheng <zhengch...@huawei.com>
>> ---
>> migration/multifd.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index 68b171f..a6838dc 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -523,6 +523,19 @@ static void multifd_send_terminate_threads(Error *err)
>> }
>> }
>>
>> +static void multifd_tls_socket_close(QIOChannel *ioc, Error *err)
>> +{
>> + if (ioc &&
>> + object_dynamic_cast(OBJECT(ioc),
>> + TYPE_QIO_CHANNEL_TLS)) {
>> + /*
>> + * TLS channel is special, we need close it before
>> + * socket finalize.
>> + */
>> + qio_channel_close(ioc, &err);
>> + }
>> +}
>
> This doesn't feel quite right to me. Calling qio_channel_close will close
> both the TLS layer, and the underlying QIOChannelSocket. If the latter
> is safe to do, then we don't need the object_dynamic_cast() check, we can
> do it unconditionally whether we're using TLS or not.
>
> Having said that, I'm not sure if we actually want to be using
> qio_channel_close or not ?
>
> I would have expected that there is already code somewhere else in the
> migration layer that is closing these multifd channels, but I can't
> actually find where that happens right now. Assuming that code does
> exist though, qio_channel_shutdown(ioc, BOTH) feels like the right
> answer to unblock waiting I/O ops.
>
Hi, Daniel.
Actually, I have tried to use qio_channel_shutdown at the same place, but it
seems not work right.
the socket connection is closed by observing through 'ss' command but the
socket fds in /proc/$(qemu pid)/fd are still
residual.
The underlying QIOChannelSocket will be closed by qio_channel_socket_finalize()
through object_unref(QIOChannel) later in socket_send_channel_destroy(),
does that means it is safe to close both of TLS and tcp socket?
>> +
>> void multifd_save_cleanup(void)
>> {
>> int i;
>> @@ -542,6 +555,7 @@ void multifd_save_cleanup(void)
>> MultiFDSendParams *p = &multifd_send_state->params[i];
>> Error *local_err = NULL;
>>
>> + multifd_tls_socket_close(p->c, NULL);
>> socket_send_channel_destroy(p->c);
>> p->c = NULL;
>> qemu_mutex_destroy(&p->mutex);
>> --
>> 1.8.3.1
>>
>
> Regards,
> Daniel
>
--
Regards.
Chuan
