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@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