Re: [Qemu-devel] [PATCH 2/7] runstate: Add runstate store

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2961 bytes --]

On 10/22/2014 05:40 AM, Markus Armbruster wrote:
>> I think the question here really comes from RunState being an enum defined
>> in qapi-schema.json; so we could use that directly in the migration stream
>> if we were guaranteed that the encoding of that enum wasn't going to change.
>> Does qapi make any guarantees about the enum encoding?
> 
> qapi-code-gen.txt in master is silent on the matter:
> 
>     === Enumeration types ===
> 
>     An enumeration type is a dictionary containing a single key whose value is a
>     list of strings.  An example enumeration is:
> 
>      { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
> 
> Eric's "drop qapi nested structs" series improves the spec a lot.

And I still need to find time to supply the next revision of that series...

>     The enumeration values are passed as strings over the QMP protocol,
>     but are encoded as C enum integral values in generated code.  While
>     the C code starts numbering at 0, it is better to use explicit
>     comparisons to enum values than implicit comparisons to 0; the C code
>     will also include a generated enum member ending in _MAX for tracking
>     the size of the enum, useful when using common functions for
>     converting between strings and enum values.  Since the wire format
>     always passes by name, it is acceptable to reorder or add new
>     enumeration members in any location without breaking QMP clients;
>     however, removing enum values would break compatibility.  For any
>     complex type that has a field that will only contain a finite set of
>     string values, using an enum type for that field is better than
>     open-coding the field to be type 'str'.
> 
> I figure relying on the QAPI code generator assigning values 0, 1, 2 in
> order is fair.  If you want to guarantee that more explicitly in the
> spec, patch welcome (on top of Eric's, please).

Yes, the generator guarantees that the order that enums are listed in a
.json file will be the 0, 1, 2, ... values assigned in the C code.  I'll
add that in my revision.  You cannot rely on the C values being stable
unless the .json file takes care to not reorder names for the enum
members; if we have enums where the C code is going to rely on a
particular ordering, we probably ought to document that in the .json
file (since MOST enums are designed for the C code to use them solely by
symbolic name and not by specific value).

> 
> A test or build-time assertion checking the values don't change would be
> prudent.  A comment next to the enum definition warning against
> incompatible changes wouldn't hurt.

Indeed, if you are going to rely on something that the generator happens
to give but does not guarantee, then additional build-time checking and
documentation is called for.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]