[libvirt] [PATCH v4 1/6] virfdstream: Check for thread error more frequently
John Ferlan
jferlan at redhat.com
Sat Jul 8 12:50:13 UTC 2017
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 at redhat.com>
> ---
> 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 at redhat.com>
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?
[...]
More information about the libvir-list
mailing list