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

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; + }