Re: [Qemu-devel] [PATCH v4 for-4.0 4/7] libvhost-user: Support tracking inflight I/O in shared memory

[Michael S. Tsirkin]

On Fri, Jan 18, 2019 at 11:59:50AM +0800, Jason Wang wrote:
> 
> On 2019/1/18 上午11:32, Yongji Xie wrote:
> > On Thu, 17 Jan 2019 at 17:57, Jason Wang <address@hidden> wrote:
> > > 
> > > On 2019/1/15 下午10:51, Yongji Xie wrote:
> > > > > Well, this may work but here're my points:
> > > > > 
> > > > > 1) The code want to recover from backed crash by introducing extra 
> > > > > space
> > > > > to store inflight data, but it still depends on the backend to set/get
> > > > > the inflight state
> > > > > 
> > > > > 2) Since the backend could be killed at any time, the backend must 
> > > > > have
> > > > > the ability to recover from the partial inflight state
> > > > > 
> > > > > So it looks to me 1) tends to be self-contradictory and 2) tends to be
> > > > > recursive. The above lines show how tricky could the code looks like.
> > > > > 
> > > > > Solving this at vhost-user level through at backend is probably wrong.
> > > > > It's time to consider the support from virtio itself.
> > > > > 
> > > > I agree that supporting this in virtio level may be better. For
> > > > example, resubmitting inflight I/O once DEVICE_NEEDS_RESET is set in
> > > > Stefan's proposal. But I still think QEMU should be able to provide
> > > > this ability too. Supposed that one vhost-user backend need to support
> > > > multiple VMs. We can't enable reconnect ability until all VMs' guest
> > > > driver support the new feature. It's limited.
> > > 
> > > That's the way virtio evolves.
> > > 
> > > 
> > > >    But if QEMU have the
> > > > ability to store inflight buffer, the backend could at least have a
> > > > chance to support this case.
> > > 
> > > The problem is, you need a careful designed protocol described somewhere
> > That's what we should discuss in detail in this series.
> 
> 
> Well, I ask some questions for this patch, but it looks like they were still
> not answered. No?
> 

Oh absolutely. I can't say I like the implementation,
I think it's both not the most robust and suboptimal.

> > 
> > > (is vhost-user.txt a good place for this?). And this work will be
> > > (partial) duplicated for the future support from virtio spec itself.
> > > 
> > I think the duplicated code is to maintain the inflight descriptor
> > list which should be in backend. That's not main work in this series.
> > And backend could choose to include it or not.
> 
> 
> You need to have a documentation to describe the protocol. Otherwise, it
> would be very hard for other backend to implement.
> 
> Thanks


Meaning how the inflight descriptors are saved in the buffer.
Yes I agree.

> 
> > 
> > Thanks,
> > Yongji
> >