Re: [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

[George Dunlap <george.dunlap@eu.citrix.com>]

On 07/01/2015 12:16 PM, Chen, Tiejun wrote:
> On 2015/7/1 18:57, George Dunlap wrote:
>> On 07/01/2015 11:26 AM, Chen, Tiejun wrote:
>>>>>> 1. By default, the domain policy is RELAXED (See above,
>>>>>> libxl__rdm_setdefault()).
>>>>>>
>>>>>> 2. By default, the policy for individual devices is STRICT (see
>>>>>> libxl_pci.c:libxl__device_pci_setdefault())
>>>>>>
>>>>>> 3. If the domain policy is set to STRICT, this overrides per-device
>>>>>> policy
>>>>>>
>>>>>> 4. If the domain policy is set to RELAXED, I don't see that having an
>>>>>> effect on individual devices
>>>>>
>>>>> This is our rule, and this is why I think you need to take a look at
>>>>> patch #00, our design and all patch head descriptions,
>>>>>
>>>>> "Default per-device RDM policy is 'strict', while default global RDM
>>>>> policy is 'relaxed'. When both policies are specified on a given
>>>>> region,
>>>>> 'strict' is always preferred."
>>>>
>>>> It looks like you didn't finish reading my message.  I suggest you
>>>> do so:
>>>
>>> Okay.
>>>
>>>>
>>>>>> If I'm correct, then #3 means it's not possible to have devices for a
>>>>>> domain *default* to strict, but to be relaxed in individual
>>>>>> instances.
>>>>>> If you had five devices you wanted strict, and only one device you
>>>>>> wanted to be relaxed (because you knew it didn't matter), you'd have
>>>>>> to set reserved=strict for all the other devices, rather than just
>>>>>> being able to set the domain setting to strict and set
>>>>>> reserve=relaxed
>>>>>> for the one.
>>>>>>
>>>>>> I think that both violates the principle of least surprise, and is
>>>>>> less useful.
>>>>
>>>
>>> So what's you idea to follow our requirement?
>>
>> So consider the following config snippet:
>>
>> ---
>> rdm="reserve=relaxed"
>>
>> pci=['01:00.1,msitranslate=1']
>> ----
>>
>> What should the policy for that device be?
>>
>> According to your policy document, it seems to me like it should be
>> "relaxed", since the domain default* is set to "relaxed" and nothing
> 
> Why? "strict" should be in this case.

OK, I think I see where the problem is.  I had expected the domain-wide
setting to be a default which was overridden by per-device policies (see
pci_permissive and friends).  So when I saw "global default RDM policy"
confirmation bias caused me to interpret it as what I expected to see --
the domain setting as the default, which the local setting could override.

I see now that in your documentation you consistently talk about two
different policies, each of which have their own defaults, and that the
effective permissions for a device end up being the intersection of the
two (i.e., only relaxed of both are relaxed; strict under all other
circumstances).

> Why are you saying this is not our expectation? Just let me pick up that
> description *again*,
> 
> "Default per-device RDM policy is 'strict', while default global RDM
> policy is 'relaxed'. When both policies are specified on a given region,
> 'strict' is always preferred."

Look, if I haven't understood what you meant by the exact same words the
first 4 times I read it, simply repeating the same exact words is not
going to be helpful.  Ideally you need to try go understand where my
misunderstanding is coming from and explain where I've misunderstood
something; or, at least you need to try to use different words, or
explain how the words you're using apply to the given situation.

>> This interface doesn't make any sense to me.  Why, if the "global
> 
> If you have any objection to our solution, and if you can't find any
> reasonable answer from our design, just please ping Jan or Kevin because
> I'm really not that person who can address this kind of change at this
> point in this high level.

And you have no idea why that design was chosen; you're just doing what
you're told?

I was involved in the design discussion, and from the very beginning I
probably saw your plan but misunderstood it.  I wouldn't be surprised if
some others didn't quite understand what they were agreeing to.

This way of doing things is different than the way we do it with most
other options relating to pci devices (e.g., pci_permissive,
pci_msitranslate, pci_sieze, &c).  All of those options use a "default"
semantic: the domain-wide setting takes effect only if it's not set
locally.  If the syntax looks the same but the semantics is different,
many people will be confused.  If we're going to have the domain-wide
policy override the per-device policy, then the naming should make that
clear; for instance, "override=(strict|relaxed|none)", or
"strict_override=(1|0)".

I don't happen to think these "override" semantics are actually going to
turn out to be that useful; I do think a "default" semantic would be
useful.  But I'd be content if the name of the current setting were
switched to "override" to make the semantics more clear.  We can always
add in "default" at some later point if we really want.

 -George