Re: [PATCH] virtio: console: Make resizing compliant with virtio spec

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Mar 19, 2025 at 06:13:08PM +0100, Halil Pasic wrote:
> On Wed, 19 Mar 2025 11:00:06 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > > > I was mistaken in my earlier reply - I had missed this
> > > > virtio_console_resize definition in the spec.  So indeed there's a
> > > > discrepancy in Linux kernel and the spec's ordering for the control
> > > > message.
> > > > 
> > > > OK, that needs fixing someplace.  Perhaps in the kernel (like your
> > > > orig. patch), but with an accurate commit message.  
> > > 
> > > So should I send a patch v2 or should the spec be changed instead? Or
> > > would you like to first await the opinion of the spec maintainers?
> > > 
> > > The mail I initially sent to the virtio mailing list seems to have
> > > fallen on deaf ears. I now added Michael Tsirkin to this thread so that
> > > things might get going.  
> > 
> > 
> > If we can fix the driver to fit the spec, that's best.
> > We generally try to avoid changing the spec just because
> > drivers are buggy.
> 
> I think the call if fixing the driver is possible needs to be made by
> the maintainers of the driver. Fixing the driver IMHO implies that
> if this is seeing any usage in the wild where it properly works a
> fix on the driver side would imply a function regression. But any
> implementers should have complained. So IMHO it is not unreasonable to
> assume that this is not seeing any usage in the wild.
> 
> And people would still have the opportunity to catch the regression
> during testing and complain about it.
> 
> I agree with Michael, changing the spec because of a buggy
> implementation should rather be the exception than the rule. And AFAIK
> it is not like we have declared something a reference implementation,
> so in that sense the implementation in Linux is just one implementation.
> 
> I suppose making it runtime configurable via module parameter is an
> overkill at this point.
> 
> So based no what we know I'm slightly in favor of let us just fix it
> in Linux and see if anybody complains.
> 
> Another thing I noticed during looking at this. AFAICT Linux does not
> seem to handle endiannes here. And AFAIU the message is supposed to hold
> le16 that is little endian u16! Maximilian, is this in your opinion
> something we need to fix as well? Or am I just missing the conversion?
> 
> Regards,
> Halil

Agreed, still, please do a bit of research on open source hypervisors
at least (rust-vmm?) and include the info which ones you checked.