Re: [Xen-devel] [PATCH 7/9] xen: use domid check in is_hardware_domain

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.04.13 at 22:13, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
> Instead of checking is_privileged to determine if a domain should
> control the hardware, check that the domain_id is equal to zero (which
> is currently the only domain for which is_privileged is true).  This
> allows other places where domain_id is checked for zero to be replaced
> with is_hardware_domain.

Isn't this moving in the wrong direction? I.e. wouldn't you rather
want to see the domid == 0 checks go away, rather than replacing
the ->is_privileged one in is_hardware_domain()?

> --- a/xen/common/cpupool.c
> +++ b/xen/common/cpupool.c
> @@ -546,13 +546,16 @@ int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op)
>      {
>          struct domain *d;
>  
> -        ret = -EINVAL;
> -        if ( op->domid == 0 )
> -            break;
>          ret = -ESRCH;
>          d = rcu_lock_domain_by_id(op->domid);
>          if ( d == NULL )
>              break;
> +        if ( is_hardware_domain(d) )

Here this is surely wrong - the operation has nothing to do with
hardware control. This would need something like is_control_domain().

Which puts under question the earlier changes in this series: Wouldn't
you be better of having is_control_domain() resolving to
->is_privileged and is_hardware_domain() resolving to domid == 0
(for the time being), and use the two according to the respective
contexts?

> +        {
> +            ret = -EINVAL;
> +            rcu_unlock_domain(d);
> +            break;
> +        }
>          if ( d->cpupool == NULL )
>          {
>              ret = -EINVAL;
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -235,7 +235,7 @@ struct domain *domain_create(
>      if ( domcr_flags & DOMCRF_hvm )
>          d->is_hvm = 1;
>  
> -    if ( domid == 0 )
> +    if ( is_hardware_domain(d) )

And this one isn't hardware related either. While the subsequent ones
both are.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel