Re: [PATCH v2] libxc: avoid clobbering errno in xc_domain_pod_target()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.12.2021 15:00, Bertrand Marquis wrote:
>> On 10 Dec 2021, at 13:54, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 10.12.2021 14:50, Bertrand Marquis wrote:
>>>> On 10 Dec 2021, at 13:11, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>
>>>> do_memory_op() supplies return value and has "errno" set the usual way.
>>>> Don't overwrite "errno" with 1 (aka EPERM on at least Linux). There's
>>>> also no reason to overwrite "err".
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>>
>> Thanks.
>>
>>> But if err can really only be 0 or -1, I do wonder if the else forcing err 
>>> to 0 could
>>> be removed but I must say I have no idea if do_memory_op could return a 
>>> value >0.
>>
>> Indeed - see ...
>>
>>>> ---
>>>> While the hypervisor side of the hypercall gives the impression of being
>>>> able to return positive values as of 637a283f17eb ("PoD: Allow
>>>> pod_set_cache_target hypercall to be preempted"), due to the use of
>>>> "rc >= 0" there, afaict that's not actually the case. IOW "err" can
>>>> really only be 0 or -1 here, and hence its setting to zero may also be
>>>> worthwhile to drop.
>>>> ---
>>
>> ... this.
> 
> So the else should be dropped then, why not doing it and just mentioning it 
> there ?

Well, I'd like confirmation by a maintainer. There are a few aspects to how
things are done in the tool stack which I'm not always aware of. IOW there
might be reasons to keep things as they are after this variant of the patch.

Jan