From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v3][PATCH 11/16] tools: introduce some new parameters to set rdm policy Date: Mon, 15 Jun 2015 09:19:12 +0800 Message-ID: <557E2810.9050004@intel.com> References: <1433985325-16676-1-git-send-email-tiejun.chen@intel.com> <1433985325-16676-12-git-send-email-tiejun.chen@intel.com> <20150612160258.GW14606@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150612160258.GW14606@zion.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: kevin.tian@intel.com, ian.campbell@citrix.com, andrew.cooper3@citrix.com, tim@xen.org, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, jbeulich@suse.com, yang.z.zhang@intel.com, Ian.Jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org 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 >> --- > [...] >> } >> >> 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