On Tue, 15 Jan 2019 at 20:54, Michael S. Tsirkin <m...@redhat.com> wrote: > > 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. > >
OK, I think I get your point. So we need something like: region's size = max_queue_size * 32 byte + xxx byte (if any) Right? > > > > > > > > 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? > Yes, that looks better. Will update it in v5. Thanks, Yongji
