[libvirt] [PATCH v3 6/6] virStream*All: Report error if a callback fails

Michal Privoznik mprivozn at redhat.com
Tue Jun 13 11:31:33 UTC 2017


On 06/13/2017 01:03 AM, John Ferlan wrote:
> 
> 
> On 06/05/2017 04:23 AM, Michal Privoznik wrote:
>> All of these four functions (virStreamRecvAll, virStreamSendAll,
>> virStreamSparseRecvAll, virStreamSparseSendAll) take one or more
>> callback that handle various aspects of streams. However, if any
> 
> s/callback/callback functions/
> 
>> of them fails no error is reported therefore caller does not know
>> what went wrong.
>>
>> At the same time, we silently presumed callbacks to set errno on
>> failure. With this change we should document it explicitly as the
> 
> s/should//
> 
>> error is not properly reported.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  include/libvirt/libvirt-stream.h | 17 +++++++++++++-
>>  src/libvirt-stream.c             | 50 +++++++++++++++++++++++++++++++++-------
>>  2 files changed, 58 insertions(+), 9 deletions(-)
>>

>> @@ -595,9 +595,15 @@ virStreamSendAll(virStreamPtr stream,
>>  
>>      for (;;) {
>>          int got, offset = 0;
>> +
>> +        errno = 0;
>>          got = (handler)(stream, bytes, want, opaque);
>> -        if (got < 0)
>> +        if (got < 0) {
>> +            if (errno == 0)
>> +                errno = EIO;
>> +            virReportSystemError(errno, "%s", _("send handler failed"));
> 
> Since errno and vir*LastError are not necessarily linked - do we really
> want to write an error message in the event that a different message was
> already in the pipeline?
> 
> That is should all of the new virReportSystemError calls be prefixed by:
> 
>    if (!virGetLastError())
> 
> or perhaps use virGetLastErrorMessage ?

I don't think so. The only path we can get to this
virReportSystemError() call is by @handler failing. However, @handler is
expected not to set any libvirt error. Therefore there is no message in
the pipeline.

Michal




More information about the libvir-list mailing list