[libvirt] [PATCH] fdstream: Report error from the I/O thread

John Ferlan posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180606141546.14944-1-jferlan@redhat.com
Test syntax-check passed
src/util/virfdstream.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
[libvirt] [PATCH] fdstream: Report error from the I/O thread
Posted by John Ferlan 5 years, 10 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1529059

Commit id 0fe4aa14 added the thread specific error message
reporting (or save) to virFDStreamEvent; however, as processing
goes via virStream{Send|SendHole|Recv} via calls from
daemonStreamHandle{WriteData|Hole|Read} the last error
gets reset in the main libvirt API's thus, whatever error
may have been set as last error will be cleared prior to
the error paths using it resulting in the generic error
on the client side.

For each of the paths that check threadQuit or threadErr,
check if threadErr was set and set it agian if there isn't
a last error (e.g. some other failure) set so that the
message can be provided back to the client.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/util/virfdstream.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index e4973a2bd0..8189559964 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -795,8 +795,13 @@ static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes)
         char *buf;
 
         if (fdst->threadQuit || fdst->threadErr) {
-            virReportSystemError(EBADF, "%s",
-                                 _("cannot write to stream"));
+
+            /* virStreamSend will virResetLastError possibly set
+             * by virFDStreamEvent */
+            if (fdst->threadErr && !virGetLastError())
+                virSetError(fdst->threadErr);
+            else
+                virReportSystemError(EBADF, "%s", _("cannot write to stream"));
             goto cleanup;
         }
 
@@ -875,8 +880,13 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
         while (!(msg = fdst->msg)) {
             if (fdst->threadQuit || fdst->threadErr) {
                 if (nbytes) {
-                    virReportSystemError(EBADF, "%s",
-                                         _("stream is not open"));
+                    /* virStreamRecv will virResetLastError possibly set
+                     * by virFDStreamEvent */
+                    if (fdst->threadErr && !virGetLastError())
+                        virSetError(fdst->threadErr);
+                    else
+                        virReportSystemError(EBADF, "%s",
+                                             _("stream is not open"));
                 } else {
                     ret = 0;
                 }
@@ -976,8 +986,12 @@ virFDStreamSendHole(virStreamPtr st,
          * might mess up file position for the thread. */
 
         if (fdst->threadQuit || fdst->threadErr) {
-            virReportSystemError(EBADF, "%s",
-                                 _("stream is not open"));
+            /* virStreamSendHole will virResetLastError possibly set
+             * by virFDStreamEvent */
+            if (fdst->threadErr && !virGetLastError())
+                virSetError(fdst->threadErr);
+            else
+                virReportSystemError(EBADF, "%s", _("stream is not open"));
             goto cleanup;
         }
 
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fdstream: Report error from the I/O thread
Posted by Michal Privoznik 5 years, 10 months ago
On 06/06/2018 04:15 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1529059
> 
> Commit id 0fe4aa14 added the thread specific error message
> reporting (or save) to virFDStreamEvent; however, as processing
> goes via virStream{Send|SendHole|Recv} via calls from
> daemonStreamHandle{WriteData|Hole|Read} the last error
> gets reset in the main libvirt API's thus, whatever error
> may have been set as last error will be cleared prior to
> the error paths using it resulting in the generic error
> on the client side.
> 
> For each of the paths that check threadQuit or threadErr,
> check if threadErr was set and set it agian if there isn't
> a last error (e.g. some other failure) set so that the
> message can be provided back to the client.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/util/virfdstream.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list