Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Jason A. Donenfeld]

On Mon, Oct 24, 2022 at 7:40 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Mon, 24 Oct 2022 at 14:20, Markus Armbruster <armbru@redhat.com> wrote:
> >>
> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >>
> >> > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> 
> >> > wrote:
> >> >>
> >> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >> >> > Markus: if we add a new value to the ShutdownCause enumeration,
> >> >> > how annoying is it if we decide we don't want it later? I guess
> >> >> > we can just leave it in the enum unused... (In this case we're
> >> >> > using it for purely internal purposes and it won't ever actually
> >> >> > wind up in any QMP events.)
> >> >>
> >> >> Deleting enumeration values is a compatibility issue only if the value
> >> >> is usable in QMP input.
> >> >>
> >> >> "Purely internal" means it cannot occur in QMP output, and any attempt
> >> >> to use it in input fails.  Aside: feels a bit fragile.
> >> >
> >> > In this case there are as far as I can see no QMP input commands
> >> > which use the enum at all -- it's only used in events, which are
> >> > always output, I think.
> >>
> >> They are.
> >>
> >> Ascertaining "not used in QMP input" is pretty easy, and "not used in
> >> CLI" isn't hard.  My point is that uses could creep in later.  This is
> >> what makes "purely internal" fragile.
> >
> > True. But otoh if there's a meaningful use of the enum constant in
> > input in future we'll want to keep it around anyway.
> >
> > I guess what I'm asking is: do you specifically want this patch
> > done some other way, or to require that "mark some values as
> > internal-only" feature in the QAPI generator, or are you OK with
> > it as-is?  QMP/QAPI is your area, so your call...
>
> QAPI was designed to specify QMP.  We pretty soon discovered that QAPI
> types can be useful elsewhere, and added some to the schema without
> marking them.  Introspection doesn't show these.  Generated
> documentation does.  Known shortcoming of the doc generator.
>
> This case differs in that we're adding an internal-only member to a type
> that is used by QMP.  Both introspection and documentation will show it.
>
> Interface introspection and (especially!) documentation showing stuff
> that doesn't exist in the interface is certainly less than ideal.
> However, I don't want to hold this patch hostage to getting QAPI
> shortcomings addressed.
>
> Instead, I want QMP documentation to make clear that this value cannot
> actually occur.
>
> Fair?

Made mention of it in v4, just posted.

20221025004327.568476-2-Jason@zx2c4.com/#Z31qapi:run-state.json">https://lore.kernel.org/all/20221025004327.568476-2-Jason@zx2c4.com/#Z31qapi:run-state.json