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

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(a)redhat.com&gt; > --- > 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 ?