[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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




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);
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]