Re: [virtio] [PATCH] ccw: split descriptor/available/used rings

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Mon, 7 Oct 2013 17:57:25 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Oct 07, 2013 at 04:32:17PM +0200, Cornelia Huck wrote:
> > On Mon, 23 Sep 2013 13:39:30 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > Resolves VIRTIO-23.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  virtio-v1.0-wd01-part1-specification.txt | 23 ++++++++++++++++++++++-
> > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/virtio-v1.0-wd01-part1-specification.txt b/virtio-v1.0-wd01-part1-specification.txt
> > > index cafc34f..08cb857 100644
> > > --- a/virtio-v1.0-wd01-part1-specification.txt
> > > +++ b/virtio-v1.0-wd01-part1-specification.txt
> > > @@ -1187,7 +1187,28 @@ Afterwards, CCW_CMD_SET_VQ is issued by the guest to inform the
> > >  host about the location used for its queue. The transmitted
> > >  structure is
> > > 
> > > -struct vq_info_block {
> > > +struct vq_info_block_legacy {
> > 
> > That's not legacy, no?
> > 
> > > +	__u64 desc;
> > > +	__u64 avail;
> > > +	__u64 used;
> > > +	__u16 index;
> > > +	__u16 num;
> > > +	__u32 padding;
> > > +} __attribute__ ((packed));
> > 
> > Hm. This means the legacy and non-legacy cases transmit two
> > incompatible structures with the same channel command. I'm not sure
> > whether this is a good idea.
> > 
> > The structures are different in length, and the host will fail if the
> > structure transmitted is too short, but it is possible for the guest to
> > transmit the longer structure to a legacy host and turn off the length
> > check. Sure, that should not happen, but I'm not too fond of this
> > structure.
> > 
> > So we should either:
> > 
> > - extend the old structure in a compatible way
> > - use a new channel command for this purpose (and prepend the structure
> >   with a version field in case we want to change it again in the future)
> 
> This would burden 1.0 spec with legacy considerations.
> IMO that's ugly.
> 
> Rusty sent a proposal where guest sets features
> before configuring VQs.
> 
> If we vote this in, host can detect whether guest acked the VIRTIO_1
> feature bit and handle the command appropriately.

Yes, it can. I'd still prefer to make this as failsafe as possible.
Simply extending the structure would satisfy this (and there is
precedent for this - it has been done e.g. for SenseID).