Re: [PATCH v2 0/6] virtio-console: notify about the terminal size

[Daniel P . Berrangé]

On Wed, Jun 24, 2020 at 01:26:34PM +0200, Szymon Lukasz wrote:
> Also there is a problem with the virtio spec and Linux Kernel
> implementation, the order of fields in virtio_console_resize struct
> differs between the kernel and the spec. I do not know if there is any
> implementation of the virtio-console driver that handles resize messages
> and uses a different order than Linux.

Well this is a bit of a mess :-(

The main virtio_console_config struct has cols, then rows.

The Linux impl of resizing appears to have arrived in 2010, and created
a new struct with rows, then cols.

commit 8345adbf96fc1bde7d9846aadbe5af9b2ae90882
Author: Amit Shah <amit.shah@redhat.com>
Date:   Thu May 6 02:05:09 2010 +0530

    virtio: console: Accept console size along with resize control message
    
    The VIRTIO_CONSOLE_RESIZE control message sent to us by the host now
    contains the new {rows, cols} values for the console. This ensures each
    console port gets its own size, and we don't depend on the config-space
    rows and cols values at all now.
    
    Signed-off-by: Amit Shah <amit.shah@redhat.com>
    CC: Christian Borntraeger <borntraeger@de.ibm.com>
    CC: linuxppc-dev@ozlabs.org
    CC: Kusanagi Kouichi <slash@ac.auone-net.jp>
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


The virtio spec documenting this came 4 years later in 2014 and documented
the resize struct with cols, then rows, which differs from Linux impl,
but matches ordering of the main virtio_console_config:

commit 908cfaa782e950d6656d947599d7a6c9fb16cad1
Author: rusty <rusty@0c8fb4dd-22a2-4bb5-bc14-6c75a5f43652>
Date:   Wed Feb 12 03:15:57 2014 +0000

    Feedback #6: Applied
    
    As per minutes:
            https://lists.oasis-open.org/archives/virtio/201402/msg00121.html
    
    Signed-off-by: Rusty Russell <rusty@au1.ibm.com>
    
    git-svn-id: https://tools.oasis-open.org/version-control/svn/virtio@237 
0c8fb4dd-22a2-4bb5-bc14-6c75a5f43652

I can understand why it is desirable for the resize struct to match
the order of the initial config struct.  I'm guessing it just wasn't
realized that the Linux impl was inverted for resize

The FreeBSD impl of virtio-console doesn't do resize:

  
https://github.com/freebsd/freebsd/blob/master/sys/dev/virtio/console/virtio_console.c#L874

Not sure what other impls are going to be around, but I feel like
Linux is going to be the most commonly deployed by orders of magnitude.

So I'd say QEMU should match Linux, and the spec should be fixed.


Have you reported this bug to the virtio spec people directly yet ?

I don't see an issue open at

  https://github.com/oasis-tcs/virtio-spec/issues/

so I think one should be filed there

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|