4:56 p.m.
I've been working on an RFC patch-set to implement resizing for
consoles in QEMU. Now that the patch-set is in review I've turned my
attention to bringing this feature to libvirt.
With the QEMU patch-set there are two ways to resize a pty chardev.
1. send a TIOCSWINSZ ioctl
2. send a QMP msg
The first approach doesn't work under Libvirt. Pty chardevs are proxied
over a FIFO meaning the ioctl wouldn't get to libvirt. Furthermore,
this approach is incompatible with remote management which libvirt
seems to go to great lengths to support.
Sending a QMP message is a QEMU specific feature. It seems to me that
adding a new handle (something like domainResizeConsole) would have to
be added to the virHypervisorDriver struct. It would be really nifty if
the resize handle could be implemented in the virStream, however it
seems that all the virStreamDriver handlers are common code (aka shared
across all hypervisors). From what I've seen it seems that the only way
to switch from common code to hypervisor specific code would be over
the virHypervisorDriver. As it stands it seems to me that the best
course of action would be adding a new handle to the
virHypervisorDriver, but I'm unsure whether that's really the right
call.
Furthermore, if we don't want to broadcast a QMP message to every
chardev, the handle needs to determine the alias with which the chardev
can be adressed in QMP. in qemuDomainOpenConsole the alias for the
console is set and added as the callback argument for the internal
close function. There doesn't seem to be a way to get the alias of a
console from virStream. It seems that I need to add an alias string
field to the virStream or the virStreamFDData. I could however be
mistaken and there is already a way to get the alias for a virStream,
hence this email
TLDR I have the following 2 questions:
- Should the resize handle be implemented in the virHypervisorDriver
directly or in the virStreamDriver?
- Is there a way to get the QMP alias of a chardev from a virStream?
Thanks in advance
Max
6:58 p.m.
On Thu, Mar 20, 2025 at 08:56:04AM +0100, Maximilian Immanuel Brandtner wrote:
I've been working on an RFC patch-set to implement resizing for
consoles in QEMU. Now that the patch-set is in review I've turned my
attention to bringing this feature to libvirt.
Can you point to the QEMU patchset URL ?
With the QEMU patch-set there are two ways to resize a pty chardev.
1. send a TIOCSWINSZ ioctl
2. send a QMP msg
The first approach doesn't work under Libvirt. Pty chardevs are proxied
over a FIFO meaning the ioctl wouldn't get to libvirt. Furthermore,
this approach is incompatible with remote management which libvirt
seems to go to great lengths to support.
So we have the flow
virsh console (or equiv mgmt equiv)
-> virStream
-> libvirtd/virtqemud
-> virStream
-> PTY
-> QEMU chardev
-> guest serial port / paravirt console
NB I've always wanted to extend our console stream handling to support
UNIX socket / TCP socket chardev instead of the PTY, as the connection
semantics are so much saner to deal with.
Sending a QMP message is a QEMU specific feature. It seems to me
that
adding a new handle (something like domainResizeConsole) would have to
be added to the virHypervisorDriver struct. It would be really nifty if
the resize handle could be implemented in the virStream, however it
seems that all the virStreamDriver handlers are common code (aka shared
across all hypervisors). From what I've seen it seems that the only way
to switch from common code to hypervisor specific code would be over
the virHypervisorDriver. As it stands it seems to me that the best
course of action would be adding a new handle to the
virHypervisorDriver, but I'm unsure whether that's really the right
call.
How is the window resize supposed to be passed to the QEMU chardev
backend for the different chardev backend types ?
TLDR I have the following 2 questions:
- Should the resize handle be implemented in the virHypervisorDriver
directly or in the virStreamDriver?
Unclear currently. I did think about this a long while back and tried
to hack something into virStream, but never completed it and it was
not as simple as it first seemed
- Is there a way to get the QMP alias of a chardev from a virStream?
No, currently the stream only knows about the underlying host OS channel,
nothing about the hypervisor
With 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 :|
7:09 p.m.
On Thu, 2025-03-20 at 09:58 +0000, Daniel P. Berrangé wrote:
On Thu, Mar 20, 2025 at 08:56:04AM +0100, Maximilian Immanuel
Brandtner wrote:
> I've been working on an RFC patch-set to implement resizing for
> consoles in QEMU. Now that the patch-set is in review I've turned
> my
> attention to bringing this feature to libvirt.
Can you point to the QEMU patchset URL ?
It's not yet on the upstream mailing list as it's still in internal
review. It should soon be sent to the upstream mailing list.
> With the QEMU patch-set there are two ways to resize a pty chardev.
> 1. send a TIOCSWINSZ ioctl
> 2. send a QMP msg
>
> The first approach doesn't work under Libvirt. Pty chardevs are
> proxied
> over a FIFO meaning the ioctl wouldn't get to libvirt. Furthermore,
> this approach is incompatible with remote management which libvirt
> seems to go to great lengths to support.
So we have the flow
virsh console (or equiv mgmt equiv)
-> virStream
-> libvirtd/virtqemud
-> virStream
-> PTY
-> QEMU chardev
-> guest serial port / paravirt console
NB I've always wanted to extend our console stream handling to
support
UNIX socket / TCP socket chardev instead of the PTY, as the
connection
semantics are so much saner to deal with.
> Sending a QMP message is a QEMU specific feature. It seems to me
> that
> adding a new handle (something like domainResizeConsole) would have
> to
> be added to the virHypervisorDriver struct. It would be really
> nifty if
> the resize handle could be implemented in the virStream, however it
> seems that all the virStreamDriver handlers are common code (aka
> shared
> across all hypervisors). From what I've seen it seems that the only
> way
> to switch from common code to hypervisor specific code would be
> over
> the virHypervisorDriver. As it stands it seems to me that the best
> course of action would be adding a new handle to the
> virHypervisorDriver, but I'm unsure whether that's really the right
> call.
How is the window resize supposed to be passed to the QEMU chardev
backend for the different chardev backend types ?
For all chardevs that support chr_resize the QMP properties winsize_row
and winsize_col are added to the chardev. In HMP the request would look
as follows:
`qom-set /chardevs/console winsize_row 24`
`qom-set /charedvs/console winsize_col 80`
For now winsize_row and winsize_col are implemented as two seperate
integers for the sake of simplicity, however I would be open to
changing it.
These two QMP properties are the same for all chardevs whose backends
support resize so it doesn't matter if the chardev uses virtio or some
other backend when it comes to changing the winsize over QMP.
> TLDR I have the following 2 questions:
> - Should the resize handle be implemented in the
> virHypervisorDriver
> directly or in the virStreamDriver?
Unclear currently. I did think about this a long while back and tried
to hack something into virStream, but never completed it and it was
not as simple as it first seemed
> - Is there a way to get the QMP alias of a chardev from a
> virStream?
No, currently the stream only knows about the underlying host OS
channel,
nothing about the hypervisor
Would it make more sense to add the QMP alias to the virStream or the
virFDStreamData?
With regards,
Daniel
7:28 p.m.
On Thu, Mar 20, 2025 at 11:09:41AM +0100, Maximilian Immanuel Brandtner wrote:
On Thu, 2025-03-20 at 09:58 +0000, Daniel P. Berrangé wrote:
> On Thu, Mar 20, 2025 at 08:56:04AM +0100, Maximilian Immanuel
> Brandtner wrote:
> > I've been working on an RFC patch-set to implement resizing for
> > consoles in QEMU. Now that the patch-set is in review I've turned
> > my
> > attention to bringing this feature to libvirt.
>
> Can you point to the QEMU patchset URL ?
It's not yet on the upstream mailing list as it's still in internal
review. It should soon be sent to the upstream mailing list.
Urgh, saying the QEMU patch-set is in review, when you've not even posted
it yet is more than a little mis-leading :-( From an upstream POV, we
would expect QEMU patches to be published, rather than asking us to design
an libvirt interface against a private QEMU design we can't even see.
With 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 :|
8:51 p.m.
On Thu, 2025-03-20 at 10:28 +0000, Daniel P. Berrangé wrote:
On Thu, Mar 20, 2025 at 11:09:41AM +0100, Maximilian Immanuel
Brandtner wrote:
> On Thu, 2025-03-20 at 09:58 +0000, Daniel P. Berrangé wrote:
> > On Thu, Mar 20, 2025 at 08:56:04AM +0100, Maximilian Immanuel
> > Brandtner wrote:
> > > I've been working on an RFC patch-set to implement resizing for
> > > consoles in QEMU. Now that the patch-set is in review I've
> > > turned
> > > my
> > > attention to bringing this feature to libvirt.
> >
> > Can you point to the QEMU patchset URL ?
>
> It's not yet on the upstream mailing list as it's still in internal
> review. It should soon be sent to the upstream mailing list.
Urgh, saying the QEMU patch-set is in review, when you've not even
posted
it yet is more than a little mis-leading :-( From an upstream POV, we
would expect QEMU patches to be published, rather than asking us to
design
an libvirt interface against a private QEMU design we can't even see.
With regards,
Daniel
Understandable, I just wanted some feedback on how I should implement
the feature in libvirt, so that I wouldn't create something that would
have to be majorly reworked.
8:56 p.m.
On Thu, Mar 20, 2025 at 12:51:35PM +0100, Maximilian Immanuel Brandtner wrote:
On Thu, 2025-03-20 at 10:28 +0000, Daniel P. Berrangé wrote:
> On Thu, Mar 20, 2025 at 11:09:41AM +0100, Maximilian Immanuel
> Brandtner wrote:
> > On Thu, 2025-03-20 at 09:58 +0000, Daniel P. Berrangé wrote:
> > > On Thu, Mar 20, 2025 at 08:56:04AM +0100, Maximilian Immanuel
> > > Brandtner wrote:
> > > > I've been working on an RFC patch-set to implement resizing for
> > > > consoles in QEMU. Now that the patch-set is in review I've
> > > > turned
> > > > my
> > > > attention to bringing this feature to libvirt.
> > >
> > > Can you point to the QEMU patchset URL ?
> >
> > It's not yet on the upstream mailing list as it's still in internal
> > review. It should soon be sent to the upstream mailing list.
>
> Urgh, saying the QEMU patch-set is in review, when you've not even
> posted
> it yet is more than a little mis-leading :-( From an upstream POV, we
> would expect QEMU patches to be published, rather than asking us to
> design
> an libvirt interface against a private QEMU design we can't even see.
Understandable, I just wanted some feedback on how I should implement
the feature in libvirt, so that I wouldn't create something that would
have to be majorly reworked.
FWIW, that latter point is why I generally suggest that contributors for
any project post patches publically as soon as possible, and not delay
for the sake of reviews in private. Any colleagues are free to review the
patches publically on the project lists/gitforce/etc, at the same time as
any other community member. This reduces the need for the author to answer
the same questions multiple times, and also means they can more quickly
learn whether the patch design is likely to be acceptable, and thus not
waste time reviewing something which would then need to be redesigned.
With 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 :|
7:05 p.m.
On Thu, Mar 20, 2025 at 08:56:04 +0100, Maximilian Immanuel Brandtner wrote:
I've been working on an RFC patch-set to implement resizing for
consoles in QEMU. Now that the patch-set is in review I've turned my
attention to bringing this feature to libvirt.
With the QEMU patch-set there are two ways to resize a pty chardev.
1. send a TIOCSWINSZ ioctl
2. send a QMP msg
The first approach doesn't work under Libvirt. Pty chardevs are proxied
over a FIFO meaning the ioctl wouldn't get to libvirt. Furthermore,
this approach is incompatible with remote management which libvirt
seems to go to great lengths to support.
Yes, with the remote console support we have in virsh the console will
be always proxied so direct approach would not work.
Only if a client application decided to access the pty directly based on
the path it gets from libvirt.
Sending a QMP message is a QEMU specific feature. It seems to me
that
adding a new handle (something like domainResizeConsole) would have to
be added to the virHypervisorDriver struct. It would be really nifty if
the resize handle could be implemented in the virStream, however it
seems that all the virStreamDriver handlers are common code (aka shared
across all hypervisors). From what I've seen it seems that the only way
to switch from common code to hypervisor specific code would be over
the virHypervisorDriver. As it stands it seems to me that the best
course of action would be adding a new handle to the
virHypervisorDriver, but I'm unsure whether that's really the right
call.
virStream is universal; not used only for console but also for
"uploading" files in the storage driver or for "tunnelled" migration.
Thus apart from not being qemu-only it's not even used for similar
features.
Furthermore, if we don't want to broadcast a QMP message to
every
chardev, the handle needs to determine the alias with which the chardev
can be adressed in QMP. in qemuDomainOpenConsole the alias for the
console is set and added as the callback argument for the internal
close function. There doesn't seem to be a way to get the alias of a
console from virStream. It seems that I need to add an alias string
field to the virStream or the virStreamFDData. I could however be
mistaken and there is already a way to get the alias for a virStream,
hence this email
We already support special messages with stream to transport holes for
sparse files.
You could implement new set of stream APIs similarly to virStreamSendHole
which would send the console size.
Then in virFDStreamThread you could issue ioctls on receive of the
message.
Libvirt supports console also for unix-socket backed chardevs. I presume
that would require the QMP command approach instead. That will likely
reuqire more plumbing because on the first glance we don't seem to
have the access to the VM object from the stream code. A callback could
potentially solve that.
TLDR I have the following 2 questions:
- Should the resize handle be implemented in the virHypervisorDriver
directly or in the virStreamDriver?
- Is there a way to get the QMP alias of a chardev from a virStream?
I'd suggest to either implement it as another type of data transported
via the stream or a standalone API.
The client application which uses the stream knows which console it
opened so it does have enough data to issue the proper API call with
parameters identical to how it opened the console.
The approach of adding stream APIs looks a bit cleaner on the first
glance but as I've noted I'm not sure how complicated it will be to
access the VM object if QMP command will be needed.
7:18 p.m.
On Thu, Mar 20, 2025 at 11:05:04AM +0100, Peter Krempa via Devel wrote:
Libvirt supports console also for unix-socket backed chardevs. I presume
that would require the QMP command approach instead. That will likely
reuqire more plumbing because on the first glance we don't seem to
have the access to the VM object from the stream code. A callback could
potentially solve that.
I thought we didn't support UNIX socket chardevs for the
console because in qemuDomainOpenConsole we have
if (chr->source->type != VIR_DOMAIN_CHR_TYPE_PTY) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("character device %1$s is not using a PTY"),
dev_name ? dev_name : NULLSTR(chr->info.alias));
goto cleanup;
}
and yet virChrdevOpen supports it since
commit 66a0664974b6200b9184ea6b4ca9d816f6924f8c
Author: John Eckersberg <jeckersb(a)redhat.com>
Date: Wed Jan 2 10:38:53 2013 -0500
conf: Add unix socket support to virChrdevOpen
so surely our check in qemuDomainOpenConsole has prevented
this ever being used.
>
> TLDR I have the following 2 questions:
> - Should the resize handle be implemented in the virHypervisorDriver
> directly or in the virStreamDriver?
> - Is there a way to get the QMP alias of a chardev from a virStream?
I'd suggest to either implement it as another type of data transported
via the stream or a standalone API.
The client application which uses the stream knows which console it
opened so it does have enough data to issue the proper API call with
parameters identical to how it opened the console.
The approach of adding stream APIs looks a bit cleaner on the first
glance but as I've noted I'm not sure how complicated it will be to
access the VM object if QMP command will be needed.
If we consider that resize is just one, of possibly many, control commands
you might want over a terminal, then we would want a general callback
that can be registered with the stream impl
virStreamControlCommand(virStreamPtr st,
virTypedParameter params,
unsigned int nparams);
and an internal method
typedef int (*virFDStreamControlCommandCallback)(virFDStream *st,
virTypedParameter params,
unsigned int nparams,
void *opaque)
virFDStreamSetInternalControlCommandCallback(virFDStream,
virFDStreamControlCommandCallback cb,
void *opaque,
virFreeCallback freecb)
which could be used to pass the 'virDomainObjPt' as the 'opaque' param
With 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 :|
13
days inactive
13
days old
7 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Maximilian Immanuel Brandtner -
Peter Krempa