[libvirt] [PATCH] Fix reporting of i/o errors by iohelper process

[Eric Blake]

On 07/16/2014 08:30 AM, Eric Blake wrote:
> On 07/16/2014 08:12 AM, Jason J. Herne wrote:
>> From: "Jason J. Herne" <jjherne at us.ibm.com>
>>
>> libvirt_iohelper is a helper process that is exec'ed and used to handle I/O
>> during a Qemu managed save operation. Due to a missing call to
>> virFileWrapperFdClose, all I/O error messages reported by iohelper are lost.
>>
>> This patch adds a call to virFileWrapperFdClose to the cleanup phase of
>> qemuDomainSaveMemory.
>>
>> This patch also modifies virFileWrapperFdClose such that errors are only
>> reported when the length of the err_msg buffer is > 0. Before now, the
>> existence of the buffer would trigger error reporting in virFileWrapperFdClose.
>>
>> Signed-off-by: Jason J. Herne <jjherne at us.ibm.com>
>> ---
>>  src/qemu/qemu_driver.c | 1 +
>>  src/util/virfile.c     | 2 +-
>>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> Nice! Thanks for persevering on this one.  Congrats on your first
> libvirt patch.

Sadly, the more I look at this, the more I wonder how it can fix anything.

> 
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index ecccf6c..8d78805 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -3015,6 +3015,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
>>  
>>   cleanup:
>>      VIR_FORCE_CLOSE(fd);
>> +    virFileWrapperFdClose(wrapperFd);
>>      virFileWrapperFdFree(wrapperFd);
>>      VIR_FREE(xml);

But qemuDomainSaveMemory() already has a call to virFileWrapperFdClose()
earlier on; worse, the current implementation of virFileWrapperFdClose()
is not designed to be called twice if an error occurred (rather, if
there is an error, two calls will end up logging the error twice, when
once would have been sufficient).  How are you getting to a point where
the cleanup label is reached without going through the earlier close?

Is there some easy setup you used in testing this patch, so that I can
reproduce a scenario where iohelper will reliably fail?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140716/02049873/attachment-0001.sig>