Re: [Xen-devel] [PATCH 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 23 Aug 2017, Zhongze Liu wrote:
> Add the parsing utils for the newly introduced libxl_static_sshm struct
> to the libxl/libxlu_* family. And add realated parsing code in xl to
> parse the struct from xl config files. This is for the proposal "Allow
> setting up shared memory areas between VMs from xl config file" (see [1]).
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03047.html
> 
> Signed-off-by: Zhongze Liu <blackskygg@xxxxxxxxx>
> 
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxx
> ---
>  tools/libxl/Makefile      |   2 +-
>  tools/libxl/libxlu_sshm.c | 210 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxlutil.h   |   6 ++
>  tools/xl/xl_parse.c       |  24 +++++-
>  4 files changed, 240 insertions(+), 2 deletions(-)
>  create mode 100644 tools/libxl/libxlu_sshm.c
> 
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 082af8f716..3b63fb2cad 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -175,7 +175,7 @@ AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h 
> _paths.h \
>  AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
>  AUTOSRCS += _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c
>  LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
> -     libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o
> +     libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o libxlu_sshm.o
>  $(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
>  
>  $(TEST_PROG_OBJS) _libxl.api-for-check: CFLAGS += $(CFLAGS_libxentoollog)
> diff --git a/tools/libxl/libxlu_sshm.c b/tools/libxl/libxlu_sshm.c
> new file mode 100644
> index 0000000000..8647665213
> --- /dev/null
> +++ b/tools/libxl/libxlu_sshm.c
> @@ -0,0 +1,210 @@
> +#include "libxl_osdeps.h" /* must come before any other headers */
> +#include "libxlu_internal.h"
> +
> +#include <ctype.h>
> +
> +#define PARAM_RE(EXPR) "^\\s*" EXPR "\\s*(,|$)"
> +#define WORD_RE         "([_a-zA-Z0-9]+)"
> +#define EQU_RE         PARAM_RE(WORD_RE "\\s*=\\s*" WORD_RE)
> +
> +#define PAGE_SIZE_MASK ((uint64_t)0xfff)

You can probably use XC_PAGE_MASK that is already defined?
Otherwise I would name this XLU_PAGE_MASK and move the definition to one
of the libxlu headers.

I am not the most qualified person to review xlu code, but this patch
looks OK to me.


> +#define RET_INVAL(msg, curr_str)  do {              \
> +        xlu__sshm_err(cfg, msg, curr_str);          \
> +        rc = EINVAL;                                \
> +        goto out;                                   \
> +    } while(0)
> +
> +/* set a member in libxl_static_shm and report an error if it's respecified,
> + * @curr_str indicates the head of the remaining string. */
> +#define SET_VAL(var, name, type, value, curr_str)  do {                 \
> +        if ((var) != LIBXL_SSHM_##type##_UNKNOWN && (var) != value) {   \
> +            RET_INVAL("\"" name "\" respecified", curr_str);            \
> +        }                                                               \
> +        (var) = value;                                                  \
> +    } while(0)
> +
> +
> +static void xlu__sshm_err(XLU_Config *cfg, const char *msg,
> +                          const char *curr_str) {
> +    fprintf(cfg->report,
> +            "%s: config parsing error in shared_memory: %s at '%s'\n",
> +            cfg->config_source, msg, curr_str);
> +}
> +
> +static int parse_prot(XLU_Config *cfg, char *str, libxl_sshm_prot *prot)
> +{
> +    int rc;
> +    libxl_sshm_prot new_prot;
> +
> +    if (!strcmp(str, "rw")) {
> +        new_prot = LIBXL_SSHM_PROT_RW;
> +    } else {
> +        RET_INVAL("invalid permission flags", str);
> +    }
> +
> +    SET_VAL(*prot, "permission flags", PROT, new_prot, str);
> +
> +    rc = 0;
> +
> + out:
> +    return rc;
> +}
> +
> +static int parse_cachepolicy(XLU_Config *cfg, char *str,
> +                             libxl_sshm_cachepolicy *policy)
> +{
> +    int rc;
> +    libxl_sshm_cachepolicy new_policy;
> +
> +    if (!strcmp(str, "ARM_normal")) {
> +        new_policy = LIBXL_SSHM_CACHEPOLICY_ARM_NORMAL;
> +    } else if (!strcmp(str, "x86_normal")) {
> +        new_policy = LIBXL_SSHM_CACHEPOLICY_X86_NORMAL;
> +    } else {
> +        RET_INVAL("invalid cache policy", str);
> +    }
> +
> +    SET_VAL(*policy, "cache policy", CACHEPOLICY, new_policy, str);
> +    rc = 0;
> +
> + out:
> +    return rc;
> +}
> +
> +/* handle key = value pairs */
> +static int handle_equ(XLU_Config *cfg, char *key, char *val,
> +                      libxl_static_shm *sshm)
> +{
> +    int rc;
> +
> +    if (!strcmp(key, "id")) {
> +        if (strlen(val) > LIBXL_SSHM_ID_MAXLEN) { RET_INVAL("id too long", 
> val); }
> +        if (sshm->id && !strcmp(sshm->id, val)) {
> +            RET_INVAL("id respecified", val);
> +        }
> +
> +        if (NULL == (sshm->id = strdup(val))) {
> +            fprintf(stderr, "sshm parser out of memory\n");
> +            rc = ENOMEM;
> +            goto out;
> +        }
> +    } else if (!strcmp(key, "role")) {
> +        libxl_sshm_role new_role;
> +
> +        if (!strcmp("master", val)) {
> +            new_role = LIBXL_SSHM_ROLE_MASTER;
> +        } else if (!strcmp("slave", val)) {
> +            new_role = LIBXL_SSHM_ROLE_SLAVE;
> +        } else {
> +            RET_INVAL("invalid role", val);
> +        }
> +
> +        SET_VAL(sshm->role, "role", ROLE, new_role, val);
> +    } else if (!strcmp(key, "begin") ||
> +               !strcmp(key, "end") ||
> +               !strcmp(key, "offset")) {
> +        char *endptr;
> +        int base = 10;
> +        uint64_t new_addr;
> +
> +        /* Could be in hex form. Note that we don't need to check the length 
> here,
> +         * for val[] is NULL-terminated */
> +        if ('0' == val[0] && 'x' == val[1]) { base = 16; }
> +        new_addr = strtoull(val, &endptr, base);
> +        if (ERANGE == errno || *endptr) {
> +            RET_INVAL("invalid begin/end/offset", val);
> +        }
> +        if (new_addr & PAGE_SIZE_MASK)
> +            RET_INVAL("begin/end/offset is not a multiple of 4K", val);
> +
> +        /* begin or end */
> +        if ('b' == key[0]) {
> +            SET_VAL(sshm->begin, "beginning address", RANGE, new_addr, val);
> +        } else if('e' == key[0]){
> +            SET_VAL(sshm->end, "ending address", RANGE, new_addr, val);
> +        } else {
> +            SET_VAL(sshm->offset, "offset", RANGE, new_addr, val);
> +        }
> +    } else if (!strcmp(key, "prot")) {
> +        rc = parse_prot(cfg, val, &sshm->prot);
> +        if (rc) { goto out; }
> +    } else if (!strcmp(key, "cache_policy")) {
> +        rc = parse_cachepolicy(cfg, val, &sshm->cache_policy);
> +        if (rc) { goto out; }
> +    } else {
> +        RET_INVAL("invalid option", key);
> +    }
> +
> +    rc = 0;
> +
> + out:
> +    return rc;
> +}
> +
> +int xlu_sshm_parse(XLU_Config *cfg, const char *spec,
> +                   libxl_static_shm *sshm)
> +{
> +    int rc;
> +    regex_t equ_rec;
> +    char *buf2 = NULL, *ptr = NULL;
> +    regmatch_t pmatch[3];
> +
> +    rc = regcomp(&equ_rec, EQU_RE, REG_EXTENDED);
> +    if (rc) {
> +        fprintf(stderr, "sshm parser failed to initialize\n");
> +        goto out;
> +    }
> +
> +    if (NULL == (buf2 = ptr = strdup(spec))) {
> +        fprintf(stderr, "sshm parser out of memory\n");
> +        rc = ENOMEM;
> +        goto out;
> +    }
> +
> +    /* main parsing loop */
> +    while (true) {
> +        if (!*ptr) { break; }
> +        if (regexec(&equ_rec, ptr, 3, pmatch, 0))
> +            RET_INVAL("unrecognized token", ptr);
> +
> +        ptr[pmatch[1].rm_eo] = '\0';
> +        ptr[pmatch[2].rm_eo] = '\0';
> +        rc = handle_equ(cfg, ptr + pmatch[1].rm_so,
> +                        ptr + pmatch[2].rm_so, sshm);
> +        if (rc) { goto out; }
> +
> +        ptr += pmatch[0].rm_eo;
> +    }
> +
> +    if (*ptr) { RET_INVAL("invalid syntax", ptr); }
> +
> +    /* do some early checks */
> +    if (!sshm->id) {
> +        RET_INVAL("id not specified", spec);
> +    }
> +    if (sshm->begin == LIBXL_SSHM_RANGE_UNKNOWN) {
> +        RET_INVAL("begin address not specified", spec);
> +    }
> +    if (sshm->end == LIBXL_SSHM_RANGE_UNKNOWN) {
> +        RET_INVAL("end address not specified", spec);
> +    }
> +    if (sshm->begin > sshm->end) {
> +        RET_INVAL("begin address larger that end address", spec);
> +    }
> +
> +    rc = 0;
> +
> + out:
> +    if (buf2) { free(buf2); }
> +    regfree(&equ_rec);
> +    return rc;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
> index e81b644c01..ee39cb5bdc 100644
> --- a/tools/libxl/libxlutil.h
> +++ b/tools/libxl/libxlutil.h
> @@ -118,6 +118,12 @@ int xlu_rdm_parse(XLU_Config *cfg, libxl_rdm_reserve 
> *rdm, const char *str);
>  int xlu_vif_parse_rate(XLU_Config *cfg, const char *rate,
>                         libxl_device_nic *nic);
>  
> +/*
> + * static shared memory specification parsing
> + */
> +int xlu_sshm_parse(XLU_Config *cfg, const char *spec,
> +                   libxl_static_shm *sshm);
> +
>  #endif /* LIBXLUTIL_H */
>  
>  /*
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 5c2bf17222..82d955b8b9 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -813,7 +813,7 @@ void parse_config_data(const char *config_source,
>      long l, vcpus = 0;
>      XLU_Config *config;
>      XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms,
> -                   *usbctrls, *usbdevs, *p9devs;
> +                   *usbctrls, *usbdevs, *p9devs, *sshms;
>      XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian, *dtdevs,
>                     *mca_caps;
>      int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian, 
> num_mca_caps;
> @@ -1392,6 +1392,28 @@ void parse_config_data(const char *config_source,
>          }
>      }
>  
> +    if (!xlu_cfg_get_list (config, "static_shm", &sshms, 0, 0)) {
> +        d_config->num_sshms = 0;
> +        d_config->sshms = NULL;
> +        while ((buf = xlu_cfg_get_listitem (sshms, d_config->num_sshms)) != 
> NULL) {
> +            libxl_static_shm *sshm;
> +            char *buf2 = strdup(buf);
> +            int ret;
> +
> +            sshm = ARRAY_EXTEND_INIT_NODEVID(d_config->sshms,
> +                                             d_config->num_sshms,
> +                                             libxl_static_shm_init);
> +            ret = xlu_sshm_parse(config, buf2, sshm);
> +            if (ret) {
> +                fprintf(stderr,
> +                        "xl: Invalid argument for static_shm: %s", buf2);
> +                exit(EXIT_FAILURE);
> +            }
> +
> +            free(buf2);
> +        }
> +    }
> +
>      if (!xlu_cfg_get_list(config, "p9", &p9devs, 0, 0)) {
>          libxl_device_p9 *p9;
>          char *security_model = NULL;

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel