On Tue, Dec 07, 2021 at 04:34:41PM +0100, Michal Privoznik wrote:
The aim of this function is to look at a virNetClientStream and
tell whether the incoming packet (if there's one) contains data
(type VIR_NET_STREAM) or a hole (type VIR_NET_STREAM_HOLE) and
how big the section is. This function will be called from the
remote driver in one of future commits.
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/libvirt_remote.syms | 1 +
src/rpc/virnetclientstream.c | 61 ++++++++++++++++++++++++++++++++++++
src/rpc/virnetclientstream.h | 4 +++
3 files changed, 66 insertions(+)
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 942e1013a6..07d22e368b 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -66,6 +66,7 @@ virNetClientStreamEOF;
virNetClientStreamEventAddCallback;
virNetClientStreamEventRemoveCallback;
virNetClientStreamEventUpdateCallback;
+virNetClientStreamInData;
virNetClientStreamMatches;
virNetClientStreamNew;
virNetClientStreamQueuePacket;
diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c
index 1ba6167a1d..ffc702cdc3 100644
--- a/src/rpc/virnetclientstream.c
+++ b/src/rpc/virnetclientstream.c
@@ -801,3 +801,64 @@ bool virNetClientStreamEOF(virNetClientStream *st)
{
return st->incomingEOF;
}
+
+
+int virNetClientStreamInData(virNetClientStream *st,
+ int *inData,
+ long long *length)
+{
+ int ret = 0;
+ virNetMessage *msg = NULL;
+
+ if (!st->allowSkip) {
The object should be already locked here I presume.
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Holes are not supported with this stream"));
+ return -1;
+ }
+
+ virObjectLock(st);
+
+ if (virNetClientStreamCheckState(st) < 0)
+ goto cleanup;
+
+ msg = st->rx;
+
+ if (!msg) {
+ /* No incoming message. This means that the stream is at its end. In
+ * this case, virStreamInData() should set both inData and length to
+ * zero and return success. */
+ *inData = 0;
+ *length = 0;
+ } else if (msg->header.type == VIR_NET_STREAM) {
+ *inData = 1;
+ *length = msg->bufferLength - msg->bufferOffset;
+ } else if (msg->header.type == VIR_NET_STREAM_HOLE) {
+ *inData = 0;
+
+ if (st->holeLength == 0 &&
+ virNetClientStreamHandleHole(NULL, st) < 0)
I was never a fried with our way of decoding some data and I guess that this is one of the occasions and st->holeLength just shows whether the message is already decoded. One of the problems I have here is that most of the functions double check their input states, although this one does not, making it called conditionally, which seems weird ...
+ goto cleanup; + + *length = st->holeLength; + st->holeLength = 0; + + /* virNetClientStreamHandleHole() called above did pop the message from + * the queue (and freed it). Instead of trying to push it back let's + * just signal to the caller what we did. */
... especially when this comment makes it seem like it relies on that function being called. Anyway, it looks fine to me, so with the locking issue above fixed, Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
