[re-added list], please keep conversation on the list by using
'reply-all'
On Fri, May 15, 2026 at 13:03:42 +0200, Lucas Kornicki wrote:
> Thanks Peter
>
> On 5/15/26 10:59, Peter Krempa wrote:
> > On Wed, May 13, 2026 at 12:20:31 +0200, Lucas Kornicki wrote:
> > > Add a generic domain event that fires when libvirt detects a state
> > > change on any virtio-serial channel of a domain (connected /
> > > disconnected). The existing VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE
> > > event is restricted to the QEMU guest agent channel
> > > ("org.qemu.guest_agent.0"), making it impossible for management
> > > applications to observe lifecycle transitions of other channels
> > > (custom guest agents, SPICE, etc.) without polling the domain XML
> > > status file.
> > >
> > > The new event is emitted for every virtio-serial channel, including
> > > the guest agent channel, and carries the affected channels name.
> > >
> > > The hypervisor must support virtio-serial port state notifications
> > > (e.g. QEMU's VSERPORT_CHANGE event) for the event to be delivered.
> > >
> > > Forward-port of the v2 series originally posted to libvirt-devel in
> > > 2016 by Matt Broadstone<[email protected]>.
> > I'd expect that authorship is preserved if you forward port the patch.
> > Did you change it substantially? Did the original author include a
> > 'signed-off-by' tag?
>
> There was no sign off.
> I started working on this before I became aware of the original patch,
> so I just hand picked most of the boilerplate that comes along with a new
> event.
Ah I see. IMO your authorship is okay then. I'd perhaps suggest
rewording it to "Some parts were lifted from original post to ..."
> If you want I can rewrite the author to Matt but what about the sign-off?
No for the authorship and definitely no for sign-off, we can't conjure
that one for someone without
>
> > > Signed-off-by: Lucas Kornicki<[email protected]>
> > > ---
> > > examples/c/misc/event-test.c | 57 +++++++++++++++++
> > > include/libvirt/libvirt-domain.h | 65 ++++++++++++++++++++
> > > src/conf/domain_event.c | 95 +++++++++++++++++++++++++++++
> > > src/conf/domain_event.h | 12 ++++
> > > src/libvirt_private.syms | 2 +
> > > src/qemu/qemu_driver.c | 8 +++
> > > src/qemu/qemu_process.c | 17 ++++--
> > Please seaprate the implementation in the qemu driver from teh other
> > changes which add the public side of the event.
> >
> By public you mean the remote / virsh parts below and not the public
> headers?
> Those would be hard to do without.
No, virsh and public API needs to go together because there are compile
time checks that enforce them. The tree still has to compile after each
patch.
>
> Also, the conflicting commit (bbc7d1a2a0d58788520a97a2be439c94a0153d3c)
> seems to combine those parts the same way I am doing here.
That is correct. What I meant is splitting off the part where you make
the qemu driver emit the events. Thus IIRC everything this patch does
under src/qemu.
>
> > > src/remote/remote_daemon_dispatch.c | 34 +++++++++++
> > > src/remote/remote_driver.c | 34 +++++++++++
> > > src/remote/remote_protocol.x | 16 ++++-
> > > src/remote_protocol-structs | 8 +++
> > > tools/virsh-domain-event.c | 35 +++++++++++
> > > 12 files changed, 377 insertions(+), 6 deletions(-)
> > [...]
One thing I likely forgot to mention is that you should also add compile
time check which forces that the _LAST parts of the both events mirrot
themselves for now so that if anyone adds another field to either of
them they stay in sync.
