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

[Markus Armbruster <armbru@redhat.com>]

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?