Re: [libvirt] [PATCH] tools: console: Relax stream EOF handling
(CCing Nikolay Shirokovskiy, author of 29f2b5248c )
On 8/7/19 8:34 AM, Roman Bolshakov wrote:
An attempt to poweroff a VM from inside triggers the error for
existing
session of virsh console and it returns with non-zero exit code:
error: internal error: console stream EOF
The message and status code are misleading because there's no real
error.
Fixes: 29f2b5248c6 ("tools: console: pass stream/fd errors to user")
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tools/virsh-console.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/tools/virsh-console.c b/tools/virsh-console.c
index e16f841e57..408a745b0f 100644
--- a/tools/virsh-console.c
+++ b/tools/virsh-console.c
@@ -172,10 +172,6 @@ virConsoleEventOnStream(virStreamPtr st,
if (got == -2)
goto cleanup; /* blocking */
if (got <= 0) {
- if (got == 0)
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("console stream EOF"));
-
I appreciate if we can remove the 'console stream EOF' error message
that started to appear even in regular VM shutdown, but I'm not sure
if this is the right fix.
'got' is the result of virStreamRecv(). Seeing the code documentation of
this function we have:
(src/libvirt-stream.c)
* Returns the number of bytes read, which may be less
* than requested.
*
* Returns 0 when the end of the stream is reached, at
* which time the caller should invoke virStreamFinish()
* to get confirmation of stream completion.
*
* Returns -1 upon error [....]
And this is the doc for virStreamFinish():
* This method is a synchronization point for all asynchronous
* errors, so if this returns a success code the application can
* be sure that all data has been successfully processed.
In the existing code we're not calling 'virStreamFinish()' to assert whether
the got=0 from virStreamRecv is a successful shutdown or not, we're
simply reporting an internal error assuming that something went wrong.
Shouldn't we call virStreamFinish if got=0 and only report an error if
virStreamFinish returns -1? Does that suffice to fix this error message
being thrown when the VM does a regular shutdown?
Thanks,
DHB
virConsoleShutdown(con);
goto cleanup;
}