[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 02/21] libxl: introduce a way to mark fields as deprecated in the idl
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@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |