Re: [v11][PATCH 16/16] tools: parse to enable new rdm policy parameters

[Ian Jackson <Ian.Jackson@eu.citrix.com>]

Tiejun Chen writes ("[v11][PATCH 16/16] tools: parse to enable new rdm policy parameters"):
> This patch parses to enable user configurable parameters to specify
> RDM resource and according policies which are defined previously,
> 
> Global RDM parameter:
>     rdm = "strategy=host,policy=strict/relaxed"
> Per-device RDM parameter:
>     pci = [ 'sbdf, rdm_policy=strict/relaxed' ]
> 
> Default per-device RDM policy is same as default global RDM policy as being
> 'relaxed'. And the per-device policy would override the global policy like
> others.
> 
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Wei, as you know, I am double-checking the acks on this series.

You acked v4.  But v11 is somewhat different.  Your ack was retained
and my subject line search did not turn up any authority for that.

The v* message by the submitter says:

> v8:
> 
> * Clean some codes style issues.
> 
> v7:
> 
> * Just sync with the fallout of renaming parameters from patch #10.
> 
> v6:
> 
> * Just sync those renames introduced by patch #10.
> 
> v5:
> 
> * Need a rebase after we make all rdm variables specific to .hvm.
> * Like other pci option, the per-device policy always follows
>   the global policy by default.

v4 does not apply in the context of the current series.

Below is the interdiff between v4 and v11, generated with `diff -b'.
(Sorry for handing you an interdiff.)

Please ack/nack.

Thanks,
Ian.
(wearing committer hat)

--- /u/iwj/mail/t	2015-07-22 15:01:34.483483917 +0100
+++ ../d/0016-tools-parse-to-enable-new-rdm-policy-parameters.patch	2015-07-22 13:56:44.286133093 +0100
@@ -1,41 +1,34 @@
+From 2e8335644daf6d3f081aaa9aaf55597c6a245f36 Mon Sep 17 00:00:00 2001
 From: Tiejun Chen <tiejun.chen@intel.com>
-To: <xen-devel@lists.xen.org>
-CC: Ian Jackson <ian.jackson@eu.citrix.com>, Stefano Stabellini
-	<stefano.stabellini@eu.citrix.com>, Ian Campbell <ian.campbell@citrix.com>,
-	Wei Liu <wei.liu2@citrix.com>
-Subject: [v4][PATCH 19/19] tools: parse to enable new rdm policy parameters
-Date: Tue, 23 Jun 2015 17:57:30 +0800
+Date: Wed, 22 Jul 2015 01:39:58 +0000
+Subject: [PATCH 16/16] tools: parse to enable new rdm policy parameters
 
 This patch parses to enable user configurable parameters to specify
-RDM resource and according policies,
+RDM resource and according policies which are defined previously,
 
 Global RDM parameter:
-    rdm = "type=none/host,reserve=strict/relaxed"
+    rdm = "strategy=host,policy=strict/relaxed"
 Per-device RDM parameter:
-    pci = [ 'sbdf, rdm_reserve=strict/relaxed' ]
+    pci = [ 'sbdf, rdm_policy=strict/relaxed' ]
 
-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.
+Default per-device RDM policy is same as default global RDM policy as being
+'relaxed'. And the per-device policy would override the global policy like
+others.
 
 CC: Ian Jackson <ian.jackson@eu.citrix.com>
 CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
 CC: Ian Campbell <ian.campbell@citrix.com>
 CC: Wei Liu <wei.liu2@citrix.com>
+Acked-by: Wei Liu <wei.liu2@citrix.com>
 Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
 ---
-v4:
-
-* Separated from current patch #11 to parse/enable our rdm policy parameters
-  since its make a lot sense and these stuffs are specific to xl/libxlu.
-
- tools/libxl/libxlu_pci.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
- tools/libxl/libxlutil.h  |  4 +++
- tools/libxl/xl_cmdimpl.c | 10 ++++++
- 3 files changed, 106 insertions(+)
+ tools/libxl/libxlu_pci.c |   92 +++++++++++++++++++++++++++++++++++++++++++++-
+ tools/libxl/libxlutil.h  |    4 ++
+ tools/libxl/xl_cmdimpl.c |   13 +++++++
+ 3 files changed, 108 insertions(+), 1 deletion(-)
 
 diff --git a/tools/libxl/libxlu_pci.c b/tools/libxl/libxlu_pci.c
-index 26fb143..9255878 100644
+index 26fb143..026413b 100644
 --- a/tools/libxl/libxlu_pci.c
 +++ b/tools/libxl/libxlu_pci.c
 @@ -42,6 +42,9 @@ static int pcidev_struct_fill(libxl_device_pci *pcidev, unsigned int domain,
@@ -43,30 +36,32 @@
  #define STATE_OPTIONS_V 7
  #define STATE_TERMINAL  8
 +#define STATE_TYPE      9
-+#define STATE_RDM_TYPE      10
-+#define STATE_RESERVE_FLAG      11
++#define STATE_RDM_STRATEGY      10
++#define STATE_RESERVE_POLICY    11
  int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str)
  {
      unsigned state = STATE_DOMAIN;
-@@ -143,6 +146,17 @@ int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str
+@@ -143,7 +146,18 @@ int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str
                      pcidev->permissive = atoi(tok);
                  }else if ( !strcmp(optkey, "seize") ) {
                      pcidev->seize = atoi(tok);
-+                }else if ( !strcmp(optkey, "rdm_reserve") ) {
-+                    if ( !strcmp(tok, "strict") ) {
-+                        pcidev->rdm_reserve = LIBXL_RDM_RESERVE_FLAG_STRICT;
-+                    } else if ( !strcmp(tok, "relaxed") ) {
-+                        pcidev->rdm_reserve = LIBXL_RDM_RESERVE_FLAG_RELAXED;
+-                }else{
++                } else if (!strcmp(optkey, "rdm_policy")) {
++                    if (!strcmp(tok, "strict")) {
++                        pcidev->rdm_policy = LIBXL_RDM_RESERVE_POLICY_STRICT;
++                    } else if (!strcmp(tok, "relaxed")) {
++                        pcidev->rdm_policy = LIBXL_RDM_RESERVE_POLICY_RELAXED;
 +                    } else {
 +                        XLU__PCI_ERR(cfg, "%s is not an valid PCI RDM property"
-+                                          " flag: 'strict' or 'relaxed'.",
++                                          " policy: 'strict' or 'relaxed'.",
 +                                     tok);
 +                        goto parse_error;
 +                    }
-                 }else{
++                } else {
                      XLU__PCI_ERR(cfg, "Unknown PCI BDF option: %s", optkey);
                  }
-@@ -167,6 +181,84 @@ parse_error:
+                 tok = ptr + 1;
+@@ -167,6 +181,82 @@ parse_error:
      return ERROR_INVAL;
  }
  
@@ -82,35 +77,33 @@
 +        switch(state) {
 +        case STATE_TYPE:
 +            if (*ptr == '=') {
-+                state = STATE_RDM_TYPE;
++                state = STATE_RDM_STRATEGY;
 +                *ptr = '\0';
-+                if (strcmp(tok, "type")) {
++                if (strcmp(tok, "strategy")) {
 +                    XLU__PCI_ERR(cfg, "Unknown RDM state option: %s", tok);
 +                    goto parse_error;
 +                }
 +                tok = ptr + 1;
 +            }
 +            break;
-+        case STATE_RDM_TYPE:
++        case STATE_RDM_STRATEGY:
 +            if (*ptr == '\0' || *ptr == ',') {
-+                state = STATE_RESERVE_FLAG;
++                state = STATE_RESERVE_POLICY;
 +                *ptr = '\0';
 +                if (!strcmp(tok, "host")) {
-+                    rdm->type = LIBXL_RDM_RESERVE_TYPE_HOST;
-+                } else if (!strcmp(tok, "none")) {
-+                    rdm->type = LIBXL_RDM_RESERVE_TYPE_NONE;
++                    rdm->strategy = LIBXL_RDM_RESERVE_STRATEGY_HOST;
 +                } else {
-+                    XLU__PCI_ERR(cfg, "Unknown RDM type option: %s", tok);
++                    XLU__PCI_ERR(cfg, "Unknown RDM strategy option: %s", tok);
 +                    goto parse_error;
 +                }
 +                tok = ptr + 1;
 +            }
 +            break;
-+        case STATE_RESERVE_FLAG:
++        case STATE_RESERVE_POLICY:
 +            if (*ptr == '=') {
 +                state = STATE_OPTIONS_V;
 +                *ptr = '\0';
-+                if (strcmp(tok, "reserve")) {
++                if (strcmp(tok, "policy")) {
 +                    XLU__PCI_ERR(cfg, "Unknown RDM property value: %s", tok);
 +                    goto parse_error;
 +                }
@@ -122,11 +115,11 @@
 +                state = STATE_TERMINAL;
 +                *ptr = '\0';
 +                if (!strcmp(tok, "strict")) {
-+                    rdm->reserve = LIBXL_RDM_RESERVE_FLAG_STRICT;
++                    rdm->policy = LIBXL_RDM_RESERVE_POLICY_STRICT;
 +                } else if (!strcmp(tok, "relaxed")) {
-+                    rdm->reserve = LIBXL_RDM_RESERVE_FLAG_RELAXED;
++                    rdm->policy = LIBXL_RDM_RESERVE_POLICY_RELAXED;
 +                } else {
-+                    XLU__PCI_ERR(cfg, "Unknown RDM property flag value: %s",
++                    XLU__PCI_ERR(cfg, "Unknown RDM property policy value: %s",
 +                                 tok);
 +                    goto parse_error;
 +                }
@@ -167,34 +160,36 @@
  /*
   * Vif rate parsing.
 diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
-index c7a12b1..85d74fd 100644
+index 615b78b..d102439 100644
 --- a/tools/libxl/xl_cmdimpl.c
 +++ b/tools/libxl/xl_cmdimpl.c
-@@ -1923,6 +1923,14 @@ skip_vfb:
+@@ -1980,6 +1980,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;
++            b_info->u.hvm.rdm.strategy = rdm.strategy;
++            b_info->u.hvm.rdm.policy = rdm.policy;
 +        }
 +    }
 +
      if (!xlu_cfg_get_list (config, "pci", &pcis, 0, 0)) {
          d_config->num_pcidevs = 0;
          d_config->pcidevs = NULL;
-@@ -1937,6 +1945,8 @@ skip_vfb:
+@@ -1993,6 +2001,11 @@ skip_vfb:
              pcidev->power_mgmt = pci_power_mgmt;
              pcidev->permissive = pci_permissive;
              pcidev->seize = pci_seize;
-+            /* We'd like to force reserve rdm specific to a device by default.*/
-+            pcidev->rdm_reserve = LIBXL_RDM_RESERVE_FLAG_STRICT;
-             if (!xlu_pci_parse_bdf(config, pcidev, buf))
-                 d_config->num_pcidevs++;
-         }
++            /*
++             * Like other pci option, the per-device policy always follows
++             * the global policy by default.
++             */
++            pcidev->rdm_policy = b_info->u.hvm.rdm.policy;
+             e = xlu_pci_parse_bdf(config, pcidev, buf);
+             if (e) {
+                 fprintf(stderr,
 -- 
-1.9.1
-
+1.7.10.4