Re: [PATCH v4 07/10] qmp: add chardev-resize command

[Filip Hejsek <filip.hejsek@gmail.com>]

On Tue, 2025-09-16 at 15:07 +0200, Markus Armbruster wrote:
> Filip Hejsek <filip.hejsek@gmail.com> writes:
> 
> > On Mon, 2025-09-15 at 08:35 +0200, Markus Armbruster wrote:
> > > Filip Hejsek <filip.hejsek@gmail.com> writes:
> > > 
> > > > On Fri, 2025-09-12 at 16:01 +0200, Markus Armbruster wrote:
> > > > > Cc: libvirt
> > > > > 
> > > > > Filip Hejsek <filip.hejsek@gmail.com> writes:
> > > > > 
> > > > > > From: Szymon Lukasz <noh4hss@gmail.com>
> > > > > > 
> > > > > > [...]
> > > > > >  
> > > > > > +##
> > > > > > +# @chardev-resize:
> > > > > 
> > > > > This name doesn't tell me what is being resized.  PATCH 04 uses
> > > > > "winsize", which is better.  The (losely) related SIGWINCH suggests
> > > > > "window change" or "window size change".  Below, you use "terminal
> > > > > size".
> > > > 
> > > > How about chardev-console-resize? That would match the name of the
> > > > virtio event (VIRTIO_CONSOLE_RESIZE).
> > > 
> > > Not bad.  It could become slightly bad if we make devices other than
> > > "consoles" make us of it.  Would that be possible?
> > 
> > I don't think the size has any meaning for devices that are not
> > connected to a console, although the code does not care whether it
> > actually is a console and simply has a size for every chardev.
> 
> Double-checking: the command works for any ChardevBackendKind, doesn't
> it?

Yes. For some (e.g. stdio) it will clash with builtin resize detection,
but it can still be used (last update wins).

Maybe using the command should be prohibited for some device types?

> > I guess I could also rename it to chardev-window-resize
> > or chardev-set-window-size. Let me know if you prefer one of these.
> 
> I think I'd prefer "window" or "terminal".
> 
> "resize" and "set size" suggest that the command initiates a size
> change.  Not true, it notifies of a size change.  Maybe
> "chardev-window-size-changed", "chardev-terminal-size-changed",
> "chardev-window-resized", or "chardev-terminal-resized".

OK, then I'll use "chardev-window-size-changed".

> > > > > 
> > > > 
> [...]
> Another question...  'vc' chardevs accept optional @rows, @cols (see
> ChardevVC).  Is this the same size or something else?

Well, yes and no. @cols + @rows control the actual size of the console
screen buffer, while the chardev size is only used to inform the guest
about the size. @cols and @rows can also be unset, in which case the
size will be determined automatically from display and font size.

This patch series does not yet implement size propagation for the 'vc'
device. I have WIP patches for that, but there is something I'm not
sure how to do, so I will likely send an RFC first.

> > > A clearly invalid size.  I guess it effectively means "unknown size".
> > > Should we document that?
> > 
> > Probably. 0x0 is I think also the default size in the Linux kernel, but
> > I don't think the Linux kernel documents this.
> 
> How does 0 x 0 behave compared to a valid size like 80 x 24?

In these patches it is not treated specially (apart from being the
default). I think the Linux kernel doesn't treat it specially either.
Terminal programs generally interpret it as unknown size and use other
methods to obtain the size like environment variables, the terminfo
database, or defaulting to 80x24. Example:

   $ python -c 'import termios; termios.tcsetwinsize(0, (0,0))'
   $ tput cols
   80

> 
> [...]
> > > > > Do we need a way to query the size?
> > > > 
> > > > I don't think it is necessary. What would be the usecase for that?
> > > 
> > > I don't know, but it's my standard question when I see an interface to
> > > set something without an interface to get it.  Its purpose is to make us
> > > think, not to make us at the get blindly.
> > 
> > I guess it might be useful for debugging. If the size is not propagated
> > correctly, one might query it to find out on which side the problem is.
> 
> We have query-chardev.  It doesn't return much.

I'm not sure what you're implying. Shall I add the size there?
> > > > >