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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list