On 05/03/2020 16:24, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH v2 07/17] libxc/restore: STATIC_DATA_END
> inference for v2 compatibility"):
>> On 24/02/2020 17:32, Ian Jackson wrote:
>>> These 17 lines appears twice, in basically identical form. Could it
>>> be refactored ?
>> Not really, no.
>>
>> The error handling (i.e. half of those 17 lines) is different, making it
>> somewhat awkward to fit into a static inline.
> You could handle that with a small bit of code around one of the call
> sites to adjust the error handling. (Also, what a mess, but I guess
> we're here now...)
... which is the majority the code you're trying to abstract away.
>
>> More importantly however, by design, common code can't call
>> arch-specific code without a restore_ops hook. Deduping these would
>> require breaking the restriction which is currently doing a decent job
>> of keeping x86-isms out of common code.
> I'm afraid you're going to have to explain that to me at a bit greater
> length. The biggest thing that is confusing me about your statement
> here is that your patch is indeed adding x86-specific code to a common
> file. But also I don't understand the comment about restore_ops
> hooks - do you mean something in restore_callbacks ?
No. restore_callbacks are plumbing between libxl-save-helper and libxc.
restore_ops are internal to the restore code, and come in x86_pv and
x86_hvm flavours, with ARM existing in some theoretical future. The
design of the common save/restore code had deliberate measures put in
place to make it hard to get arch-specific details slip into common
code, so porting to different architectures didn't have to start by
doing a bunch of cleanup.
tl;dr I could put an #ifdef x86'd static inline in the root common
header (xc_sr_common.h), but the overall complexity is greater than what
is presented here.
~Andrew
