Re: [Qemu-devel] [PATCH RFC 1/6] io: only allow return path for socket typed

From: "Daniel P. Berrange" <berrange@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: qemu-devel@nongnu.org,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Juan Quintela <quintela@redhat.com>
Subject: Re: [Qemu-devel] [PATCH RFC 1/6] io: only allow return path for socket typed
Date: Fri, 19 May 2017 09:30:10 +0100	[thread overview]
Message-ID: <20170519083010.GE4912@redhat.com> (raw)
In-Reply-To: <20170519082538.GD4912@redhat.com>

On Fri, May 19, 2017 at 09:25:38AM +0100, Daniel P. Berrange wrote:
> On Fri, May 19, 2017 at 02:43:27PM +0800, Peter Xu wrote:
> > We don't really have a return path for the other types yet. Let's check
> > this when .get_return_path() is called.
> > 
> > For this, we introduce a new feature bit, and set it up only for socket
> > typed IO channels.
> > 
> > This will help detect earlier failure for postcopy, e.g., logically
> > speaking postcopy cannot work with "exec:". Before this patch, when we
> > try to migrate with "migrate -d exec:cat>out", we'll hang the system.
> > With this patch, we'll get:
> > 
> > (qemu) migrate -d exec:cat>out
> > Unable to open return-path for postcopy
> 
> This is wrong - post-copy migration *can* work with exec: - it just entirely
> depends on what command you are running. Your example ran a command which is
> unidirectional, but if you ran 'exec:socat ...' you would have a fully
> bidirectional channel. Actually the channel is always bi-directional, but
> 'cat' simply won't ever send data back to QEMU.
> 
> If QEMU hangs when the other end doesn't send data back, that actually seems
> like a potentially serious bug in migration code. Even if using the normal
> 'tcp' migration protocol, if the target QEMU server hangs and fails to
> send data to QEMU on the return path, the source QEMU must never hang.

BTW, if you want to simplify the code in this area at all, then arguably
we should get rid of the "get_return_path" helper method entirely. We're
not actually opening any new connections - we're just creating a second
QEMUFile that uses the same underlying QIOChannel object. All we would
need is for the QEMUFile to have a separate 'buf' field management in
QEMUFile for the read & write directions.  Then all the code would be
able to just use the single QEMUFile for read & write getting rid of this
concept of "opening a return path" which doens't actually do anything at
the underlying data transport level.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|