[libvirt] [PATCH v3 26/31] daemonStreamHandleRead: Wire up seekable stream

Michal Privoznik posted 31 patches 8 years, 8 months ago
[libvirt] [PATCH v3 26/31] daemonStreamHandleRead: Wire up seekable stream
Posted by Michal Privoznik 8 years, 8 months ago
Whenever client is able to receive some data from stream
daemonStreamHandleRead is called. But now the behaviour of this
function needs to be changed a bit. Previously it just read data
from underlying file (of chardev or whatever) and sent those
through the stream to client. This model will not work any longer
because it does not differentiate whether underlying file is in
data or hole section. Therefore, at the beginning of this
function add code that checks this situation and acts
accordingly.
So after the this, when wanting to send some data we always check
whether we are not in a hole and if so, skip it an inform client
about its size.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 daemon/stream.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/daemon/stream.c b/daemon/stream.c
index 57ddfe830..284499912 100644
--- a/daemon/stream.c
+++ b/daemon/stream.c
@@ -29,6 +29,7 @@
 #include "virlog.h"
 #include "virnetserverclient.h"
 #include "virerror.h"
+#include "libvirt_internal.h"
 
 #define VIR_FROM_THIS VIR_FROM_STREAMS
 
@@ -53,6 +54,7 @@ struct daemonClientStream {
     bool tx;
 
     bool allowSkip;
+    size_t dataLen; /* How much data is there remaining until we see a hole */
 
     daemonClientStreamPtr next;
 };
@@ -796,6 +798,8 @@ daemonStreamHandleRead(virNetServerClientPtr client,
     size_t bufferLen = VIR_NET_MESSAGE_LEGACY_PAYLOAD_MAX;
     int ret = -1;
     int rv;
+    int inData = 0;
+    long long length = 0;
 
     VIR_DEBUG("client=%p, stream=%p tx=%d closed=%d",
               client, stream, stream->tx, stream->closed);
@@ -820,6 +824,58 @@ daemonStreamHandleRead(virNetServerClientPtr client,
     if (!(msg = virNetMessageNew(false)))
         goto cleanup;
 
+    if (stream->allowSkip && !stream->dataLen) {
+        /* Handle skip. We want to send some data to the client. But we might
+         * be in a hole. Seek to next data. But if we are in data already, just
+         * carry on. */
+
+        rv = virStreamInData(stream->st, &inData, &length);
+        VIR_DEBUG("rv=%d inData=%d length=%lld", rv, inData, length);
+
+        if (rv < 0) {
+            if (virNetServerProgramSendStreamError(remoteProgram,
+                                                   client,
+                                                   msg,
+                                                   &rerr,
+                                                   stream->procedure,
+                                                   stream->serial) < 0)
+                goto cleanup;
+            msg = NULL;
+
+            /* We're done with this call */
+            goto done;
+        } else {
+            if (!inData && length) {
+                stream->tx = false;
+                msg->cb = daemonStreamMessageFinished;
+                msg->opaque = stream;
+                stream->refs++;
+                if (virNetServerProgramSendStreamHole(remoteProgram,
+                                                      client,
+                                                      msg,
+                                                      stream->procedure,
+                                                      stream->serial,
+                                                      length,
+                                                      0) < 0)
+                    goto cleanup;
+
+                msg = NULL;
+
+                /* We have successfully sent stream skip to the  other side.
+                 * To keep streams in sync seek locally too. */
+                virStreamSendHole(stream->st, length, 0);
+                /* We're done with this call */
+                goto done;
+            }
+        }
+
+        stream->dataLen = length;
+    }
+
+    if (stream->allowSkip &&
+        bufferLen > stream->dataLen)
+        bufferLen = stream->dataLen;
+
     rv = virStreamRecv(stream->st, buffer, bufferLen);
     if (rv == -2) {
         /* Should never get this, since we're only called when we know
@@ -834,6 +890,8 @@ daemonStreamHandleRead(virNetServerClientPtr client,
             goto cleanup;
         msg = NULL;
     } else {
+        stream->dataLen -= rv;
+
         stream->tx = false;
         if (rv == 0)
             stream->recvEOF = true;
@@ -851,6 +909,7 @@ daemonStreamHandleRead(virNetServerClientPtr client,
         msg = NULL;
     }
 
+ done:
     ret = 0;
  cleanup:
     VIR_FREE(buffer);
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 26/31] daemonStreamHandleRead: Wire up seekable stream
Posted by John Ferlan 8 years, 8 months ago

On 05/16/2017 10:04 AM, Michal Privoznik wrote:
> Whenever client is able to receive some data from stream
> daemonStreamHandleRead is called. But now the behaviour of this
> function needs to be changed a bit. Previously it just read data
> from underlying file (of chardev or whatever) and sent those
> through the stream to client. This model will not work any longer
> because it does not differentiate whether underlying file is in
> data or hole section. Therefore, at the beginning of this
> function add code that checks this situation and acts
> accordingly.

empty
 between paragraphs
> So after the this, when wanting to send some data we always check
> whether we are not in a hole and if so, skip it an inform client

s/it an inform/it and inform/

> about its size.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  daemon/stream.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/daemon/stream.c b/daemon/stream.c
> index 57ddfe830..284499912 100644
> --- a/daemon/stream.c
> +++ b/daemon/stream.c
> @@ -29,6 +29,7 @@
>  #include "virlog.h"
>  #include "virnetserverclient.h"
>  #include "virerror.h"
> +#include "libvirt_internal.h"

Oh, right virStreamInData is not publicly available... (patch 10
comments)...

>  
>  #define VIR_FROM_THIS VIR_FROM_STREAMS
>  
> @@ -53,6 +54,7 @@ struct daemonClientStream {
>      bool tx;
>  
>      bool allowSkip;
> +    size_t dataLen; /* How much data is there remaining until we see a hole */
>  
>      daemonClientStreamPtr next;
>  };
> @@ -796,6 +798,8 @@ daemonStreamHandleRead(virNetServerClientPtr client,
>      size_t bufferLen = VIR_NET_MESSAGE_LEGACY_PAYLOAD_MAX;
>      int ret = -1;
>      int rv;
> +    int inData = 0;
> +    long long length = 0;
>  
>      VIR_DEBUG("client=%p, stream=%p tx=%d closed=%d",
>                client, stream, stream->tx, stream->closed);
> @@ -820,6 +824,58 @@ daemonStreamHandleRead(virNetServerClientPtr client,
>      if (!(msg = virNetMessageNew(false)))
>          goto cleanup;
>  
> +    if (stream->allowSkip && !stream->dataLen) {

dataLen == 0 ?

(I know, same thing - I guess it's just one of those visual things for
me...)


> +        /* Handle skip. We want to send some data to the client. But we might
> +         * be in a hole. Seek to next data. But if we are in data already, just
> +         * carry on. */
> +
> +        rv = virStreamInData(stream->st, &inData, &length);
> +        VIR_DEBUG("rv=%d inData=%d length=%lld", rv, inData, length);
> +
> +        if (rv < 0) {
> +            if (virNetServerProgramSendStreamError(remoteProgram,
> +                                                   client,
> +                                                   msg,
> +                                                   &rerr,
> +                                                   stream->procedure,
> +                                                   stream->serial) < 0)
> +                goto cleanup;
> +            msg = NULL;
> +
> +            /* We're done with this call */
> +            goto done;
> +        } else {
> +            if (!inData && length) {
> +                stream->tx = false;
> +                msg->cb = daemonStreamMessageFinished;
> +                msg->opaque = stream;
> +                stream->refs++;
> +                if (virNetServerProgramSendStreamHole(remoteProgram,
> +                                                      client,
> +                                                      msg,
> +                                                      stream->procedure,
> +                                                      stream->serial,
> +                                                      length,
> +                                                      0) < 0)
> +                    goto cleanup;
> +
> +                msg = NULL;
> +
> +                /* We have successfully sent stream skip to the  other side.

Extra space between "the  other"

> +                 * To keep streams in sync seek locally too. */
> +                virStreamSendHole(stream->st, length, 0);
> +                /* We're done with this call */
> +                goto done;
> +            }
> +        }
> +
> +        stream->dataLen = length;
> +    }
> +
> +    if (stream->allowSkip &&
> +        bufferLen > stream->dataLen)
> +        bufferLen = stream->dataLen;
> +
>      rv = virStreamRecv(stream->st, buffer, bufferLen);
>      if (rv == -2) {
>          /* Should never get this, since we're only called when we know
> @@ -834,6 +890,8 @@ daemonStreamHandleRead(virNetServerClientPtr client,
>              goto cleanup;
>          msg = NULL;
>      } else {
> +        stream->dataLen -= rv;
> +

Since dataLen is only "set" if stream->allowSkip - should this be fenced
similarly?

Not that I see ->dataLen being used for anything other than sparse
stream mgmt...

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

>          stream->tx = false;
>          if (rv == 0)
>              stream->recvEOF = true;
> @@ -851,6 +909,7 @@ daemonStreamHandleRead(virNetServerClientPtr client,
>          msg = NULL;
>      }
>  
> + done:
>      ret = 0;
>   cleanup:
>      VIR_FREE(buffer);
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list