Re: [PATCH 08/11] mos6522: add "info via" HMP command for debugging

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-devel@nongnu.org, "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	Laurent@vivier.eu
Subject: Re: [PATCH 08/11] mos6522: add "info via" HMP command for debugging
Date: Tue, 8 Feb 2022 13:03:52 +0000	[thread overview]
Message-ID: <YgJqOCmbgMt3oMl1@work-vm> (raw)
In-Reply-To: <60d34033-fd73-249d-97d2-886ece0ef190@ilande.co.uk>

* Mark Cave-Ayland (mark.cave-ayland@ilande.co.uk) wrote:
> On 08/02/2022 11:52, Daniel P. Berrangé wrote:
> 
> (cut)
> 
> > > > The proposed device-specific command uses a mechanism originally made
> > > > for modules instead (more on that below).
> > > > 
> > > > I think we should make up our minds which way we want device-specific
> > > > commands done, then do *all* of them that way.
> > > 
> > > I think device specific commands make sense, but I think it would
> > > probably be better if we had an 'info dev $name' and that a method on
> > > the device rather than registering each one separately.
> > > I'd assume that this would be a QMP level thing that got unwrapped at
> > > HMP.
> > > 
> > > But that's not a problem for this contribution; someone else can figure
> > > that out later.
> > 
> > Actually I think this would solve a problem with this contribution.
> > This patch implements a QMP command but never registers it, so it
> > isn't actually accessible via QMP. It only registers the HMP wrapper
> > which rather defeats the point of doing it via the QMP HumanReadableText
> > approach.
> > 
> > I'm guessing this was done because we don't have ability to dynamically
> > register QMP commands at runtime.  I don't know how hard/easy it is
> > to address this, and perhaps we don't even want to.  It was a problem
> > for me when previously converting HMP info commands to QMP and I
> > didn't get a solution, so didn't convert anything where the command
> > impl was dynamically registered at runtime. This basically excluded
> > converting devices that have been split into loadable modules.
> > 
> > If we had a general  'info dev-debug' (HMP) and  'x-query-dev-debug'
> > commands, then we could side-step the QMP limitation, as both thue
> > HMP and QMP comamnds could be statically registered. The devices
> > then only need to register  a callback at runtime which should be
> > easier to deal with.
> 
> That could work, or even just a "debug via" without using "info" for brevity.
> 
> You could even add the callback to DeviceClass so that the registration
> takes place as part of class_init() using a function such as
> device_class_register_debug("name", callback).

Yes, that was what I was imagining

Dave

> 
> ATB,
> 
> Mark.
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK