Re: virtio-vsock requires 'disable-legacy=on' in QEMU 5.1

[Cornelia Huck]

On Thu, 13 Aug 2020 14:04:15 +0200
Stefano Garzarella <sgarzare@redhat.com> wrote:

> On Thu, Aug 13, 2020 at 12:37:37PM +0200, Cornelia Huck wrote:
> > On Thu, 13 Aug 2020 12:24:30 +0200
> > Stefano Garzarella <sgarzare@redhat.com> wrote:
> >   
> > > On Thu, Aug 13, 2020 at 11:28:20AM +0200, Cornelia Huck wrote:  
> > > > We basically have three possible ways to deal with this:
> > > > 
> > > > - Force it to modern (i.e., what you have been doing; would need the
> > > >   equivalent changes in ccw as well.)    
> > > 
> > > Oo, thanks for pointing out ccw!
> > > I don't know ccw well, in this case should we set dev->max_rev to 1 or 2
> > > to force to modern?  
> > 
> > No, ->max_rev is the wrong side of the limit :) You want  
> 
> Well :-) Thanks!
> 
> > 
> >     ccw_dev->force_revision_1 = true;
> > 
> > in _instance_init() (see e.g. virtio-ccw-gpu.c).
> >   
> > >   
> > > >   Pro: looks like the cleanest approach.
> > > >   Con: not sure if we would need backwards compatibility support,
> > > >   which looks hairy.    
> > > 
> > > Not sure too.  
> > 
> > Yes, I'm not sure at all how to handle user-specified values for
> > legacy/modern.

Thinking a bit more about it, I'm not sure whether we even *can*
provide backwards compatibility: we have different autoconfigurations
for PCI based upon where it is plugged, and ccw does not have a way to
turn legacy on/off, except from within the code.

> >   
> > >   
> > > > - Add vsock to the list of devices with legacy support.
> > > >   Pro: Existing setups continue to work.
> > > >   Con: If vsock is really virtio-1-only, we still carry around
> > > >   possibly broken legacy support.    
> > > 
> > > I'm not sure it is virtio-1-only, but virtio-vsock was introduced in
> > > 2016, so I supposed it is modern-only.  
> > 
> > Yes, I would guess so as well.
> >   
> > > 
> > > How can I verify that? Maybe forcing legacy mode and run some tests.  
> > 
> > Probably yes. The likeliest area with issues is probably endianness, so
> > maybe with something big endian in the mix?
> >   
> 
> Yeah, I'll try this setup!
> 
> > >   
> > > > - Do nothing, have users force legacy off. Bad idea, as ccw has no way
> > > >   to do that on the command line.
> > > > 
> > > > The first option is probably best.

The first option is now "force modern, but with no backwards
compatibility", which is not that great; but "allow legacy, even though
it should not exist" is not particularly appealing, either... what a
mess :(

> > > >    
> > > 
> > > Yeah, I agree with you!  
> > 
> > Yes, it's really a pity we only noticed this after the release; this
> > was supposed to stop new devices with legacy support creeping in, not
> > to break existing command lines :(
> >   
> 
> Yes, I forgot to test vsock stuff before the release :-(
> Maybe we should add some tests...

Speaking of tests: do you have a quick way to test vhost-vsock at hand?
Maybe I should add it to my manual repertoire...