On 06/22/2017 08:30 AM, 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@xxxxxxxxxx> > --- > src/util/virfdstream.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > Trying to keep a fresh an macroscopic view and not think about my previous myopic look at the code... I got too hung up on the "error" part (as in reporting an error in an || condition even though conceptually one would have been reported)... > diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c > index ae6f78e01..bd2355d17 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; > + I spent some cycles considering if there was any "odd possibility" that the @cb could be called twice w/ some virStreamAbort interaction, but I was able to convince myself that it shouldn't be possible. Still just want to "be sure" since virFDStreamCloseInt uses VIR_STREAM_EVENT_ERROR and also sets abortCallbackCalled to ensure it couldn't be called again. Is that something perhaps that could/should be done here as well - defensive type programming? In any case, I'm sufficiently convinced this is fine, but figured I'd ask as well! Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx> John Would it be a bad time to point out that virFDStreamJoinWorker returning -1 and setting ret = -1 in the caller doesn't really do anything since ret is immediately overwritten? [...] -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
