Re: [libvirt] [Qemu-devel] Exposing feature deprecation to machine clients

On 8/15/19 10:16 AM, Markus Armbruster wrote: > John Snow <jsnow(a)redhat.com&gt; writes:

>> I asked Markus this not too long ago; do we want to amend the QAPI >> schema specification to allow commands to return with "Warning" strings, >> or "Deprecated" stings to allow in-band deprecation notices for cases >> like these? >> >> example: >> >> { "return": {}, >> "deprecated": True, >> "warning": "Omitting filter-node-name parameter is deprecated, it will >> be required in the future" >> } >> >> There's no "error" key, so this should be recognized as success by >> compatible clients, but they'll definitely see the extra information. > > This is a compatible evolution of the QMP protocol. > >> Part of my motivation is to facilitate a more aggressive deprecation of >> legacy features by ensuring that we are able to rigorously notify users >> through any means that they need to adjust their scripts. > > Yes, we should help libvirt etc. with detecting use of deprecated > features. We discussed this at the KVM Forum 2018 BoF on deprecating > stuff. Minutes: > > Message-ID: <87mur0ls8o.fsf(a)dusky.pond.sub.org&gt; > https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html > > Last item is relevant here. > > Adding deprecation information to QMP's success response belongs to "We > can also pass the buck to the next layer up", next to "emit a QMP > event". > > Let's compare the two, i.e. "deprecation info in success response" > vs. "deprecation event". > > 1. Possible triggers > > Anything we put in the success response should only ever apply to the > (successful) command. So this one's limited to QMP commands. > > A QMP event is not limited to QMP commands. For instance, it could be > emitted for deprecated CLI features (long after the fact, in addition to > human-readable warnings on stderr), or when we detect use of a > deprecated feature only after we sent the success response, say in a > job. Neither use case is particularly convincing. Reporting use of > deprecated CLI in QMP feels like a work-around for the CLI's > machine-unfriendliness. Job-like commands should really check their > arguments upfront. > > 2. Connection to trigger > > Connecting responses to commands is the QMP protocol's responsibility. > Transmitting deprecation information in the response trivially ties it > to the offending command. > > The QMP protocol doesn't tie events to anything. Thus, a deprecation > event needs an event-specific tie to its trigger. > > The obvious way to tie it to a command mirrors how the QMP protocol ties > responses to commands: by command ID. The event either has to be sent > just to the offending monitor (currently, all events are broadcast to > all monitors), or include a suitable monitor ID. > > For non-command triggers, we'd have to invent some other tie. > > 3. Interface complexity > > Tying the event to some arbitrary trigger adds complexity. > > Do we need non-command triggers, and badly enough to justify the > additional complexity? > > 4. Implementation complexity > > Emitting an event could be as simple as > > qapi_event_send_deprecated(qmp_command_id(), > "Omitting 'filter-node-name'"); > > where qmp_command_id() returns the ID of the currently executing > command. Making qmp_command_id() work is up to the QMP core. Simple > enough as long as each QMP command runs to completion before its monitor > starts the next one. > > The event is "fire and forget". There is no warning object propagated > up the call chain into the QMP core like errors objects are. > > "Fire and forget" is ideal for letting arbitrary code decide "this is > deprecated". > > Note the QAPI schema remains untouched. > > Unlike an event, which can be emitted anywhere, the success response > gets built in the QMP core. To have the core add deprecation info to > it, we need to get the info to the core. > > If deprecation info originates in command code, like errors do, we need > to propagate it up the call chain into the QMP core like errors. > > Propagating errors is painful. It has caused massive churn all over the > place. > > I don't think we can hitch deprecation info to the existing error > propagation, since we need to take the success path back to the QMP > core, not an error path. > > Propagating a second object for warnings... thanks, but no thanks. > Probably the best argument against it. Fire-and-forget avoids the problem. Events might work just fine, but the "tie" bit seems like a yak in need of a shave. > The QMP core could provide a function for recording deprecation info for > the currently executing QMP command. This is how we used to record > errors in QMP commands, until Anthony rammed through what we have now. > The commit messages (e.g. d5ec4f27c38) provide no justification. I > dimly recall adamant (oral?) claims that recording errors in the Monitor > object cannot work for us. > > I smell a swamp. > > Can we avoid plumbing deprecation info from command code to QMP core? > Only if the QMP core itself can recognize deprecated interfaces. I > believe it can for the cases we can expose in introspecion. Let me > explain. > > Kevin recently added "features" to the QAPI schema language. The > implementation is incomplete, but that's detail. The idea is to tack a > "deprecated" feature to deprecated stuff in the QAPI schema. > That's a good idea too; but the semantics of exactly *what* was deprecated may be hard to capture. > Commands and arguments need to support features for that. > Implementation should be relatively straightforward. > > Deprecating an argument's optionalness may require a > "optional-deprecated" feature. I've seen more elegant designs, but I've > also seen plenty of uglier ones. > > Note that features are tied to schema syntax. To express semantically > conditional deprecation like "if you specify argument FOO, then not > specifying argument BAR is deprecated", we'd have to add a language for > these conditions. Uh, not now, maybe never. > > The primary use of having deprecation defined in the QAPI schema is > introspection. The BoF minutes mention this, too. > > A secondary use could be detecting use of deprecated features right in > the QMP core. No need for ad hoc code in commands, no need for plumbing > information from there to the QMP core. > > I'd like to pursue this idea, then see how well it suits our deprecation > needs. > I should clearly remember to talk to you before thinking about QMP in public, because you've thought about it much more.