Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Alex Williamson]

On Wed, 05 Dec 2018 10:09:38 +0100
Markus Armbruster <address@hidden> wrote:

> Alex Williamson <address@hidden> writes:
> 
> > Create properties to be able to define speeds and widths for PCIe
> > links.  The only tricky bit here is that our get and set callbacks
> > translate from the fixed QAPI automagic enums to those we define
> > in PCI code to represent the actual register segment value.  
> 
> QAPI can only generate enumerations with values 0, 1, 2, ...  You want
> different enumeration values, namely the actual register values.  You
> still want QAPI to get its standard mapping to and from strings.
> 
> This patch's solution is to define a non-QAPI enumeration type with the
> values you want [PATCH 1/9], then map between the enumerations in the
> PropertyInfo methods.  Works.
> 
> You could instead use the encoding chosen by QAPI for the properties,
> and map it to the register values on use.  Differently ugly.  Might be
> simpler.  Your choice to make.

As we discussed offline, I personally don't like using the magic macros
that QAPI generates outside of QAPI code; I can't git grep for them and
I need to remember to build the tree before using cscope in order to
find them.  Therefore I prefer to keep the translation to standard
macros within the QAPI code to make things more obvious, thus the
approach chosen.  I appreciate the feedback and alternative ideas.

> We could extend QAPI to permit specification of the enumeration values.
> Marc-André's work to permit conditionals makes the syntax flexible
> enough to support that.  Of course, adding QAPI features is worthwhile
> only if we get sufficient mileage out of them to result in an overall
> improvement.  Even if we decided to do it right now, I'd recommend not
> to wait for it, but instead plan to simplify after the feature lands.

Good to know.

> > Cc: Eric Blake <address@hidden>
> > Cc: Markus Armbruster <address@hidden>
> > Tested-by: Geoffrey McRae <address@hidden>
> > Signed-off-by: Alex Williamson <address@hidden>  
> 
> Reviewed-by: Markus Armbruster <address@hidden>

Thanks!

Alex