On 11/22/19 8:00 AM, Lance Liu wrote:
Thank you!
But it looks like libvirtd daemon stream module still has bug lead to
process segfault if applied this patch(if not, it will lead libvirtd
deadlock).I have got the cause, but it seems difficult to fix it with
present codes。
Bug reproduce:
1. add sleep(3) in daemonStreamFilter() so it is easily to get the
point, as follows
static int
daemonStreamFilter(virNetServerClientPtr client,
virNetMessagePtr msg,
void *opaque)
{
daemonClientStream *stream = opaque;
int ret = 0;
static int first = 1;
// if (first) {
// notify_force_console();
// VIR_ERROR("notify force console to break this client
down!client: %p", client);
// sleep(30);
// VIR_ERROR("sleep 10 finished");
// }
VIR_ERROR("stream: %p, filter_id: %d", stream, stream->filterID);
virObjectUnlock(client);
sleep(3);
virMutexLock(&stream->priv->lock);
virObjectLock(client);
2. build the libvirtd with new code, then replace the libvirtd
3. use virsh console to open a vm's console tty, then just input many
chars in the console
4. open new terminal, then use virsh console --force to break the
previous client, then you'll got libvirtd segfault
it looks because when daemonStreamEvent() with para events
as VIR_STREAM_EVENT_ERROR is called, it will remove stream and free stream,
but daemonStreamFilter still waiting stream->priv->lock mutex, thus
cause invalid memory access
Yes, this is exactly what is happening.
To fix this, I think the most effective and clean way is to drop
client->priv->lock elements, just use client object lock。But for
current code, it's not easy to do that。
also， a workaroud scheme is to add a big lock, but is ugly and low
efficient。
Any good ideas?
I think we need to get back to drawing board. In the original problem
you observed ABBA, but also was patching older libvirt. I convinced
myself that the problem still occurs even with (at that point) current
master:
1) virNetServerClientDispatchEvent() locks @client,
virNetServerClientDispatchRead()
daemonStreamFilter() which locks stream->priv->lock;
on the other hand:
2) daemonStreamEvent() locks stream->priv->lock,
virNetServerClientClose() which locks @client; /* there are other
functions called over @client which also lock @client, I've just picked
one */
So there is ABBA pattern. However, there might be another lock that is
breaking the pattern (although I don't see it now): CAB and another
thread then does CBA.
Do you have a reproducer for your original problem? Does it reproduce on
the current master (after you revert the patch I've merged of course).
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
