[libvirt] [PATCH] Revert "Ensure async packets never get marked for sync replies"

[Daniel P. Berrange]

On Tue, Sep 20, 2011 at 06:00:55PM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 20, 2011 at 06:20:48PM +0200, Michal Privoznik wrote:
> > This partially reverts commit 984840a2c292402926ad100aeea33f8859ff31a9.
> > 
> > Without this patch, client don't finish a stream which makes any
> > API involving virStream block indefinitely.
> > ---
> >  src/rpc/virnetclient.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> > index 055361d..50f46ea 100644
> > --- a/src/rpc/virnetclient.c
> > +++ b/src/rpc/virnetclient.c
> > @@ -627,6 +627,11 @@ static int virNetClientCallDispatchStream(virNetClientPtr client)
> >      case VIR_NET_CONTINUE: {
> >          if (virNetClientStreamQueuePacket(st, &client->msg) < 0)
> >              return -1;
> > +
> > +        if (thecall && thecall->expectReply) {
> > +            VIR_DEBUG("Got sync data packet completion");
> > +            thecall->mode = VIR_NET_CLIENT_MODE_COMPLETE;
> > +        }
> >          return 0;
> >      }
> 
> This doesn't work, because 'thecall' could be the RPC message associated
> with a virStreamAbort() call, in which case we'd be signalling abort
> too soon, and when the real VIR_NET_ERROR message for the abort later
> arraives we'll not know what todo with it. This is what the quoted commit
> was solving.
> 
> The problem is that with doing virStreamRecv(), we will put a fake call
> on the queue. We only want to signal completion of those fake calls.
> This fake call should have been set to have status VIR_NET_CONTINUE
> but we forgot that. So I think we need this instead:
> 
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index 055361d..57c75c0 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -627,6 +627,18 @@ static int virNetClientCallDispatchStream(virNetClientPtr client)
>      case VIR_NET_CONTINUE: {
>          if (virNetClientStreamQueuePacket(st, &client->msg) < 0)
>              return -1;
> +
> +        if (thecall && thecall->expectReply) {
> +            if (thecall->msg->header.status == VIR_NET_CONTINUE) {
> +                VIR_DEBUG("Got a synchronous confirm");
> +                thecall->mode = VIR_NET_CLIENT_MODE_COMPLETE;
> +            } else {
> +                VIR_DEBUG("Not completing call with status %d", thecall->msg->header.status);
> +            }
> +        } else {
> +            VIR_DEBUG("Got unexpected async stream finish confirmation");
> +            return -1;
> +        }

Opps, the second 'else { .... ; return -1}'  bit should not be here.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|