Re: [Xen-devel] [PATCH 11/18] xen: use XSM instead of IS_PRIV where duplicated

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.08.12 at 17:25, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
> On 08/06/2012 11:18 AM, Jan Beulich wrote:
>>>>> On 06.08.12 at 16:32, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -3366,12 +3366,12 @@ static int hvmop_set_pci_intx_level(
>>>      if ( (op.domain > 0) || (op.bus > 0) || (op.device > 31) || (op.intx > 
>>> 3) )
>>>          return -EINVAL;
>>>  
>>> -    rc = rcu_lock_remote_target_domain_by_id(op.domid, &d);
>>> -    if ( rc != 0 )
>>> -        return rc;
>>> +    d = rcu_lock_domain_by_id(op.domid);
>>> +    if ( d == NULL )
>>> +        return -ESRCH;
>>>  
>>>      rc = -EINVAL;
>>> -    if ( !is_hvm_domain(d) )
>>> +    if ( d == current->domain || !is_hvm_domain(d) )
>> 
>> What's wrong with rcu_lock_remote_target_domain_by_id() here
>> and in other places below? I think this minimally would deserve
>> a comment in the patch description, the more that this huge a
>> patch is already bad enough to look at.
> 
> The main reason for this change is that rcu_lock_remote_target_domain_by_id
> calls IS_PRIV, and this patch is attempting to remove the duplicated calls.
> Would you prefer making another rcu_lock_* function that only checks against
> current->domain and doesn't include the IS_PRIV_FOR check?

Yes, I think the restructuring should be so that no new
"d == current->domain" or alike get introduced (or at least
not as many of them as this patch did).

Jan


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