Re: [PATCH v2 14/21] libs/guest: introduce helper to check cpu policy compatibility

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

On Wed, Apr 14, 2021 at 03:36:54PM +0200, Jan Beulich wrote:
> On 13.04.2021 16:01, Roger Pau Monne wrote:
> > --- a/tools/libs/guest/xg_cpuid_x86.c
> > +++ b/tools/libs/guest/xg_cpuid_x86.c
> > @@ -925,3 +925,22 @@ int xc_cpu_policy_update_msrs(xc_interface *xch, 
> > xc_cpu_policy_t policy,
> >  
> >      return rc;
> >  }
> > +
> > +bool xc_cpu_policy_is_compatible(xc_interface *xch, const xc_cpu_policy_t 
> > host,
> > +                                 const xc_cpu_policy_t guest)
> > +{
> > +    struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
> > +    struct cpu_policy h = { &host->cpuid, &host->msr };
> > +    struct cpu_policy g = { &guest->cpuid, &guest->msr };
> > +    int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
> > +
> > +    if ( !rc )
> > +        return true;
> > +
> > +    if ( err.leaf != -1 )
> > +        ERROR("Leaf %#x subleaf %#x is not compatible", err.leaf, 
> > err.subleaf);
> > +    if ( err.msr != -1 )
> > +        ERROR("MSR index %#x is not compatible", err.msr);
> 
> Personally I'm against making assumptions like these ones about what
> (in this case) INIT_CPU_POLICY_ERRORS actually expands to (i.e. three
> times -1). I can see how alternatives to this are quickly going to
> get ugly, so I'll leave it to others to judge.

Would you like me to define a separate POLICY_ERROR? ie:

#define INIT_CPU_POLICY_ERROR -1
#define INIT_CPU_POLICY_ERRORS { INIT_CPU_POLICY_ERROR, \
                                 INIT_CPU_POLICY_ERROR, \
                                 INIT_CPU_POLICY_ERROR }

We already have a bunch of open coded -1 checks anyway, but might
prevent new ones from appearing.

Thanks, Roger.