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

["Chen, Tiejun" <tiejun.chen@intel.com>]

On 2015/6/13 0:02, Wei Liu wrote:
> On Thu, Jun 11, 2015 at 09:15:20AM +0800, Tiejun Chen wrote:
>> This patch introduces user configurable parameters to specify RDM
>> resource and according policies,
>>
>> Global RDM parameter:
>>      rdm = "type=none/host,reserve=strict/relaxed"
>> Per-device RDM parameter:
>>      pci = [ 'sbdf, rdm_reserve=strict/relaxed' ]
>>
>> Global RDM parameter, "type", allows user to specify reserved regions
>> explicitly, e.g. using 'host' to include all reserved regions reported
>> on this platform which is good to handle hotplug scenario. In the future
>> this parameter may be further extended to allow specifying random regions,
>> e.g. even those belonging to another platform as a preparation for live
>> migration with passthrough devices. Instead, 'none' means we have nothing
>> to do all reserved regions and ignore all policies, so guest work as before.
>>
>> 'strict/relaxed' policy decides how to handle conflict when reserving RDM
>> regions in pfn space. If conflict exists, 'strict' means an immediate error
>> so VM will be killed, while 'relaxed' allows moving forward with a warning
>> message thrown out.
>>
>> 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.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
> [...]
>>   }
>>
>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index 23f27d4..4dfcaf7 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -73,6 +73,17 @@ libxl_domain_type = Enumeration("domain_type", [
>>       (2, "PV"),
>>       ], init_val = "LIBXL_DOMAIN_TYPE_INVALID")
>>
>> +libxl_rdm_reserve_type = Enumeration("rdm_reserve_type", [
>> +    (0, "none"),
>> +    (1, "host"),
>> +    ], init_val = "LIBXL_RDM_RESERVE_TYPE_NONE")
>> +
>
> No need to define init_val if that value is 0.

Okay.

>
> Other than this minor nit this patch does what we've discussed before.
>
>
>> + */
>> +int xlu_rdm_parse(XLU_Config *cfg, libxl_rdm_reserve *rdm, const char *str);
>>
>>   /*
>>    * Vif rate parsing.
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index c858068..aedbd4b 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -1920,6 +1920,14 @@ skip_vfb:
>>           xlu_cfg_get_defbool(config, "e820_host", &b_info->u.pv.e820_host, 0);
>>       }
>>
>> +    if (!xlu_cfg_get_string(config, "rdm", &buf, 0)) {
>> +        libxl_rdm_reserve rdm;
>> +        if (!xlu_rdm_parse(config, &rdm, buf)) {
>> +            b_info->rdm.type = rdm.type;
>> +            b_info->rdm.reserve = rdm.reserve;
>> +        }
>> +    }
>> +
>
> You might want to consider breaking out changes to xl and libxlu to a
> final patch.
>
> My thought is that even if those changes don't break bisection (which
> I'm not very sure at this point), they are useless. If you think it is
> difficult or I'm talking non-sense, do let me know.
>

Its a little difficult but just let me try to split again if possible.

Thanks
Tiejun