qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 for-4.0 2/7] vhost-user: Support transferring


From: Yongji Xie
Subject: Re: [Qemu-devel] [PATCH v4 for-4.0 2/7] vhost-user: Support transferring inflight buffer between qemu and backend
Date: Fri, 18 Jan 2019 10:45:22 +0800

On Tue, 15 Jan 2019 at 22:18, Yongji Xie <address@hidden> wrote:
>
> On Tue, 15 Jan 2019 at 20:54, Michael S. Tsirkin <address@hidden> 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 <address@hidden> wrote:
> > > >
> > > > On Wed, Jan 09, 2019 at 07:27:23PM +0800, address@hidden 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.
>

Hi Michael,

I found a problem during implentmenting this. If we put version into
the shared buffer, QEMU will reset it when vm reset. Then if backend
restart at the same time, the version of this buffer will be lost. So
maybe qemu still need to maintain it.

Thanks,
Yongji



reply via email to

[Prev in Thread] Current Thread [Next in Thread]