Re: [libvirt] [PATCH v2 6/6] streams: Report errors if sendAll/recvAll callbacks fail
On Thu, Jun 01, 2017 at 02:02:14PM +0200, Michal Privoznik wrote:
We have couple of wrappers over our low level stream APIs:
virSreamRecvAll(), virStreamSendAll() and their sparse stream
variants. All of them take some callbacks and call them at
appropriate times. If a callback fails it aborts the whole
operation. However, if it so happens that the callback fails
without setting any error users are left with very unhelpful
error message:
error: cannot receive data from volume fedora.img
error: An error occurred, but the cause is unknown
One way to avoid it is to report error if callback hasn't.
The callbacks should never be expected to set a libvirt
error, as they are implemented by code that is outside
libvirt. The intention was really that the callbacks set
an errno value on error, since that's all you have access
to from application level impl....
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt-stream.c | 41 +++++++++++++++++++++++++++++++++--------
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
index 1594ed212..efdbc9e44 100644
--- a/src/libvirt-stream.c
+++ b/src/libvirt-stream.c
@@ -596,8 +596,12 @@ virStreamSendAll(virStreamPtr stream,
for (;;) {
int got, offset = 0;
For sanity we should set 'errno = 0' here.
got = (handler)(stream, bytes, want, opaque);
- if (got < 0)
+ if (got < 0) {
+ if (!virGetLastError())
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("send handler failed"));
goto cleanup;
+ }
...then this should do
if (errno == 0) {
errno = EIO;
}
virReportSystemError(errno, ....)
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|