Re: [Qemu-devel] [PATCH for-4.0 0/6] vhost-user-blk: Add support for backend reconnecting

[Michael S. Tsirkin]

On Thu, Dec 13, 2018 at 11:41:06AM +0800, Yongji Xie wrote:
> On Thu, 13 Dec 2018 at 10:58, Jason Wang <address@hidden> wrote:
> >
> >
> > On 2018/12/12 下午5:18, Yongji Xie wrote:
> > >>>> Ok, then we can simply forbid increasing the avail_idx in this case?
> > >>>>
> > >>>> Basically, it's a question of whether or not it's better to done it in
> > >>>> the level of virtio instead of vhost. I'm pretty sure if we expose
> > >>>> sufficient information, it could be done without touching vhost-user.
> > >>>> And we won't deal with e.g migration and other cases.
> > >>>>
> > >>> OK, I get your point. That's indeed an alternative way. But this 
> > >>> feature seems
> > >>> to be only useful to vhost-user backend.
> > >> I admit I could not think of a use case other than vhost-user.
> > >>
> > >>
> > >>>    I'm not sure whether it make sense to
> > >>> touch virtio protocol for this feature.
> > >> Some possible advantages:
> > >>
> > >> - Feature could be determined and noticed by user or management layer.
> > >>
> > >> - There's no need to invent ring layout specific protocol to record in
> > >> flight descriptors. E.g if my understanding is correct, for this series
> > >> and for the example above, it still can not work for packed virtqueue
> > >> since descriptor id is not sufficient (descriptor could be overwritten
> > >> by used one). You probably need to have a (partial) copy of descriptor
> > >> ring for this.
> > >>
> > >> - No need to deal with migration, all information was in guest memory.
> > >>
> > > Yes, we have those advantages. But seems like handle this in vhost-user
> > > level could be easier to be maintained in production environment. We can
> > > support old guest. And the bug fix will not depend on guest kernel 
> > > updating.
> >
> >
> > Yes. But the my main concern is the layout specific data structure. If
> > it could be done through a generic structure (can it?), it would be
> > fine. Otherwise, I believe we don't want another negotiation about what
> > kind of layout that backend support for reconnect.
> >
> 
> Yes, the current layout in shared memory didn't support packed virtqueue 
> because
> the information of one descriptor in descriptor ring will not be
> available once device fetch it.
> 
> I also thought about a generic structure before. But I failed... So I
> tried another way
> to acheive that in this series. In QEMU side, we just provide a shared
> memory to backend
> and we didn't define anything for this memory. In backend side, they
> should know how to
> use those memory to record inflight I/O no matter what kind of
> virtqueue they used.
> Thus,  If we updates virtqueue for new virtio spec in the feature, we
> don't need to touch
> QEMU and guest. What do you think about it?
> 
> Thanks,
> Yongji

I think that's a good direction to take, yes.
Backends need to be very careful about the layout,
with versioning etc.

-- 
MST