On Thu, 13 Aug 2020 14:04:15 +0200 Stefano Garzarella <sgarz...@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 <sgarz...@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...
