[libvirt] [PATCH] processSerialChangedEvent: Close agent monitor early

[Michal Privoznik]

On 06.05.2015 18:27, Eric Blake wrote:
> On 05/06/2015 02:46 AM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=890648
>>
>> So, imagine you've issued an API that involves guest agent. For
>> instance, you want to query guest's IP addresses. So the API acquires
>> QUERY_JOB, locks the guest agent and issues the agemt command.
> 
> s/agemt/agent/
> 
>> However, for some reason, guest agent replies to initial ping
>> correctly, but then crashes tragically while executing real command
>> (in this case guest-network-get-interfaces). Since initial ping went
>> well, libvirt thinks guest agent is accessible and awaits reply to the
>> real command. But it will never come. What will is a monitor event.
> 
> The monitor event is telling you that your side of the socket is still
> valid, but the guest's side of the agenet is no longer connected.
> 
>> Our handler (processSerialChangedEvent) will try to acquire
>> MODIFY_JOB, which will fail obviously because the other thread that's
>> executing the API already holds a job. So the event handler exits
>> early, and the QUERY_JOB is never released nor ended.
>>
>> I don't understand why, but qemu should in fact instead of sending a
>> event on the monitor close the socket.
> 
> Reads awkwardly; I think you were trying to say:
> 
> qemu should in fact be closing the socket instead of sending an event
> 
> But that doesn't make sense technically.  The fact that the agent is no
> longer open in the guest is no reason to close the monitor socket, and
> closing the agent socket is inappropriate in case the guest restarts the
> guest agent and reopens the port.  Thus, listening to events on the
> monitor socket about whether the agent socket is connected or dangling
> is sufficient for us to know if the agent will be responsive or not.
> 
>> But leave that for a different
>> discussion. Since qemu is not doing that, we must work around it and
>> close and unregister the agent's FD from the even loop ourselves. This
>> will wake up all callers stuck in waiting for agent's reply.
> 
> Why do we have to kill the agent's fd?  If the guest restarts the agent,
> how will we reconnect to it if we have killed our agent fd?

We will get another event, and the even handler will connect to the
agent instead of disconnecting from it.

> 
>> Hopefully, it will make them finish the job too and even handler can
>> acquire its job later.
> 
> I agree that we want to detect agent death events as a reason to mark
> the agent unresponsive, in such a way that any other job pending on the
> agent then gracefully aborts, but I'm not sure that this is the right
> solution.
> 

Why? The other thread is stuck at: a) poll() b) agent condition. We need
to wake it up on both points (which is exactly what qemuAgentClose does).

Michal