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

[Ian Jackson <ian.jackson@eu.citrix.com>]

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.


I have two related comments that do need addressing though: I think
your generate deprecated copy function should check that at most one
of the old and new fields is set.  And, then, to make default setting
idempotent (and avoid memory leaks etc.), it should clear the old
field.


Thanks,
Ian.

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