[libvirt] [PATCH v2] qemu_agent: Ignore expected EOFs

[Michal Privoznik]

On 23.01.2013 14:25, Peter Krempa wrote:
> On 01/22/13 15:11, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=892079
>>
>> One of my previous patches (f2a4e5f176c408) tried to fix crashing
>> libvirtd on domain detroy. However, we need to copy pattern from
>> qemuProcessHandleMonitorEOF() instead of decrementing reference
>> counter. The rationale for this is, if qemu process is dying due
>> to domain being destroyed, we obtain EOF on both the monitor and
>> agent sockets. However, if the exit is expected, qemuProcessStop
>> is called, which cleans both agent and monitor sockets up. We
>> want qemuAgentClose() to be called iff the EOF is not expected,
>> so we don't leak an FD and memory.
>> ---
>>
>> diff to v1:
>> -handle race with qemuProcessHandleMonitorEOF correctly
>>
>>   src/qemu/qemu_process.c | 20 ++++++++++++++++++--
>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 2f08215..013b2ca 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -133,14 +133,30 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
>>       virObjectLock(vm);
>>
>>       priv = vm->privateData;
>> -    if (priv->agent == agent &&
>> -        !virObjectUnref(priv->agent))
>> +
>> +    if (!priv->agent) {
>> +        VIR_DEBUG("Agent freed already");
>> +        goto unlock;
>> +    }
> 
> You shouls add to the commit message why this condition is needed.
> 
>> +
>> +    if (priv->beingDestroyed) {
>> +        VIR_DEBUG("Domain is being destroyed, agent EOF is expected");
>> +        goto unlock;
>> +    }
>> +
>> +    if (priv->agent == agent)
>>           priv->agent = NULL;
> 
> Is there a possibility for the agent to be different from the saved one
> here?

Actually no. It's a leftover from previous implementation. I have
removed the condition, extended the commit message and pushed. Thanks!

Michal