Re: [PATCH v2 1/5] x86/HVM: introduce hvm_point_entry()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/11/2023 9:34 am, Jan Beulich wrote:
> On 21.11.2023 22:30, Andrew Cooper wrote:
>> On 16/11/2023 1:46 pm, Jan Beulich wrote:
>>> ... to accompany hvm_read_entry() when actual copying isn't desirable.
>>> This allows to remove open-coded stream accesses from hpet_load(),
>>> along with using the helper in hvm_load() itself.
>>>
>>> Since arch_hvm_load()'s declaration would need changing, and since the
>>> function is not used from elsewhere, purge the declaration. With that it
>>> makes little sense to keep arch_hvm_save()'s around; convert that
>>> function to static then at the same time.
>>>
>>> In hpet_load() simplify the specific case of error return that's in
>>> context anyway: There's no need to hold the lock when only updating a
>>> local variable.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Thanks.
>
>> I really do hate all of this logic and most of it wants to live in
>> userspace, but this an improvement.
>>
>> The only thing I'm a little concerned with is the name. 
>> hvm_read_entry() clearly consumes an entry, while hvm_point_entry()
>> does, but doesn't obviously convey this at a glance.
>>
>> Ideally I'd say that hvm_{get,copy}_entry() would be better nomeclature,
>> as both have at least a vague implication of unmarshalling and one
>> clearly is making a separate copy.
> I'm fine renaming the new one to hvm_get_entry(). For the existing one,
> "copy" may be marginally better than "read" / "load", but then it doesn't
> indicate direction (i.e. somewhat collides with hvm_{write,load}_entry()).
> So I'd want to leave those as they are.

Ok, get/read/write make a reasonably-ok set of names.

~Andrew