Re: [PATCH for-4.22?] domctl: correct return value of XEN_DOMCTL_[gs]etvcpuaffinity

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Jun 15, 2026 at 02:00:39PM +0200, Jan Beulich wrote:
> cpumask_to_xenctl_bitmap() may return errors. Clearing the error indicator
> of an earlier such call by a (successful) later call is misleading the
> caller. For "set", keep setting soft affinity if the hard affinity copy-
> back fails; only accumulate respective errors.
> 
> While fiddling with return values, also drop a redundant clearing of
> "ret". This eliminates a Misra C:2012 rule 2.2 ("There shall be no dead
> code") violation.
> 
> Fixes: 6e4ecc6d5884 ("sched: DOMCTL_*vcpuaffinity works with hard and soft 
> affinity")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -1705,6 +1705,7 @@ int vcpu_affinity_domctl(struct domain *
>      {
>          cpumask_var_t new_affinity, old_affinity;
>          cpumask_t *online = cpupool_domain_master_cpumask(v->domain);
> +        int hret = 0;
>  
>          /*
>           * We want to be able to restore hard affinity if we are trying
> @@ -1726,8 +1727,6 @@ int vcpu_affinity_domctl(struct domain *
>          if ( vcpuaff->flags & XEN_VCPUAFFINITY_FORCE )
>              vcpu_temporary_affinity(v, NR_CPUS, VCPU_AFFINITY_OVERRIDE);
>  
> -        ret = 0;
> -
>          /*
>           * We both set a new affinity and report back to the caller what
>           * the scheduler will be effectively using.
> @@ -1746,7 +1745,7 @@ int vcpu_affinity_domctl(struct domain *
>               * cpupool's online mask and the new hard affinity.
>               */
>              cpumask_and(new_affinity, online, unit->cpu_hard_affinity);
> -            ret = cpumask_to_xenctl_bitmap(&vcpuaff->cpumap_hard, 
> new_affinity);
> +            hret = cpumask_to_xenctl_bitmap(&vcpuaff->cpumap_hard, 
> new_affinity);
>          }
>          if ( vcpuaff->flags & XEN_VCPUAFFINITY_SOFT )
>          {
> @@ -1777,6 +1776,8 @@ int vcpu_affinity_domctl(struct domain *
>              ret = cpumask_to_xenctl_bitmap(&vcpuaff->cpumap_soft, 
> new_affinity);
>          }
>  
> +        ret = hret ?: ret;

Here you seem to have a preference to keep the error from the first
caller (possibly XEN_VCPUAFFINITY_HARD if set), but below...

> +
>   setvcpuaffinity_out:
>          free_cpumask_var(new_affinity);
>          free_cpumask_var(old_affinity);
> @@ -1788,7 +1789,7 @@ int vcpu_affinity_domctl(struct domain *
>                                             unit->cpu_hard_affinity);
>          if ( vcpuaff->flags & XEN_VCPUAFFINITY_SOFT )
>              ret = cpumask_to_xenctl_bitmap(&vcpuaff->cpumap_soft,
> -                                           unit->cpu_soft_affinity);
> +                                           unit->cpu_soft_affinity) ?: ret;

...you overwrite it with a possible error returned from the last
cpumask_to_xenctl_bitmap() call.  Don't you want to do the same, and
return the first error if possible?  IOW: store the error from the
first call into hret and use the elvis operator to set ret after both
calls.

Thanks, Roger.