All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Christian Brauner <brauner@kernel.org>
Cc: qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
	"Eric Blake" <eblake@redhat.com>,
	"Fabiano Rosas" <farosas@suse.de>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Thomas Huth" <th.huth+qemu@posteo.eu>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: Re: [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support
Date: Fri, 10 Apr 2026 17:25:42 +0100	[thread overview]
Message-ID: <adkkhkMdJxHY8rh-@redhat.com> (raw)
In-Reply-To: <20260410-kupfer-rosten-9d08af140e1a@brauner>

On Fri, Apr 10, 2026 at 09:36:02AM +0200, Christian Brauner wrote:
> On Thu, Apr 09, 2026 at 08:43:40PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Apr 09, 2026 at 09:18:17AM +0200, Christian Brauner wrote:
> > > QEMU supports runtime hotplug for chardevs, devices, block backends,
> > > and netdevs. Monitors are the only major subsystem that lacks this --
> > > all QMP monitors must be configured at launch via -mon or -qmp CLI
> > > options.
> > > 
> > > This series adds monitor-add, monitor-remove, and query-monitors QMP
> > > commands so that management tools can create independent QMP sessions
> > > on demand at runtime.
> > 
> > There's nothing inherently wrong with your proposal. It is following
> > the standard historical design practice we've taken for enabling
> > hotplug/unplug of all the other backend types. Conceptually though
> > our historical practice is more of an accident of evolution rather
> > than an ideal state.
> > 
> > By this I mean we have a general purpose object framework
> > internally and commands '-object', and 'object_add' / 'object_del'
> > for hotplug/unplug in QMP and HMP. Conceptually these could handle
> > all of our objects, devices, netdev chardevs, monitors, etc, etc,
> > removing the need for the specialized add/remove QMP commands for
> > each backend type, and the specialized CLI argsf. 
> 
> If you actually plan on converting old types you're now left with two
> ways of adding objects. And if you plan on ultimately deprecating the
> old format it's going to be very annoying to fade those out.

We have two ways to dealing with this.

A formal deprecation & removal process which lets us fully
kill off the old way of doing things across 3 releases (1
year).

  https://www.qemu.org/docs/master/about/deprecated.html

We use that alot, but for something as fundamental as the
monitor I don't think we would take that route as the
impact on users is too great. Instead we would follow the
approach of preserving the existing CLI arg and internally
rewriting it to the new syntax. So we only have one (new)
code path internally, except for a thin shim as the CLI
layer for conversion which has minimal long term burden on
maintainers.

Eventually we're likely to have a clean compat back with
new binaries, where we can drop all the legacy cruft from
the CLI.

> > The blocker has been lack of time to convert everything/anything,
> > since while the conversions are not especially difficult they are
> > certainly time consuming due to the sheer number of types that
> > exist in many cases.
> > 
> > 
> > I'm thinking the monitor, however, is a decent opportunity to
> > "do the right thing" from a design POV. We only have three classes
> > needed in QOM, the monitor generic base class, a HMP subclass and
> > a QMP subclass.
> > 
> > If we convert QMP/HMP into QOM objects, we could then use the
> > pre-existing object_add/object_del commands in both HMP and QMP.
> > 
> > I'm not asking you to do that conversion work though. I've
> > been doing some experimentation myself today to test the
> > viability of this idea and it looks promising.
> 
> How difficult is this to do? I've had bad experiences with projects
> promising to do it all very differently and taking the patch away from
> me and then not following through or like 2 years later or the feature
> just vanished into nothing. I'm not saying this is the case here but I'm
> a bit weary of not having any way to move this forward myself other than
> "any news on this?" mails. It's frustrating for both sides.
> 
> So if this is doable just give me what you got and let me see whether I
> can take it over the finish line.

I've just posted a series which gets it as far as being able to
use -object and object_add.  I don't want your use case to be
delayed indefinitely - whichever design & impl, I'd want something
to be mergeable for the next schedule release. 

> Also, I really dislike spending a lot of time understanding something
> and it all working and then being told "actually, let me do this". I
> avoid this like the plague doing this to developers in the kernel.

I'm sorry I didn't notice your series when it was the v1 posting,
as I would have liked to raise this idea earlier. It is delicate
balancing act between being accessible to new contributions, vs
trying to align with longer term design preferences.

> > Given we've got an entire 4 month dev cycle before these
> > proposed monitor-add/remove commands could get into a QEMU
> > release, I think it is worth trialling the QOM conversion
> > for a short while.
> 
> I'm not sure what your governance model is so I'm just going to ask: Is
> this just a review or is this a project level decision?

This is just my design opinion as one of many QEMU maintainers,
albeit informed by many historical discussions on this topic.

Ultimately nothing is final until a patch series is accepted
and queued by a maintainer.

Hopefully others will give some input - I'm especialy looking
at Markus (CCd) for this as our QAPI & QMP design expert/maintainer.

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 :|



      reply	other threads:[~2026-04-10 16:26 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-09  7:18 [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Christian Brauner
2026-04-09  7:18 ` [PATCH v4 1/5] monitor: store monitor id in Monitor struct Christian Brauner
2026-04-09  7:18 ` [PATCH v4 2/5] monitor/qmp: add infrastructure for safe dynamic monitor removal Christian Brauner
2026-04-09  7:18 ` [PATCH v4 3/5] qapi: add monitor-add, monitor-remove, query-monitors commands Christian Brauner
2026-04-09  7:18 ` [PATCH v4 4/5] tests/qtest: add tests for dynamic monitor add/remove Christian Brauner
2026-04-09 13:01   ` Fabiano Rosas
2026-04-09  7:18 ` [PATCH v4 5/5] tests/functional: add e2e test for dynamic QMP monitor hotplug Christian Brauner
2026-04-09 19:43 ` [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Daniel P. Berrangé
2026-04-10  7:36   ` Christian Brauner
2026-04-10 16:25     ` Daniel P. Berrangé [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=adkkhkMdJxHY8rh-@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=brauner@kernel.org \
    --cc=eblake@redhat.com \
    --cc=farosas@suse.de \
    --cc=lvivier@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=th.huth+qemu@posteo.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.