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@xxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
