[libvirt] [PATCH 1/2] rpc: protocol: Clarify VIR_NET_ERROR usage with streams
The described protocol semantics really only apply to server initiated stream messages. Document the semantics for client messages. --- AFAICT, the only time a server will receive VIR_NET_ERROR stream message from a client is via an explicit virStreamAbort call (which hits remote_driver.c:remoteStreamAbort) src/rpc/virnetprotocol.x | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetprotocol.x b/src/rpc/virnetprotocol.x index 327a334..9ce33b0 100644 --- a/src/rpc/virnetprotocol.x +++ b/src/rpc/virnetprotocol.x @@ -104,7 +104,9 @@ const VIR_NET_MESSAGE_NUM_FDS_MAX = 32; * - type == VIR_NET_STREAM * * VIR_NET_CONTINUE if more data is following * * VIR_NET_OK if stream is complete - * * VIR_NET_ERROR if stream had an error + * * VIR_NET_ERROR + * server message: stream had an error + * client message: client aborted the stream * * Payload varies according to type and status: * @@ -125,7 +127,8 @@ const VIR_NET_MESSAGE_NUM_FDS_MAX = 32; * * status == VIR_NET_CONTINUE * byte[] raw stream data * * status == VIR_NET_ERROR - * remote_error error information + * server message: remote_error error information + * client message: <empty> * * status == VIR_NET_OK * <empty> * -- 2.7.3
Every time a client aborts a stream via the virStreamAbort API, the daemon always logs an error like: error : daemonStreamHandleAbort:617 : stream aborted at client request For example, this message is logged any time the user disconnects from 'virsh console'. However this isn't really an error condition, and instead the result of a successful RPC call to abort the stream. Downgrade the message to VIR_INFO --- daemon/stream.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/daemon/stream.c b/daemon/stream.c index c892dcb..d2f85ad 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -613,8 +613,7 @@ daemonStreamHandleAbort(virNetServerClientPtr client, virStreamAbort(stream->st); if (msg->header.status == VIR_NET_ERROR) { - virReportError(VIR_ERR_RPC, - "%s", _("stream aborted at client request")); + VIR_INFO("stream aborted at client request"); } else { VIR_WARN("unexpected stream status %d", msg->header.status); virReportError(VIR_ERR_RPC, -- 2.7.3
On 04/24/2016 04:58 PM, Cole Robinson wrote:
Every time a client aborts a stream via the virStreamAbort API, the daemon always logs an error like:
error : daemonStreamHandleAbort:617 : stream aborted at client request
For example, this message is logged any time the user disconnects from 'virsh console'. However this isn't really an error condition, and instead the result of a successful RPC call to abort the stream.
Downgrade the message to VIR_INFO --- daemon/stream.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/daemon/stream.c b/daemon/stream.c index c892dcb..d2f85ad 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -613,8 +613,7 @@ daemonStreamHandleAbort(virNetServerClientPtr client, virStreamAbort(stream->st);
if (msg->header.status == VIR_NET_ERROR) { - virReportError(VIR_ERR_RPC, - "%s", _("stream aborted at client request")); + VIR_INFO("stream aborted at client request"); } else { VIR_WARN("unexpected stream status %d", msg->header.status); virReportError(VIR_ERR_RPC,
This isn't complete, that error message is expected to bubble up to the client, and just switching it to VIR_INFO isn't sufficient. I'll send a v2 - Cole
participants (1)
-
Cole Robinson