On Tue, Jan 15, 2019 at 02:46:42PM +0800, Yongji Xie wrote: > On Tue, 15 Jan 2019 at 06:25, Michael S. Tsirkin <m...@redhat.com> wrote: > > > > On Wed, Jan 09, 2019 at 07:27:23PM +0800, elohi...@gmail.com wrote: > > > @@ -382,6 +397,30 @@ If VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD protocol > > > feature is negotiated, > > > slave can send file descriptors (at most 8 descriptors in each message) > > > to master via ancillary data using this fd communication channel. > > > > > > +Inflight I/O tracking > > > +--------------------- > > > + > > > +To support slave reconnecting, slave need to track inflight I/O in a > > > +shared memory. VHOST_USER_GET_INFLIGHT_FD and VHOST_USER_SET_INFLIGHT_FD > > > +are used to transfer the memory between master and slave. And to > > > encourage > > > +consistency, we provide a recommended format for this memory: > > > > I think we should make a stronger statement and actually > > just say what the format is. Not recommend it weakly. > > > > Okey, will do it. > > > > + > > > +offset width description > > > +0x0 0x400 region for queue0 > > > +0x400 0x400 region for queue1 > > > +0x800 0x400 region for queue2 > > > +... ... ... > > > + > > > +For each virtqueue, we have a 1024 bytes region. > > > > > > Why is the size hardcoded? Why not a function of VQ size? > > > > Sorry, I didn't get your point. Should the region's size be fixed? Do > you mean we need to document a function for the region's size?
Well you are saying 0x0 to 0x400 is for queue0. How do you know that's enough? And why are 0x400 bytes necessary? After all max queue size can be very small. > > > > > The region's format is like: > > > + > > > +offset width description > > > +0x0 0x1 descriptor 0 is in use or not > > > +0x1 0x1 descriptor 1 is in use or not > > > +0x2 0x1 descriptor 2 is in use or not > > > +... ... ... > > > + > > > +For each descriptor, we use one byte to specify whether it's in use or > > > not. > > > + > > > Protocol features > > > ----------------- > > > > > > > I think that it's a good idea to have a version in this region. > > Otherwise how are you going to handle compatibility when > > this needs to be extended? > > > > I have put the version into the message's payload: VhostUserInflight. Is it > OK? > > Thanks, > Yongji I'm not sure I like it. So is qemu expected to maintain it? Reset it? Also don't you want to be able to detect that qemu has reset the buffer? If we have version 1 at a known offset that can serve both purposes. Given it only has value within the buffer why not store it there? -- MST
