All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file
@ 2021-05-16 23:14 QI Fuli
  2021-05-16 23:14 ` [RFC ndctl PATCH 1/3] ndctl, ccan: import ciniparser QI Fuli
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: QI Fuli @ 2021-05-16 23:14 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: QI Fuli

From: QI Fuli <qi.fuli@fujitsu.com>

This patch set is to rename monitor.conf to ndctl.conf, and make it a
global ndctl configuration file that all ndctl commands can refer to.

As this patch set has been pending until now, I would like to know if
current idea works or not. If yes, I will finish the documents and test.

Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>

QI Fuli (3):
  ndctl, ccan: import ciniparser
  ndctl, util: add parse-configs helper
  ndctl, rename monitor.conf to ndctl.conf

 Makefile.am                        |   8 +-
 ccan/ciniparser/ciniparser.c       | 480 +++++++++++++++++++++++++++++
 ccan/ciniparser/ciniparser.h       | 262 ++++++++++++++++
 ccan/ciniparser/dictionary.c       | 266 ++++++++++++++++
 ccan/ciniparser/dictionary.h       | 166 ++++++++++
 configure.ac                       |   8 +-
 ndctl/Makefile.am                  |   9 +-
 ndctl/monitor.c                    | 127 ++------
 ndctl/{monitor.conf => ndctl.conf} |  16 +-
 util/parse-configs.c               |  47 +++
 util/parse-configs.h               |  26 ++
 11 files changed, 1294 insertions(+), 121 deletions(-)
 create mode 100644 ccan/ciniparser/ciniparser.c
 create mode 100644 ccan/ciniparser/ciniparser.h
 create mode 100644 ccan/ciniparser/dictionary.c
 create mode 100644 ccan/ciniparser/dictionary.h
 rename ndctl/{monitor.conf => ndctl.conf} (82%)
 create mode 100644 util/parse-configs.c
 create mode 100644 util/parse-configs.h

-- 
2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [RFC ndctl PATCH 1/3] ndctl, ccan: import ciniparser
  2021-05-16 23:14 [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file QI Fuli
@ 2021-05-16 23:14 ` QI Fuli
  2021-05-16 23:14 ` [RFC ndctl PATCH 2/3] ndctl, util: add parse-configs helper QI Fuli
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: QI Fuli @ 2021-05-16 23:14 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: QI Fuli

From: QI Fuli <qi.fuli@fujitsu.com>

Import ciniparser from ccan[1].
[1] https://ccodearchive.net/info/ciniparser.html

Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
---
 Makefile.am                  |   6 +-
 ccan/ciniparser/ciniparser.c | 480 +++++++++++++++++++++++++++++++++++
 ccan/ciniparser/ciniparser.h | 262 +++++++++++++++++++
 ccan/ciniparser/dictionary.c | 266 +++++++++++++++++++
 ccan/ciniparser/dictionary.h | 166 ++++++++++++
 5 files changed, 1179 insertions(+), 1 deletion(-)
 create mode 100644 ccan/ciniparser/ciniparser.c
 create mode 100644 ccan/ciniparser/ciniparser.h
 create mode 100644 ccan/ciniparser/dictionary.c
 create mode 100644 ccan/ciniparser/dictionary.h

diff --git a/Makefile.am b/Makefile.am
index 60a1998..960b5e9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -64,7 +64,11 @@ libccan_a_SOURCES = \
 	ccan/array_size/array_size.h \
 	ccan/minmax/minmax.h \
 	ccan/short_types/short_types.h \
-	ccan/endian/endian.h
+	ccan/endian/endian.h \
+	ccan/ciniparser/ciniparser.h \
+	ccan/ciniparser/ciniparser.c \
+	ccan/ciniparser/dictionary.h \
+	ccan/ciniparser/dictionary.c
 
 noinst_LIBRARIES += libutil.a
 libutil_a_SOURCES = \
diff --git a/ccan/ciniparser/ciniparser.c b/ccan/ciniparser/ciniparser.c
new file mode 100644
index 0000000..527f837
--- /dev/null
+++ b/ccan/ciniparser/ciniparser.c
@@ -0,0 +1,480 @@
+/* Copyright (c) 2000-2007 by Nicolas Devillard.
+ * Copyright (x) 2009 by Tim Post <tinkertim@gmail.com>
+ * MIT License
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+/** @addtogroup ciniparser
+ * @{
+ */
+/**
+ *  @file ciniparser.c
+ *  @author N. Devillard
+ *  @date Sep 2007
+ *  @version 3.0
+ *  @brief Parser for ini files.
+ */
+
+#include <ctype.h>
+#include <ccan/ciniparser/ciniparser.h>
+
+#define ASCIILINESZ      (1024)
+#define INI_INVALID_KEY  ((char*) NULL)
+
+/**
+ * This enum stores the status for each parsed line (internal use only).
+ */
+typedef enum _line_status_ {
+	LINE_UNPROCESSED,
+	LINE_ERROR,
+	LINE_EMPTY,
+	LINE_COMMENT,
+	LINE_SECTION,
+	LINE_VALUE
+} line_status;
+
+
+/**
+ * @brief Convert a string to lowercase.
+ * @param s String to convert.
+ * @return ptr to statically allocated string.
+ *
+ * This function returns a pointer to a statically allocated string
+ * containing a lowercased version of the input string. Do not free
+ * or modify the returned string! Since the returned string is statically
+ * allocated, it will be modified at each function call (not re-entrant).
+ */
+static char *strlwc(const char *s)
+{
+	static char l[ASCIILINESZ+1];
+	int i;
+
+	if (s == NULL)
+		return NULL;
+
+	for (i = 0; s[i] && i < ASCIILINESZ; i++)
+		l[i] = tolower(s[i]);
+	l[i] = '\0';
+	return l;
+}
+
+/**
+ * @brief Remove blanks at the beginning and the end of a string.
+ * @param s String to parse.
+ * @return ptr to statically allocated string.
+ *
+ * This function returns a pointer to a statically allocated string,
+ * which is identical to the input string, except that all blank
+ * characters at the end and the beg. of the string have been removed.
+ * Do not free or modify the returned string! Since the returned string
+ * is statically allocated, it will be modified at each function call
+ * (not re-entrant).
+ */
+static char *strstrip(const char *s)
+{
+	static char l[ASCIILINESZ+1];
+	unsigned int i, numspc;
+
+	if (s == NULL)
+		return NULL;
+
+	while (isspace(*s))
+		s++;
+
+	for (i = numspc = 0; s[i] && i < ASCIILINESZ; i++) {
+		l[i] = s[i];
+		if (isspace(l[i]))
+			numspc++;
+		else
+			numspc = 0;
+	}
+	l[i - numspc] = '\0';
+	return l;
+}
+
+/**
+ * @brief Load a single line from an INI file
+ * @param input_line Input line, may be concatenated multi-line input
+ * @param section Output space to store section
+ * @param key Output space to store key
+ * @param value Output space to store value
+ * @return line_status value
+ */
+static
+line_status ciniparser_line(char *input_line, char *section,
+	char *key, char *value)
+{
+	line_status sta;
+	char line[ASCIILINESZ+1];
+	int	 len;
+
+	strcpy(line, strstrip(input_line));
+	len = (int) strlen(line);
+
+	if (len < 1) {
+		/* Empty line */
+		sta = LINE_EMPTY;
+	} else if (line[0] == '#') {
+		/* Comment line */
+		sta = LINE_COMMENT;
+	} else if (line[0] == '[' && line[len-1] == ']') {
+		/* Section name */
+		sscanf(line, "[%[^]]", section);
+		strcpy(section, strstrip(section));
+		strcpy(section, strlwc(section));
+		sta = LINE_SECTION;
+	} else if (sscanf (line, "%[^=] = \"%[^\"]\"", key, value) == 2
+		   ||  sscanf (line, "%[^=] = '%[^\']'", key, value) == 2
+		   ||  sscanf (line, "%[^=] = %[^;#]", key, value) == 2) {
+		/* Usual key=value, with or without comments */
+		strcpy(key, strstrip(key));
+		strcpy(key, strlwc(key));
+		strcpy(value, strstrip(value));
+		/*
+		 * sscanf cannot handle '' or "" as empty values
+		 * this is done here
+		 */
+		if (!strcmp(value, "\"\"") || (!strcmp(value, "''"))) {
+			value[0] = 0;
+		}
+		sta = LINE_VALUE;
+	} else if (sscanf(line, "%[^=] = %[;#]", key, value) == 2
+		||  sscanf(line, "%[^=] %[=]", key, value) == 2) {
+		/*
+		 * Special cases:
+		 * key=
+		 * key=;
+		 * key=#
+		 */
+		strcpy(key, strstrip(key));
+		strcpy(key, strlwc(key));
+		value[0] = 0;
+		sta = LINE_VALUE;
+	} else {
+		/* Generate syntax error */
+		sta = LINE_ERROR;
+	}
+	return sta;
+}
+
+/* The remaining public functions are documented in ciniparser.h */
+
+int ciniparser_getnsec(dictionary *d)
+{
+	int i;
+	int nsec;
+
+	if (d == NULL)
+		return -1;
+
+	nsec = 0;
+	for (i = 0; i < d->size; i++) {
+		if (d->key[i] == NULL)
+			continue;
+		if (strchr(d->key[i], ':') == NULL) {
+			nsec ++;
+		}
+	}
+
+	return nsec;
+}
+
+char *ciniparser_getsecname(dictionary *d, int n)
+{
+	int i;
+	int foundsec;
+
+	if (d == NULL || n < 0)
+		return NULL;
+
+	if (n == 0)
+		n ++;
+
+	foundsec = 0;
+
+	for (i = 0; i < d->size; i++) {
+		if (d->key[i] == NULL)
+			continue;
+		if (! strchr(d->key[i], ':')) {
+			foundsec++;
+			if (foundsec >= n)
+				break;
+		}
+	}
+
+	if (foundsec == n) {
+		return d->key[i];
+	}
+
+	return (char *) NULL;
+}
+
+void ciniparser_dump(dictionary *d, FILE *f)
+{
+	int i;
+
+	if (d == NULL || f == NULL)
+		return;
+
+	for (i = 0; i < d->size; i++) {
+		if (d->key[i] == NULL)
+			continue;
+		if (d->val[i] != NULL) {
+			fprintf(f, "[%s]=[%s]\n", d->key[i], d->val[i]);
+		} else {
+			fprintf(f, "[%s]=UNDEF\n", d->key[i]);
+		}
+	}
+
+	return;
+}
+
+void ciniparser_dump_ini(dictionary *d, FILE *f)
+{
+	int i, j;
+	char keym[ASCIILINESZ+1];
+	int nsec;
+	char *secname;
+	int seclen;
+
+	if (d == NULL || f == NULL)
+		return;
+
+	memset(keym, 0, ASCIILINESZ + 1);
+
+	nsec = ciniparser_getnsec(d);
+	if (nsec < 1) {
+		/* No section in file: dump all keys as they are */
+		for (i = 0; i < d->size; i++) {
+			if (d->key[i] == NULL)
+				continue;
+			fprintf(f, "%s = %s\n", d->key[i], d->val[i]);
+		}
+		return;
+	}
+
+	for (i = 0; i < nsec; i++) {
+		secname = ciniparser_getsecname(d, i);
+		seclen  = (int)strlen(secname);
+		fprintf(f, "\n[%s]\n", secname);
+		snprintf(keym, ASCIILINESZ + 1, "%s:", secname);
+		for (j = 0; j < d->size; j++) {
+			if (d->key[j] == NULL)
+				continue;
+			if (!strncmp(d->key[j], keym, seclen+1)) {
+				fprintf(f, "%-30s = %s\n",
+					d->key[j]+seclen+1,
+					d->val[j] ? d->val[j] : "");
+			}
+		}
+	}
+	fprintf(f, "\n");
+
+	return;
+}
+
+char *ciniparser_getstring(dictionary *d, const char *key, char *def)
+{
+	char *lc_key;
+	char *sval;
+
+	if (d == NULL || key == NULL)
+		return def;
+
+	lc_key = strlwc(key);
+	sval = dictionary_get(d, lc_key, def);
+
+	return sval;
+}
+
+int ciniparser_getint(dictionary *d, const char *key, int notfound)
+{
+	char *str;
+
+	str = ciniparser_getstring(d, key, INI_INVALID_KEY);
+
+	if (str == INI_INVALID_KEY)
+		return notfound;
+
+	return (int) strtol(str, NULL, 10);
+}
+
+double ciniparser_getdouble(dictionary *d, const char *key, double notfound)
+{
+	char *str;
+
+	str = ciniparser_getstring(d, key, INI_INVALID_KEY);
+
+	if (str == INI_INVALID_KEY)
+		return notfound;
+
+	return atof(str);
+}
+
+int ciniparser_getboolean(dictionary *d, const char *key, int notfound)
+{
+	char *c;
+	int ret;
+
+	c = ciniparser_getstring(d, key, INI_INVALID_KEY);
+	if (c == INI_INVALID_KEY)
+		return notfound;
+
+	switch(c[0]) {
+	case 'y': case 'Y': case '1': case 't': case 'T':
+		ret = 1;
+		break;
+	case 'n': case 'N': case '0': case 'f': case 'F':
+		ret = 0;
+		break;
+	default:
+		ret = notfound;
+		break;
+	}
+
+	return ret;
+}
+
+int ciniparser_find_entry(dictionary *ini, char *entry)
+{
+	int found = 0;
+
+	if (ciniparser_getstring(ini, entry, INI_INVALID_KEY) != INI_INVALID_KEY) {
+		found = 1;
+	}
+
+	return found;
+}
+
+int ciniparser_set(dictionary *d, char *entry, char *val)
+{
+	return dictionary_set(d, strlwc(entry), val);
+}
+
+void ciniparser_unset(dictionary *ini, char *entry)
+{
+	dictionary_unset(ini, strlwc(entry));
+}
+
+dictionary *ciniparser_load(const char *ininame)
+{
+	FILE *in;
+	char line[ASCIILINESZ+1];
+	char section[ASCIILINESZ+1];
+	char key[ASCIILINESZ+1];
+	char tmp[ASCIILINESZ+1];
+	char val[ASCIILINESZ+1];
+	int  last = 0, len, lineno = 0, errs = 0;
+	dictionary *dict;
+
+	if ((in = fopen(ininame, "r")) == NULL) {
+		fprintf(stderr, "ciniparser: cannot open %s\n", ininame);
+		return NULL;
+	}
+
+	dict = dictionary_new(0);
+	if (!dict) {
+		fclose(in);
+		return NULL;
+	}
+
+	memset(line, 0, ASCIILINESZ + 1);
+	memset(section, 0, ASCIILINESZ + 1);
+	memset(key, 0, ASCIILINESZ + 1);
+	memset(val, 0, ASCIILINESZ + 1);
+	last = 0;
+
+	while (fgets(line+last, ASCIILINESZ-last, in)!=NULL) {
+		lineno++;
+		len = (int) strlen(line)-1;
+		/* Safety check against buffer overflows */
+		if (line[len] != '\n') {
+			fprintf(stderr,
+					"ciniparser: input line too long in %s (%d)\n",
+					ininame,
+					lineno);
+			dictionary_del(dict);
+			fclose(in);
+			return NULL;
+		}
+
+		/* Get rid of \n and spaces at end of line */
+		while ((len >= 0) &&
+				((line[len] == '\n') || (isspace(line[len])))) {
+			line[len] = 0;
+			len--;
+		}
+
+		/* Detect multi-line */
+		if (len >= 0 && line[len] == '\\') {
+			/* Multi-line value */
+			last = len;
+			continue;
+		}
+
+		switch (ciniparser_line(line, section, key, val)) {
+		case LINE_EMPTY:
+		case LINE_COMMENT:
+			break;
+
+		case LINE_SECTION:
+			errs = dictionary_set(dict, section, NULL);
+			break;
+
+		case LINE_VALUE:
+			snprintf(tmp, ASCIILINESZ + 1, "%s:%s", section, key);
+			errs = dictionary_set(dict, tmp, val);
+			break;
+
+		case LINE_ERROR:
+			fprintf(stderr, "ciniparser: syntax error in %s (%d):\n",
+					ininame, lineno);
+			fprintf(stderr, "-> %s\n", line);
+			errs++;
+			break;
+
+		default:
+			break;
+		}
+		memset(line, 0, ASCIILINESZ);
+		last = 0;
+		if (errs < 0) {
+			fprintf(stderr, "ciniparser: memory allocation failure\n");
+			break;
+		}
+	}
+
+	if (errs) {
+		dictionary_del(dict);
+		dict = NULL;
+	}
+
+	fclose(in);
+
+	return dict;
+}
+
+void ciniparser_freedict(dictionary *d)
+{
+	dictionary_del(d);
+}
+
+/** @}
+ */
diff --git a/ccan/ciniparser/ciniparser.h b/ccan/ciniparser/ciniparser.h
new file mode 100644
index 0000000..b61c1d6
--- /dev/null
+++ b/ccan/ciniparser/ciniparser.h
@@ -0,0 +1,262 @@
+#ifndef _INIPARSER_H_
+#define _INIPARSER_H_
+
+/* Copyright (c) 2000-2007 by Nicolas Devillard.
+ * Copyright (x) 2009 by Tim Post <tinkertim@gmail.com>
+ * MIT License
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+/** @addtogroup ciniparser
+ * @{
+ */
+
+/**
+ * @file    ciniparser.h
+ * @author  N. Devillard
+ * @date    Sep 2007
+ * @version 3.0
+ * @brief   Parser for ini files.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "dictionary.h"
+
+#define ciniparser_getstr(d, k)  ciniparser_getstring(d, k, NULL)
+#define ciniparser_setstr        ciniparser_setstring
+
+/**
+ * @brief    Get number of sections in a dictionary
+ * @param    d   Dictionary to examine
+ * @return   int Number of sections found in dictionary, -1 on error
+ *
+ * This function returns the number of sections found in a dictionary.
+ * The test to recognize sections is done on the string stored in the
+ * dictionary: a section name is given as "section" whereas a key is
+ * stored as "section:key", thus the test looks for entries that do not
+ * contain a colon.
+ *
+ * This clearly fails in the case a section name contains a colon, but
+ * this should simply be avoided.
+ */
+int ciniparser_getnsec(dictionary *d);
+
+/**
+ * @brief    Get name for section n in a dictionary.
+ * @param    d   Dictionary to examine
+ * @param    n   Section number (from 0 to nsec-1).
+ * @return   Pointer to char string, NULL on error
+ *
+ * This function locates the n-th section in a dictionary and returns
+ * its name as a pointer to a string statically allocated inside the
+ * dictionary. Do not free or modify the returned string!
+ */
+char *ciniparser_getsecname(dictionary *d, int n);
+
+/**
+ * @brief    Save a dictionary to a loadable ini file
+ * @param    d   Dictionary to dump
+ * @param    f   Opened file pointer to dump to
+ * @return   void
+ *
+ * This function dumps a given dictionary into a loadable ini file.
+ * It is Ok to specify @c stderr or @c stdout as output files.
+ */
+void ciniparser_dump_ini(dictionary *d, FILE *f);
+
+/**
+ * @brief    Dump a dictionary to an opened file pointer.
+ * @param    d   Dictionary to dump.
+ * @param    f   Opened file pointer to dump to.
+ * @return   void
+ *
+ * This function prints out the contents of a dictionary, one element by
+ * line, onto the provided file pointer. It is OK to specify @c stderr
+ * or @c stdout as output files. This function is meant for debugging
+ * purposes mostly.
+ */
+void ciniparser_dump(dictionary *d, FILE *f);
+
+/**
+ * @brief    Get the string associated to a key
+ * @param    d       Dictionary to search
+ * @param    key     Key string to look for
+ * @param    def     Default value to return if key not found.
+ * @return   pointer to statically allocated character string
+ *
+ * This function queries a dictionary for a key. A key as read from an
+ * ini file is given as "section:key". If the key cannot be found,
+ * the pointer passed as 'def' is returned.
+ * The returned char pointer is pointing to a string allocated in
+ * the dictionary, do not free or modify it.
+ */
+char *ciniparser_getstring(dictionary *d, const char *key, char *def);
+
+/**
+ * @brief    Get the string associated to a key, convert to an int
+ * @param    d Dictionary to search
+ * @param    key Key string to look for
+ * @param    notfound Value to return in case of error
+ * @return   integer
+ *
+ * This function queries a dictionary for a key. A key as read from an
+ * ini file is given as "section:key". If the key cannot be found,
+ * the notfound value is returned.
+ *
+ * Supported values for integers include the usual C notation
+ * so decimal, octal (starting with 0) and hexadecimal (starting with 0x)
+ * are supported. Examples:
+ *
+ * - "42"      ->  42
+ * - "042"     ->  34 (octal -> decimal)
+ * - "0x42"    ->  66 (hexa  -> decimal)
+ *
+ * Warning: the conversion may overflow in various ways. Conversion is
+ * totally outsourced to strtol(), see the associated man page for overflow
+ * handling.
+ *
+ * Credits: Thanks to A. Becker for suggesting strtol()
+ */
+int ciniparser_getint(dictionary *d, const char *key, int notfound);
+
+/**
+ * @brief    Get the string associated to a key, convert to a double
+ * @param    d Dictionary to search
+ * @param    key Key string to look for
+ * @param    notfound Value to return in case of error
+ * @return   double
+ *
+ * This function queries a dictionary for a key. A key as read from an
+ * ini file is given as "section:key". If the key cannot be found,
+ * the notfound value is returned.
+ */
+double ciniparser_getdouble(dictionary *d, const char *key, double notfound);
+
+/**
+ * @brief    Get the string associated to a key, convert to a boolean
+ * @param    d Dictionary to search
+ * @param    key Key string to look for
+ * @param    notfound Value to return in case of error
+ * @return   integer
+ *
+ * This function queries a dictionary for a key. A key as read from an
+ * ini file is given as "section:key". If the key cannot be found,
+ * the notfound value is returned.
+ *
+ * A true boolean is found if one of the following is matched:
+ *
+ * - A string starting with 'y'
+ * - A string starting with 'Y'
+ * - A string starting with 't'
+ * - A string starting with 'T'
+ * - A string starting with '1'
+ *
+ * A false boolean is found if one of the following is matched:
+ *
+ * - A string starting with 'n'
+ * - A string starting with 'N'
+ * - A string starting with 'f'
+ * - A string starting with 'F'
+ * - A string starting with '0'
+ *
+ * The notfound value returned if no boolean is identified, does not
+ * necessarily have to be 0 or 1.
+ */
+int ciniparser_getboolean(dictionary *d, const char *key, int notfound);
+
+/**
+ * @brief    Set an entry in a dictionary.
+ * @param    ini     Dictionary to modify.
+ * @param    entry   Entry to modify (entry name)
+ * @param    val     New value to associate to the entry.
+ * @return   int 0 if Ok, -1 otherwise.
+ *
+ * If the given entry can be found in the dictionary, it is modified to
+ * contain the provided value. If it cannot be found, -1 is returned.
+ * It is Ok to set val to NULL.
+ */
+int ciniparser_setstring(dictionary *ini, char *entry, char *val);
+
+/**
+ * @brief    Delete an entry in a dictionary
+ * @param    ini     Dictionary to modify
+ * @param    entry   Entry to delete (entry name)
+ * @return   void
+ *
+ * If the given entry can be found, it is deleted from the dictionary.
+ */
+void ciniparser_unset(dictionary *ini, char *entry);
+
+/**
+ * @brief    Finds out if a given entry exists in a dictionary
+ * @param    ini     Dictionary to search
+ * @param    entry   Name of the entry to look for
+ * @return   integer 1 if entry exists, 0 otherwise
+ *
+ * Finds out if a given entry exists in the dictionary. Since sections
+ * are stored as keys with NULL associated values, this is the only way
+ * of querying for the presence of sections in a dictionary.
+ */
+int ciniparser_find_entry(dictionary *ini, char *entry) ;
+
+/**
+ * @brief    Parse an ini file and return an allocated dictionary object
+ * @param    ininame Name of the ini file to read.
+ * @return   Pointer to newly allocated dictionary
+ *
+ * This is the parser for ini files. This function is called, providing
+ * the name of the file to be read. It returns a dictionary object that
+ * should not be accessed directly, but through accessor functions
+ * instead.
+ *
+ * The returned dictionary must be freed using ciniparser_freedict().
+ */
+dictionary *ciniparser_load(const char *ininame);
+
+/**
+ * @brief    Free all memory associated to an ini dictionary
+ * @param    d Dictionary to free
+ * @return   void
+ *
+ * Free all memory associated to an ini dictionary.
+ * It is mandatory to call this function before the dictionary object
+ * gets out of the current context.
+ */
+void ciniparser_freedict(dictionary *d);
+
+/**
+ * @brief Set an item in the dictionary
+ * @param d      Dictionary object created by ciniparser_load()
+ * @param entry  Entry in the dictionary to manipulate
+ * @param val    Value to assign to the entry
+ * @return       0 on success, -1 on error
+ *
+ * Remember that string values are converted by ciniparser_getboolean(),
+ * ciniparser_getdouble(), etc. It is also OK to set an entry to NULL.
+ */
+int ciniparser_set(dictionary *d, char *entry, char *val);
+
+#endif
+/** @}
+ */
diff --git a/ccan/ciniparser/dictionary.c b/ccan/ciniparser/dictionary.c
new file mode 100644
index 0000000..19dd641
--- /dev/null
+++ b/ccan/ciniparser/dictionary.c
@@ -0,0 +1,266 @@
+/* Copyright (c) 2000-2007 by Nicolas Devillard.
+ * Copyright (x) 2009 by Tim Post <tinkertim@gmail.com>
+ * MIT License
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+/** @addtogroup ciniparser
+ * @{
+ */
+/**
+ * @file dictionary.c
+ * @author N. Devillard
+ * @date Sep 2007
+ * @version $Revision: 1.27 $
+ * @brief Implements a dictionary for string variables.
+ *
+ * This module implements a simple dictionary object, i.e. a list
+ * of string/string associations. This object is useful to store e.g.
+ * information retrieved from a configuration file (ini files).
+ */
+
+#include "dictionary.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+/** Maximum value size for integers and doubles. */
+#define MAXVALSZ	1024
+
+/** Minimal allocated number of entries in a dictionary */
+#define DICTMINSZ	128
+
+/** Invalid key token */
+#define DICT_INVALID_KEY	((char*)-1)
+
+/**
+ * @brief Double the allocated size associated to a pointer
+ * @param size the current allocated size
+ * @return re-allocated pointer on success, NULL on failure
+ */
+static void *mem_double(void *ptr, int size)
+{
+	void *newptr;
+
+	newptr = calloc(2 * size, 1);
+	if (newptr == NULL) {
+		return NULL;
+	}
+	memcpy(newptr, ptr, size);
+	free(ptr);
+	return newptr;
+}
+
+/* The remaining exposed functions are documented in dictionary.h */
+
+unsigned dictionary_hash(const char *key)
+{
+	int len;
+	unsigned hash;
+	int i;
+
+	len = strlen(key);
+	for (hash = 0, i = 0; i < len; i++) {
+		hash += (unsigned) key[i];
+		hash += (hash << 10);
+		hash ^= (hash >> 6);
+	}
+	hash += (hash << 3);
+	hash ^= (hash >> 11);
+	hash += (hash << 15);
+	return hash;
+}
+
+dictionary *dictionary_new(int size)
+{
+	dictionary *d;
+
+	/* If no size was specified, allocate space for DICTMINSZ */
+	if (size<DICTMINSZ) size=DICTMINSZ;
+
+	if (!(d = (dictionary *) calloc(1, sizeof(dictionary)))) {
+		return NULL;
+	}
+	d->size = size;
+	d->val  = (char **) calloc(size, sizeof(char *));
+	d->key  = (char **) calloc(size, sizeof(char *));
+	d->hash = (unsigned int *) calloc(size, sizeof(unsigned));
+	return d;
+}
+
+void dictionary_del(dictionary *d)
+{
+	int i;
+
+	if (d == NULL)
+		return;
+	for (i = 0; i < d->size; i++) {
+		if (d->key[i] != NULL)
+			free(d->key[i]);
+		if (d->val[i] != NULL)
+			free(d->val[i]);
+	}
+	free(d->val);
+	free(d->key);
+	free(d->hash);
+	free(d);
+	return;
+}
+
+char *dictionary_get(dictionary *d, const char *key, char *def)
+{
+	unsigned hash;
+	int i;
+
+	hash = dictionary_hash(key);
+	for (i=0; i < d->size; i++) {
+		if (d->key[i] == NULL)
+			continue;
+		/* Compare hash */
+		if (hash == d->hash[i]) {
+			/* Compare string, to avoid hash collisions */
+			if (!strcmp(key, d->key[i])) {
+				return d->val[i];
+			}
+		}
+	}
+	return def;
+}
+
+int dictionary_set(dictionary *d, const char *key, char *val)
+{
+	int i;
+	unsigned hash;
+
+	if (d==NULL || key==NULL)
+		return -1;
+
+	/* Compute hash for this key */
+	hash = dictionary_hash(key);
+	/* Find if value is already in dictionary */
+	if (d->n > 0) {
+		for (i = 0; i < d->size; i++) {
+			if (d->key[i] == NULL)
+				continue;
+			/* Same hash value */
+			if (hash == d->hash[i]) {
+				/* Same key */
+				if (!strcmp(key, d->key[i])) {
+					/* Found a value: modify and return */
+					if (d->val[i] != NULL)
+						free(d->val[i]);
+					d->val[i] = val ? strdup(val) : NULL;
+					/* Value has been modified: return */
+					return 0;
+				}
+			}
+		}
+	}
+
+	/* Add a new value
+	 * See if dictionary needs to grow */
+	if (d->n == d->size) {
+		/* Reached maximum size: reallocate dictionary */
+		d->val  = (char **) mem_double(d->val, d->size * sizeof(char *));
+		d->key  = (char **) mem_double(d->key, d->size * sizeof(char *));
+		d->hash = (unsigned int *)
+			mem_double(d->hash, d->size * sizeof(unsigned));
+		if ((d->val == NULL) || (d->key == NULL) || (d->hash == NULL))
+			/* Cannot grow dictionary */
+			return -1;
+		/* Double size */
+		d->size *= 2;
+	}
+
+	/* Insert key in the first empty slot */
+	for (i = 0; i < d->size; i++) {
+		if (d->key[i] == NULL) {
+			/* Add key here */
+			break;
+		}
+	}
+	/* Copy key */
+	d->key[i] = strdup(key);
+	d->val[i] = val ? strdup(val) : NULL;
+	d->hash[i] = hash;
+	d->n ++;
+	return 0;
+}
+
+void dictionary_unset(dictionary *d, const char *key)
+{
+	unsigned hash;
+	int i;
+
+	if (key == NULL)
+		return;
+
+	hash = dictionary_hash(key);
+	for (i = 0; i < d->size; i++) {
+		if (d->key[i] == NULL)
+			continue;
+		/* Compare hash */
+		if (hash == d->hash[i]) {
+			/* Compare string, to avoid hash collisions */
+			if (!strcmp(key, d->key[i])) {
+				/* Found key */
+				break;
+			}
+		}
+	}
+	if (i >= d->size)
+		/* Key not found */
+		return;
+
+	free(d->key[i]);
+	d->key[i] = NULL;
+	if (d->val[i]!=NULL) {
+		free(d->val[i]);
+		d->val[i] = NULL;
+	}
+	d->hash[i] = 0;
+	d->n --;
+	return;
+}
+
+void dictionary_dump(dictionary *d, FILE *out)
+{
+	int i;
+
+	if (d == NULL || out == NULL)
+		return;
+	if (d->n < 1) {
+		fprintf(out, "empty dictionary\n");
+		return;
+	}
+	for (i = 0; i < d->size; i++) {
+		if (d->key[i]) {
+			fprintf(out, "%20s\t[%s]\n",
+				d->key[i],
+				d->val[i] ? d->val[i] : "UNDEF");
+		}
+	}
+	return;
+}
+
+/** @}
+ */
diff --git a/ccan/ciniparser/dictionary.h b/ccan/ciniparser/dictionary.h
new file mode 100644
index 0000000..a94ea1a
--- /dev/null
+++ b/ccan/ciniparser/dictionary.h
@@ -0,0 +1,166 @@
+#ifndef _DICTIONARY_H_
+#define _DICTIONARY_H_
+
+/* Copyright (c) 2000-2007 by Nicolas Devillard.
+ * Copyright (x) 2009 by Tim Post <tinkertim@gmail.com>
+ * MIT License
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+/** @addtogroup ciniparser
+ * @{
+ */
+/**
+ * @file    dictionary.h
+ * @author  N. Devillard
+ * @date    Sep 2007
+ * @version $Revision: 1.12 $
+ * @brief   Implements a dictionary for string variables.
+ *
+ * This module implements a simple dictionary object, i.e. a list
+ * of string/string associations. This object is useful to store e.g.
+ * information retrieved from a configuration file (ini files).
+ */
+
+
+/**
+ * @brief Dictionary object
+ * @param n Number of entries in the dictionary
+ * @param size Storage size
+ * @param val List of string values
+ * @param key List of string keys
+ * @param hash List of hash values for keys
+ *
+ * This object contains a list of string/string associations. Each
+ * association is identified by a unique string key. Looking up values
+ * in the dictionary is speeded up by the use of a (hopefully collision-free)
+ * hash function.
+ */
+typedef struct _dictionary_ {
+	int n;
+	int size;
+	char **val;
+	char **key;
+	unsigned *hash;
+} dictionary;
+
+/**
+ * @brief Compute the hash key for a string.
+ * @param key Character string to use for key.
+ * @return 1 unsigned int on at least 32 bits.
+ *
+ * This hash function has been taken from an Article in Dr Dobbs Journal.
+ * This is normally a collision-free function, distributing keys evenly.
+ * The key is stored anyway in the struct so that collision can be avoided
+ * by comparing the key itself in last resort.
+ */
+unsigned dictionary_hash(const char *key);
+
+/**
+ * @brief Create a new dictionary object.
+ * @param size Optional initial size of the dictionary.
+ * @return allocated dictionary object on success, NULL on failure
+ *
+ * This function allocates a new dictionary object of given size and returns
+ * it. If you do not know in advance (roughly) the number of entries in the
+ * dictionary, give size=0.
+ */
+dictionary *dictionary_new(int size);
+
+/**
+ * @brief Delete a dictionary object
+ * @param d dictionary object to deallocate.
+ * @return void
+ *
+ * Deallocate a dictionary object and all memory associated to it.
+ */
+void dictionary_del(dictionary *vd);
+
+/**
+ * @brief Get a value from a dictionary.
+ * @param d dictionary object to search.
+ * @param key Key to look for in the dictionary.
+ * @param def Default value to return if key not found.
+ * @return 1 pointer to internally allocated character string.
+ *
+ * This function locates a key in a dictionary and returns a pointer to its
+ * value, or the passed 'def' pointer if no such key can be found in
+ * dictionary. The returned character pointer points to data internal to the
+ * dictionary object, you should not try to free it or modify it.
+ */
+char *dictionary_get(dictionary *d, const char *key, char *def);
+
+/**
+ * @brief Set a value in a dictionary.
+ * @param d dictionary object to modify.
+ * @param key Key to modify or add.
+ * @param val Value to add.
+ * @return int 0 if Ok, anything else otherwise
+ *
+ * If the given key is found in the dictionary, the associated value is
+ * replaced by the provided one. If the key cannot be found in the
+ * dictionary, it is added to it.
+ *
+ * It is Ok to provide a NULL value for val, but NULL values for the dictionary
+ * or the key are considered as errors: the function will return immediately
+ * in such a case.
+ *
+ * Notice that if you dictionary_set a variable to NULL, a call to
+ * dictionary_get will return a NULL value: the variable will be found, and
+ * its value (NULL) is returned. In other words, setting the variable
+ * content to NULL is equivalent to deleting the variable from the
+ * dictionary. It is not possible (in this implementation) to have a key in
+ * the dictionary without value.
+ *
+ * This function returns non-zero in case of failure.
+ */
+int dictionary_set(dictionary *vd, const char *key, char *val);
+
+/**
+ * @brief Delete a key in a dictionary
+ * @param d dictionary object to modify.
+ * @param key Key to remove.
+ * @return void
+ *
+ * This function deletes a key in a dictionary. Nothing is done if the
+ * key cannot be found.
+ */
+void dictionary_unset(dictionary *d, const char *key);
+
+/**
+ * @brief Dump a dictionary to an opened file pointer.
+ * @param d Dictionary to dump
+ * @param out Opened file pointer
+ * @return void
+ *
+ * Dumps a dictionary onto an opened file pointer. Key pairs are printed out
+ * as @c [Key]=[Value], one per line. It is Ok to provide stdout or stderr as
+ * output file pointers.
+ */
+void dictionary_dump(dictionary *d, FILE *out);
+
+#endif
+/** @}
+ */
-- 
2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [RFC ndctl PATCH 2/3] ndctl, util: add parse-configs helper
  2021-05-16 23:14 [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file QI Fuli
  2021-05-16 23:14 ` [RFC ndctl PATCH 1/3] ndctl, ccan: import ciniparser QI Fuli
@ 2021-05-16 23:14 ` QI Fuli
  2021-05-16 23:14 ` [RFC ndctl PATCH 3/3] ndctl, rename monitor.conf to ndctl.conf QI Fuli
  2021-06-02  5:31 ` [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file Verma, Vishal L
  3 siblings, 0 replies; 10+ messages in thread
From: QI Fuli @ 2021-05-16 23:14 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: QI Fuli

From: QI Fuli <qi.fuli@fujitsu.com>

Add parse-config helper to help ndctl commands read the ndctl global
configuration file.

Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
---
 Makefile.am          |  2 ++
 util/parse-configs.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
 util/parse-configs.h | 26 ++++++++++++++++++++++++
 3 files changed, 75 insertions(+)
 create mode 100644 util/parse-configs.c
 create mode 100644 util/parse-configs.h

diff --git a/Makefile.am b/Makefile.am
index 960b5e9..6e50741 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -74,6 +74,8 @@ noinst_LIBRARIES += libutil.a
 libutil_a_SOURCES = \
 	util/parse-options.c \
 	util/parse-options.h \
+	util/parse-configs.c \
+	util/parse-configs.h \
 	util/usage.c \
 	util/size.c \
 	util/main.c \
diff --git a/util/parse-configs.c b/util/parse-configs.c
new file mode 100644
index 0000000..d404786
--- /dev/null
+++ b/util/parse-configs.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2021, FUJITSU LIMITED. ALL rights reserved.
+
+#include <errno.h>
+#include <util/parse-configs.h>
+#include <util/strbuf.h>
+#include <ccan/ciniparser/ciniparser.h>
+
+static void set_value(const char **value, char *val)
+{
+	struct strbuf buf = STRBUF_INIT;
+	size_t len = *value ? strlen(*value) : 0;
+
+	if (!val)
+		return;
+
+	if (len) {
+		strbuf_add(&buf, *value, len);
+		strbuf_addstr(&buf, " ");
+	}
+	strbuf_addstr(&buf, val);
+	*value = strbuf_detach(&buf, NULL);
+}
+
+int parse_configs(const char *config_file, const struct config *configs)
+{
+	dictionary *dic;
+
+	dic = ciniparser_load(config_file);
+	if (!dic)
+		return -errno;
+
+	for (; configs->type != CONFIG_END; configs++) {
+		switch (configs->type) {
+		case CONFIG_STRING:
+			set_value((const char **)configs->value,
+					ciniparser_getstring(dic,
+					configs->key, configs->defval));
+			break;
+		case CONFIG_END:
+			break;
+		}
+	}
+
+	ciniparser_freedict(dic);
+	return 0;
+}
diff --git a/util/parse-configs.h b/util/parse-configs.h
new file mode 100644
index 0000000..31886f7
--- /dev/null
+++ b/util/parse-configs.h
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2021, FUJITSU LIMITED. ALL rights reserved.
+
+#include <stdbool.h>
+#include <stdint.h>
+#include <util/util.h>
+
+enum parse_conf_type {
+	CONFIG_STRING,
+	CONFIG_END,
+};
+
+struct config {
+	enum parse_conf_type type;
+	const char *key;
+	void *value;
+	void *defval;
+};
+
+#define check_vtype(v, type) ( BUILD_BUG_ON_ZERO(!__builtin_types_compatible_p(typeof(v), type)) + v )
+
+#define CONF_END() { .type = CONFIG_END }
+#define CONF_STR(k,v,d) \
+	{ .type = CONFIG_STRING, .key = (k), .value = check_vtype(v, const char **), .defval = (d) }
+
+int parse_configs(const char *config_file, const struct config *configs);
-- 
2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [RFC ndctl PATCH 3/3] ndctl, rename monitor.conf to ndctl.conf
  2021-05-16 23:14 [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file QI Fuli
  2021-05-16 23:14 ` [RFC ndctl PATCH 1/3] ndctl, ccan: import ciniparser QI Fuli
  2021-05-16 23:14 ` [RFC ndctl PATCH 2/3] ndctl, util: add parse-configs helper QI Fuli
@ 2021-05-16 23:14 ` QI Fuli
  2021-06-02  6:33   ` Verma, Vishal L
  2021-06-02  5:31 ` [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file Verma, Vishal L
  3 siblings, 1 reply; 10+ messages in thread
From: QI Fuli @ 2021-05-16 23:14 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: QI Fuli

From: QI Fuli <qi.fuli@fujitsu.com>

Rename monitor.conf to ndctl.conf, and make it a ndclt global
configuration file that all commands can refer to.
Refactor monitor to make it work with ndctl.conf.

Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
---
 configure.ac                       |   8 +-
 ndctl/Makefile.am                  |   9 +-
 ndctl/monitor.c                    | 127 +++++------------------------
 ndctl/{monitor.conf => ndctl.conf} |  16 +++-
 4 files changed, 40 insertions(+), 120 deletions(-)
 rename ndctl/{monitor.conf => ndctl.conf} (82%)

diff --git a/configure.ac b/configure.ac
index 5ec8d2f..ab2d8a3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -171,10 +171,10 @@ fi
 AC_SUBST([systemd_unitdir])
 AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
 
-ndctl_monitorconfdir=${sysconfdir}/ndctl
-ndctl_monitorconf=monitor.conf
-AC_SUBST([ndctl_monitorconfdir])
-AC_SUBST([ndctl_monitorconf])
+ndctl_confdir=${sysconfdir}/ndctl
+ndctl_conf=ndctl.conf
+AC_SUBST([ndctl_confdir])
+AC_SUBST([ndctl_conf])
 
 daxctl_modprobe_datadir=${datadir}/daxctl
 daxctl_modprobe_data=daxctl.conf
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index a63b1e0..b107b3d 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -7,7 +7,7 @@ BUILT_SOURCES = config.h
 config.h: $(srcdir)/Makefile.am
 	$(AM_V_GEN) echo "/* Autogenerated by ndctl/Makefile.am */" >$@ && \
 	echo '#define NDCTL_CONF_FILE \
-		"$(ndctl_monitorconfdir)/$(ndctl_monitorconf)"' >>$@
+		"$(ndctl_confdir)/$(ndctl_conf)"' >>$@
 	$(AM_V_GEN) echo '#define NDCTL_KEYS_DIR  "$(ndctl_keysdir)"' >>$@
 
 ndctl_SOURCES = ndctl.c \
@@ -42,7 +42,7 @@ keys_configdir = $(ndctl_keysdir)
 keys_config_DATA = $(ndctl_keysreadme)
 endif
 
-EXTRA_DIST += keys.readme monitor.conf ndctl-monitor.service
+EXTRA_DIST += keys.readme ndctl.conf ndctl-monitor.service
 
 if ENABLE_DESTRUCTIVE
 ndctl_SOURCES += ../test/blk_namespaces.c \
@@ -54,6 +54,7 @@ ndctl_LDADD =\
 	lib/libndctl.la \
 	../daxctl/lib/libdaxctl.la \
 	../libutil.a \
+	../libccan.a \
 	$(UUID_LIBS) \
 	$(KMOD_LIBS) \
 	$(JSON_LIBS)
@@ -73,8 +74,8 @@ ndctl_SOURCES += ../test/libndctl.c \
 		 test.c
 endif
 
-monitor_configdir = $(ndctl_monitorconfdir)
-monitor_config_DATA = $(ndctl_monitorconf)
+ndctl_configdir = $(ndctl_confdir)
+ndctl_config_DATA = $(ndctl_conf)
 
 if ENABLE_SYSTEMD_UNITS
 systemd_unit_DATA = ndctl-monitor.service
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
index ca36179..ea707d2 100644
--- a/ndctl/monitor.c
+++ b/ndctl/monitor.c
@@ -10,11 +10,13 @@
 #include <util/filter.h>
 #include <util/util.h>
 #include <util/parse-options.h>
+#include <util/parse-configs.h>
 #include <util/strbuf.h>
 #include <ndctl/config.h>
 #include <ndctl/ndctl.h>
 #include <ndctl/libndctl.h>
 #include <sys/epoll.h>
+#include <stdbool.h>
 #define BUF_SIZE 2048
 
 /* reuse the core log helpers for the monitor logger */
@@ -463,113 +465,6 @@ out:
 	return rc;
 }
 
-static void parse_config(const char **arg, char *key, char *val, char *ident)
-{
-	struct strbuf value = STRBUF_INIT;
-	size_t arg_len = *arg ? strlen(*arg) : 0;
-
-	if (!ident || !key || (strcmp(ident, key) != 0))
-		return;
-
-	if (arg_len) {
-		strbuf_add(&value, *arg, arg_len);
-		strbuf_addstr(&value, " ");
-	}
-	strbuf_addstr(&value, val);
-	*arg = strbuf_detach(&value, NULL);
-}
-
-static int read_config_file(struct ndctl_ctx *ctx, struct monitor *_monitor,
-		struct util_filter_params *_param)
-{
-	FILE *f;
-	size_t len = 0;
-	int line = 0, rc = 0;
-	char *buf = NULL, *seek, *value, *config_file;
-
-	if (_monitor->config_file)
-		config_file = strdup(_monitor->config_file);
-	else
-		config_file = strdup(NDCTL_CONF_FILE);
-	if (!config_file) {
-		fail("strdup default config file failed\n");
-		rc = -ENOMEM;
-		goto out;
-	}
-
-	buf = malloc(BUF_SIZE);
-	if (!buf) {
-		fail("malloc read config-file buf error\n");
-		rc = -ENOMEM;
-		goto out;
-	}
-	seek = buf;
-
-	f = fopen(config_file, "r");
-	if (!f) {
-		if (_monitor->config_file) {
-			err(&monitor, "config-file: %s cannot be opened\n",
-				config_file);
-			rc = -errno;
-		}
-		goto out;
-	}
-
-	while (fgets(seek, BUF_SIZE, f)) {
-		value = NULL;
-		line++;
-
-		while (isspace(*seek))
-			seek++;
-
-		if (*seek == '#' || *seek == '\0')
-			continue;
-
-		value = strchr(seek, '=');
-		if (!value) {
-			fail("config-file syntax error, skip line[%i]\n", line);
-			continue;
-		}
-
-		value[0] = '\0';
-		value++;
-
-		while (isspace(value[0]))
-			value++;
-
-		len = strlen(seek);
-		if (len == 0)
-			continue;
-		while (isspace(seek[len-1]))
-			len--;
-		seek[len] = '\0';
-
-		len = strlen(value);
-		if (len == 0)
-			continue;
-		while (isspace(value[len-1]))
-			len--;
-		value[len] = '\0';
-
-		if (len == 0)
-			continue;
-
-		parse_config(&_param->bus, "bus", value, seek);
-		parse_config(&_param->dimm, "dimm", value, seek);
-		parse_config(&_param->region, "region", value, seek);
-		parse_config(&_param->namespace, "namespace", value, seek);
-		parse_config(&_monitor->dimm_event, "dimm-event", value, seek);
-
-		if (!_monitor->log)
-			parse_config(&_monitor->log, "log", value, seek);
-	}
-	fclose(f);
-out:
-	free(buf);
-	free(config_file);
-	return rc;
-}
-
 int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
 {
 	const struct option options[] = {
@@ -601,6 +496,19 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
 		"ndctl monitor [<options>]",
 		NULL
 	};
+	const struct config configs[] = {
+		CONF_STR("core:bus", &param.bus, NULL),
+		CONF_STR("core:region", &param.region, NULL),
+		CONF_STR("core:dimm", &param.dimm, NULL),
+		CONF_STR("core:namespace", &param.namespace, NULL),
+		CONF_STR("monitor:bus", &param.bus, NULL),
+		CONF_STR("monitor:region", &param.region, NULL),
+		CONF_STR("monitor:dimm", &param.dimm, NULL),
+		CONF_STR("monitor:namespace", &param.namespace, NULL),
+		CONF_STR("monitor:dimm-event", &monitor.dimm_event, NULL),
+		//CONF_FILE("monitor:log", &monitor.log, NULL),
+		CONF_END(),
+	};
 	const char *prefix = "./";
 	struct util_filter_ctx fctx = { 0 };
 	struct monitor_filter_arg mfa = { 0 };
@@ -621,7 +529,10 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
 	else
 		monitor.ctx.log_priority = LOG_INFO;
 
-	rc = read_config_file(ctx, &monitor, &param);
+	if (monitor.config_file)
+		rc = parse_configs(monitor.config_file, configs);
+	else
+		rc = parse_configs(NDCTL_CONF_FILE, configs);
 	if (rc)
 		goto out;
 
diff --git a/ndctl/monitor.conf b/ndctl/ndctl.conf
similarity index 82%
rename from ndctl/monitor.conf
rename to ndctl/ndctl.conf
index 934e2c0..85343b0 100644
--- a/ndctl/monitor.conf
+++ b/ndctl/ndctl.conf
@@ -1,14 +1,22 @@
-# This is the main ndctl monitor configuration file. It contains the
-# configuration directives that give ndctl monitor instructions.
-# You can change the configuration of ndctl monitor by editing this
+# This is the ndctl global configuration file. It contains the
+# configuration directives that give ndctl instructions.
+# You can change the configuration of ndctl by editing this
 # file or using [--config-file=<file>] option to override this one.
-# The changed value will work after restart ndctl monitor service.
+# The changed value will work after restart ndctl services.
 
 # In this file, lines starting with a hash (#) are comments.
 # The configurations should follow <key> = <value> style.
 # Multiple space-separated values are allowed, but except the following
 # characters: : ? / \ % " ' $ & ! * { } [ ] ( ) = < > @
 
+[core]
+# The values in the [core] section work for all ndctl commands.
+# dimm = all
+# bus = all
+# region = all
+# namespace = all
+
+[monitor]
 # The objects to monitor are filtered via dimm's name by setting key "dimm".
 # If this value is different from the value of [--dimm=<value>] option,
 # both of the values will work.
-- 
2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* Re: [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file
  2021-05-16 23:14 [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file QI Fuli
                   ` (2 preceding siblings ...)
  2021-05-16 23:14 ` [RFC ndctl PATCH 3/3] ndctl, rename monitor.conf to ndctl.conf QI Fuli
@ 2021-06-02  5:31 ` Verma, Vishal L
  2021-06-02 16:47   ` Dan Williams
  3 siblings, 1 reply; 10+ messages in thread
From: Verma, Vishal L @ 2021-06-02  5:31 UTC (permalink / raw)
  To: fukuri.sai, nvdimm; +Cc: qi.fuli

[switching to the new mailing list]

On Mon, 2021-05-17 at 08:14 +0900, QI Fuli wrote:
> From: QI Fuli <qi.fuli@fujitsu.com>
> 
> This patch set is to rename monitor.conf to ndctl.conf, and make it a
> global ndctl configuration file that all ndctl commands can refer to.
> 
> As this patch set has been pending until now, I would like to know if
> current idea works or not. If yes, I will finish the documents and test.
> 
> Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>

Hi Qi,

Thanks for picking up on this! The approach generally looks good to me,
I think we can definitely move forward with this direction.

One thing that stands out is - I don't think we can simply rename the
existing monitor.conf. We have to keep supporting the 'legacy'
monitor.conf so that we don't break any deployments. I'd suggest
keeping the old monitor.conf as is, and continuing to parse it as is,
but also adding a new ndctl.conf as you have done.

We can indicate that 'monitor.conf' is legacy, and any new features
will only get added to the new global config to encourage migration to
the new config. Perhaps we can even provide a helper script to migrate
the old config to new - but I think it needs to be a user triggered
action.

This is timely as I also need to go add some config related
functionality to daxctl, and basing it on this would be perfect, so I'd
love to get this series merged in soon.

Thanks again!
-Vishal

> 
> QI Fuli (3):
>   ndctl, ccan: import ciniparser
>   ndctl, util: add parse-configs helper
>   ndctl, rename monitor.conf to ndctl.conf
> 
>  Makefile.am                        |   8 +-
>  ccan/ciniparser/ciniparser.c       | 480 +++++++++++++++++++++++++++++
>  ccan/ciniparser/ciniparser.h       | 262 ++++++++++++++++
>  ccan/ciniparser/dictionary.c       | 266 ++++++++++++++++
>  ccan/ciniparser/dictionary.h       | 166 ++++++++++
>  configure.ac                       |   8 +-
>  ndctl/Makefile.am                  |   9 +-
>  ndctl/monitor.c                    | 127 ++------
>  ndctl/{monitor.conf => ndctl.conf} |  16 +-
>  util/parse-configs.c               |  47 +++
>  util/parse-configs.h               |  26 ++
>  11 files changed, 1294 insertions(+), 121 deletions(-)
>  create mode 100644 ccan/ciniparser/ciniparser.c
>  create mode 100644 ccan/ciniparser/ciniparser.h
>  create mode 100644 ccan/ciniparser/dictionary.c
>  create mode 100644 ccan/ciniparser/dictionary.h
>  rename ndctl/{monitor.conf => ndctl.conf} (82%)
>  create mode 100644 util/parse-configs.c
>  create mode 100644 util/parse-configs.h
> 


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

* Re: [RFC ndctl PATCH 3/3] ndctl, rename monitor.conf to ndctl.conf
  2021-05-16 23:14 ` [RFC ndctl PATCH 3/3] ndctl, rename monitor.conf to ndctl.conf QI Fuli
@ 2021-06-02  6:33   ` Verma, Vishal L
  0 siblings, 0 replies; 10+ messages in thread
From: Verma, Vishal L @ 2021-06-02  6:33 UTC (permalink / raw)
  To: fukuri.sai, nvdimm; +Cc: qi.fuli

On Mon, 2021-05-17 at 08:14 +0900, QI Fuli wrote:
> From: QI Fuli <qi.fuli@fujitsu.com>
> 
> Rename monitor.conf to ndctl.conf, and make it a ndclt global
> configuration file that all commands can refer to.
> Refactor monitor to make it work with ndctl.conf.
> 
> Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> ---
>  configure.ac                       |   8 +-
>  ndctl/Makefile.am                  |   9 +-
>  ndctl/monitor.c                    | 127 +++++------------------------
>  ndctl/{monitor.conf => ndctl.conf} |  16 +++-
>  4 files changed, 40 insertions(+), 120 deletions(-)
>  rename ndctl/{monitor.conf => ndctl.conf} (82%)
> 

[snip]

> @@ -601,6 +496,19 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
>  		"ndctl monitor [<options>]",
>  		NULL
>  	};
> +	const struct config configs[] = {
> +		CONF_STR("core:bus", &param.bus, NULL),
> +		CONF_STR("core:region", &param.region, NULL),
> +		CONF_STR("core:dimm", &param.dimm, NULL),
> +		CONF_STR("core:namespace", &param.namespace, NULL),
> +		CONF_STR("monitor:bus", &param.bus, NULL),
> +		CONF_STR("monitor:region", &param.region, NULL),
> +		CONF_STR("monitor:dimm", &param.dimm, NULL),
> +		CONF_STR("monitor:namespace", &param.namespace, NULL),
> +		CONF_STR("monitor:dimm-event", &monitor.dimm_event, NULL),
> +		//CONF_FILE("monitor:log", &monitor.log, NULL),
> +		CONF_END(),
> +	};
>  	const char *prefix = "./";
>  	struct util_filter_ctx fctx = { 0 };
>  	struct monitor_filter_arg mfa = { 0 };
> @@ -621,7 +529,10 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
>  	else
>  		monitor.ctx.log_priority = LOG_INFO;
>  
> -	rc = read_config_file(ctx, &monitor, &param);
> +	if (monitor.config_file)
> +		rc = parse_configs(monitor.config_file, configs);
> +	else
> +		rc = parse_configs(NDCTL_CONF_FILE, configs);
>  	if (rc)
>  		goto out;

If I'm reading this right, it looks like this is set up so that params
from the config file will override any params from the command line,
which I think can be surprising behavior. The command line should
always override anything in the config file.

I suspect we'll need to collect CLI params in a 'param_cli' structure,
config params in a 'param_config' structure, and then consolidate them
to create the final 'param' structure that gets passed to
util_filter_walk etc.


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

* Re: [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file
  2021-06-02  5:31 ` [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file Verma, Vishal L
@ 2021-06-02 16:47   ` Dan Williams
  2021-06-02 17:15     ` Verma, Vishal L
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Williams @ 2021-06-02 16:47 UTC (permalink / raw)
  To: Verma, Vishal L; +Cc: fukuri.sai, nvdimm, qi.fuli

On Tue, Jun 1, 2021 at 10:31 PM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> [switching to the new mailing list]
>
> On Mon, 2021-05-17 at 08:14 +0900, QI Fuli wrote:
> > From: QI Fuli <qi.fuli@fujitsu.com>
> >
> > This patch set is to rename monitor.conf to ndctl.conf, and make it a
> > global ndctl configuration file that all ndctl commands can refer to.
> >
> > As this patch set has been pending until now, I would like to know if
> > current idea works or not. If yes, I will finish the documents and test.
> >
> > Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
>
> Hi Qi,
>
> Thanks for picking up on this! The approach generally looks good to me,
> I think we can definitely move forward with this direction.
>
> One thing that stands out is - I don't think we can simply rename the
> existing monitor.conf. We have to keep supporting the 'legacy'
> monitor.conf so that we don't break any deployments. I'd suggest
> keeping the old monitor.conf as is, and continuing to parse it as is,
> but also adding a new ndctl.conf as you have done.
>
> We can indicate that 'monitor.conf' is legacy, and any new features
> will only get added to the new global config to encourage migration to
> the new config. Perhaps we can even provide a helper script to migrate
> the old config to new - but I think it needs to be a user triggered
> action.
>
> This is timely as I also need to go add some config related
> functionality to daxctl, and basing it on this would be perfect, so I'd
> love to get this series merged in soon.

I wonder if ndctl should treat /etc/ndctl like a conf.d directory of
which all files with the .conf suffix are concatenated into one
combined configuration file. I.e. something that maintains legacy, but
also allows for config fragments to be deployed individually.

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

* Re: [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file
  2021-06-02 16:47   ` Dan Williams
@ 2021-06-02 17:15     ` Verma, Vishal L
  2021-06-17  0:25       ` qi.fuli
  0 siblings, 1 reply; 10+ messages in thread
From: Verma, Vishal L @ 2021-06-02 17:15 UTC (permalink / raw)
  To: Williams, Dan J; +Cc: fukuri.sai, nvdimm, qi.fuli

On Wed, 2021-06-02 at 09:47 -0700, Dan Williams wrote:
> On Tue, Jun 1, 2021 at 10:31 PM Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
> > 
> > [switching to the new mailing list]
> > 
> > On Mon, 2021-05-17 at 08:14 +0900, QI Fuli wrote:
> > > From: QI Fuli <qi.fuli@fujitsu.com>
> > > 
> > > This patch set is to rename monitor.conf to ndctl.conf, and make it a
> > > global ndctl configuration file that all ndctl commands can refer to.
> > > 
> > > As this patch set has been pending until now, I would like to know if
> > > current idea works or not. If yes, I will finish the documents and test.
> > > 
> > > Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> > 
> > Hi Qi,
> > 
> > Thanks for picking up on this! The approach generally looks good to me,
> > I think we can definitely move forward with this direction.
> > 
> > One thing that stands out is - I don't think we can simply rename the
> > existing monitor.conf. We have to keep supporting the 'legacy'
> > monitor.conf so that we don't break any deployments. I'd suggest
> > keeping the old monitor.conf as is, and continuing to parse it as is,
> > but also adding a new ndctl.conf as you have done.
> > 
> > We can indicate that 'monitor.conf' is legacy, and any new features
> > will only get added to the new global config to encourage migration to
> > the new config. Perhaps we can even provide a helper script to migrate
> > the old config to new - but I think it needs to be a user triggered
> > action.
> > 
> > This is timely as I also need to go add some config related
> > functionality to daxctl, and basing it on this would be perfect, so I'd
> > love to get this series merged in soon.
> 
> I wonder if ndctl should treat /etc/ndctl like a conf.d directory of
> which all files with the .conf suffix are concatenated into one
> combined configuration file. I.e. something that maintains legacy, but
> also allows for config fragments to be deployed individually.

Agreed, this would be the most flexible. ciniparser doesn't seem to
support multiple files directly, but perhaps ndctl can, early on, load
up multiple files/dictionaries, and stash them in ndctl_ctx. Then there
can be accessor functions to retrieve specific conf strings from that
as needed by different commands.


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

* RE: [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file
  2021-06-02 17:15     ` Verma, Vishal L
@ 2021-06-17  0:25       ` qi.fuli
  2021-07-08  6:34         ` Verma, Vishal L
  0 siblings, 1 reply; 10+ messages in thread
From: qi.fuli @ 2021-06-17  0:25 UTC (permalink / raw)
  To: 'Verma, Vishal L', Williams, Dan J; +Cc: fukuri.sai, nvdimm

> On Wed, 2021-06-02 at 09:47 -0700, Dan Williams wrote:
> > On Tue, Jun 1, 2021 at 10:31 PM Verma, Vishal L
> > <vishal.l.verma@intel.com> wrote:
> > >
> > > [switching to the new mailing list]
> > >
> > > On Mon, 2021-05-17 at 08:14 +0900, QI Fuli wrote:
> > > > From: QI Fuli <qi.fuli@fujitsu.com>
> > > >
> > > > This patch set is to rename monitor.conf to ndctl.conf, and make
> > > > it a global ndctl configuration file that all ndctl commands can refer to.
> > > >
> > > > As this patch set has been pending until now, I would like to know
> > > > if current idea works or not. If yes, I will finish the documents and test.
> > > >
> > > > Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> > >
> > > Hi Qi,
> > >
> > > Thanks for picking up on this! The approach generally looks good to
> > > me, I think we can definitely move forward with this direction.
> > >
> > > One thing that stands out is - I don't think we can simply rename
> > > the existing monitor.conf. We have to keep supporting the 'legacy'
> > > monitor.conf so that we don't break any deployments. I'd suggest
> > > keeping the old monitor.conf as is, and continuing to parse it as
> > > is, but also adding a new ndctl.conf as you have done.
> > >
> > > We can indicate that 'monitor.conf' is legacy, and any new features
> > > will only get added to the new global config to encourage migration
> > > to the new config. Perhaps we can even provide a helper script to
> > > migrate the old config to new - but I think it needs to be a user
> > > triggered action.
> > >
> > > This is timely as I also need to go add some config related
> > > functionality to daxctl, and basing it on this would be perfect, so
> > > I'd love to get this series merged in soon.
> >
> > I wonder if ndctl should treat /etc/ndctl like a conf.d directory of
> > which all files with the .conf suffix are concatenated into one
> > combined configuration file. I.e. something that maintains legacy, but
> > also allows for config fragments to be deployed individually.
> 
> Agreed, this would be the most flexible. ciniparser doesn't seem to support
> multiple files directly, but perhaps ndctl can, early on, load up multiple
> files/dictionaries, and stash them in ndctl_ctx. Then there can be accessor
> functions to retrieve specific conf strings from that as needed by different
> commands.

Thank you very much for the comments.

I also agree, and I am working on the v2 patch set now.
I found that the style of ndctl.conf is different from monitor.conf, since there is no section name in monitor.conf.
Do I need to keep the legacy style in monitor.conf, i.e. Can I add the section name [monitor] to monitor.conf?

Best regards,
QI

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

* Re: [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file
  2021-06-17  0:25       ` qi.fuli
@ 2021-07-08  6:34         ` Verma, Vishal L
  0 siblings, 0 replies; 10+ messages in thread
From: Verma, Vishal L @ 2021-07-08  6:34 UTC (permalink / raw)
  To: Williams, Dan J, qi.fuli; +Cc: fukuri.sai, nvdimm

On Thu, 2021-06-17 at 00:25 +0000, qi.fuli@fujitsu.com wrote:
> > On Wed, 2021-06-02 at 09:47 -0700, Dan Williams wrote:
> > > On Tue, Jun 1, 2021 at 10:31 PM Verma, Vishal L
> > > <vishal.l.verma@intel.com> wrote:
> > > > 
> > > > [switching to the new mailing list]
> > > > 
> > > > On Mon, 2021-05-17 at 08:14 +0900, QI Fuli wrote:
> > > > > From: QI Fuli <qi.fuli@fujitsu.com>
> > > > > 
> > > > > This patch set is to rename monitor.conf to ndctl.conf, and make
> > > > > it a global ndctl configuration file that all ndctl commands can refer to.
> > > > > 
> > > > > As this patch set has been pending until now, I would like to know
> > > > > if current idea works or not. If yes, I will finish the documents and test.
> > > > > 
> > > > > Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> > > > 
> > > > Hi Qi,
> > > > 
> > > > Thanks for picking up on this! The approach generally looks good to
> > > > me, I think we can definitely move forward with this direction.
> > > > 
> > > > One thing that stands out is - I don't think we can simply rename
> > > > the existing monitor.conf. We have to keep supporting the 'legacy'
> > > > monitor.conf so that we don't break any deployments. I'd suggest
> > > > keeping the old monitor.conf as is, and continuing to parse it as
> > > > is, but also adding a new ndctl.conf as you have done.
> > > > 
> > > > We can indicate that 'monitor.conf' is legacy, and any new features
> > > > will only get added to the new global config to encourage migration
> > > > to the new config. Perhaps we can even provide a helper script to
> > > > migrate the old config to new - but I think it needs to be a user
> > > > triggered action.
> > > > 
> > > > This is timely as I also need to go add some config related
> > > > functionality to daxctl, and basing it on this would be perfect, so
> > > > I'd love to get this series merged in soon.
> > > 
> > > I wonder if ndctl should treat /etc/ndctl like a conf.d directory of
> > > which all files with the .conf suffix are concatenated into one
> > > combined configuration file. I.e. something that maintains legacy, but
> > > also allows for config fragments to be deployed individually.
> > 
> > Agreed, this would be the most flexible. ciniparser doesn't seem to support
> > multiple files directly, but perhaps ndctl can, early on, load up multiple
> > files/dictionaries, and stash them in ndctl_ctx. Then there can be accessor
> > functions to retrieve specific conf strings from that as needed by different
> > commands.
> 
> Thank you very much for the comments.
> 
> I also agree, and I am working on the v2 patch set now.
> I found that the style of ndctl.conf is different from monitor.conf,
> since there is no section name in monitor.conf.
> Do I need to keep the legacy style in monitor.conf, i.e. Can I add
> the section name [monitor] to monitor.conf?

Sorry about the delayed reply, I missed this thread. I think adding it
to the sample monitor.conf is harmless, but I'm not sure that gets us
anything.. The goal is to continue to support any monitor.confs of the
old style that have already been deployed. Those wouldn't have a
[monitor] section header.

I think we'd have to treat the specific file /etc/ndctl/monitor.conf as
a special case, and just parse it the old way. We can make it so that a
new style [monitor] section is incompatible with the old style
monitor.conf (error out and fail if this is detected), and we won't
ever add any new functionality to the old monitor.conf to encourage
people to switch. We can additionally print a warning to stderr if the
old style file is being used.


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

end of thread, other threads:[~2021-07-08  6:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-16 23:14 [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file QI Fuli
2021-05-16 23:14 ` [RFC ndctl PATCH 1/3] ndctl, ccan: import ciniparser QI Fuli
2021-05-16 23:14 ` [RFC ndctl PATCH 2/3] ndctl, util: add parse-configs helper QI Fuli
2021-05-16 23:14 ` [RFC ndctl PATCH 3/3] ndctl, rename monitor.conf to ndctl.conf QI Fuli
2021-06-02  6:33   ` Verma, Vishal L
2021-06-02  5:31 ` [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file Verma, Vishal L
2021-06-02 16:47   ` Dan Williams
2021-06-02 17:15     ` Verma, Vishal L
2021-06-17  0:25       ` qi.fuli
2021-07-08  6:34         ` Verma, Vishal L

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.