Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig

[Eduardo Habkost <ehabkost@redhat.com>]

On Wed, Jun 13, 2018 at 02:53:37PM +0100, Daniel P. Berrangé wrote:
> On Wed, Jun 13, 2018 at 10:47:45AM -0300, Eduardo Habkost wrote:
> > On Tue, Jun 12, 2018 at 09:49:14AM +0100, Dr. David Alan Gilbert wrote:
> > [...]
> > > > > People have been trying to add qom-get etc for quite a while (I tried a
> > > > > couple of years ago); it gets stuck in type display issues.  I've not
> > > > > directly seen a need for those other variants, but qom-get is something
> > > > > I'd love to have, still that's a job for another patch.
> > > > 
> > > > Yes.
> > > > 
> > > > > 'info qom-tree' is very very useful when debugging qemu to see what the
> > > > > basic state we're building is; it's primarily for debugging.
> > > > 
> > > > I'm not at all opposed to enabling qom-tree, but I want its QMP building
> > > > blocks enabled as well then.  I think enabling their HMP buddies as well
> > > > would only make sense.
> > > 
> > > Hmm; OK, enabling qom-list, qom-get, qom-set, qom-list-types,
> > > qom-list-properties for qmp.
> > > qom-list and qom-set enabled in HMP.
> > 
> > I would prefer to avoid enabling qom-set in preconfig mode unless
> > we have a good reason for that.  I don't trust all existing
> > property setters to not crash or break if the machine is not
> > initialized yet.
> 
> If we're in early startup, the impact of a crash is pretty minor - QEMU
> will simply exit, which is not significantly differnt from if a setter
> handled it "correctly" by setting an Error **errp. A mgmt app that uses
> this and finds a setter which crashes will detect the problem pretty
> quickly & report bugs.
> 
> QEMU is complex enough that we're unlikely to ever find broken setters
> by code inspection. So if we don't allow it in preconfig mode, then
> I doubt we'll ever find & fix the problems.
> 
> So, IMHO, we should allow qom-set and just fix problems as they come
> to light, as we would for any other part of QEMU which has plenty of
> scope for crashers in rarely tested codepaths.

Good points.

I'm still worried about making qom-set confused with a supported
configuration API, and have applications starting to rely on it.

But I guess this problem could be solved by properly documenting
qom-set as a debugging aid, not a supported API.

-- 
Eduardo