When the I/O thread quits (e.g. due to an I/O error, lseek()
error, whatever), any subsequent virFDStream API should return
error too. Moreover, when invoking stream event callback, we must
set the VIR_STREAM_EVENT_ERROR flag so that the callback knows
something bad happened.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virfdstream.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index 7ee58be13..ebd0f6cf1 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -312,6 +312,9 @@ static void virFDStreamEvent(int watch ATTRIBUTE_UNUSED,
         return;
     }
 
+    if (fdst->threadErr)
+        events |= VIR_STREAM_EVENT_ERROR;
+
     cb = fdst->cb;
     cbopaque = fdst->opaque;
     ff = fdst->ff;
@@ -764,6 +767,9 @@ static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes)
         return -1;
     }
 
+    if (fdst->threadErr)
+        return -1;
+
     if (!fdst) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("stream is not open"));
@@ -844,6 +850,9 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
         return -1;
     }
 
+    if (fdst->threadErr)
+        return -1;
+
     if (!fdst) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("stream is not open"));
@@ -959,6 +968,9 @@ virFDStreamSendHole(virStreamPtr st,
         fdst->offset += length;
     }
 
+    if (fdst->threadErr)
+        goto cleanup;
+
     if (fdst->thread) {
         /* Things are a bit complicated here. If FDStream is in a
          * read mode, then if the message at the queue head is
@@ -1018,6 +1030,9 @@ virFDStreamInData(virStreamPtr st,
 
     virObjectLock(fdst);
 
+    if (fdst->threadErr)
+        goto cleanup;
+
     if (fdst->thread) {
         virFDStreamMsgPtr msg;
 
-- 
2.13.0

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

On Tue, May 30, 2017 at 12:44:22PM +0200, Michal Privoznik wrote:
>When the I/O thread quits (e.g. due to an I/O error, lseek()
>error, whatever), any subsequent virFDStream API should return
>error too. Moreover, when invoking stream event callback, we must
>set the VIR_STREAM_EVENT_ERROR flag so that the callback knows
>something bad happened.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/util/virfdstream.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
>diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
>index 7ee58be13..ebd0f6cf1 100644
>--- a/src/util/virfdstream.c
>+++ b/src/util/virfdstream.c
>@@ -312,6 +312,9 @@ static void virFDStreamEvent(int watch ATTRIBUTE_UNUSED,
>         return;
>     }
>
>+    if (fdst->threadErr)
>+        events |= VIR_STREAM_EVENT_ERROR;
>+
>     cb = fdst->cb;
>     cbopaque = fdst->opaque;
>     ff = fdst->ff;
>@@ -764,6 +767,9 @@ static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes)
>         return -1;
>     }
>
>+    if (fdst->threadErr)
>+        return -1;
>+

It feels like this should be done after locking the object.

>     if (!fdst) {

Not mentioning it looks like it can be NULL before this check.

>         virReportError(VIR_ERR_INTERNAL_ERROR,
>                        "%s", _("stream is not open"));
>@@ -844,6 +850,9 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
>         return -1;
>     }
>
>+    if (fdst->threadErr)
>+        return -1;
>+
>     if (!fdst) {

Same here.

I have no iSCSI to test it with, but it looks OK otherwise.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list