Re: [PATCH v4 00/35] monitor: turn QMP and HMP into QOM objects

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jun 23, 2026 at 03:54:54PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Jun 23, 2026 at 3:53 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Tue, Jun 23, 2026 at 12:38:10PM +0100, Daniel P. Berrangé wrote:
> > > On Tue, Jun 23, 2026 at 01:03:46PM +0200, Peter Krempa wrote:
> > > > On Tue, Jun 23, 2026 at 10:55:16 +0100, Daniel P. Berrangé via Devel wrote:
> > > > > Conceptually -object and object_add/object_del should be sufficient
> > > > > for essentially all QEMU configuration....if only we ported all our
> > > > > internal custom backends/devices/etc to QOM. That is of course a big
> > > > > job which is why it hasn't happened.
> > > > >
> > > > > This series started with the premise that the monitor is one of the
> > > > > easier areas to convert since we have no more than three classes,
> > > > > a common base, and QMP and HMP subclasses[1]. So why not give it a
> > > > > go and thus unlock the ability to dynamically create/delete monitors
> > > > > in QMP/HMP.
> > > > >
> > > > > This series does the conversion in a great many small steps to better
> > > > > understand the implications at each stage.
> > > > >
> > > > > The high level outcome of this series is
> > > > >
> > > > >  * HMP and QMP monitors are QOM objects, 'monitor-hmp' and
> > > > >    'monitor-qmp' respectively
> > > > >
> > > > >  * Both can be cold plugged and hot plugged. QMP only, can
> > > > >    also be hot unplugged.
> > > > >
> > > > >  * '-mon' is obsolete, deprecated and replaced by '-object',
> > > > >    but -monitor, -qmp and kept  as high level syntax sugar
> > > >
> > > > I was giving this a spin by implementing libvirt support for it. The
> > > > basic usage seems to work well but I've encoutered a regression in
> > > > behaviour which happens both when the QMP monitor is instantiated using
> > > > the new syntax but also the old one
> > > > ( -mon chardev=charmonitor,id=monitor,mode=control)
> > > >
> > > > When libvirt wants to re-connect to a qemu running with these patches it
> > > > re-issues a 'qmp_capabilities' command after connecting.
> > > >
> > > > With these patches qemu does not respond to that command even when the
> > > > old syntax is used. This makes libvirt stuck when re-connecting to VMs.
> > >
> > > Oh that's odd, thanks for the warning. I'll investigate this.
> >
> > /face-palm - a misplaced a line in the auto-delete code, and did not
> > have test coverage of re-opening a monitor. Try testing libvirt with
> > this additional change on top
> >
> > diff --git a/monitor/qmp.c b/monitor/qmp.c
> > index f6b7fe65bb..10d651ca5c 100644
> > --- a/monitor/qmp.c
> > +++ b/monitor/qmp.c
> > @@ -613,7 +613,6 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
> >          qobject_unref(data);
> >          break;
> >      case CHR_EVENT_CLOSED:
> > -        mon->delete_pending = true;
> >          /*
> >           * Note: this is only useful when the output of the chardev
> >           * backend is still open.  For example, when the backend is
> > @@ -629,6 +628,7 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
> >          case MONITOR_QMP_CLOSE_ACTION_NONE:
> >              break; /* nada */
> >          case MONITOR_QMP_CLOSE_ACTION_DELETE:
> > +            mon->delete_pending = true;
> >              /*
> >               * Do NOT run in the AIO context associated with the
> >               * monitor. We need to run in the default AIO context
> >
> >
> 
> Good, I just asked the same in my review :)
> 
> btw, you didn't collect my r-b from v3.

Oh sorry, I will get those.


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|