Re: [Xen-devel] [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Sep 20, 2017 at 04:46:16PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 02/21] libxl: introduce a way to mark 
> fields as deprecated in the idl"):
> > The deprecation involves generating a function that copies the
> > deprecated fields into it's new location if the new location has not
> > been set.
> 
> Hi.  We had an IRL conversation which I will summarise.
> 
> 
> The first issue is about the scoping and context of the deprecated_by
> annotations.  The arrangement you have is that the field name in
> deprecated_by is a (textual) reference to an (implicit) enclosing
> struct which has copy_deprecated_fn specified.
> 
> This is kind of OK but it feels a bit limited and irregular to me.
> The practical consequence is that this can be used to bring fields out
> into the toplevel, but it is difficult to use it in other ways (for
> example, to move a field from one substruct to another, or to
> deprecate fields which are part of named substrutures rather than
> anonymous ones and which might therefore appear in several places).
> 
> We discussed how this might be done better.  To me it seems like the
> only really plausible alternative was to replace the
> `deprecated_by' and `copy_deprecated_fn' annotations with a single
> annotation in the parent structure, something like
>   deprecated_fields=['u.hvm','u',['bootloader_args',
>                                   'timer_mode', ...]
> or maybe even
>   deprecated_fields=['u.hvm','u',[('timer_mode_new_name','timer_mode')]]
> 
> I really don't want to ask you to implement this general scheme now,
> but if you feel like it you could perhaps experiment and see how it
> seems.

Hello,

Thanks for the review. I've fixed all the other comments on the series
and started an osstest flight, my aim is to post a new version of the
series using the same deprecation idl mechanism ASAP.

The motivation for doing this is that I think we really need stable
PVHv2 support in Xen, and it must be in the next release.

Regarding your proposal for deprecating fields, I was thinking of
something along the lines of:

deprecated_fields=[['u.hvm.timer_mode', 'timer_mode'],
                   ['u.pv.bootloader', 'booloader'],
                   ...]

Something like a list of tuples (I'm not sure this is the right
terminology in python?), where the first element is the deprecated
field, and the second one is the new position.

This has the extra complication that when parsing the types gentypes
needs to split the strings based on the '.' or '->' delimiters, and
then iterate inside of the structure object to find the actual
element. AFAICT this is doable, but not as simple as my current
approach.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel