[Qemu-devel] [PATCH for-2.5 0/4] Expose ErrorClass through introspection
I noticed that introspection was not documenting either qmp_capabilities nor the ErrorClass enum. I think this is worth fixing for 2.5 when introspection is brand new, so that if we later extend the ErrorClass enum or add future capability negotiation (and in particular if such additions get backported in downstream builds), a client will be able to use introspection to learn whether the new features are supported, regardless of the qemu version. Note that this also adds qmp_capabilities to 'query-commands'. Yes, this is borderline, and you may decide that it doesn't deserve to be called a bug and should wait for 2.6. Eric Blake (3): qapi: Add type.is_empty() helper qapi: Fix command with named empty argument type qapi: Expose ErrorClass through introspection Marc-André Lureau (1): monitor: use qapi for qmp_capabilities command docs/qmp-spec.txt | 16 monitor.c | 8 ++-- qapi-schema.json| 32 qmp-commands.hx | 4 ++-- scripts/qapi-commands.py| 6 +++--- scripts/qapi-event.py | 6 +++--- scripts/qapi-types.py | 2 +- scripts/qapi.py | 3 +++ tests/qapi-schema/qapi-schema-test.json | 2 ++ tests/qapi-schema/qapi-schema-test.out | 2 ++ tests/test-qmp-commands.c | 5 + 11 files changed, 75 insertions(+), 11 deletions(-) -- 2.4.3
Re: [Qemu-devel] [PATCH for-2.5 0/4] Expose ErrorClass through introspection
Eric Blake writes: > I noticed that introspection was not documenting either > qmp_capabilities nor the ErrorClass enum. I think this is worth > fixing for 2.5 when introspection is brand new, so that if we later > extend the ErrorClass enum or add future capability negotiation (and > in particular if such additions get backported in downstream builds), > a client will be able to use introspection to learn whether the new > features are supported, regardless of the qemu version. > > Note that this also adds qmp_capabilities to 'query-commands'. > > Yes, this is borderline, and you may decide that it doesn't deserve > to be called a bug and should wait for 2.6. Before I discuss the error class proposal in more detail, a preliminary remark: error classes are a leftover from the days of "rich" error objects, and any new use of an error class other than ERROR_CLASS_GENERIC_ERROR is immediately suspect. I'm not saying that we won't add such uses anymore, just that there's a significant bar to overcome, which we haven't for quite some time now. I think I could be persuaded that a client might be able to use knowledge on what error classes a specific command can produce. Of course, presence of an error class doesn't tell what actual error conditions map to it, i.e. the client still needs to make assumptions on the meaning of error classes. Humans make those, too, but humans can read the contract in the comments. The value of a global list of error classes seems even more dubious, though. Existence of an error class by itself guarantees nothing. How would a client use the information? Assume that existence of a class implies a certain command uses it in a certain way? That's an even bigger jump than above. I guess using the presence or absence of an error class as a witness for a certain feature or behavior could work. Seems practical when the written contract guarantees the connection between the two (de jure connection), or the commit that introduces the feature or behavior also adds or removes the error class (de facto connecton). This applies both to a global list of error classes and to per-command lists. Example 1: MigrationExpected Before commit 1e99814 "qmp: handle stop/cont in INMIGRATE state", cont could fail with error MigrationExpected. Libvirt dealt with it by trying again. Commit 1e99814 made cont just work, and dropped the error class. The error class was never used for anything else. Exposing a global list of error classes like your patch does would permit detecting the presence of this commit. However, detecting it is pointless: to deal with its absence, you have to loop on MigrationExpected anyway, and that code works just fine with and without the commit. Example 2: Unwanted DeviceNotFound dropped During the 2.3 development cycle, a few unwanted uses of DeviceNotFound crept in. Commits 5b347c5, f3cf80e, 6ec46ad backed them out before the release. For the sake of argument, ignore the fact that these unwanted DeviceNotFound never made it to a release, and if they had, we would've left them in, because taking them out would've been an ABI break. A client could use a per-command error class list to detect them, but not a global error class list. But what could it do with the information? If DeviceNotFound is there, the client can handle it specially, if not, it can't, and I can't see how knowing it would make a difference. Example 3 (hypothetical): New error class to support a client's need Say we discover that a client wants to handle a certain error specially, and we decide to make that possible by changing its error class from GenericError to something specific to that error. Hypothetical, because changing an error's error class is an ABI break, and we normally don't do that. The client could then refrain from using the command in certain ways unless it uses the specific error class for this error. Detecting that by finding the error class in the global list of error classes works only if the error class is new, and only works as long as it doesn't get used for other things that then get backported without the original use. Detecting it by finding the error class in the command's list of error classes would be less brittle. Example 4: Use per-command error list to catch unwanted error class If we declare a command's errors, we can detect undeclared errors at run time. This should help catching unwanted ones early (see example 2). Having to declare error classes may facilitate proper review of new uses of funky error classes. None of these examples is a particularly convincing use case. Can you think of better ones? Finally, what happens if error class introspection misses 2.5 and makes a later version? If we add a global error class list like this patch does, a client has to assume that anything th
Re: [Qemu-devel] [PATCH for-2.5 0/4] Expose ErrorClass through introspection
Eric Blake writes: > I noticed that introspection was not documenting either > qmp_capabilities nor the ErrorClass enum. I think this is worth > fixing for 2.5 when introspection is brand new, so that if we later > extend the ErrorClass enum or add future capability negotiation (and > in particular if such additions get backported in downstream builds), > a client will be able to use introspection to learn whether the new > features are supported, regardless of the qemu version. > > Note that this also adds qmp_capabilities to 'query-commands'. > > Yes, this is borderline, and you may decide that it doesn't deserve > to be called a bug and should wait for 2.6. Two entirely separate issues: introspecting qmp_capabilities and introspecting error classes. This reply is about the former. I'll discuss the latter in a separate message. Introspecting qmp_capabilities with query-qmp-schema is kind of academic, because by the time you can query-qmp-schema, you're already done using qmp_capabilities. That's why docs/qmp-spec.txt nails down qmp_capabilities, unlike other commands. Unfortunately, it's a bit screwy. Let's have a closer look. Right after connect, the server sends a greeting that includes a list of available capabilities (docs/qmp-spec.txt section 2.2 Server Greeting), currently none. The only valid command then is qmp_capabilities (section 4. Capabilities Negotiation). The command is meant to let clients enable capabilities from the greeting, but we've neglected to specify how. Fortunately, QMP rejects all arguments, so we can still fix that by adding optional arguments, either a list of capabilities to enable, or a (boolean) argument for each capability. My point is: unlike other commands, qmp_capabilities is baked into the protocol. Introspection is useful to examine *variable* aspects. With the hole in the protocol specification plugged, the only variable aspect of qmp_capabilities are the capabilities, and the practical way to introspect those is the server greeting. Qapifying qmp_capabilities can make sense regardless. But why does it need to be rushed into 2.5 then?
Re: [Qemu-devel] [PATCH for-2.5 0/4] Expose ErrorClass through introspection
On 11/12/2015 03:48 AM, Markus Armbruster wrote: > Eric Blake writes: > >> I noticed that introspection was not documenting either >> qmp_capabilities nor the ErrorClass enum. I think this is worth >> fixing for 2.5 when introspection is brand new, so that if we later >> extend the ErrorClass enum or add future capability negotiation (and >> in particular if such additions get backported in downstream builds), >> a client will be able to use introspection to learn whether the new >> features are supported, regardless of the qemu version. >> >> Note that this also adds qmp_capabilities to 'query-commands'. >> >> Yes, this is borderline, and you may decide that it doesn't deserve >> to be called a bug and should wait for 2.6. Or in other words, I posted this series precisely for this conversation. The mere fact that you have questions means that it is NOT appropriate for 2.5 this late in the game. But to answer you... > > Two entirely separate issues: introspecting qmp_capabilities and > introspecting error classes. This reply is about the former. I'll > discuss the latter in a separate message. > > Introspecting qmp_capabilities with query-qmp-schema is kind of > academic, because by the time you can query-qmp-schema, you're already > done using qmp_capabilities. That's why docs/qmp-spec.txt nails down > qmp_capabilities, unlike other commands. Unfortunately, it's a bit > screwy. Let's have a closer look. > > Right after connect, the server sends a greeting that includes a list of > available capabilities (docs/qmp-spec.txt section 2.2 Server Greeting), > currently none. > > The only valid command then is qmp_capabilities (section 4. Capabilities > Negotiation). The command is meant to let clients enable capabilities > from the greeting, but we've neglected to specify how. Fortunately, QMP > rejects all arguments, so we can still fix that by adding optional > arguments, either a list of capabilities to enable, or a (boolean) > argument for each capability. > > My point is: unlike other commands, qmp_capabilities is baked into the > protocol. Introspection is useful to examine *variable* aspects. With > the hole in the protocol specification plugged, the only variable aspect > of qmp_capabilities are the capabilities, and the practical way to > introspect those is the server greeting. Good point - if we ever add a capability, a client that knows how to request the capability can easily learn whether the server will honor the request by reading the server's advertisements. > > Qapifying qmp_capabilities can make sense regardless. But why does it > need to be rushed into 2.5 then? No need for the rush after all. You are correct that qapi-fying qmp_capabilities will NOT help the client learn anything it couldn't have already learned from the server greeting. And if we don't rush qmp_capabilities into 2.5 (patch 3), then we also don't need to rush in a fix for explicitly empty types (patch 2) or a way to detect empty types (patch 1). As for patch 4 - we could still expose ErrorClass, via some means other than qmp_capabilities, if desired; but that's a discussion for your other mail. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for-2.5 0/4] Expose ErrorClass through introspection
On 11/12/2015 03:48 AM, Markus Armbruster wrote: > Eric Blake writes: > >> I noticed that introspection was not documenting either >> qmp_capabilities nor the ErrorClass enum. I think this is worth >> fixing for 2.5 when introspection is brand new, so that if we later >> extend the ErrorClass enum or add future capability negotiation (and >> in particular if such additions get backported in downstream builds), >> a client will be able to use introspection to learn whether the new >> features are supported, regardless of the qemu version. >> >> Note that this also adds qmp_capabilities to 'query-commands'. >> >> Yes, this is borderline, and you may decide that it doesn't deserve >> to be called a bug and should wait for 2.6. > > Before I discuss the error class proposal in more detail, a preliminary > remark: error classes are a leftover from the days of "rich" error > objects, and any new use of an error class other than > ERROR_CLASS_GENERIC_ERROR is immediately suspect. I'm not saying that > we won't add such uses anymore, just that there's a significant bar to > overcome, which we haven't for quite some time now. > > I think I could be persuaded that a client might be able to use > knowledge on what error classes a specific command can produce. Of > course, presence of an error class doesn't tell what actual error > conditions map to it, i.e. the client still needs to make assumptions on > the meaning of error classes. Humans make those, too, but humans can > read the contract in the comments. Indeed - knowing the global set of possible errors is NOT the same as knowing the set of command-specific errors; and the latter is more likely to be useful. But exposing the latter would require per-command documentation in the .json files, and is certainly not doable in time for 2.5. > > The value of a global list of error classes seems even more dubious, > though. Existence of an error class by itself guarantees nothing. How > would a client use the information? Assume that existence of a class > implies a certain command uses it in a certain way? That's an even > bigger jump than above. > > I guess using the presence or absence of an error class as a witness for > a certain feature or behavior could work. Seems practical when the > written contract guarantees the connection between the two (de jure > connection), or the commit that introduces the feature or behavior also > adds or removes the error class (de facto connecton). This applies both > to a global list of error classes and to per-command lists. > [snip good examples] > > None of these examples is a particularly convincing use case. Can you > think of better ones? No. > > Finally, what happens if error class introspection misses 2.5 and makes > a later version? > It would not be the first time that libvirt has been coded along the lines of "if the information is available, trust it; if not, go by a hard-coded list that matched the upstream state prior to the information being made available". It's a bit more work on clients, but not insurmountable. > If we add a global error class list like this patch does, a client has > to assume that anything that doesn't has one has the usual error > classes: GenericError, CommandNotFound, DeviceEncrypted, > DeviceNotActive, DeviceNotFound, KVMMissingCap[*]. Whether "doesn't has > one" is "doesn't has one in query-qmp-schema" or "doesn't even have > query-qmp-schema" doesn't matter. Correct - a client can easily hard-code the correct information for all older qemu (at worse, it will miss an error case that has been backported - but in all likelihood, if backporting an error was that important, downstream could also backport the way to introspect for it). > > If we add per-command error class lists, it's the same, only the > assumptions become a bit more involved. > > Of course, the fewer discernible versions of introspection we have, the > better. If we can figure out what we need in time to get it into the > very first version, great! But it's awfully late for 2.5, and I'm not > at all sure we know what we need. Perhaps we can find out quickly, but > let's not rush the job. > > > [*] Ancient versions may also have MigrationExpected (see above), but > backporting introspection that far, but not backporting the fix for the > migration stop/cont race would be insane. Therefore, I agree with you that there is no rush to get this into 2.5. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
