[Xen-devel] [PATCH] libxlu: Handle += in config files

* [Xen-devel] [PATCH] libxlu: Handle += in config files
@ 2019-08-13 14:48 Anthony PERARD
  2019-08-13 15:06 ` Andrew Cooper
  0 siblings, 1 reply; 9+ messages in thread
From: Anthony PERARD @ 2019-08-13 14:48 UTC (permalink / raw)
  To: xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Ian Jackson, Wei Liu

Handle += of both strings and lists.

If += is used for config options expected to be numbers, then a
warning is printed and the config option ignored (because xl ignores
config options with errors).

This is to be used for development purposes, where modifying config
option can be done on the `xl create' command line.

One could have a cmdline= in the cfg file, and specify cmdline+= on
the `xl create` command line without the need to write the whole
cmdline in `xl' command line but simply append to the one in the cfg file.
Or add an extra vif or disk by simply having "vif += [ '', ];" in the
`xl' cmdline.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    Commiter, the libxlu_cfg_?.[hc] files needs to be regenerated. (with make)
    
    This is a different proposal to Andrew's patch:
    <20190805144910.20223-1-andrew.cooper3@citrix.com>
    [PATCH] tools/xl: Make extra= usable in combination with cmdline=

 tools/libxl/libxlu_cfg.c      | 100 +++++++++++++++++++++++++++++++++-
 tools/libxl/libxlu_cfg_i.h    |   1 +
 tools/libxl/libxlu_cfg_l.l    |   1 +
 tools/libxl/libxlu_cfg_y.y    |   4 +-
 tools/libxl/libxlu_internal.h |   1 +
 tools/libxl/libxlutil.h       |   5 ++
 6 files changed, 109 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c
index 5838f6885e..72815d25dd 100644
--- a/tools/libxl/libxlu_cfg.c
+++ b/tools/libxl/libxlu_cfg.c
@@ -276,6 +276,14 @@ int xlu_cfg_get_long(const XLU_Config *cfg, const char *n,
     char *ep;
 
     e= find_atom(cfg,n,&set,dont_warn);  if (e) return e;
+    if (set->op == XLU_OP_ADDITION) {
+        if (!dont_warn)
+            fprintf(cfg->report,
+                    "%s:%d: warning: can't use += with numbers"
+                    " for parameter `%s'\n",
+                    cfg->config_source, set->lineno, n);
+        return EINVAL;
+    }
     errno= 0; l= strtol(set->value->u.string, &ep, 0);
     e= errno;
     if (errno) {
@@ -450,23 +458,111 @@ void xlu__cfg_list_append(CfgParseContext *ctx,
     list->u.list.nvalues++;
 }
 
+static int xlu__cfg_concat_vals(CfgParseContext *ctx,
+                                XLU_ConfigValue *prev,
+                                XLU_ConfigValue *to_add)
+{
+    int r;
+
+    if (prev->type != to_add->type) {
+        xlu__cfgl_lexicalerror(ctx,
+                           "can't add [list] to \"string\" or vice versa");
+        return EINVAL;
+    }
+
+    switch (to_add->type) {
+    case XLU_STRING: {
+        char *new_string = NULL;
+
+        r = asprintf(&new_string, "%s%s", prev->u.string,
+                     to_add->u.string);
+        if (r < 0) {
+            return errno;
+        }
+        free(to_add->u.string);
+        to_add->u.string = new_string;
+        return 0;
+    }
+    case XLU_LIST: {
+        XLU_ConfigList *const prev_list = &prev->u.list;
+        XLU_ConfigList *const cur_list = &to_add->u.list;
+        int nvalues;
+
+        if (prev->u.list.nvalues > INT_MAX - to_add->u.list.nvalues) {
+            return ERANGE;
+        }
+        nvalues = prev->u.list.nvalues + to_add->u.list.nvalues;
+
+        if (nvalues >= cur_list->avalues) {
+            XLU_ConfigValue **new_vals;
+            new_vals = realloc(cur_list->values,
+                               nvalues * sizeof(*new_vals));
+            if (!new_vals) {
+                return ENOMEM;
+            }
+            cur_list->avalues = nvalues;
+            cur_list->values = new_vals;
+        }
+
+        /* make space for `prev' into `to_add' */
+        memmove(cur_list->values + prev_list->nvalues,
+                cur_list->values,
+                cur_list->nvalues * sizeof(XLU_ConfigValue *));
+        /* move values from `prev' to `to_add' as the list in `prev' will
+         * not be reachable by find(). */
+        memcpy(cur_list->values,
+               prev_list->values,
+               prev_list->nvalues * sizeof(XLU_ConfigValue *));
+        cur_list->nvalues = nvalues;
+        prev_list->nvalues = 0;
+        memset(prev_list->values, 0,
+               prev_list->nvalues * sizeof(XLU_ConfigValue *));
+        return 0;
+    }
+    default:
+        abort();
+    }
+    return -1;
+}
+
 void xlu__cfg_set_store(CfgParseContext *ctx, char *name,
+                        enum XLU_Operation op,
                         XLU_ConfigValue *val, int lineno) {
     XLU_ConfigSetting *set;
+    int r;
 
-    if (ctx->err) return;
+    if (ctx->err) goto out;
 
     assert(name);
+
+    if (op == XLU_OP_ADDITION) {
+        /* If we have += concatenate with previous value with same name */
+        XLU_ConfigSetting *prev_set = find(ctx->cfg, name);
+        if (prev_set) {
+            r = xlu__cfg_concat_vals(ctx, prev_set->value, val);
+            if (r) {
+                ctx->err = r;
+                goto out;
+            }
+        }
+    }
+
     set = malloc(sizeof(*set));
     if (!set) {
         ctx->err = errno;
-        return;
+        goto out;
     }
     set->name= name;
     set->value = val;
+    set->op = op;
     set->lineno= lineno;
     set->next= ctx->cfg->settings;
     ctx->cfg->settings= set;
+    return;
+out:
+    assert(ctx->err);
+    free(name);
+    xlu__cfg_value_free(val);
 }
 
 char *xlu__cfgl_strdup(CfgParseContext *ctx, const char *src) {
diff --git a/tools/libxl/libxlu_cfg_i.h b/tools/libxl/libxlu_cfg_i.h
index 1b59b3312f..87b19df311 100644
--- a/tools/libxl/libxlu_cfg_i.h
+++ b/tools/libxl/libxlu_cfg_i.h
@@ -24,6 +24,7 @@
 
 void xlu__cfg_set_free(XLU_ConfigSetting *set);
 void xlu__cfg_set_store(CfgParseContext*, char *name,
+                        enum XLU_Operation op,
                         XLU_ConfigValue *val, int lineno);
 XLU_ConfigValue *xlu__cfg_string_mk(CfgParseContext *ctx,
                                     char *atom, YYLTYPE *loc);
diff --git a/tools/libxl/libxlu_cfg_l.l b/tools/libxl/libxlu_cfg_l.l
index e0ea8cfcb3..390d6e2c2b 100644
--- a/tools/libxl/libxlu_cfg_l.l
+++ b/tools/libxl/libxlu_cfg_l.l
@@ -66,6 +66,7 @@ void xlu__cfg_yyset_column(int  column_no, yyscan_t yyscanner);
 ,                       { GOT(','); }
 \[                      { GOT('['); }
 \]                      { GOT(']'); }
+\+\=                    { GOT(OP_ADD); }
 \=                      { GOT('='); }
 \;                      { GOT(';'); }
 
diff --git a/tools/libxl/libxlu_cfg_y.y b/tools/libxl/libxlu_cfg_y.y
index a923f7672d..020fc63eb3 100644
--- a/tools/libxl/libxlu_cfg_y.y
+++ b/tools/libxl/libxlu_cfg_y.y
@@ -38,6 +38,7 @@
 %token <string>                IDENT STRING NUMBER NEWLINE
 %type <string>            atom
 %destructor { free($$); } atom IDENT STRING NUMBER
+%token OP_ADD "+="
 
 %type <value>                             value valuelist values
 %destructor { xlu__cfg_value_free($$); }  value valuelist values
@@ -54,7 +55,8 @@ stmt:   assignment endstmt
  |      endstmt
  |      error NEWLINE
 
-assignment: IDENT '=' value { xlu__cfg_set_store(ctx,$1,$3,@3.first_line); }
+assignment: IDENT '=' value { xlu__cfg_set_store(ctx,$1,XLU_OP_ASSIGNMENT,$3,@3.first_line); }
+ |          IDENT "+=" value { xlu__cfg_set_store(ctx,$1,XLU_OP_ADDITION,$3,@3.first_line); }
 
 endstmt: NEWLINE
  |      ';'
diff --git a/tools/libxl/libxlu_internal.h b/tools/libxl/libxlu_internal.h
index 0acdde38f4..1f7559ecd9 100644
--- a/tools/libxl/libxlu_internal.h
+++ b/tools/libxl/libxlu_internal.h
@@ -53,6 +53,7 @@ typedef struct XLU_ConfigSetting { /* transparent */
     struct XLU_ConfigSetting *next;
     char *name;
     XLU_ConfigValue *value;
+    enum XLU_Operation op;
     int lineno;
 } XLU_ConfigSetting;
 
diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
index e81b644c01..057cc25cb2 100644
--- a/tools/libxl/libxlutil.h
+++ b/tools/libxl/libxlutil.h
@@ -25,6 +25,11 @@ enum XLU_ConfigValueType {
     XLU_LIST,
 };
 
+enum XLU_Operation {
+    XLU_OP_ASSIGNMENT = 0,
+    XLU_OP_ADDITION,
+};
+
 /* Unless otherwise stated, all functions return an errno value. */
 typedef struct XLU_Config XLU_Config;
 typedef struct XLU_ConfigList XLU_ConfigList;
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread