Peter Maydell <peter.mayd...@linaro.org> writes: > On Mon, 24 Oct 2022 at 14:20, Markus Armbruster <arm...@redhat.com> wrote: >> >> Peter Maydell <peter.mayd...@linaro.org> writes: >> >> > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <arm...@redhat.com> wrote: >> >> >> >> Peter Maydell <peter.mayd...@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?
