We first produce this bug in rhel7.4's libvir daemon。For easily produce the bug, the step can be as follows:
1. add sleep(3) in daemonStreamFilter() pre virMutexLock(&stream->priv->lock), then build libvirtd bin executable, then restart libvirtd
2. use virsh console open one vm's console, for this console（the vm's kernel need console=ttyS0 boot parameter,then just input from keyboard on and on
3. use virsh console --force to break the previous console session, than you will get libvirt daemon deadlock。
And for the problem client->privData to be released problem, only virNetServerClientClose() will free client->privData and client, I think this can be fixed by remove virNetServerClientClose() from daemonStreamEvent(),
and replace with virNetServerClientImmediateClose(), so virNetServerProcessClients() will test the session would be closed。
Michal Privoznik <mprivozn@xxxxxxxxxx> 于2019年11月25日周一 下午5:38写道：
On 11/25/19 8:34 AM, LanceLiu wrote:
> ---
> src/libvirt_remote.syms | 1 +
> src/remote/remote_daemon_stream.c | 10 +++++++++-
> src/rpc/virnetserverclient.c | 12 ++++++++++++
> src/rpc/virnetserverclient.h | 2 ++
> 4 files changed, 24 insertions(+), 1 deletion(-)
Please format commit messages following title + message format (look at
git log how other messages are formatted).
>
> diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
> index 0493467..c32e234 100644
> --- a/src/libvirt_remote.syms
> +++ b/src/libvirt_remote.syms
> @@ -173,6 +173,7 @@ virNetServerClientPreExecRestart;
> virNetServerClientRemoteAddrStringSASL;
> virNetServerClientRemoteAddrStringURI;
> virNetServerClientRemoveFilter;
> +virNetServerClientCheckFilterExist;
> virNetServerClientSendMessage;
> virNetServerClientSetAuthLocked;
> virNetServerClientSetAuthPendingLocked;
> diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c
> index 82cadb6..de0dca3 100644
> --- a/src/remote/remote_daemon_stream.c
> +++ b/src/remote/remote_daemon_stream.c
> @@ -292,10 +292,18 @@ daemonStreamFilter(virNetServerClientPtr client,
> {
> daemonClientStream *stream = opaque;
> int ret = 0;
> + daemonClientPrivatePtr priv = NULL;
> + int filter_id = stream->filterID;
>
> virObjectUnlock(client);
> + priv = virNetServerClientGetPrivateData(client);
This is not needed.
> virMutexLock(&stream->priv->lock);
> virObjectLock(client);
> + if (!virNetServerClientCheckFilterExist(client, filter_id)) {
> + VIR_WARN("this daemon stream filter: %d have been deleted!", filter_id);
> + ret = -1;
> + goto cleanup;
> + }
>
> if (msg->header.type != VIR_NET_STREAM &&
> msg->header.type != VIR_NET_STREAM_HOLE)
> @@ -317,7 +325,7 @@ daemonStreamFilter(virNetServerClientPtr client,
> ret = 1;
>
> cleanup:
> - virMutexUnlock(&stream->priv->lock);
> + virMutexUnlock(&priv->lock);
This is not needed: stream->priv and priv are the same structure.
> return ret;
> }
>
Anyway, this still doesn't work. Problem is, that if a stream is
removed, the private data might be removed too and hence
virMutexLock(&stream->priv->lock) will do something undefined (besides
accessing freed memory). In my testing the daemon deadlocks because it's
trying to lock stream-priv->lock which is locked.
As I said in the other thread - we need to re-evaluate the first commit.
Do you have a reproducer for the original problem please?
Michal
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
