[libvirt] [PATCH] virNetClientStreamQueuePacket: Set st->incomingEOF on the end of stream
While reworking client side of streams, I had to postpone payload decoding so that stream holes and stream data can be distinguished in virNetClientStreamRecvPacket. That's merely what 18944b7aea46d does. However, I accidentally removed one important bit: when server sends us an empty STREAM packet (with no payload) - meaning end of stream - st->incomingEOF flag needs to be set. It used to be before I touched the code. After I removed it, virNetClientStreamRecvPacket will try to fetch more data from the stream, but it will never come. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Thanks to Martin who helped me debug this. src/rpc/virnetclientstream.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index a9bf271dc..54729c84f 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -278,6 +278,15 @@ int virNetClientStreamQueuePacket(virNetClientStreamPtr st, VIR_DEBUG("Incoming stream message: stream=%p message=%p", st, msg); + if (msg->bufferLength == msg->bufferOffset) { + /* No payload means end of the stream. */ + virObjectLock(st); + st->incomingEOF = true; + virNetClientStreamEventTimerUpdate(st); + virObjectUnlock(st); + return 0; + } + /* Unfortunately, we must allocate new message as the one we * get in @msg is going to be cleared later in the process. */ -- 2.13.0
On 06/06/2017 06:53 AM, Michal Privoznik wrote:
While reworking client side of streams, I had to postpone payload decoding so that stream holes and stream data can be distinguished in virNetClientStreamRecvPacket. That's merely what 18944b7aea46d does. However, I accidentally removed one important bit: when server sends us an empty STREAM packet (with no payload) - meaning end of stream - st->incomingEOF flag needs to be set. It used to be before I touched the code. After I removed it, virNetClientStreamRecvPacket will try to fetch more data from the stream, but it will never come.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Thanks to Martin who helped me debug this.
src/rpc/virnetclientstream.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index a9bf271dc..54729c84f 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -278,6 +278,15 @@ int virNetClientStreamQueuePacket(virNetClientStreamPtr st,
VIR_DEBUG("Incoming stream message: stream=%p message=%p", st, msg);
+ if (msg->bufferLength == msg->bufferOffset) { + /* No payload means end of the stream. */ + virObjectLock(st); + st->incomingEOF = true; + virNetClientStreamEventTimerUpdate(st); + virObjectUnlock(st); + return 0; + } +
Your description of the problem and fix helps a reviewer with limited experience in this part of the code base :-). Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
On 06/07/2017 05:51 PM, Jim Fehlig wrote:
On 06/06/2017 06:53 AM, Michal Privoznik wrote:
While reworking client side of streams, I had to postpone payload decoding so that stream holes and stream data can be distinguished in virNetClientStreamRecvPacket. That's merely what 18944b7aea46d does. However, I accidentally removed one important bit: when server sends us an empty STREAM packet (with no payload) - meaning end of stream - st->incomingEOF flag needs to be set. It used to be before I touched the code. After I removed it, virNetClientStreamRecvPacket will try to fetch more data from the stream, but it will never come.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Thanks to Martin who helped me debug this.
src/rpc/virnetclientstream.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Regards, Jim
Thank you. Pushed now. Michal
participants (2)
-
Jim Fehlig -
Michal Privoznik