* Mark Cave-Ayland (mark.cave-ayl...@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 / dgilb...@redhat.com / Manchester, UK
