* [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl
@ 2021-12-06 22:28 Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 01/12] ndctl, util: add iniparser helper Vishal Verma
` (12 more replies)
0 siblings, 13 replies; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, Vishal Verma
Changes since v1[1]:
- Collect review tags
- Fix 'make clean' removing the reconfigure script from the source tree
(Fenghua, Qi)
- Documentation wordsmithing (Dan)
- Fix line break after declarations in parse-configs.c (Dan)
- Move daxctl config files to its own directory, /etc/daxctl/ (Dan)
- Improve failure mode in the absence of a configs directory
- Rename {nd,dax}ctl_{get|set}_configs to
{nd,dax}ctl_{get|set}_configs_dir
- Exit with success if -C is specified, but no matching config section
is found.
- Refuse to proceed if CLI options are passed in along with -C (Dan)
- In the config file, rename: s/[auto-online foo]/[reconfigure-device foo/
and s/uuid/nvdimm.uuid/ (Dan)
- Teach device.c to accept /dev/daxX.Y instead of only daxX.Y and thus
remove the need for a wrapper script that systemd invokes (Dan)
These patches add policy (config file) support to daxctl. The
introductory user is daxctl-reconfigure-device. Sysadmins may wish to
use daxctl devices as system-ram, but it may be cumbersome to automate
the reconfiguration step for every device upon boot.
Introduce a new option for daxctl-reconfigure-device, --check-config.
This is at the heart of policy based reconfiguration, as it allows
daxctl to look up reconfiguration parameters for a given device from the
config system instead of the command line.
Some systemd and udev glue then automates this for every new dax device
that shows up, providing a way for the administrator to simply list all
the 'system-ram' UUIDs in a config file, and not have to worry about
anything else.
An example config file can be:
# cat /etc/ndctl/daxctl.conf
[reconfigure-device unique_identifier_foo]
nvdimm.uuid = 48d8e42c-a2f0-4312-9e70-a837faafe862
mode = system-ram
online = true
movable = false
Any file under '/etc/ndctl/' can be used - all files with a '.conf' suffix
will be considered when looking for matches.
These patches depend on the initial config file support from Qi Fuli[2].
I've re-rolled Qi's original patches as the first five patches in this
series because of a change I made for graceful handling in the case of a
missing configs directory, and also to incorporate review feedback that
applied to the dependant patches. Patch 6 onwards is the actual v2 of
the daxctl policy work.
A branch containing these patches is available at [3].
[1]: https://lore.kernel.org/nvdimm/20210831090459.2306727-1-vishal.l.verma@intel.com/
[2]: https://lore.kernel.org/nvdimm/20210824095106.104808-1-qi.fuli@fujitsu.com/
[3]: https://github.com/pmem/ndctl/tree/vv/daxctl_config_v2
QI Fuli (5):
ndctl, util: add iniparser helper
ndctl, util: add parse-configs helper
ndctl: make ndctl support configuration files
ndctl, config: add the default ndctl configuration file
ndctl, monitor: refator monitor for supporting multiple config files
Vishal Verma (7):
ndctl: Update ndctl.spec.in for 'ndctl.conf'
daxctl: Documentation updates for persistent reconfiguration
util/parse-config: refactor filter_conf_files into util/
daxctl: add basic config parsing support
util/parse-configs: add a key/value search helper
daxctl/device.c: add an option for getting params from a config file
daxctl: add systemd service and udev rule for automatic
reconfiguration
.../daxctl/daxctl-reconfigure-device.txt | 75 ++
Documentation/ndctl/ndctl-monitor.txt | 8 +-
configure.ac | 18 +-
Makefile.am | 8 +-
ndctl/lib/private.h | 1 +
daxctl/lib/libdaxctl.c | 39 +
ndctl/lib/libndctl.c | 39 +
daxctl/libdaxctl.h | 2 +
ndctl/libndctl.h | 2 +
util/dictionary.h | 175 ++++
util/iniparser.h | 360 ++++++++
util/parse-configs.h | 53 ++
daxctl/daxctl.c | 1 +
daxctl/device.c | 174 +++-
ndctl/monitor.c | 73 +-
ndctl/ndctl.c | 1 +
util/dictionary.c | 383 ++++++++
util/iniparser.c | 838 ++++++++++++++++++
util/parse-configs.c | 150 ++++
Documentation/daxctl/Makefile.am | 11 +-
Documentation/ndctl/Makefile.am | 2 +-
daxctl/90-daxctl-device.rules | 1 +
daxctl/Makefile.am | 9 +
daxctl/daxdev-reconfigure@.service | 8 +
daxctl/lib/Makefile.am | 6 +
daxctl/lib/libdaxctl.sym | 2 +
ndctl.spec.in | 4 +
ndctl/Makefile.am | 9 +-
ndctl/lib/Makefile.am | 6 +
ndctl/lib/libndctl.sym | 2 +
ndctl/ndctl.conf | 56 ++
31 files changed, 2467 insertions(+), 49 deletions(-)
create mode 100644 util/dictionary.h
create mode 100644 util/iniparser.h
create mode 100644 util/parse-configs.h
create mode 100644 util/dictionary.c
create mode 100644 util/iniparser.c
create mode 100644 util/parse-configs.c
create mode 100644 daxctl/90-daxctl-device.rules
create mode 100644 daxctl/daxdev-reconfigure@.service
create mode 100644 ndctl/ndctl.conf
base-commit: 4e646fa490ba4b782afa188dd8818b94c419924e
--
2.33.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 01/12] ndctl, util: add iniparser helper
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-07 18:24 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 02/12] ndctl, util: add parse-configs helper Vishal Verma
` (11 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, QI Fuli, Vishal Verma
From: QI Fuli <qi.fuli@fujitsu.com>
Add iniparser[1] helper for parsing ndctl global configuration files.
[1] https://github.com/ndevilla/iniparser
Link: https://lore.kernel.org/r/20210824095106.104808-2-qi.fuli@fujitsu.com
Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
Makefile.am | 6 +-
util/dictionary.h | 175 ++++++++++
util/iniparser.h | 360 ++++++++++++++++++++
util/dictionary.c | 383 +++++++++++++++++++++
util/iniparser.c | 838 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 1761 insertions(+), 1 deletion(-)
create mode 100644 util/dictionary.h
create mode 100644 util/iniparser.h
create mode 100644 util/dictionary.c
create mode 100644 util/iniparser.c
diff --git a/Makefile.am b/Makefile.am
index 60a1998..235c362 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -85,6 +85,10 @@ libutil_a_SOURCES = \
util/size.h \
util/main.h \
util/filter.h \
- util/bitmap.h
+ util/bitmap.h \
+ util/dictionary.c \
+ util/dictionary.h \
+ util/iniparser.c \
+ util/iniparser.h
nobase_include_HEADERS = daxctl/libdaxctl.h
diff --git a/util/dictionary.h b/util/dictionary.h
new file mode 100644
index 0000000..4678e38
--- /dev/null
+++ b/util/dictionary.h
@@ -0,0 +1,175 @@
+/* SPDX-License-Identifier: MIT */
+/* originally copied from https://github.com/ndevilla/iniparser */
+
+/*-------------------------------------------------------------------------*/
+/**
+ @file dictionary.h
+ @author N. Devillard
+ @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.
+ informations retrieved from a configuration file (ini files).
+*/
+/*--------------------------------------------------------------------------*/
+
+#ifndef _DICTIONARY_H_
+#define _DICTIONARY_H_
+
+/*---------------------------------------------------------------------------
+ Includes
+ ---------------------------------------------------------------------------*/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/*---------------------------------------------------------------------------
+ New types
+ ---------------------------------------------------------------------------*/
+
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Dictionary object
+
+ 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 ; /** Number of entries in dictionary */
+ ssize_t size ; /** Storage size */
+ char ** val ; /** List of string values */
+ char ** key ; /** List of string keys */
+ unsigned * hash ; /** List of hash values for keys */
+} dictionary ;
+
+
+/*---------------------------------------------------------------------------
+ Function prototypes
+ ---------------------------------------------------------------------------*/
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 1 newly allocated dictionary object.
+
+ 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(size_t 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.
+ */
+/*--------------------------------------------------------------------------*/
+const char * dictionary_get(const dictionary * d, const char * key, const 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, const 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 f 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(const dictionary * d, FILE * out);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/util/iniparser.h b/util/iniparser.h
new file mode 100644
index 0000000..9e30119
--- /dev/null
+++ b/util/iniparser.h
@@ -0,0 +1,360 @@
+/* SPDX-License-Identifier: MIT */
+/* originally copied from https://github.com/ndevilla/iniparser */
+
+/*-------------------------------------------------------------------------*/
+/**
+ @file iniparser.h
+ @author N. Devillard
+ @brief Parser for ini files.
+*/
+/*--------------------------------------------------------------------------*/
+
+#ifndef _INIPARSER_H_
+#define _INIPARSER_H_
+
+/*---------------------------------------------------------------------------
+ Includes
+ ---------------------------------------------------------------------------*/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+/*
+ * The following #include is necessary on many Unixes but not Linux.
+ * It is not needed for Windows platforms.
+ * Uncomment it if needed.
+ */
+/* #include <unistd.h> */
+
+#include "dictionary.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Configure a function to receive the error messages.
+ @param errback Function to call.
+
+ By default, the error will be printed on stderr. If a null pointer is passed
+ as errback the error callback will be switched back to default.
+ */
+/*--------------------------------------------------------------------------*/
+
+void iniparser_set_error_callback(int (*errback)(const char *, ...));
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Get number of sections in a dictionary
+ @param d Dictionary to examine
+ @return int Number of sections found in dictionary
+
+ 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.
+
+ This function returns -1 in case of error.
+ */
+/*--------------------------------------------------------------------------*/
+
+int iniparser_getnsec(const 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
+
+ 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!
+
+ This function returns NULL in case of error.
+ */
+/*--------------------------------------------------------------------------*/
+
+const char * iniparser_getsecname(const 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 iniparser_dump_ini(const dictionary * d, FILE * f);
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Save a dictionary section to a loadable ini file
+ @param d Dictionary to dump
+ @param s Section name of dictionary to dump
+ @param f Opened file pointer to dump to
+ @return void
+
+ This function dumps a given section of a given dictionary into a loadable ini
+ file. It is Ok to specify @c stderr or @c stdout as output files.
+ */
+/*--------------------------------------------------------------------------*/
+
+void iniparser_dumpsection_ini(const dictionary * d, const char * s, 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 iniparser_dump(const dictionary * d, FILE * f);
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Get the number of keys in a section of a dictionary.
+ @param d Dictionary to examine
+ @param s Section name of dictionary to examine
+ @return Number of keys in section
+ */
+/*--------------------------------------------------------------------------*/
+int iniparser_getsecnkeys(const dictionary * d, const char * s);
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Get the number of keys in a section of a dictionary.
+ @param d Dictionary to examine
+ @param s Section name of dictionary to examine
+ @param keys Already allocated array to store the keys in
+ @return The pointer passed as `keys` argument or NULL in case of error
+
+ This function queries a dictionary and finds all keys in a given section.
+ The keys argument should be an array of pointers which size has been
+ determined by calling `iniparser_getsecnkeys` function prior to this one.
+
+ Each pointer in the returned char pointer-to-pointer is pointing to
+ a string allocated in the dictionary; do not free or modify them.
+ */
+/*--------------------------------------------------------------------------*/
+const char ** iniparser_getseckeys(const dictionary * d, const char * s, const char ** keys);
+
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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.
+ */
+/*--------------------------------------------------------------------------*/
+const char * iniparser_getstring(const dictionary * d, const char * key, const 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 iniparser_getint(const dictionary * d, const char * key, int notfound);
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Get the string associated to a key, convert to an long 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.
+ */
+/*--------------------------------------------------------------------------*/
+long int iniparser_getlongint(const dictionary * d, const char * key, long 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 iniparser_getdouble(const 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 iniparser_getboolean(const 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, the entry is created.
+ It is Ok to set val to NULL.
+ */
+/*--------------------------------------------------------------------------*/
+int iniparser_set(dictionary * ini, const char * entry, const 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 iniparser_unset(dictionary * ini, const 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 iniparser_find_entry(const dictionary * ini, const 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 iniparser_freedict().
+ */
+/*--------------------------------------------------------------------------*/
+dictionary * iniparser_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 iniparser_freedict(dictionary * d);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/util/dictionary.c b/util/dictionary.c
new file mode 100644
index 0000000..55d63ff
--- /dev/null
+++ b/util/dictionary.c
@@ -0,0 +1,383 @@
+/* SPDX-License-Identifier: MIT */
+/* originally copied from https://github.com/ndevilla/iniparser */
+
+/*-------------------------------------------------------------------------*/
+/**
+ @file dictionary.c
+ @author N. Devillard
+ @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.
+ informations retrieved from a configuration file (ini files).
+*/
+/*--------------------------------------------------------------------------*/
+
+/*---------------------------------------------------------------------------
+ Includes
+ ---------------------------------------------------------------------------*/
+#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)
+
+/*---------------------------------------------------------------------------
+ Private functions
+ ---------------------------------------------------------------------------*/
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Duplicate a string
+ @param s String to duplicate
+ @return Pointer to a newly allocated string, to be freed with free()
+
+ This is a replacement for strdup(). This implementation is provided
+ for systems that do not have it.
+ */
+/*--------------------------------------------------------------------------*/
+static char * xstrdup(const char * s)
+{
+ char * t ;
+ size_t len ;
+ if (!s)
+ return NULL ;
+
+ len = strlen(s) + 1 ;
+ t = (char*) malloc(len) ;
+ if (t) {
+ memcpy(t, s, len) ;
+ }
+ return t ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Double the size of the dictionary
+ @param d Dictionary to grow
+ @return This function returns non-zero in case of failure
+ */
+/*--------------------------------------------------------------------------*/
+static int dictionary_grow(dictionary * d)
+{
+ char ** new_val ;
+ char ** new_key ;
+ unsigned * new_hash ;
+
+ new_val = (char**) calloc(d->size * 2, sizeof *d->val);
+ new_key = (char**) calloc(d->size * 2, sizeof *d->key);
+ new_hash = (unsigned*) calloc(d->size * 2, sizeof *d->hash);
+ if (!new_val || !new_key || !new_hash) {
+ /* An allocation failed, leave the dictionary unchanged */
+ if (new_val)
+ free(new_val);
+ if (new_key)
+ free(new_key);
+ if (new_hash)
+ free(new_hash);
+ return -1 ;
+ }
+ /* Initialize the newly allocated space */
+ memcpy(new_val, d->val, d->size * sizeof(char *));
+ memcpy(new_key, d->key, d->size * sizeof(char *));
+ memcpy(new_hash, d->hash, d->size * sizeof(unsigned));
+ /* Delete previous data */
+ free(d->val);
+ free(d->key);
+ free(d->hash);
+ /* Actually update the dictionary */
+ d->size *= 2 ;
+ d->val = new_val;
+ d->key = new_key;
+ d->hash = new_hash;
+ return 0 ;
+}
+
+/*---------------------------------------------------------------------------
+ Function codes
+ ---------------------------------------------------------------------------*/
+/*-------------------------------------------------------------------------*/
+/**
+ @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)
+{
+ size_t len ;
+ unsigned hash ;
+ size_t i ;
+
+ if (!key)
+ return 0 ;
+
+ 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 ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Create a new dictionary object.
+ @param size Optional initial size of the dictionary.
+ @return 1 newly allocated dictionary object.
+
+ 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(size_t size)
+{
+ dictionary * d ;
+
+ /* If no size was specified, allocate space for DICTMINSZ */
+ if (size<DICTMINSZ) size=DICTMINSZ ;
+
+ d = (dictionary*) calloc(1, sizeof *d) ;
+
+ if (d) {
+ d->size = size ;
+ d->val = (char**) calloc(size, sizeof *d->val);
+ d->key = (char**) calloc(size, sizeof *d->key);
+ d->hash = (unsigned*) calloc(size, sizeof *d->hash);
+ }
+ return d ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 * d)
+{
+ ssize_t 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 ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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.
+ */
+/*--------------------------------------------------------------------------*/
+const char * dictionary_get(const dictionary * d, const char * key, const char * def)
+{
+ unsigned hash ;
+ ssize_t 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 ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 * d, const char * key, const char * val)
+{
+ ssize_t 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 ;
+ if (hash==d->hash[i]) { /* Same hash value */
+ if (!strcmp(key, d->key[i])) { /* Same key */
+ /* Found a value: modify and return */
+ if (d->val[i]!=NULL)
+ free(d->val[i]);
+ d->val[i] = (val ? xstrdup(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 */
+ if (dictionary_grow(d) != 0)
+ return -1;
+ }
+
+ /* Insert key in the first empty slot. Start at d->n and wrap at
+ d->size. Because d->n < d->size this will necessarily
+ terminate. */
+ for (i=d->n ; d->key[i] ; ) {
+ if(++i == d->size) i = 0;
+ }
+ /* Copy key */
+ d->key[i] = xstrdup(key);
+ d->val[i] = (val ? xstrdup(val) : NULL) ;
+ d->hash[i] = hash;
+ d->n ++ ;
+ return 0 ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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)
+{
+ unsigned hash ;
+ ssize_t i ;
+
+ if (key == NULL || d == 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 ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Dump a dictionary to an opened file pointer.
+ @param d Dictionary to dump
+ @param f 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(const dictionary * d, FILE * out)
+{
+ ssize_t 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/util/iniparser.c b/util/iniparser.c
new file mode 100644
index 0000000..9e2023a
--- /dev/null
+++ b/util/iniparser.c
@@ -0,0 +1,838 @@
+/* SPDX-License-Identifier: MIT */
+/* originally copied from https://github.com/ndevilla/iniparser */
+
+/*-------------------------------------------------------------------------*/
+/**
+ @file iniparser.c
+ @author N. Devillard
+ @brief Parser for ini files.
+*/
+/*--------------------------------------------------------------------------*/
+/*---------------------------- Includes ------------------------------------*/
+#include <ctype.h>
+#include <stdarg.h>
+#include "iniparser.h"
+
+/*---------------------------- Defines -------------------------------------*/
+#define ASCIILINESZ (1024)
+#define INI_INVALID_KEY ((char*)-1)
+
+/*---------------------------------------------------------------------------
+ Private to this module
+ ---------------------------------------------------------------------------*/
+/**
+ * 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 in String to convert.
+ @param out Output buffer.
+ @param len Size of the out buffer.
+ @return ptr to the out buffer or NULL if an error occured.
+
+ This function convert a string into lowercase.
+ At most len - 1 elements of the input string will be converted.
+ */
+/*--------------------------------------------------------------------------*/
+static const char * strlwc(const char * in, char *out, unsigned len)
+{
+ unsigned i ;
+
+ if (in==NULL || out == NULL || len==0) return NULL ;
+ i=0 ;
+ while (in[i] != '\0' && i < len-1) {
+ out[i] = (char)tolower((int)in[i]);
+ i++ ;
+ }
+ out[i] = '\0';
+ return out ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Duplicate a string
+ @param s String to duplicate
+ @return Pointer to a newly allocated string, to be freed with free()
+
+ This is a replacement for strdup(). This implementation is provided
+ for systems that do not have it.
+ */
+/*--------------------------------------------------------------------------*/
+static char * xstrdup(const char * s)
+{
+ char * t ;
+ size_t len ;
+ if (!s)
+ return NULL ;
+
+ len = strlen(s) + 1 ;
+ t = (char*) malloc(len) ;
+ if (t) {
+ memcpy(t, s, len) ;
+ }
+ return t ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Remove blanks at the beginning and the end of a string.
+ @param str String to parse and alter.
+ @return unsigned New size of the string.
+ */
+/*--------------------------------------------------------------------------*/
+static unsigned strstrip(char * s)
+{
+ char *last = NULL ;
+ char *dest = s;
+
+ if (s==NULL) return 0;
+
+ last = s + strlen(s);
+ while (isspace((int)*s) && *s) s++;
+ while (last > s) {
+ if (!isspace((int)*(last-1)))
+ break ;
+ last -- ;
+ }
+ *last = (char)0;
+
+ memmove(dest,s,last - s + 1);
+ return last - s;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Default error callback for iniparser: wraps `fprintf(stderr, ...)`.
+ */
+/*--------------------------------------------------------------------------*/
+static int default_error_callback(const char *format, ...)
+{
+ int ret;
+ va_list argptr;
+ va_start(argptr, format);
+ ret = vfprintf(stderr, format, argptr);
+ va_end(argptr);
+ return ret;
+}
+
+static int (*iniparser_error_callback)(const char*, ...) = default_error_callback;
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Configure a function to receive the error messages.
+ @param errback Function to call.
+
+ By default, the error will be printed on stderr. If a null pointer is passed
+ as errback the error callback will be switched back to default.
+ */
+/*--------------------------------------------------------------------------*/
+void iniparser_set_error_callback(int (*errback)(const char *, ...))
+{
+ if (errback) {
+ iniparser_error_callback = errback;
+ } else {
+ iniparser_error_callback = default_error_callback;
+ }
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Get number of sections in a dictionary
+ @param d Dictionary to examine
+ @return int Number of sections found in dictionary
+
+ 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.
+
+ This function returns -1 in case of error.
+ */
+/*--------------------------------------------------------------------------*/
+int iniparser_getnsec(const 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 ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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
+
+ 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!
+
+ This function returns NULL in case of error.
+ */
+/*--------------------------------------------------------------------------*/
+const char * iniparser_getsecname(const dictionary * d, int n)
+{
+ int i ;
+ int foundsec ;
+
+ if (d==NULL || n<0) return NULL ;
+ foundsec=0 ;
+ for (i=0 ; i<d->size ; i++) {
+ if (d->key[i]==NULL)
+ continue ;
+ if (strchr(d->key[i], ':')==NULL) {
+ foundsec++ ;
+ if (foundsec>n)
+ break ;
+ }
+ }
+ if (foundsec<=n) {
+ return NULL ;
+ }
+ return d->key[i] ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 iniparser_dump(const 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 ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 iniparser_dump_ini(const dictionary * d, FILE * f)
+{
+ int i ;
+ int nsec ;
+ const char * secname ;
+
+ if (d==NULL || f==NULL) return ;
+
+ nsec = iniparser_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 = iniparser_getsecname(d, i) ;
+ iniparser_dumpsection_ini(d, secname, f);
+ }
+ fprintf(f, "\n");
+ return ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Save a dictionary section to a loadable ini file
+ @param d Dictionary to dump
+ @param s Section name of dictionary to dump
+ @param f Opened file pointer to dump to
+ @return void
+
+ This function dumps a given section of a given dictionary into a loadable ini
+ file. It is Ok to specify @c stderr or @c stdout as output files.
+ */
+/*--------------------------------------------------------------------------*/
+void iniparser_dumpsection_ini(const dictionary * d, const char * s, FILE * f)
+{
+ int j ;
+ char keym[ASCIILINESZ+1];
+ int seclen ;
+
+ if (d==NULL || f==NULL) return ;
+ if (! iniparser_find_entry(d, s)) return ;
+
+ seclen = (int)strlen(s);
+ fprintf(f, "\n[%s]\n", s);
+ sprintf(keym, "%s:", s);
+ 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 ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Get the number of keys in a section of a dictionary.
+ @param d Dictionary to examine
+ @param s Section name of dictionary to examine
+ @return Number of keys in section
+ */
+/*--------------------------------------------------------------------------*/
+int iniparser_getsecnkeys(const dictionary * d, const char * s)
+{
+ int seclen, nkeys ;
+ char keym[ASCIILINESZ+1];
+ int j ;
+
+ nkeys = 0;
+
+ if (d==NULL) return nkeys;
+ if (! iniparser_find_entry(d, s)) return nkeys;
+
+ seclen = (int)strlen(s);
+ strlwc(s, keym, sizeof(keym));
+ keym[seclen] = ':';
+
+ for (j=0 ; j<d->size ; j++) {
+ if (d->key[j]==NULL)
+ continue ;
+ if (!strncmp(d->key[j], keym, seclen+1))
+ nkeys++;
+ }
+
+ return nkeys;
+
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Get the number of keys in a section of a dictionary.
+ @param d Dictionary to examine
+ @param s Section name of dictionary to examine
+ @param keys Already allocated array to store the keys in
+ @return The pointer passed as `keys` argument or NULL in case of error
+
+ This function queries a dictionary and finds all keys in a given section.
+ The keys argument should be an array of pointers which size has been
+ determined by calling `iniparser_getsecnkeys` function prior to this one.
+
+ Each pointer in the returned char pointer-to-pointer is pointing to
+ a string allocated in the dictionary; do not free or modify them.
+ */
+/*--------------------------------------------------------------------------*/
+const char ** iniparser_getseckeys(const dictionary * d, const char * s, const char ** keys)
+{
+ int i, j, seclen ;
+ char keym[ASCIILINESZ+1];
+
+ if (d==NULL || keys==NULL) return NULL;
+ if (! iniparser_find_entry(d, s)) return NULL;
+
+ seclen = (int)strlen(s);
+ strlwc(s, keym, sizeof(keym));
+ keym[seclen] = ':';
+
+ i = 0;
+
+ for (j=0 ; j<d->size ; j++) {
+ if (d->key[j]==NULL)
+ continue ;
+ if (!strncmp(d->key[j], keym, seclen+1)) {
+ keys[i] = d->key[j];
+ i++;
+ }
+ }
+
+ return keys;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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.
+ */
+/*--------------------------------------------------------------------------*/
+const char * iniparser_getstring(const dictionary * d, const char * key, const char * def)
+{
+ const char * lc_key ;
+ const char * sval ;
+ char tmp_str[ASCIILINESZ+1];
+
+ if (d==NULL || key==NULL)
+ return def ;
+
+ lc_key = strlwc(key, tmp_str, sizeof(tmp_str));
+ sval = dictionary_get(d, lc_key, def);
+ return sval ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @brief Get the string associated to a key, convert to an long int
+ @param d Dictionary to search
+ @param key Key string to look for
+ @param notfound Value to return in case of error
+ @return long 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()
+ */
+/*--------------------------------------------------------------------------*/
+long int iniparser_getlongint(const dictionary * d, const char * key, long int notfound)
+{
+ const char * str ;
+
+ str = iniparser_getstring(d, key, INI_INVALID_KEY);
+ if (str==INI_INVALID_KEY) return notfound ;
+ return strtol(str, NULL, 0);
+}
+
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 iniparser_getint(const dictionary * d, const char * key, int notfound)
+{
+ return (int)iniparser_getlongint(d, key, 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 iniparser_getdouble(const dictionary * d, const char * key, double notfound)
+{
+ const char * str ;
+
+ str = iniparser_getstring(d, key, INI_INVALID_KEY);
+ if (str==INI_INVALID_KEY) return notfound ;
+ return atof(str);
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 iniparser_getboolean(const dictionary * d, const char * key, int notfound)
+{
+ int ret ;
+ const char * c ;
+
+ c = iniparser_getstring(d, key, INI_INVALID_KEY);
+ if (c==INI_INVALID_KEY) return notfound ;
+ if (c[0]=='y' || c[0]=='Y' || c[0]=='1' || c[0]=='t' || c[0]=='T') {
+ ret = 1 ;
+ } else if (c[0]=='n' || c[0]=='N' || c[0]=='0' || c[0]=='f' || c[0]=='F') {
+ ret = 0 ;
+ } else {
+ ret = notfound ;
+ }
+ return ret;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 iniparser_find_entry(const dictionary * ini, const char * entry)
+{
+ int found=0 ;
+ if (iniparser_getstring(ini, entry, INI_INVALID_KEY)!=INI_INVALID_KEY) {
+ found = 1 ;
+ }
+ return found ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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, the entry is created.
+ It is Ok to set val to NULL.
+ */
+/*--------------------------------------------------------------------------*/
+int iniparser_set(dictionary * ini, const char * entry, const char * val)
+{
+ char tmp_str[ASCIILINESZ+1];
+ return dictionary_set(ini, strlwc(entry, tmp_str, sizeof(tmp_str)), 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 iniparser_unset(dictionary * ini, const char * entry)
+{
+ char tmp_str[ASCIILINESZ+1];
+ dictionary_unset(ini, strlwc(entry, tmp_str, sizeof(tmp_str)));
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 iniparser_line(
+ const char * input_line,
+ char * section,
+ char * key,
+ char * value)
+{
+ line_status sta ;
+ char * line = NULL;
+ size_t len ;
+
+ line = xstrdup(input_line);
+ len = strstrip(line);
+
+ sta = LINE_UNPROCESSED ;
+ if (len<1) {
+ /* Empty line */
+ sta = LINE_EMPTY ;
+ } else if (line[0]=='#' || line[0]==';') {
+ /* Comment line */
+ sta = LINE_COMMENT ;
+ } else if (line[0]=='[' && line[len-1]==']') {
+ /* Section name */
+ sscanf(line, "[%[^]]", section);
+ strstrip(section);
+ strlwc(section, section, len);
+ sta = LINE_SECTION ;
+ } else if (sscanf (line, "%[^=] = \"%[^\"]\"", key, value) == 2
+ || sscanf (line, "%[^=] = '%[^\']'", key, value) == 2) {
+ /* Usual key=value with quotes, with or without comments */
+ strstrip(key);
+ strlwc(key, key, len);
+ /* Don't strip spaces from values surrounded with quotes */
+ sta = LINE_VALUE ;
+ } else if (sscanf (line, "%[^=] = %[^;#]", key, value) == 2) {
+ /* Usual key=value without quotes, with or without comments */
+ strstrip(key);
+ strlwc(key, key, len);
+ 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=#
+ */
+ strstrip(key);
+ strlwc(key, key, len);
+ value[0]=0 ;
+ sta = LINE_VALUE ;
+ } else {
+ /* Generate syntax error */
+ sta = LINE_ERROR ;
+ }
+
+ free(line);
+ return sta ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 iniparser_freedict().
+ */
+/*--------------------------------------------------------------------------*/
+dictionary * iniparser_load(const char * ininame)
+{
+ FILE * in ;
+
+ char line [ASCIILINESZ+1] ;
+ char section [ASCIILINESZ+1] ;
+ char key [ASCIILINESZ+1] ;
+ char tmp [(ASCIILINESZ * 2) + 2] ;
+ char val [ASCIILINESZ+1] ;
+
+ int last=0 ;
+ int len ;
+ int lineno=0 ;
+ int errs=0;
+ int mem_err=0;
+
+ dictionary * dict ;
+
+ if ((in=fopen(ininame, "r"))==NULL) {
+ iniparser_error_callback("iniparser: cannot open %s\n", ininame);
+ return NULL ;
+ }
+
+ dict = dictionary_new(0) ;
+ if (!dict) {
+ fclose(in);
+ return NULL ;
+ }
+
+ memset(line, 0, ASCIILINESZ);
+ memset(section, 0, ASCIILINESZ);
+ memset(key, 0, ASCIILINESZ);
+ memset(val, 0, ASCIILINESZ);
+ last=0 ;
+
+ while (fgets(line+last, ASCIILINESZ-last, in)!=NULL) {
+ lineno++ ;
+ len = (int)strlen(line)-1;
+ if (len<=0)
+ continue;
+ /* Safety check against buffer overflows */
+ if (line[len]!='\n' && !feof(in)) {
+ iniparser_error_callback(
+ "iniparser: 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-- ;
+ }
+ if (len < 0) { /* Line was entirely \n and/or spaces */
+ len = 0;
+ }
+ /* Detect multi-line */
+ if (line[len]=='\\') {
+ /* Multi-line value */
+ last=len ;
+ continue ;
+ } else {
+ last=0 ;
+ }
+ switch (iniparser_line(line, section, key, val)) {
+ case LINE_EMPTY:
+ case LINE_COMMENT:
+ break ;
+
+ case LINE_SECTION:
+ mem_err = dictionary_set(dict, section, NULL);
+ break ;
+
+ case LINE_VALUE:
+ sprintf(tmp, "%s:%s", section, key);
+ mem_err = dictionary_set(dict, tmp, val);
+ break ;
+
+ case LINE_ERROR:
+ iniparser_error_callback(
+ "iniparser: syntax error in %s (%d):\n-> %s\n",
+ ininame,
+ lineno,
+ line);
+ errs++ ;
+ break;
+
+ default:
+ break ;
+ }
+ memset(line, 0, ASCIILINESZ);
+ last=0;
+ if (mem_err<0) {
+ iniparser_error_callback("iniparser: memory allocation failure\n");
+ break ;
+ }
+ }
+ if (errs) {
+ dictionary_del(dict);
+ dict = NULL ;
+ }
+ fclose(in);
+ return dict ;
+}
+
+/*-------------------------------------------------------------------------*/
+/**
+ @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 iniparser_freedict(dictionary * d)
+{
+ dictionary_del(d);
+}
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 02/12] ndctl, util: add parse-configs helper
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 01/12] ndctl, util: add iniparser helper Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-07 21:58 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 03/12] ndctl: make ndctl support configuration files Vishal Verma
` (10 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, QI Fuli, Vishal Verma
From: QI Fuli <qi.fuli@fujitsu.com>
Add parse-config util to help ndctl commands parse ndctl global
configuration files.
Link: https://lore.kernel.org/r/20210824095106.104808-3-qi.fuli@fujitsu.com
Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
Makefile.am | 2 ++
util/parse-configs.h | 34 ++++++++++++++++++
util/parse-configs.c | 82 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 118 insertions(+)
create mode 100644 util/parse-configs.h
create mode 100644 util/parse-configs.c
diff --git a/Makefile.am b/Makefile.am
index 235c362..af55f0e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -70,6 +70,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.h b/util/parse-configs.h
new file mode 100644
index 0000000..f70f58f
--- /dev/null
+++ b/util/parse-configs.h
@@ -0,0 +1,34 @@
+// 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,
+ MONITOR_CALLBACK,
+};
+
+struct config;
+typedef int parse_conf_cb(const struct config *, const char *config_file);
+
+struct config {
+ enum parse_conf_type type;
+ const char *key;
+ void *value;
+ void *defval;
+ parse_conf_cb *callback;
+};
+
+#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) }
+#define CONF_MONITOR(k,f) \
+ { .type = MONITOR_CALLBACK, .key = (k), .callback = (f)}
+
+int parse_configs_prefix(const char *__config_file, const char *prefix,
+ const struct config *configs);
diff --git a/util/parse-configs.c b/util/parse-configs.c
new file mode 100644
index 0000000..44dcff4
--- /dev/null
+++ b/util/parse-configs.c
@@ -0,0 +1,82 @@
+// 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 <util/iniparser.h>
+
+static void set_str_val(const char **value, const 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);
+}
+
+static int parse_config_file(const char *config_file,
+ const struct config *configs)
+{
+ dictionary *dic;
+
+ dic = iniparser_load(config_file);
+ if (!dic)
+ return -errno;
+
+ for (; configs->type != CONFIG_END; configs++) {
+ switch (configs->type) {
+ case CONFIG_STRING:
+ set_str_val((const char **)configs->value,
+ iniparser_getstring(dic,
+ configs->key, configs->defval));
+ break;
+ case MONITOR_CALLBACK:
+ case CONFIG_END:
+ break;
+ }
+ }
+
+ iniparser_freedict(dic);
+ return 0;
+}
+
+int parse_configs_prefix(const char *__config_files, const char *prefix,
+ const struct config *configs)
+{
+ char *config_files, *save;
+ const char *config_file;
+ int rc;
+
+ config_files = strdup(__config_files);
+ if (!config_files)
+ return -ENOMEM;
+
+ for (config_file = strtok_r(config_files, " ", &save); config_file;
+ config_file = strtok_r(NULL, " ", &save)) {
+
+ if (strncmp(config_file, "./", 2) != 0)
+ fix_filename(prefix, &config_file);
+
+ if ((configs->type == MONITOR_CALLBACK) &&
+ (strcmp(config_file, configs->key) == 0))
+ rc = configs->callback(configs, configs->key);
+ else
+ rc = parse_config_file(config_file, configs);
+
+ if (rc)
+ goto end;
+ }
+
+ end:
+ free(config_files);
+ return rc;
+
+}
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 03/12] ndctl: make ndctl support configuration files
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 01/12] ndctl, util: add iniparser helper Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 02/12] ndctl, util: add parse-configs helper Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-07 22:51 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 04/12] ndctl, config: add the default ndctl configuration file Vishal Verma
` (9 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, QI Fuli, Vishal Verma
From: QI Fuli <qi.fuli@fujitsu.com>
Add ndctl_configs to ndctl_ctx for supporting ndctl global configuration
files. All files with .conf suffix under {sysconfdir}/ndctl can be
regarded as global configuration files that all ndctl commands can refer
to. Add ndctl_set_configs() public function for setting ndctl default
configuration files' path. Add ndctl_get_configs() public function for
getting ndctl configuration files' path form ndctl_ctx.
Link: https://lore.kernel.org/r/20210824095106.104808-4-qi.fuli@fujitsu.com
Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
configure.ac | 4 ++--
ndctl/lib/private.h | 1 +
ndctl/lib/libndctl.c | 54 ++++++++++++++++++++++++++++++++++++++++++
ndctl/libndctl.h | 2 ++
ndctl/ndctl.c | 1 +
ndctl/Makefile.am | 5 ++--
ndctl/lib/Makefile.am | 4 ++++
ndctl/lib/libndctl.sym | 2 ++
8 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index dc39dbe..42a66e1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -171,9 +171,9 @@ fi
AC_SUBST([systemd_unitdir])
AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
-ndctl_monitorconfdir=${sysconfdir}/ndctl
+ndctl_confdir=${sysconfdir}/ndctl
ndctl_monitorconf=monitor.conf
-AC_SUBST([ndctl_monitorconfdir])
+AC_SUBST([ndctl_confdir])
AC_SUBST([ndctl_monitorconf])
daxctl_modprobe_datadir=${datadir}/daxctl
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 8f4510e..f4ca71f 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -129,6 +129,7 @@ struct ndctl_ctx {
int regions_init;
void *userdata;
struct list_head busses;
+ const char *configs;
int busses_init;
struct udev *udev;
struct udev_queue *udev_queue;
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 536e142..6b68e3a 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -24,6 +24,7 @@
#include <util/util.h>
#include <util/size.h>
#include <util/sysfs.h>
+#include <util/strbuf.h>
#include <ndctl/libndctl.h>
#include <ndctl/namespace.h>
#include <daxctl/libdaxctl.h>
@@ -265,6 +266,58 @@ NDCTL_EXPORT void ndctl_set_userdata(struct ndctl_ctx *ctx, void *userdata)
ctx->userdata = userdata;
}
+static int filter_conf(const struct dirent *dir)
+{
+ if (!dir)
+ return 0;
+
+ if (dir->d_type == DT_REG) {
+ const char *ext = strrchr(dir->d_name, '.');
+ if ((!ext) || (ext == dir->d_name))
+ return 0;
+ if (strcmp(ext, ".conf") == 0)
+ return 1;
+ }
+
+ return 0;
+}
+
+NDCTL_EXPORT void ndctl_set_configs_dir(struct ndctl_ctx **ctx, char *conf_dir)
+{
+ struct dirent **namelist;
+ struct strbuf value = STRBUF_INIT;
+ int rc;
+
+ if ((!ctx) || (!conf_dir))
+ return;
+
+ rc = scandir(conf_dir, &namelist, filter_conf, alphasort);
+ if (rc == -1) {
+ if (errno != ENOENT)
+ err(*ctx, "scandir for configs failed: %s\n",
+ strerror(errno));
+ return;
+ }
+
+ while (rc--) {
+ if (value.len)
+ strbuf_addstr(&value, " ");
+ strbuf_addstr(&value, conf_dir);
+ strbuf_addstr(&value, "/");
+ strbuf_addstr(&value, namelist[rc]->d_name);
+ free(namelist[rc]);
+ }
+ (*ctx)->configs = strbuf_detach(&value, NULL);
+ free(namelist);
+}
+
+NDCTL_EXPORT const char *ndctl_get_configs_dir(struct ndctl_ctx *ctx)
+{
+ if (ctx == NULL)
+ return NULL;
+ return ctx->configs;
+}
+
/**
* ndctl_new - instantiate a new library context
* @ctx: context to establish
@@ -331,6 +384,7 @@ NDCTL_EXPORT int ndctl_new(struct ndctl_ctx **ctx)
c->daxctl_ctx = daxctl_ctx;
return 0;
+
err_ctx:
daxctl_unref(daxctl_ctx);
err_daxctl:
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 87d07b7..883a56f 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -92,6 +92,8 @@ int ndctl_get_log_priority(struct ndctl_ctx *ctx);
void ndctl_set_log_priority(struct ndctl_ctx *ctx, int priority);
void ndctl_set_userdata(struct ndctl_ctx *ctx, void *userdata);
void *ndctl_get_userdata(struct ndctl_ctx *ctx);
+void ndctl_set_configs_dir(struct ndctl_ctx **ctx, char *conf_dir);
+const char *ndctl_get_configs_dir(struct ndctl_ctx *ctx);
enum ndctl_persistence_domain {
PERSISTENCE_NONE = 0,
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 31d2c5e..0f908db 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -125,6 +125,7 @@ int main(int argc, const char **argv)
rc = ndctl_new(&ctx);
if (rc)
goto out;
+ ndctl_set_configs_dir(&ctx, NDCTL_CONF_DIR);
main_handle_internal_command(argc, argv, ctx, commands,
ARRAY_SIZE(commands), PROG_NDCTL);
ndctl_unref(ctx);
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index a63b1e0..1caa031 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -7,8 +7,9 @@ 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_monitorconf)"' >>$@
$(AM_V_GEN) echo '#define NDCTL_KEYS_DIR "$(ndctl_keysdir)"' >>$@
+ $(AM_V_GEN) echo '#define NDCTL_CONF_DIR "$(ndctl_confdir)"' >>$@
ndctl_SOURCES = ndctl.c \
builtin.h \
@@ -73,7 +74,7 @@ ndctl_SOURCES += ../test/libndctl.c \
test.c
endif
-monitor_configdir = $(ndctl_monitorconfdir)
+monitor_configdir = $(ndctl_confdir)
monitor_config_DATA = $(ndctl_monitorconf)
if ENABLE_SYSTEMD_UNITS
diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
index e15bb22..f741c44 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -14,6 +14,10 @@ libndctl_la_SOURCES =\
../../util/log.h \
../../util/sysfs.c \
../../util/sysfs.h \
+ ../../util/strbuf.h \
+ ../../util/strbuf.c \
+ ../../util/wrapper.c \
+ ../../util/usage.c \
../../util/fletcher.h \
dimm.c \
inject.c \
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 58afb74..6e1b510 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -454,4 +454,6 @@ LIBNDCTL_25 {
LIBNDCTL_26 {
ndctl_bus_nfit_translate_spa;
+ ndctl_set_configs_dir;
+ ndctl_get_configs_dir;
} LIBNDCTL_25;
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 04/12] ndctl, config: add the default ndctl configuration file
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (2 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 03/12] ndctl: make ndctl support configuration files Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-08 16:00 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 05/12] ndctl, monitor: refator monitor for supporting multiple config files Vishal Verma
` (8 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, QI Fuli, Vishal Verma
From: QI Fuli <qi.fuli@fujitsu.com>
Install ndctl/ndctl.conf as the default ndctl configuration file.
Link: https://lore.kernel.org/r/20210824095106.104808-5-qi.fuli@fujitsu.com
Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
configure.ac | 2 ++
ndctl/Makefile.am | 4 +++-
ndctl/ndctl.conf | 56 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 61 insertions(+), 1 deletion(-)
create mode 100644 ndctl/ndctl.conf
diff --git a/configure.ac b/configure.ac
index 42a66e1..9e1c6db 100644
--- a/configure.ac
+++ b/configure.ac
@@ -172,8 +172,10 @@ AC_SUBST([systemd_unitdir])
AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
ndctl_confdir=${sysconfdir}/ndctl
+ndctl_conf=ndctl.conf
ndctl_monitorconf=monitor.conf
AC_SUBST([ndctl_confdir])
+AC_SUBST([ndctl_conf])
AC_SUBST([ndctl_monitorconf])
daxctl_modprobe_datadir=${datadir}/daxctl
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 1caa031..fceb3ab 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -43,7 +43,7 @@ keys_configdir = $(ndctl_keysdir)
keys_config_DATA = $(ndctl_keysreadme)
endif
-EXTRA_DIST += keys.readme monitor.conf ndctl-monitor.service
+EXTRA_DIST += keys.readme monitor.conf ndctl-monitor.service ndctl.conf
if ENABLE_DESTRUCTIVE
ndctl_SOURCES += ../test/blk_namespaces.c \
@@ -74,6 +74,8 @@ ndctl_SOURCES += ../test/libndctl.c \
test.c
endif
+ndctl_configdir = $(ndctl_confdir)
+ndctl_config_DATA = $(ndctl_conf)
monitor_configdir = $(ndctl_confdir)
monitor_config_DATA = $(ndctl_monitorconf)
diff --git a/ndctl/ndctl.conf b/ndctl/ndctl.conf
new file mode 100644
index 0000000..04d322d
--- /dev/null
+++ b/ndctl/ndctl.conf
@@ -0,0 +1,56 @@
+# This is the default ndctl configuration file. It contains the
+# configuration directives that give ndctl instructions.
+# Ndctl supports multiple configuration files. All files with the
+# .conf suffix under {sysconfdir}/ndctl can be regarded as ndctl
+# configuration files.
+
+# In this file, lines starting with a hash (#) are comments.
+# The configurations should be in a [section] and follow <key> = <value>
+# style. Multiple space-separated values are allowed, but except the
+# following characters: : ? / \ % " ' $ & ! * { } [ ] ( ) = < > @
+
+[core]
+# The values in [core] section work for all ndctl sub commands.
+# dimm = all
+# bus = all
+# region = all
+# namespace = all
+
+[monitor]
+# The values in [monitor] section work for ndctl monitor.
+# You can change the configuration of ndctl monitor by editing this
+# file or use [--config-file=<file>] option to override this one.
+# The changed value will work after restart ndctl monitor service.
+
+# 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.
+# dimm = all
+
+# The objects to monitor are filtered via its parent bus by setting key "bus".
+# If this value is different from the value of [--bus=<value>] option,
+# both of the values will work.
+# bus = all
+
+# The objects to monitor are filtered via region by setting key "region".
+# If this value is different from the value of [--region=<value>] option,
+# both of the values will work.
+# region = all
+
+# The objects to monitor are filtered via namespace by setting key "namespace".
+# If this value is different from the value of [--namespace=<value>] option,
+# both of the values will work.
+# namespace = all
+
+# The DIMM events to monitor are filtered via event type by setting key
+# "dimm-event". If this value is different from the value of
+# [--dimm-event=<value>] option, both of the values will work.
+# dimm-event = all
+
+# Users can choose to output the notifications to syslog (log=syslog),
+# to standard output (log=standard) or to write into a special file (log=<file>)
+# by setting key "log". If this value is in conflict with the value of
+# [--log=<value>] option, this value will be ignored.
+# Note: Setting value to "standard" or relative path for <file> will not work
+# when running moniotr as a daemon.
+# log = /var/log/ndctl/monitor.log
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 05/12] ndctl, monitor: refator monitor for supporting multiple config files
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (3 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 04/12] ndctl, config: add the default ndctl configuration file Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-08 16:00 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 06/12] ndctl: Update ndctl.spec.in for 'ndctl.conf' Vishal Verma
` (7 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, QI Fuli, Vishal Verma
From: QI Fuli <qi.fuli@fujitsu.com>
Refactor ndctl monitor by using parse-configs helper to support multiple
configuration files.
Link: https://lore.kernel.org/r/20210824095106.104808-6-qi.fuli@fujitsu.com
Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
Documentation/ndctl/ndctl-monitor.txt | 8 +--
ndctl/monitor.c | 73 ++++++++++++++-------------
Documentation/ndctl/Makefile.am | 2 +-
3 files changed, 44 insertions(+), 39 deletions(-)
diff --git a/Documentation/ndctl/ndctl-monitor.txt b/Documentation/ndctl/ndctl-monitor.txt
index dbc9070..8c8c35b 100644
--- a/Documentation/ndctl/ndctl-monitor.txt
+++ b/Documentation/ndctl/ndctl-monitor.txt
@@ -21,8 +21,8 @@ objects and dumping the json format notifications to syslog, standard
output or a logfile.
The objects to monitor and smart events to notify can be selected by
-setting options and/or the configuration file at
-{ndctl_monitorconfdir}/{ndctl_monitorconf}
+setting options and/or configuration files with .conf suffix under
+{ndctl_confdir}
Both, the values in configuration file and in options will work. If
there is a conflict, the values in options will override the values in
@@ -81,8 +81,8 @@ will not work if "--daemon" is specified.
-c::
--config-file=::
- Provide the config file to use. This overrides the default config
- typically found in {ndctl_monitorconfdir}
+ Provide the config file(s) to use. This overrides the default config
+ typically found in {ndctl_confdir}
--daemon::
Run a monitor as a daemon.
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
index ca36179..6bf3160 100644
--- a/ndctl/monitor.c
+++ b/ndctl/monitor.c
@@ -10,6 +10,7 @@
#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>
@@ -28,7 +29,7 @@
static struct monitor {
const char *log;
- const char *config_file;
+ const char *configs;
const char *dimm_event;
FILE *log_file;
bool daemon;
@@ -463,7 +464,7 @@ out:
return rc;
}
-static void parse_config(const char **arg, char *key, char *val, char *ident)
+static void set_monitor_conf(const char **arg, char *key, char *val, char *ident)
{
struct strbuf value = STRBUF_INIT;
size_t arg_len = *arg ? strlen(*arg) : 0;
@@ -479,39 +480,25 @@ static void parse_config(const char **arg, char *key, char *val, char *ident)
*arg = strbuf_detach(&value, NULL);
}
-static int read_config_file(struct ndctl_ctx *ctx, struct monitor *_monitor,
- struct util_filter_params *_param)
+static int parse_monitor_config(const struct config *configs,
+ const char *config_file)
{
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;
- }
+ char *buf = NULL, *seek, *value;
buf = malloc(BUF_SIZE);
if (!buf) {
fail("malloc read config-file buf error\n");
- rc = -ENOMEM;
- goto out;
+ return -ENOMEM;
}
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;
- }
+ err(&monitor, "%s cannot be opened\n", config_file);
+ rc = -errno;
goto out;
}
@@ -554,19 +541,18 @@ static int read_config_file(struct ndctl_ctx *ctx, struct monitor *_monitor,
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);
+ set_monitor_conf(¶m.bus, "bus", value, seek);
+ set_monitor_conf(¶m.dimm, "dimm", value, seek);
+ set_monitor_conf(¶m.region, "region", value, seek);
+ set_monitor_conf(¶m.namespace, "namespace", value, seek);
+ set_monitor_conf(&monitor.dimm_event, "dimm-event", value, seek);
- if (!_monitor->log)
- parse_config(&_monitor->log, "log", value, seek);
+ if (!monitor.log)
+ set_monitor_conf(&monitor.log, "log", value, seek);
}
fclose(f);
out:
free(buf);
- free(config_file);
return rc;
}
@@ -585,8 +571,8 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
OPT_FILENAME('l', "log", &monitor.log,
"<file> | syslog | standard",
"where to output the monitor's notification"),
- OPT_FILENAME('c', "config-file", &monitor.config_file,
- "config-file", "override the default config"),
+ OPT_STRING('c', "config-file", &monitor.configs,
+ "config-file", "override default configs"),
OPT_BOOLEAN('\0', "daemon", &monitor.daemon,
"run ndctl monitor as a daemon"),
OPT_BOOLEAN('u', "human", &monitor.human,
@@ -601,7 +587,20 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
"ndctl monitor [<options>]",
NULL
};
- const char *prefix = "./";
+ const struct config configs[] = {
+ CONF_MONITOR(NDCTL_CONF_FILE, parse_monitor_config),
+ CONF_STR("core:bus", ¶m.bus, NULL),
+ CONF_STR("core:region", ¶m.region, NULL),
+ CONF_STR("core:dimm", ¶m.dimm, NULL),
+ CONF_STR("core:namespace", ¶m.namespace, NULL),
+ CONF_STR("monitor:bus", ¶m.bus, NULL),
+ CONF_STR("monitor:region", ¶m.region, NULL),
+ CONF_STR("monitor:dimm", ¶m.dimm, NULL),
+ CONF_STR("monitor:namespace", ¶m.namespace, NULL),
+ CONF_STR("monitor:dimm-event", &monitor.dimm_event, NULL),
+ CONF_END(),
+ };
+ const char *prefix = "./", *ndctl_configs;
struct util_filter_ctx fctx = { 0 };
struct monitor_filter_arg mfa = { 0 };
int i, rc;
@@ -621,7 +620,13 @@ 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, ¶m);
+ ndctl_configs = ndctl_get_configs_dir(ctx);
+ if (monitor.configs)
+ rc = parse_configs_prefix(monitor.configs, prefix, configs);
+ else if (ndctl_configs)
+ rc = parse_configs_prefix(ndctl_configs, prefix, configs);
+ else
+ rc = 0;
if (rc)
goto out;
diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index f0d5b21..37855cc 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -59,7 +59,7 @@ CLEANFILES = $(man1_MANS)
.ONESHELL:
attrs.adoc: $(srcdir)/Makefile.am
$(AM_V_GEN) cat <<- EOF >$@
- :ndctl_monitorconfdir: $(ndctl_monitorconfdir)
+ :ndctl_confdir: $(ndctl_confdir)
:ndctl_monitorconf: $(ndctl_monitorconf)
:ndctl_keysdir: $(ndctl_keysdir)
EOF
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 06/12] ndctl: Update ndctl.spec.in for 'ndctl.conf'
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (4 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 05/12] ndctl, monitor: refator monitor for supporting multiple config files Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-08 21:47 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 07/12] daxctl: Documentation updates for persistent reconfiguration Vishal Verma
` (6 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, Vishal Verma, QI Fuli
The new config system introduces and installs a sample config file
called ndctl.conf. Update the RPM spec to include this in the %files
section for ndctl.
Cc: QI Fuli <qi.fuli@fujitsu.com>
Reviewed-by: QI Fuli <qi.fuli@jp.fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
ndctl.spec.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/ndctl.spec.in b/ndctl.spec.in
index 0563b2d..07c36ec 100644
--- a/ndctl.spec.in
+++ b/ndctl.spec.in
@@ -118,6 +118,7 @@ make check
%{_sysconfdir}/modprobe.d/nvdimm-security.conf
%config(noreplace) %{_sysconfdir}/ndctl/monitor.conf
+%config(noreplace) %{_sysconfdir}/ndctl/ndctl.conf
%files -n daxctl
%defattr(-,root,root)
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 07/12] daxctl: Documentation updates for persistent reconfiguration
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (5 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 06/12] ndctl: Update ndctl.spec.in for 'ndctl.conf' Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-08 21:51 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 08/12] util/parse-config: refactor filter_conf_files into util/ Vishal Verma
` (5 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, Vishal Verma
Add a man page update describing how daxctl-reconfigure-device(1) can
be used for persistent reconfiguration of a daxctl device using a
config file.
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
.../daxctl/daxctl-reconfigure-device.txt | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/Documentation/daxctl/daxctl-reconfigure-device.txt b/Documentation/daxctl/daxctl-reconfigure-device.txt
index f112b3c..aa87d45 100644
--- a/Documentation/daxctl/daxctl-reconfigure-device.txt
+++ b/Documentation/daxctl/daxctl-reconfigure-device.txt
@@ -162,6 +162,15 @@ include::region-option.txt[]
brought online automatically and immediately with the 'online_movable'
policy. Use this option to disable the automatic onlining behavior.
+-C::
+--check-config::
+ Get reconfiguration parameters from the global daxctl config file.
+ This is typically used when daxctl-reconfigure-device is called from
+ a systemd-udevd device unit file. The reconfiguration proceeds only
+ if the match parameters in a 'reconfigure-device' section of the
+ config match the dax device specified on the command line. See the
+ 'PERSISTENT RECONFIGURATION' section for more details.
+
include::movable-options.txt[]
-f::
@@ -183,6 +192,64 @@ include::human-option.txt[]
include::verbose-option.txt[]
+PERSISTENT RECONFIGURATION
+--------------------------
+
+The 'mode' of a daxctl device is not persistent across reboots by default. This
+is because the device itself does not hold any metadata that hints at what mode
+it was set to, or is intended to be used. The default mode for such a device
+on boot is 'devdax'.
+
+The administrator may set policy such that certain dax devices are always
+reconfigured into a target configuration every boot. This is accomplished via a
+daxctl config file.
+
+The config file may have multiple sections influencing different aspects of
+daxctl operation. The section of interest for persistent reconfiguration is
+'reconfigure-device'. The format of this is as follows:
+
+----
+[reconfigure-device <unique_subsection_name>]
+nvdimm.uuid = <NVDIMM namespace uuid>
+mode = <desired reconfiguration mode> (default: system-ram)
+online = <true|false> (default: true)
+movable = <true|false> (default: true)
+----
+
+Here is an example of a config snippet for managing three devdax namespaces,
+one is left in devdax mode, the second is changed to system-ram mode with
+default options (online, movable), and the third is set to system-ram mode,
+the memory is onlined, but not movable.
+
+Note that the 'subsection name' can be arbitrary, and is only used to
+identify a specific config section. It does not have to match the 'device
+name' (e.g. 'dax0.0' etc).
+
+----
+[reconfigure-device dax0]
+nvdimm.uuid = ed93e918-e165-49d8-921d-383d7b9660c5
+mode = devdax
+
+[reconfigure-device dax1]
+nvdimm.uuid = f36d02ff-1d9f-4fb9-a5b9-8ceb10a00fe3
+mode = system-ram
+
+[reconfigure-device dax2]
+nvdimm.uuid = f36d02ff-1d9f-4fb9-a5b9-8ceb10a00fe3
+mode = system-ram
+online = true
+movable = false
+----
+
+The following example can be used to create a devdax mode namespace, and
+simultaneously add the newly created namespace to the config file for
+system-ram conversion.
+
+----
+ndctl create-namespace --mode=devdax | \
+ jq -r "\"[reconfigure-device $(uuidgen)]\", \"nvdimm.uuid = \(.uuid)\", \"mode = system-ram\"" >> $config_path
+----
+
include::../copyright.txt[]
SEE ALSO
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 08/12] util/parse-config: refactor filter_conf_files into util/
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (6 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 07/12] daxctl: Documentation updates for persistent reconfiguration Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-08 21:52 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 09/12] daxctl: add basic config parsing support Vishal Verma
` (4 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, Vishal Verma, QI Fuli
Move filter_conf() into util/parse-configs.c as filter_conf_files() so
that it can be reused by the config parser in daxctl.
Cc: QI Fuli <qi.fuli@fujitsu.com>
Reviewed-by: QI Fuli <qi.fuli@jp.fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
ndctl/lib/libndctl.c | 19 ++-----------------
util/parse-configs.h | 4 ++++
util/parse-configs.c | 17 +++++++++++++++++
daxctl/lib/Makefile.am | 2 ++
ndctl/lib/Makefile.am | 2 ++
5 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 6b68e3a..9d33005 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -25,6 +25,7 @@
#include <util/size.h>
#include <util/sysfs.h>
#include <util/strbuf.h>
+#include <util/parse-configs.h>
#include <ndctl/libndctl.h>
#include <ndctl/namespace.h>
#include <daxctl/libdaxctl.h>
@@ -266,22 +267,6 @@ NDCTL_EXPORT void ndctl_set_userdata(struct ndctl_ctx *ctx, void *userdata)
ctx->userdata = userdata;
}
-static int filter_conf(const struct dirent *dir)
-{
- if (!dir)
- return 0;
-
- if (dir->d_type == DT_REG) {
- const char *ext = strrchr(dir->d_name, '.');
- if ((!ext) || (ext == dir->d_name))
- return 0;
- if (strcmp(ext, ".conf") == 0)
- return 1;
- }
-
- return 0;
-}
-
NDCTL_EXPORT void ndctl_set_configs_dir(struct ndctl_ctx **ctx, char *conf_dir)
{
struct dirent **namelist;
@@ -291,7 +276,7 @@ NDCTL_EXPORT void ndctl_set_configs_dir(struct ndctl_ctx **ctx, char *conf_dir)
if ((!ctx) || (!conf_dir))
return;
- rc = scandir(conf_dir, &namelist, filter_conf, alphasort);
+ rc = scandir(conf_dir, &namelist, filter_conf_files, alphasort);
if (rc == -1) {
if (errno != ENOENT)
err(*ctx, "scandir for configs failed: %s\n",
diff --git a/util/parse-configs.h b/util/parse-configs.h
index f70f58f..491aebb 100644
--- a/util/parse-configs.h
+++ b/util/parse-configs.h
@@ -1,8 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
// Copyright (C) 2021, FUJITSU LIMITED. ALL rights reserved.
+#include <dirent.h>
#include <stdbool.h>
#include <stdint.h>
+#include <string.h>
#include <util/util.h>
enum parse_conf_type {
@@ -11,6 +13,8 @@ enum parse_conf_type {
MONITOR_CALLBACK,
};
+int filter_conf_files(const struct dirent *dir);
+
struct config;
typedef int parse_conf_cb(const struct config *, const char *config_file);
diff --git a/util/parse-configs.c b/util/parse-configs.c
index 44dcff4..aac129b 100644
--- a/util/parse-configs.c
+++ b/util/parse-configs.c
@@ -6,6 +6,23 @@
#include <util/strbuf.h>
#include <util/iniparser.h>
+int filter_conf_files(const struct dirent *dir)
+{
+ if (!dir)
+ return 0;
+
+ if (dir->d_type == DT_REG) {
+ const char *ext = strrchr(dir->d_name, '.');
+
+ if ((!ext) || (ext == dir->d_name))
+ return 0;
+ if (strcmp(ext, ".conf") == 0)
+ return 1;
+ }
+
+ return 0;
+}
+
static void set_str_val(const char **value, const char *val)
{
struct strbuf buf = STRBUF_INIT;
diff --git a/daxctl/lib/Makefile.am b/daxctl/lib/Makefile.am
index 25efd83..db2351e 100644
--- a/daxctl/lib/Makefile.am
+++ b/daxctl/lib/Makefile.am
@@ -15,6 +15,8 @@ libdaxctl_la_SOURCES =\
../../util/sysfs.h \
../../util/log.c \
../../util/log.h \
+ ../../util/parse-configs.h \
+ ../../util/parse-configs.c \
libdaxctl.c
libdaxctl_la_LIBADD =\
diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
index f741c44..8020eb4 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -19,6 +19,8 @@ libndctl_la_SOURCES =\
../../util/wrapper.c \
../../util/usage.c \
../../util/fletcher.h \
+ ../../util/parse-configs.h \
+ ../../util/parse-configs.c \
dimm.c \
inject.c \
nfit.c \
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 09/12] daxctl: add basic config parsing support
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (7 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 08/12] util/parse-config: refactor filter_conf_files into util/ Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 10/12] util/parse-configs: add a key/value search helper Vishal Verma
` (3 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, Vishal Verma, QI Fuli
Add support similar to ndctl and libndctl for parsing config files. This
allows storing a config file path/list in the daxctl_ctx, and adds APIs
for setting and retrieving it.
Cc: QI Fuli <qi.fuli@fujitsu.com>
Reviewed-by: QI Fuli <qi.fuli@jp.fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
.../daxctl/daxctl-reconfigure-device.txt | 8 ++++
configure.ac | 3 ++
daxctl/lib/libdaxctl.c | 39 +++++++++++++++++++
daxctl/libdaxctl.h | 2 +
Documentation/daxctl/Makefile.am | 11 +++++-
daxctl/Makefile.am | 1 +
daxctl/lib/Makefile.am | 4 ++
daxctl/lib/libdaxctl.sym | 2 +
8 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/Documentation/daxctl/daxctl-reconfigure-device.txt b/Documentation/daxctl/daxctl-reconfigure-device.txt
index aa87d45..09556cc 100644
--- a/Documentation/daxctl/daxctl-reconfigure-device.txt
+++ b/Documentation/daxctl/daxctl-reconfigure-device.txt
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
+include::attrs.adoc[]
+
daxctl-reconfigure-device(1)
============================
@@ -250,6 +252,12 @@ ndctl create-namespace --mode=devdax | \
jq -r "\"[reconfigure-device $(uuidgen)]\", \"nvdimm.uuid = \(.uuid)\", \"mode = system-ram\"" >> $config_path
----
+The default location for daxctl config files is under {daxctl_confdir}/,
+and any file with a '.conf' suffix at this location is considered. It is
+acceptable to have multiple files containing ini-style config sections,
+but the {section, subsection} tuple must be unique across all config files
+under {daxctl_confdir}/.
+
include::../copyright.txt[]
SEE ALSO
diff --git a/configure.ac b/configure.ac
index 9e1c6db..e779b51 100644
--- a/configure.ac
+++ b/configure.ac
@@ -178,6 +178,9 @@ AC_SUBST([ndctl_confdir])
AC_SUBST([ndctl_conf])
AC_SUBST([ndctl_monitorconf])
+daxctl_confdir=${sysconfdir}/daxctl
+AC_SUBST([daxctl_confdir])
+
daxctl_modprobe_datadir=${datadir}/daxctl
daxctl_modprobe_data=daxctl.conf
AC_SUBST([daxctl_modprobe_datadir])
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 860bd9c..2de3616 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -17,6 +17,8 @@
#include <util/log.h>
#include <util/sysfs.h>
#include <util/iomem.h>
+#include <util/strbuf.h>
+#include <util/parse-configs.h>
#include <daxctl/libdaxctl.h>
#include "libdaxctl-private.h"
@@ -37,6 +39,7 @@ struct daxctl_ctx {
struct log_ctx ctx;
int refcount;
void *userdata;
+ const char *configs;
int regions_init;
struct list_head regions;
struct kmod_ctx *kmod_ctx;
@@ -68,6 +71,42 @@ DAXCTL_EXPORT void daxctl_set_userdata(struct daxctl_ctx *ctx, void *userdata)
ctx->userdata = userdata;
}
+DAXCTL_EXPORT void daxctl_set_configs_dir(struct daxctl_ctx **ctx, char *conf_dir)
+{
+ struct dirent **namelist;
+ struct strbuf value = STRBUF_INIT;
+ int rc;
+
+ if ((!ctx) || (!conf_dir))
+ return;
+
+ rc = scandir(conf_dir, &namelist, filter_conf_files, alphasort);
+ if (rc == -1) {
+ if (errno != ENOENT)
+ err(*ctx, "scandir for configs failed: %s\n",
+ strerror(errno));
+ return;
+ }
+
+ while (rc--) {
+ if (value.len)
+ strbuf_addstr(&value, " ");
+ strbuf_addstr(&value, conf_dir);
+ strbuf_addstr(&value, "/");
+ strbuf_addstr(&value, namelist[rc]->d_name);
+ free(namelist[rc]);
+ }
+ (*ctx)->configs = strbuf_detach(&value, NULL);
+ free(namelist);
+}
+
+DAXCTL_EXPORT const char *daxctl_get_configs_dir(struct daxctl_ctx *ctx)
+{
+ if (ctx == NULL)
+ return NULL;
+ return ctx->configs;
+}
+
/**
* daxctl_new - instantiate a new library context
* @ctx: context to establish
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index 683ae9c..6be09d9 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -28,6 +28,8 @@ int daxctl_get_log_priority(struct daxctl_ctx *ctx);
void daxctl_set_log_priority(struct daxctl_ctx *ctx, int priority);
void daxctl_set_userdata(struct daxctl_ctx *ctx, void *userdata);
void *daxctl_get_userdata(struct daxctl_ctx *ctx);
+void daxctl_set_configs_dir(struct daxctl_ctx **ctx, char *conf_dir);
+const char *daxctl_get_configs_dir(struct daxctl_ctx *ctx);
struct daxctl_region;
struct daxctl_region *daxctl_new_region(struct daxctl_ctx *ctx, int id,
diff --git a/Documentation/daxctl/Makefile.am b/Documentation/daxctl/Makefile.am
index 5991731..9c43e61 100644
--- a/Documentation/daxctl/Makefile.am
+++ b/Documentation/daxctl/Makefile.am
@@ -33,11 +33,20 @@ EXTRA_DIST = $(man1_MANS)
CLEANFILES = $(man1_MANS)
+.ONESHELL:
+attrs.adoc: $(srcdir)/Makefile.am
+ $(AM_V_GEN) cat <<- EOF >$@
+ :daxctl_confdir: $(daxctl_confdir)
+ :daxctl_conf: $(daxctl_conf)
+ :ndctl_keysdir: $(ndctl_keysdir)
+ EOF
+
XML_DEPS = \
../../version.m4 \
../copyright.txt \
Makefile \
- $(CONFFILE)
+ $(CONFFILE) \
+ attrs.adoc
RM ?= rm -f
diff --git a/daxctl/Makefile.am b/daxctl/Makefile.am
index 9b1313a..a5c6de1 100644
--- a/daxctl/Makefile.am
+++ b/daxctl/Makefile.am
@@ -10,6 +10,7 @@ config.h: $(srcdir)/Makefile.am
"$(daxctl_modprobe_datadir)/$(daxctl_modprobe_data)"' >>$@ && \
echo '#define DAXCTL_MODPROBE_INSTALL \
"$(sysconfdir)/modprobe.d/$(daxctl_modprobe_data)"' >>$@
+ $(AM_V_GEN) echo '#define DAXCTL_CONF_DIR "$(daxctl_confdir)"' >>$@
daxctl_SOURCES =\
daxctl.c \
diff --git a/daxctl/lib/Makefile.am b/daxctl/lib/Makefile.am
index db2351e..7a53598 100644
--- a/daxctl/lib/Makefile.am
+++ b/daxctl/lib/Makefile.am
@@ -13,6 +13,10 @@ libdaxctl_la_SOURCES =\
../../util/iomem.h \
../../util/sysfs.c \
../../util/sysfs.h \
+ ../../util/strbuf.h \
+ ../../util/strbuf.c \
+ ../../util/wrapper.c \
+ ../../util/usage.c \
../../util/log.c \
../../util/log.h \
../../util/parse-configs.h \
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index a13e93d..7098aba 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -96,4 +96,6 @@ LIBDAXCTL_9 {
global:
daxctl_dev_will_auto_online_memory;
daxctl_dev_has_online_memory;
+ daxctl_set_configs_dir;
+ daxctl_get_configs_dir;
} LIBDAXCTL_8;
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 10/12] util/parse-configs: add a key/value search helper
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (8 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 09/12] daxctl: add basic config parsing support Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 11/12] daxctl/device.c: add an option for getting params from a config file Vishal Verma
` (2 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, Vishal Verma, QI Fuli
Add a new config query type called CONFIG_SEARCH_SECTION, which searches
all loaded config files based on a query criteria of: specified section
name, specified key/value pair within that section, and can return other
key/values from the section that matched the search criteria.
This allows for multiple named subsections, where a subsection name is
of the type: '[section subsection]'.
Cc: QI Fuli <qi.fuli@fujitsu.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: QI Fuli <qi.fuli@jp.fujitsu.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
util/parse-configs.h | 15 +++++++++++++
util/parse-configs.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+)
diff --git a/util/parse-configs.h b/util/parse-configs.h
index 491aebb..6dcc01c 100644
--- a/util/parse-configs.h
+++ b/util/parse-configs.h
@@ -9,6 +9,7 @@
enum parse_conf_type {
CONFIG_STRING,
+ CONFIG_SEARCH_SECTION,
CONFIG_END,
MONITOR_CALLBACK,
};
@@ -20,6 +21,10 @@ typedef int parse_conf_cb(const struct config *, const char *config_file);
struct config {
enum parse_conf_type type;
+ const char *section;
+ const char *search_key;
+ const char *search_val;
+ const char *get_key;
const char *key;
void *value;
void *defval;
@@ -31,6 +36,16 @@ struct config {
#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) }
+#define CONF_SEARCH(s, sk, sv, gk, v, d) \
+{ \
+ .type = CONFIG_SEARCH_SECTION, \
+ .section = (s), \
+ .search_key = (sk), \
+ .search_val = (sv), \
+ .get_key = (gk), \
+ .value = check_vtype(v, const char **), \
+ .defval = (d) \
+}
#define CONF_MONITOR(k,f) \
{ .type = MONITOR_CALLBACK, .key = (k), .callback = (f)}
diff --git a/util/parse-configs.c b/util/parse-configs.c
index aac129b..ca1daef 100644
--- a/util/parse-configs.c
+++ b/util/parse-configs.c
@@ -39,6 +39,54 @@ static void set_str_val(const char **value, const char *val)
*value = strbuf_detach(&buf, NULL);
}
+static const char *search_section_kv(dictionary *d, const struct config *c)
+{
+ int i;
+
+ for (i = 0; i < iniparser_getnsec(d); i++) {
+ const char *cur_sec_full = iniparser_getsecname(d, i);
+ char *cur_sec = strdup(cur_sec_full);
+ const char *search_val, *ret_val;
+ const char *delim = " \t\n\r";
+ char *save, *cur, *query;
+
+ if (!cur_sec)
+ return NULL;
+ if (!c->section || !c->search_key || !c->search_val || !c->get_key) {
+ fprintf(stderr, "warning: malformed config query, skipping\n");
+ return NULL;
+ }
+
+ cur = strtok_r(cur_sec, delim, &save);
+ if ((cur == NULL) || (strcmp(cur, c->section) != 0))
+ goto out_sec;
+
+ if (asprintf(&query, "%s:%s", cur_sec_full, c->search_key) < 0)
+ goto out_sec;
+ search_val = iniparser_getstring(d, query, NULL);
+ if (!search_val)
+ goto out_query;
+ if (strcmp(search_val, c->search_val) != 0)
+ goto out_query;
+
+ /* we're now in a matching section */
+ free(query);
+ if (asprintf(&query, "%s:%s", cur_sec_full, c->get_key) < 0)
+ goto out_sec;
+ ret_val = iniparser_getstring(d, query, NULL);
+ free(query);
+ free(cur_sec);
+ return ret_val;
+
+out_query:
+ free(query);
+out_sec:
+ free(cur_sec);
+ }
+
+ return NULL;
+}
+
static int parse_config_file(const char *config_file,
const struct config *configs)
{
@@ -55,6 +103,9 @@ static int parse_config_file(const char *config_file,
iniparser_getstring(dic,
configs->key, configs->defval));
break;
+ case CONFIG_SEARCH_SECTION:
+ set_str_val((const char **)configs->value,
+ search_section_kv(dic, configs));
case MONITOR_CALLBACK:
case CONFIG_END:
break;
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 11/12] daxctl/device.c: add an option for getting params from a config file
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (9 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 10/12] util/parse-configs: add a key/value search helper Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 12/12] daxctl: add systemd service and udev rule for automatic reconfiguration Vishal Verma
2021-12-06 23:10 ` [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Verma, Vishal L
12 siblings, 0 replies; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, Vishal Verma, QI Fuli
Add a new option to daxctl-reconfigure-device that allows it to
comprehend the new global config system in ndctl/daxctl. With this, the
reconfigure-device command can query the config to match a specific
device UUID, and operate using the parameters supplied in that INI
section.
This is in preparation to make daxctl device reconfiguration (usually
as system-ram) policy based, so that reconfiguration can happen
automatically on boot.
Cc: QI Fuli <qi.fuli@fujitsu.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
daxctl/daxctl.c | 1 +
daxctl/device.c | 174 ++++++++++++++++++++++++++++++++++++++++++++-
daxctl/Makefile.am | 1 +
3 files changed, 174 insertions(+), 2 deletions(-)
diff --git a/daxctl/daxctl.c b/daxctl/daxctl.c
index 928814c..41b35e5 100644
--- a/daxctl/daxctl.c
+++ b/daxctl/daxctl.c
@@ -95,6 +95,7 @@ int main(int argc, const char **argv)
rc = daxctl_new(&ctx);
if (rc)
goto out;
+ daxctl_set_configs_dir(&ctx, DAXCTL_CONF_DIR);
main_handle_internal_command(argc, argv, ctx, commands,
ARRAY_SIZE(commands), PROG_DAXCTL);
daxctl_unref(ctx);
diff --git a/daxctl/device.c b/daxctl/device.c
index a427b7d..7b02420 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -14,8 +14,10 @@
#include <util/filter.h>
#include <json-c/json.h>
#include <json-c/json_util.h>
+#include <ndctl/libndctl.h>
#include <daxctl/libdaxctl.h>
#include <util/parse-options.h>
+#include <util/parse-configs.h>
#include <ccan/array_size/array_size.h>
static struct {
@@ -25,6 +27,7 @@ static struct {
const char *size;
const char *align;
const char *input;
+ bool check_config;
bool no_online;
bool no_movable;
bool force;
@@ -65,6 +68,9 @@ enum device_action {
ACTION_DESTROY,
};
+#define CONF_SECTION "reconfigure-device"
+#define CONF_NVDIMM_UUID_STR "nvdimm.uuid"
+
#define BASE_OPTIONS() \
OPT_STRING('r', "region", ¶m.region, "region-id", "filter by region"), \
OPT_BOOLEAN('u', "human", ¶m.human, "use human friendly number formats"), \
@@ -75,7 +81,9 @@ OPT_STRING('m', "mode", ¶m.mode, "mode", "mode to switch the device to"), \
OPT_BOOLEAN('N', "no-online", ¶m.no_online, \
"don't auto-online memory sections"), \
OPT_BOOLEAN('f', "force", ¶m.force, \
- "attempt to offline memory sections before reconfiguration")
+ "attempt to offline memory sections before reconfiguration"), \
+OPT_BOOLEAN('C', "check-config", ¶m.check_config, \
+ "use config files to determine parameters for the operation")
#define CREATE_OPTIONS() \
OPT_STRING('s', "size", ¶m.size, "size", "size to switch the device to"), \
@@ -218,6 +226,137 @@ err:
return rc;
}
+static int conf_string_to_bool(const char *str)
+{
+ if (!str)
+ return INT_MAX;
+ if (strncmp(str, "t", 1) == 0 ||
+ strncmp(str, "T", 1) == 0 ||
+ strncmp(str, "y", 1) == 0 ||
+ strncmp(str, "Y", 1) == 0 ||
+ strncmp(str, "1", 1) == 0)
+ return true;
+ if (strncmp(str, "f", 1) == 0 ||
+ strncmp(str, "F", 1) == 0 ||
+ strncmp(str, "n", 1) == 0 ||
+ strncmp(str, "N", 1) == 0 ||
+ strncmp(str, "0", 1) == 0)
+ return false;
+ return INT_MAX;
+}
+
+#define conf_assign_inverted_bool(p, conf_var) \
+do { \
+ if (conf_string_to_bool(conf_var) != INT_MAX) \
+ param.p = !conf_string_to_bool(conf_var); \
+} while(0)
+
+static int parse_config_reconfig_set_params(struct daxctl_ctx *ctx, const char *device,
+ const char *uuid)
+{
+ const char *conf_online = NULL, *conf_movable = NULL;
+ const struct config configs[] = {
+ CONF_SEARCH(CONF_SECTION, CONF_NVDIMM_UUID_STR, uuid,
+ "mode", ¶m.mode, NULL),
+ CONF_SEARCH(CONF_SECTION, CONF_NVDIMM_UUID_STR, uuid,
+ "online", &conf_online, NULL),
+ CONF_SEARCH(CONF_SECTION, CONF_NVDIMM_UUID_STR, uuid,
+ "movable", &conf_movable, NULL),
+ CONF_END(),
+ };
+ const char *prefix = "./", *daxctl_configs;
+ int rc;
+
+ daxctl_configs = daxctl_get_configs_dir(ctx);
+ if (daxctl_configs == NULL)
+ return 0;
+
+ rc = parse_configs_prefix(daxctl_configs, prefix, configs);
+ if (rc < 0)
+ return rc;
+
+ conf_assign_inverted_bool(no_online, conf_online);
+ conf_assign_inverted_bool(no_movable, conf_movable);
+
+ return 0;
+}
+
+static bool daxctl_ndns_has_device(struct ndctl_namespace *ndns,
+ const char *device)
+{
+ struct daxctl_region *dax_region;
+ struct ndctl_dax *dax;
+
+ dax = ndctl_namespace_get_dax(ndns);
+ if (!dax)
+ return false;
+
+ dax_region = ndctl_dax_get_daxctl_region(dax);
+ if (dax_region) {
+ struct daxctl_dev *dev;
+
+ dev = daxctl_dev_get_first(dax_region);
+ if (dev) {
+ if (strcmp(daxctl_dev_get_devname(dev), device) == 0)
+ return true;
+ }
+ }
+ return false;
+}
+
+static int parse_config_reconfig(struct daxctl_ctx *ctx, const char *device)
+{
+ struct ndctl_namespace *ndns;
+ struct ndctl_ctx *ndctl_ctx;
+ struct ndctl_region *region;
+ struct ndctl_bus *bus;
+ struct ndctl_dax *dax;
+ int rc, found = 0;
+ char uuid_buf[40];
+ uuid_t uuid;
+
+ if (strcmp(device, "all") == 0)
+ return 0;
+
+ rc = ndctl_new(&ndctl_ctx);
+ if (rc < 0)
+ return rc;
+
+ ndctl_bus_foreach(ndctl_ctx, bus) {
+ ndctl_region_foreach(bus, region) {
+ ndctl_namespace_foreach(region, ndns) {
+ if (daxctl_ndns_has_device(ndns, device)) {
+ dax = ndctl_namespace_get_dax(ndns);
+ if (!dax)
+ continue;
+ ndctl_dax_get_uuid(dax, uuid);
+ found = 1;
+ }
+ }
+ }
+ }
+
+ if (!found) {
+ fprintf(stderr, "no UUID match for %s found in config files\n",
+ device);
+ return 0;
+ }
+
+ uuid_unparse(uuid, uuid_buf);
+ return parse_config_reconfig_set_params(ctx, device, uuid_buf);
+}
+
+static int parse_device_config(struct daxctl_ctx *ctx, const char *device,
+ enum device_action action)
+{
+ switch (action) {
+ case ACTION_RECONFIG:
+ return parse_config_reconfig(ctx, device);
+ default:
+ return 0;
+ }
+}
+
static const char *parse_device_options(int argc, const char **argv,
enum device_action action, const struct option *options,
const char *usage, struct daxctl_ctx *ctx)
@@ -228,8 +367,11 @@ static const char *parse_device_options(int argc, const char **argv,
};
unsigned long long units = 1;
int i, rc = 0;
+ char *device;
argc = parse_options(argc, argv, options, u, 0);
+ device = basename(argv[0]);
+
/* Handle action-agnostic non-option arguments */
if (argc == 0 &&
@@ -279,6 +421,34 @@ static const char *parse_device_options(int argc, const char **argv,
if (param.human)
flags |= UTIL_JSON_HUMAN;
+ /* Scan config file(s) for options. This sets param.foo accordingly */
+ if (param.check_config) {
+ if (param.mode || param.no_online || param.no_movable) {
+ fprintf(stderr,
+ "%s: -C cannot be used with --mode, --(no-)movable, or --(no-)online\n",
+ device);
+ usage_with_options(u, options);
+ }
+ rc = parse_device_config(ctx, device, action);
+ if (rc) {
+ fprintf(stderr, "error parsing config file: %s\n",
+ strerror(-rc));
+ return NULL;
+ }
+ if (!param.mode && !param.no_online && !param.no_movable) {
+ fprintf(stderr, "%s: missing or malformed config section\n",
+ device);
+ /*
+ * Exit with success since the most common case is there is
+ * no config defined for this device, and we don't want to
+ * treat that as an error. There isn't an easy way currently
+ * to distinguish between a malformed config entry from a
+ * completely missing config section.
+ */
+ exit(0);
+ }
+ }
+
/* Handle action-specific options */
switch (action) {
case ACTION_RECONFIG:
@@ -336,7 +506,7 @@ static const char *parse_device_options(int argc, const char **argv,
return NULL;
}
- return argv[0];
+ return device;
}
static int dev_online_memory(struct daxctl_dev *dev)
diff --git a/daxctl/Makefile.am b/daxctl/Makefile.am
index a5c6de1..ac97cd1 100644
--- a/daxctl/Makefile.am
+++ b/daxctl/Makefile.am
@@ -23,6 +23,7 @@ daxctl_SOURCES =\
daxctl_LDADD =\
lib/libdaxctl.la \
+ ../ndctl/lib/libndctl.la \
../libutil.a \
$(UUID_LIBS) \
$(KMOD_LIBS) \
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [ndctl PATCH v2 12/12] daxctl: add systemd service and udev rule for automatic reconfiguration
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (10 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 11/12] daxctl/device.c: add an option for getting params from a config file Vishal Verma
@ 2021-12-06 22:28 ` Vishal Verma
2021-12-06 23:10 ` [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Verma, Vishal L
12 siblings, 0 replies; 26+ messages in thread
From: Vishal Verma @ 2021-12-06 22:28 UTC (permalink / raw)
To: nvdimm; +Cc: Dan Williams, QI Fuli, fenghua.hu, Vishal Verma, QI Fuli
Install a systemd service that calls "daxctl-reconfigure-device
--check-config" with a daxctl device passed in to it via the
environment.
Install a udev rule that is triggered for every daxctl device, and
triggers the above oneshot systemd service.
On boot, whenever a daxctl device is found, udev triggers a
device-specific systemd service called, for example:
daxdev-reconfigure@-dev-dax0.0.service
This initiates a daxctl-reconfigure-device with a config lookup for the
'dax0.0' device. If the config has a '[reconfigure-device <unique_id>]'
section, it uses the information in that to set the operating mode of
the device.
If any device is in an unexpected status, 'journalctl' can be used to
view the reconfiguration log for that device, for example:
journalctl --unit daxdev-reconfigure@-dev-dax0.0.service
Update the RPM spec file to include the newly added files to the RPM
build.
Cc: QI Fuli <qi.fuli@fujitsu.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
configure.ac | 9 ++++++++-
daxctl/90-daxctl-device.rules | 1 +
daxctl/Makefile.am | 7 +++++++
daxctl/daxdev-reconfigure@.service | 8 ++++++++
ndctl.spec.in | 3 +++
5 files changed, 27 insertions(+), 1 deletion(-)
create mode 100644 daxctl/90-daxctl-device.rules
create mode 100644 daxctl/daxdev-reconfigure@.service
diff --git a/configure.ac b/configure.ac
index e779b51..dcdc15f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -160,7 +160,7 @@ AC_CHECK_FUNCS([ \
AC_ARG_WITH([systemd],
AS_HELP_STRING([--with-systemd],
- [Enable systemd functionality (monitor). @<:@default=yes@:>@]),
+ [Enable systemd functionality. @<:@default=yes@:>@]),
[], [with_systemd=yes])
if test "x$with_systemd" = "xyes"; then
@@ -186,6 +186,13 @@ daxctl_modprobe_data=daxctl.conf
AC_SUBST([daxctl_modprobe_datadir])
AC_SUBST([daxctl_modprobe_data])
+AC_ARG_WITH(udevrulesdir,
+ [AS_HELP_STRING([--with-udevrulesdir=DIR], [udev rules.d directory])],
+ [UDEVRULESDIR="$withval"],
+ [UDEVRULESDIR='${prefix}/lib/udev/rules.d']
+)
+AC_SUBST(UDEVRULESDIR)
+
AC_ARG_WITH([keyutils],
AS_HELP_STRING([--with-keyutils],
[Enable keyutils functionality (security). @<:@default=yes@:>@]), [], [with_keyutils=yes])
diff --git a/daxctl/90-daxctl-device.rules b/daxctl/90-daxctl-device.rules
new file mode 100644
index 0000000..ee0670f
--- /dev/null
+++ b/daxctl/90-daxctl-device.rules
@@ -0,0 +1 @@
+ACTION=="add", SUBSYSTEM=="dax", TAG+="systemd", ENV{SYSTEMD_WANTS}="daxdev-reconfigure@$env{DEVNAME}.service"
diff --git a/daxctl/Makefile.am b/daxctl/Makefile.am
index ac97cd1..6d41016 100644
--- a/daxctl/Makefile.am
+++ b/daxctl/Makefile.am
@@ -28,3 +28,10 @@ daxctl_LDADD =\
$(UUID_LIBS) \
$(KMOD_LIBS) \
$(JSON_LIBS)
+
+udevrulesdir = $(UDEVRULESDIR)
+udevrules_DATA = 90-daxctl-device.rules
+
+if ENABLE_SYSTEMD_UNITS
+systemd_unit_DATA = daxdev-reconfigure@.service
+endif
diff --git a/daxctl/daxdev-reconfigure@.service b/daxctl/daxdev-reconfigure@.service
new file mode 100644
index 0000000..13d570c
--- /dev/null
+++ b/daxctl/daxdev-reconfigure@.service
@@ -0,0 +1,8 @@
+[Unit]
+Description=Automatic daxctl device reconfiguration
+Documentation=man:daxctl-reconfigure-device(1)
+
+[Service]
+Type=forking
+GuessMainPID=false
+ExecStart=/bin/sh -c "exec daxctl reconfigure-device --check-config %I"
diff --git a/ndctl.spec.in b/ndctl.spec.in
index 07c36ec..2c33664 100644
--- a/ndctl.spec.in
+++ b/ndctl.spec.in
@@ -26,6 +26,7 @@ BuildRequires: pkgconfig(json-c)
BuildRequires: pkgconfig(bash-completion)
BuildRequires: pkgconfig(systemd)
BuildRequires: keyutils-libs-devel
+BuildRequires: systemd-rpm-macros
%description
Utility library for managing the "libnvdimm" subsystem. The "libnvdimm"
@@ -126,6 +127,8 @@ make check
%{_bindir}/daxctl
%{_mandir}/man1/daxctl*
%{_datadir}/daxctl/daxctl.conf
+%{_unitdir}/daxdev-reconfigure@.service
+%config %{_udevrulesdir}/90-daxctl-device.rules
%files -n LNAME
%defattr(-,root,root)
--
2.33.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
` (11 preceding siblings ...)
2021-12-06 22:28 ` [ndctl PATCH v2 12/12] daxctl: add systemd service and udev rule for automatic reconfiguration Vishal Verma
@ 2021-12-06 23:10 ` Verma, Vishal L
2021-12-07 11:24 ` qi.fuli
12 siblings, 1 reply; 26+ messages in thread
From: Verma, Vishal L @ 2021-12-06 23:10 UTC (permalink / raw)
To: nvdimm; +Cc: Williams, Dan J, Hu, Fenghua, qi.fuli
On Mon, 2021-12-06 at 15:28 -0700, Vishal Verma wrote:
> Changes since v1[1]:
> - Collect review tags
> - Fix 'make clean' removing the reconfigure script from the source tree
> (Fenghua, Qi)
> - Documentation wordsmithing (Dan)
> - Fix line break after declarations in parse-configs.c (Dan)
> - Move daxctl config files to its own directory, /etc/daxctl/ (Dan)
> - Improve failure mode in the absence of a configs directory
> - Rename {nd,dax}ctl_{get|set}_configs to
> {nd,dax}ctl_{get|set}_configs_dir
> - Exit with success if -C is specified, but no matching config section
> is found.
> - Refuse to proceed if CLI options are passed in along with -C (Dan)
> - In the config file, rename: s/[auto-online foo]/[reconfigure-device foo/
> and s/uuid/nvdimm.uuid/ (Dan)
> - Teach device.c to accept /dev/daxX.Y instead of only daxX.Y and thus
> remove the need for a wrapper script that systemd invokes (Dan)
>
> These patches add policy (config file) support to daxctl. The
> introductory user is daxctl-reconfigure-device. Sysadmins may wish to
> use daxctl devices as system-ram, but it may be cumbersome to automate
> the reconfiguration step for every device upon boot.
>
> Introduce a new option for daxctl-reconfigure-device, --check-config.
> This is at the heart of policy based reconfiguration, as it allows
> daxctl to look up reconfiguration parameters for a given device from the
> config system instead of the command line.
>
> Some systemd and udev glue then automates this for every new dax device
> that shows up, providing a way for the administrator to simply list all
> the 'system-ram' UUIDs in a config file, and not have to worry about
> anything else.
>
> An example config file can be:
>
> # cat /etc/ndctl/daxctl.conf
Missed updating this, it should be /etc/daxctl/daxctl.conf
>
> [reconfigure-device unique_identifier_foo]
> nvdimm.uuid = 48d8e42c-a2f0-4312-9e70-a837faafe862
> mode = system-ram
> online = true
> movable = false
>
> Any file under '/etc/ndctl/' can be used - all files with a '.conf' suffix
And this should be '/etc/daxctl/'
> will be considered when looking for matches.
>
> These patches depend on the initial config file support from Qi Fuli[2].
>
> I've re-rolled Qi's original patches as the first five patches in this
> series because of a change I made for graceful handling in the case of a
> missing configs directory, and also to incorporate review feedback that
> applied to the dependant patches. Patch 6 onwards is the actual v2 of
> the daxctl policy work.
>
> A branch containing these patches is available at [3].
>
> [1]: https://lore.kernel.org/nvdimm/20210831090459.2306727-1-vishal.l.verma@intel.com/
> [2]: https://lore.kernel.org/nvdimm/20210824095106.104808-1-qi.fuli@fujitsu.com/
> [3]: https://github.com/pmem/ndctl/tree/vv/daxctl_config_v2
>
> QI Fuli (5):
> ndctl, util: add iniparser helper
> ndctl, util: add parse-configs helper
> ndctl: make ndctl support configuration files
> ndctl, config: add the default ndctl configuration file
> ndctl, monitor: refator monitor for supporting multiple config files
>
> Vishal Verma (7):
> ndctl: Update ndctl.spec.in for 'ndctl.conf'
> daxctl: Documentation updates for persistent reconfiguration
> util/parse-config: refactor filter_conf_files into util/
> daxctl: add basic config parsing support
> util/parse-configs: add a key/value search helper
> daxctl/device.c: add an option for getting params from a config file
> daxctl: add systemd service and udev rule for automatic
> reconfiguration
>
> .../daxctl/daxctl-reconfigure-device.txt | 75 ++
> Documentation/ndctl/ndctl-monitor.txt | 8 +-
> configure.ac | 18 +-
> Makefile.am | 8 +-
> ndctl/lib/private.h | 1 +
> daxctl/lib/libdaxctl.c | 39 +
> ndctl/lib/libndctl.c | 39 +
> daxctl/libdaxctl.h | 2 +
> ndctl/libndctl.h | 2 +
> util/dictionary.h | 175 ++++
> util/iniparser.h | 360 ++++++++
> util/parse-configs.h | 53 ++
> daxctl/daxctl.c | 1 +
> daxctl/device.c | 174 +++-
> ndctl/monitor.c | 73 +-
> ndctl/ndctl.c | 1 +
> util/dictionary.c | 383 ++++++++
> util/iniparser.c | 838 ++++++++++++++++++
> util/parse-configs.c | 150 ++++
> Documentation/daxctl/Makefile.am | 11 +-
> Documentation/ndctl/Makefile.am | 2 +-
> daxctl/90-daxctl-device.rules | 1 +
> daxctl/Makefile.am | 9 +
> daxctl/daxdev-reconfigure@.service | 8 +
> daxctl/lib/Makefile.am | 6 +
> daxctl/lib/libdaxctl.sym | 2 +
> ndctl.spec.in | 4 +
> ndctl/Makefile.am | 9 +-
> ndctl/lib/Makefile.am | 6 +
> ndctl/lib/libndctl.sym | 2 +
> ndctl/ndctl.conf | 56 ++
> 31 files changed, 2467 insertions(+), 49 deletions(-)
> create mode 100644 util/dictionary.h
> create mode 100644 util/iniparser.h
> create mode 100644 util/parse-configs.h
> create mode 100644 util/dictionary.c
> create mode 100644 util/iniparser.c
> create mode 100644 util/parse-configs.c
> create mode 100644 daxctl/90-daxctl-device.rules
> create mode 100644 daxctl/daxdev-reconfigure@.service
> create mode 100644 ndctl/ndctl.conf
>
>
> base-commit: 4e646fa490ba4b782afa188dd8818b94c419924e
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl
2021-12-06 23:10 ` [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Verma, Vishal L
@ 2021-12-07 11:24 ` qi.fuli
0 siblings, 0 replies; 26+ messages in thread
From: qi.fuli @ 2021-12-07 11:24 UTC (permalink / raw)
To: 'Verma, Vishal L', nvdimm; +Cc: Williams, Dan J, Hu, Fenghua
> Subject: Re: [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl
>
> On Mon, 2021-12-06 at 15:28 -0700, Vishal Verma wrote:
> > Changes since v1[1]:
> > - Collect review tags
> > - Fix 'make clean' removing the reconfigure script from the source tree
> > (Fenghua, Qi)
> > - Documentation wordsmithing (Dan)
> > - Fix line break after declarations in parse-configs.c (Dan)
> > - Move daxctl config files to its own directory, /etc/daxctl/ (Dan)
> > - Improve failure mode in the absence of a configs directory
> > - Rename {nd,dax}ctl_{get|set}_configs to
> > {nd,dax}ctl_{get|set}_configs_dir
> > - Exit with success if -C is specified, but no matching config section
> > is found.
> > - Refuse to proceed if CLI options are passed in along with -C (Dan)
> > - In the config file, rename: s/[auto-online foo]/[reconfigure-device foo/
> > and s/uuid/nvdimm.uuid/ (Dan)
> > - Teach device.c to accept /dev/daxX.Y instead of only daxX.Y and thus
> > remove the need for a wrapper script that systemd invokes (Dan)
> >
> > These patches add policy (config file) support to daxctl. The
> > introductory user is daxctl-reconfigure-device. Sysadmins may wish to
> > use daxctl devices as system-ram, but it may be cumbersome to automate
> > the reconfiguration step for every device upon boot.
> >
> > Introduce a new option for daxctl-reconfigure-device, --check-config.
> > This is at the heart of policy based reconfiguration, as it allows
> > daxctl to look up reconfiguration parameters for a given device from
> > the config system instead of the command line.
> >
> > Some systemd and udev glue then automates this for every new dax
> > device that shows up, providing a way for the administrator to simply
> > list all the 'system-ram' UUIDs in a config file, and not have to
> > worry about anything else.
> >
> > An example config file can be:
> >
> > # cat /etc/ndctl/daxctl.conf
>
> Missed updating this, it should be /etc/daxctl/daxctl.conf
>
> >
> > [reconfigure-device unique_identifier_foo]
> > nvdimm.uuid = 48d8e42c-a2f0-4312-9e70-a837faafe862
> > mode = system-ram
> > online = true
> > movable = false
> >
> > Any file under '/etc/ndctl/' can be used - all files with a '.conf'
> > suffix
>
> And this should be '/etc/daxctl/'
>
> > will be considered when looking for matches.
> >
> > These patches depend on the initial config file support from Qi Fuli[2].
> >
> > I've re-rolled Qi's original patches as the first five patches in this
> > series because of a change I made for graceful handling in the case of
> > a missing configs directory, and also to incorporate review feedback
> > that applied to the dependant patches. Patch 6 onwards is the actual
> > v2 of the daxctl policy work.
> >
> > A branch containing these patches is available at [3].
> >
> > [1]:
> > https://lore.kernel.org/nvdimm/20210831090459.2306727-1-vishal.l.verma
> > @intel.com/
> > [2]:
> > https://lore.kernel.org/nvdimm/20210824095106.104808-1-qi.fuli@fujitsu
> > .com/
> > [3]: https://github.com/pmem/ndctl/tree/vv/daxctl_config_v2
> >
> > QI Fuli (5):
> > ndctl, util: add iniparser helper
> > ndctl, util: add parse-configs helper
> > ndctl: make ndctl support configuration files
> > ndctl, config: add the default ndctl configuration file
> > ndctl, monitor: refator monitor for supporting multiple config files
Hi Vishal,
Thank you very for the work.
I made a patch[1] to fix test/monitor.sh for the new style config file.
Could you please also pick it?
[1] https://lore.kernel.org/all/20210914024119.99711-1-qi.fuli@fujitsu.com/
Best,
QI Fuli
> >
> > Vishal Verma (7):
> > ndctl: Update ndctl.spec.in for 'ndctl.conf'
> > daxctl: Documentation updates for persistent reconfiguration
> > util/parse-config: refactor filter_conf_files into util/
> > daxctl: add basic config parsing support
> > util/parse-configs: add a key/value search helper
> > daxctl/device.c: add an option for getting params from a config file
> > daxctl: add systemd service and udev rule for automatic
> > reconfiguration
> >
> > .../daxctl/daxctl-reconfigure-device.txt | 75 ++
> > Documentation/ndctl/ndctl-monitor.txt | 8 +-
> > configure.ac | 18 +-
> > Makefile.am | 8 +-
> > ndctl/lib/private.h | 1 +
> > daxctl/lib/libdaxctl.c | 39 +
> > ndctl/lib/libndctl.c | 39 +
> > daxctl/libdaxctl.h | 2 +
> > ndctl/libndctl.h | 2 +
> > util/dictionary.h | 175 ++++
> > util/iniparser.h | 360 ++++++++
> > util/parse-configs.h | 53 ++
> > daxctl/daxctl.c | 1 +
> > daxctl/device.c | 174 +++-
> > ndctl/monitor.c | 73 +-
> > ndctl/ndctl.c | 1 +
> > util/dictionary.c | 383 ++++++++
> > util/iniparser.c | 838
> ++++++++++++++++++
> > util/parse-configs.c | 150 ++++
> > Documentation/daxctl/Makefile.am | 11 +-
> > Documentation/ndctl/Makefile.am | 2 +-
> > daxctl/90-daxctl-device.rules | 1 +
> > daxctl/Makefile.am | 9 +
> > daxctl/daxdev-reconfigure@.service | 8 +
> > daxctl/lib/Makefile.am | 6 +
> > daxctl/lib/libdaxctl.sym | 2 +
> > ndctl.spec.in | 4 +
> > ndctl/Makefile.am | 9 +-
> > ndctl/lib/Makefile.am | 6 +
> > ndctl/lib/libndctl.sym | 2 +
> > ndctl/ndctl.conf | 56 ++
> > 31 files changed, 2467 insertions(+), 49 deletions(-) create mode
> > 100644 util/dictionary.h create mode 100644 util/iniparser.h create
> > mode 100644 util/parse-configs.h create mode 100644 util/dictionary.c
> > create mode 100644 util/iniparser.c create mode 100644
> > util/parse-configs.c create mode 100644 daxctl/90-daxctl-device.rules
> > create mode 100644 daxctl/daxdev-reconfigure@.service
> > create mode 100644 ndctl/ndctl.conf
> >
> >
> > base-commit: 4e646fa490ba4b782afa188dd8818b94c419924e
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 01/12] ndctl, util: add iniparser helper
2021-12-06 22:28 ` [ndctl PATCH v2 01/12] ndctl, util: add iniparser helper Vishal Verma
@ 2021-12-07 18:24 ` Dan Williams
2021-12-07 21:50 ` Verma, Vishal L
0 siblings, 1 reply; 26+ messages in thread
From: Dan Williams @ 2021-12-07 18:24 UTC (permalink / raw)
To: Vishal Verma; +Cc: Linux NVDIMM, QI Fuli, Hu, Fenghua, QI Fuli
On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> From: QI Fuli <qi.fuli@fujitsu.com>
>
> Add iniparser[1] helper for parsing ndctl global configuration files.
> [1] https://github.com/ndevilla/iniparser
Is there a reason this is being copied rather than linked? The ccan
code was copied because no distribution packaged it up into library
form, but iniparser ships in Fedora and Ubuntu. Unless ndctl needs to
maintain a fork I would prefer to link just like json-c is linked.
Even if ndctl needs a fork I would expect that to be called out here
with a plan to eventually get that forked feature into the upstream
iniparser project.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 01/12] ndctl, util: add iniparser helper
2021-12-07 18:24 ` Dan Williams
@ 2021-12-07 21:50 ` Verma, Vishal L
0 siblings, 0 replies; 26+ messages in thread
From: Verma, Vishal L @ 2021-12-07 21:50 UTC (permalink / raw)
To: Williams, Dan J; +Cc: nvdimm, qi.fuli, Hu, Fenghua, qi.fuli
On Tue, 2021-12-07 at 10:24 -0800, Dan Williams wrote:
> On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
> >
> > From: QI Fuli <qi.fuli@fujitsu.com>
> >
> > Add iniparser[1] helper for parsing ndctl global configuration files.
> > [1] https://github.com/ndevilla/iniparser
>
> Is there a reason this is being copied rather than linked? The ccan
> code was copied because no distribution packaged it up into library
> form, but iniparser ships in Fedora and Ubuntu. Unless ndctl needs to
> maintain a fork I would prefer to link just like json-c is linked.
> Even if ndctl needs a fork I would expect that to be called out here
> with a plan to eventually get that forked feature into the upstream
> iniparser project.
Ah I didn't realize it was packaged. I don't think we needed any
changes to the core package - I'll change it to a linked dependency and
remove the fork.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 02/12] ndctl, util: add parse-configs helper
2021-12-06 22:28 ` [ndctl PATCH v2 02/12] ndctl, util: add parse-configs helper Vishal Verma
@ 2021-12-07 21:58 ` Dan Williams
0 siblings, 0 replies; 26+ messages in thread
From: Dan Williams @ 2021-12-07 21:58 UTC (permalink / raw)
To: Vishal Verma; +Cc: Linux NVDIMM, QI Fuli, Hu, Fenghua, QI Fuli
On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> From: QI Fuli <qi.fuli@fujitsu.com>
>
> Add parse-config util to help ndctl commands parse ndctl global
> configuration files.
Can this add a bit more description / quick summary of how these APIs are used?
>
> Link: https://lore.kernel.org/r/20210824095106.104808-3-qi.fuli@fujitsu.com
This link is pointing to the original posting I would expect that this
gets deleted and replaced with the final Link: to the version of the
patch that eventually gets applied. I assume you just pulled the
patches down and this link get appended in the resend.
> Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
> Makefile.am | 2 ++
> util/parse-configs.h | 34 ++++++++++++++++++
> util/parse-configs.c | 82 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 118 insertions(+)
> create mode 100644 util/parse-configs.h
> create mode 100644 util/parse-configs.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 235c362..af55f0e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -70,6 +70,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.h b/util/parse-configs.h
> new file mode 100644
> index 0000000..f70f58f
> --- /dev/null
> +++ b/util/parse-configs.h
> @@ -0,0 +1,34 @@
> +// 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,
> + MONITOR_CALLBACK,
> +};
> +
> +struct config;
> +typedef int parse_conf_cb(const struct config *, const char *config_file);
> +
> +struct config {
> + enum parse_conf_type type;
> + const char *key;
> + void *value;
> + void *defval;
> + parse_conf_cb *callback;
> +};
> +
> +#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) }
> +#define CONF_MONITOR(k,f) \
> + { .type = MONITOR_CALLBACK, .key = (k), .callback = (f)}
> +
> +int parse_configs_prefix(const char *__config_file, const char *prefix,
> + const struct config *configs);
> diff --git a/util/parse-configs.c b/util/parse-configs.c
> new file mode 100644
> index 0000000..44dcff4
> --- /dev/null
> +++ b/util/parse-configs.c
> @@ -0,0 +1,82 @@
> +// 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 <util/iniparser.h>
> +
> +static void set_str_val(const char **value, const 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);
> +}
> +
> +static int parse_config_file(const char *config_file,
> + const struct config *configs)
> +{
> + dictionary *dic;
> +
> + dic = iniparser_load(config_file);
> + if (!dic)
> + return -errno;
> +
> + for (; configs->type != CONFIG_END; configs++) {
> + switch (configs->type) {
> + case CONFIG_STRING:
> + set_str_val((const char **)configs->value,
> + iniparser_getstring(dic,
> + configs->key, configs->defval));
> + break;
> + case MONITOR_CALLBACK:
> + case CONFIG_END:
> + break;
> + }
> + }
> +
> + iniparser_freedict(dic);
> + return 0;
> +}
> +
> +int parse_configs_prefix(const char *__config_files, const char *prefix,
> + const struct config *configs)
> +{
> + char *config_files, *save;
> + const char *config_file;
> + int rc;
> +
> + config_files = strdup(__config_files);
> + if (!config_files)
> + return -ENOMEM;
> +
> + for (config_file = strtok_r(config_files, " ", &save); config_file;
> + config_file = strtok_r(NULL, " ", &save)) {
> +
> + if (strncmp(config_file, "./", 2) != 0)
> + fix_filename(prefix, &config_file);
> +
> + if ((configs->type == MONITOR_CALLBACK) &&
> + (strcmp(config_file, configs->key) == 0))
> + rc = configs->callback(configs, configs->key);
> + else
> + rc = parse_config_file(config_file, configs);
> +
> + if (rc)
> + goto end;
> + }
> +
> + end:
> + free(config_files);
> + return rc;
> +
> +}
> --
> 2.33.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 03/12] ndctl: make ndctl support configuration files
2021-12-06 22:28 ` [ndctl PATCH v2 03/12] ndctl: make ndctl support configuration files Vishal Verma
@ 2021-12-07 22:51 ` Dan Williams
2021-12-08 0:04 ` Verma, Vishal L
0 siblings, 1 reply; 26+ messages in thread
From: Dan Williams @ 2021-12-07 22:51 UTC (permalink / raw)
To: Vishal Verma; +Cc: Linux NVDIMM, QI Fuli, Hu, Fenghua, QI Fuli
On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> From: QI Fuli <qi.fuli@fujitsu.com>
>
> Add ndctl_configs to ndctl_ctx for supporting ndctl global configuration
> files. All files with .conf suffix under {sysconfdir}/ndctl can be
I expect this should be named a ".d" directory to match expectations
of when a directory supports multiple config-file snippets. Either
"{sysconfdir}/ndctl/ndctl.d", "{sysconfdir}/ndctl/conf.d",
""{sysconfdir}/ndctl.d" would be ok with me. "{sysconfdir}/ndctl" is
still needed for the security keys.
> regarded as global configuration files that all ndctl commands can refer
> to. Add ndctl_set_configs() public function for setting ndctl default
> configuration files' path. Add ndctl_get_configs() public function for
> getting ndctl configuration files' path form ndctl_ctx.
s/form/from/
Lets those ndctl_{get,set}_config_path().
>
> Link: https://lore.kernel.org/r/20210824095106.104808-4-qi.fuli@fujitsu.com
> Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
> configure.ac | 4 ++--
> ndctl/lib/private.h | 1 +
> ndctl/lib/libndctl.c | 54 ++++++++++++++++++++++++++++++++++++++++++
> ndctl/libndctl.h | 2 ++
> ndctl/ndctl.c | 1 +
> ndctl/Makefile.am | 5 ++--
> ndctl/lib/Makefile.am | 4 ++++
> ndctl/lib/libndctl.sym | 2 ++
> 8 files changed, 69 insertions(+), 4 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index dc39dbe..42a66e1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -171,9 +171,9 @@ fi
> AC_SUBST([systemd_unitdir])
> AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
>
> -ndctl_monitorconfdir=${sysconfdir}/ndctl
> +ndctl_confdir=${sysconfdir}/ndctl
> ndctl_monitorconf=monitor.conf
> -AC_SUBST([ndctl_monitorconfdir])
> +AC_SUBST([ndctl_confdir])
> AC_SUBST([ndctl_monitorconf])
>
> daxctl_modprobe_datadir=${datadir}/daxctl
> diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
> index 8f4510e..f4ca71f 100644
> --- a/ndctl/lib/private.h
> +++ b/ndctl/lib/private.h
> @@ -129,6 +129,7 @@ struct ndctl_ctx {
> int regions_init;
> void *userdata;
> struct list_head busses;
> + const char *configs;
> int busses_init;
> struct udev *udev;
> struct udev_queue *udev_queue;
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 536e142..6b68e3a 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -24,6 +24,7 @@
> #include <util/util.h>
> #include <util/size.h>
> #include <util/sysfs.h>
> +#include <util/strbuf.h>
> #include <ndctl/libndctl.h>
> #include <ndctl/namespace.h>
> #include <daxctl/libdaxctl.h>
> @@ -265,6 +266,58 @@ NDCTL_EXPORT void ndctl_set_userdata(struct ndctl_ctx *ctx, void *userdata)
> ctx->userdata = userdata;
> }
>
> +static int filter_conf(const struct dirent *dir)
> +{
> + if (!dir)
> + return 0;
> +
> + if (dir->d_type == DT_REG) {
> + const char *ext = strrchr(dir->d_name, '.');
Oh, I guess clang-format does not insert line breaks between
declarations and code.
> + if ((!ext) || (ext == dir->d_name))
> + return 0;
> + if (strcmp(ext, ".conf") == 0)
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +NDCTL_EXPORT void ndctl_set_configs_dir(struct ndctl_ctx **ctx, char *conf_dir)
ndctl_set_config_path
Why is ctx a 'struct ndctl_ctx **' instead of a 'struct ndctl_ctx *'?
> +{
> + struct dirent **namelist;
> + struct strbuf value = STRBUF_INIT;
> + int rc;
> +
> + if ((!ctx) || (!conf_dir))
I think this function needs to return an error. If someone does:
ndctl_set_config_path(NULL, "/path") nothing will notify them that
/path is not established as the config directory. The ENOENT check is
helpful, but why not tell the caller?
> + return;
> +
> + rc = scandir(conf_dir, &namelist, filter_conf, alphasort);
> + if (rc == -1) {
> + if (errno != ENOENT)
> + err(*ctx, "scandir for configs failed: %s\n",
> + strerror(errno));
> + return;
> + }
> +
> + while (rc--) {
> + if (value.len)
> + strbuf_addstr(&value, " ");
> + strbuf_addstr(&value, conf_dir);
> + strbuf_addstr(&value, "/");
> + strbuf_addstr(&value, namelist[rc]->d_name);
> + free(namelist[rc]);
> + }
Any reason this needs to be converted to a space separated list? What
if there are spaces in the path name?
This could just cache the scandir result in the ctx directly and skip
the strbuf usage.
> + (*ctx)->configs = strbuf_detach(&value, NULL);
> + free(namelist);
> +}
> +
> +NDCTL_EXPORT const char *ndctl_get_configs_dir(struct ndctl_ctx *ctx)
> +{
> + if (ctx == NULL)
> + return NULL;
> + return ctx->configs;
> +}
> +
> /**
> * ndctl_new - instantiate a new library context
> * @ctx: context to establish
> @@ -331,6 +384,7 @@ NDCTL_EXPORT int ndctl_new(struct ndctl_ctx **ctx)
> c->daxctl_ctx = daxctl_ctx;
>
> return 0;
> +
Unnecessary newline.
> err_ctx:
> daxctl_unref(daxctl_ctx);
> err_daxctl:
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index 87d07b7..883a56f 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -92,6 +92,8 @@ int ndctl_get_log_priority(struct ndctl_ctx *ctx);
> void ndctl_set_log_priority(struct ndctl_ctx *ctx, int priority);
> void ndctl_set_userdata(struct ndctl_ctx *ctx, void *userdata);
> void *ndctl_get_userdata(struct ndctl_ctx *ctx);
> +void ndctl_set_configs_dir(struct ndctl_ctx **ctx, char *conf_dir);
> +const char *ndctl_get_configs_dir(struct ndctl_ctx *ctx);
>
> enum ndctl_persistence_domain {
> PERSISTENCE_NONE = 0,
> diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
> index 31d2c5e..0f908db 100644
> --- a/ndctl/ndctl.c
> +++ b/ndctl/ndctl.c
> @@ -125,6 +125,7 @@ int main(int argc, const char **argv)
> rc = ndctl_new(&ctx);
> if (rc)
> goto out;
> + ndctl_set_configs_dir(&ctx, NDCTL_CONF_DIR);
I expect this to happen by default in ndctl_new().
In fact, ndctl_new() might want to grow a config path argument to
override the default.
> main_handle_internal_command(argc, argv, ctx, commands,
> ARRAY_SIZE(commands), PROG_NDCTL);
> ndctl_unref(ctx);
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index a63b1e0..1caa031 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -7,8 +7,9 @@ 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_monitorconf)"' >>$@
> $(AM_V_GEN) echo '#define NDCTL_KEYS_DIR "$(ndctl_keysdir)"' >>$@
> + $(AM_V_GEN) echo '#define NDCTL_CONF_DIR "$(ndctl_confdir)"' >>$@
>
> ndctl_SOURCES = ndctl.c \
> builtin.h \
> @@ -73,7 +74,7 @@ ndctl_SOURCES += ../test/libndctl.c \
> test.c
> endif
>
> -monitor_configdir = $(ndctl_monitorconfdir)
> +monitor_configdir = $(ndctl_confdir)
> monitor_config_DATA = $(ndctl_monitorconf)
>
> if ENABLE_SYSTEMD_UNITS
> diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
> index e15bb22..f741c44 100644
> --- a/ndctl/lib/Makefile.am
> +++ b/ndctl/lib/Makefile.am
> @@ -14,6 +14,10 @@ libndctl_la_SOURCES =\
> ../../util/log.h \
> ../../util/sysfs.c \
> ../../util/sysfs.h \
> + ../../util/strbuf.h \
> + ../../util/strbuf.c \
> + ../../util/wrapper.c \
> + ../../util/usage.c \
Seems this new utility code linking is not necessary per the above
recommendation to just cache the scandir results in the ctx.
> ../../util/fletcher.h \
> dimm.c \
> inject.c \
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index 58afb74..6e1b510 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -454,4 +454,6 @@ LIBNDCTL_25 {
>
> LIBNDCTL_26 {
> ndctl_bus_nfit_translate_spa;
> + ndctl_set_configs_dir;
> + ndctl_get_configs_dir;
> } LIBNDCTL_25;
> --
> 2.33.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 03/12] ndctl: make ndctl support configuration files
2021-12-07 22:51 ` Dan Williams
@ 2021-12-08 0:04 ` Verma, Vishal L
2021-12-08 0:19 ` Dan Williams
0 siblings, 1 reply; 26+ messages in thread
From: Verma, Vishal L @ 2021-12-08 0:04 UTC (permalink / raw)
To: Williams, Dan J; +Cc: nvdimm, qi.fuli, Hu, Fenghua, qi.fuli
On Tue, 2021-12-07 at 14:51 -0800, Dan Williams wrote:
> On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
> >
> > From: QI Fuli <qi.fuli@fujitsu.com>
> >
> > Add ndctl_configs to ndctl_ctx for supporting ndctl global configuration
> > files. All files with .conf suffix under {sysconfdir}/ndctl can be
>
> I expect this should be named a ".d" directory to match expectations
> of when a directory supports multiple config-file snippets. Either
> "{sysconfdir}/ndctl/ndctl.d", "{sysconfdir}/ndctl/conf.d",
> ""{sysconfdir}/ndctl.d" would be ok with me. "{sysconfdir}/ndctl" is
> still needed for the security keys.
".d" sounds good - I prefer "{sysconfdir}/ndctl.d" or even something
like "{sysconfdir}/ndctl.conf.d" to make it explicit that this is for
config files.
>
> > regarded as global configuration files that all ndctl commands can refer
> > to. Add ndctl_set_configs() public function for setting ndctl default
> > configuration files' path. Add ndctl_get_configs() public function for
> > getting ndctl configuration files' path form ndctl_ctx.
>
> s/form/from/
>
> Lets those ndctl_{get,set}_config_path().
>
>
> >
> > Link: https://lore.kernel.org/r/20210824095106.104808-4-qi.fuli@fujitsu.com
> > Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> > configure.ac | 4 ++--
> > ndctl/lib/private.h | 1 +
> > ndctl/lib/libndctl.c | 54 ++++++++++++++++++++++++++++++++++++++++++
> > ndctl/libndctl.h | 2 ++
> > ndctl/ndctl.c | 1 +
> > ndctl/Makefile.am | 5 ++--
> > ndctl/lib/Makefile.am | 4 ++++
> > ndctl/lib/libndctl.sym | 2 ++
> > 8 files changed, 69 insertions(+), 4 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index dc39dbe..42a66e1 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -171,9 +171,9 @@ fi
> > AC_SUBST([systemd_unitdir])
> > AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
> >
> > -ndctl_monitorconfdir=${sysconfdir}/ndctl
> > +ndctl_confdir=${sysconfdir}/ndctl
> > ndctl_monitorconf=monitor.conf
> > -AC_SUBST([ndctl_monitorconfdir])
> > +AC_SUBST([ndctl_confdir])
> > AC_SUBST([ndctl_monitorconf])
> >
> > daxctl_modprobe_datadir=${datadir}/daxctl
> > diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
> > index 8f4510e..f4ca71f 100644
> > --- a/ndctl/lib/private.h
> > +++ b/ndctl/lib/private.h
> > @@ -129,6 +129,7 @@ struct ndctl_ctx {
> > int regions_init;
> > void *userdata;
> > struct list_head busses;
> > + const char *configs;
> > int busses_init;
> > struct udev *udev;
> > struct udev_queue *udev_queue;
> > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > index 536e142..6b68e3a 100644
> > --- a/ndctl/lib/libndctl.c
> > +++ b/ndctl/lib/libndctl.c
> > @@ -24,6 +24,7 @@
> > #include <util/util.h>
> > #include <util/size.h>
> > #include <util/sysfs.h>
> > +#include <util/strbuf.h>
> > #include <ndctl/libndctl.h>
> > #include <ndctl/namespace.h>
> > #include <daxctl/libdaxctl.h>
> > @@ -265,6 +266,58 @@ NDCTL_EXPORT void ndctl_set_userdata(struct ndctl_ctx *ctx, void *userdata)
> > ctx->userdata = userdata;
> > }
> >
> > +static int filter_conf(const struct dirent *dir)
> > +{
> > + if (!dir)
> > + return 0;
> > +
> > + if (dir->d_type == DT_REG) {
> > + const char *ext = strrchr(dir->d_name, '.');
>
> Oh, I guess clang-format does not insert line breaks between
> declarations and code.
Oh, I'd fixed this, but I guess only in my future patch that moves this
function elsewhere. Since I'm modifying Qi's patches anyway now, I'll
squash that move patch into this, and make sure that fix is picked up
too.
>
> > + if ((!ext) || (ext == dir->d_name))
> > + return 0;
> > + if (strcmp(ext, ".conf") == 0)
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +NDCTL_EXPORT void ndctl_set_configs_dir(struct ndctl_ctx **ctx, char *conf_dir)
>
> ndctl_set_config_path
Sounds good.
>
> Why is ctx a 'struct ndctl_ctx **' instead of a 'struct ndctl_ctx *'?
Heh I'd noticed this too but forgot to fix it, will do now.
>
> > +{
> > + struct dirent **namelist;
> > + struct strbuf value = STRBUF_INIT;
> > + int rc;
> > +
> > + if ((!ctx) || (!conf_dir))
>
> I think this function needs to return an error. If someone does:
> ndctl_set_config_path(NULL, "/path") nothing will notify them that
> /path is not established as the config directory. The ENOENT check is
> helpful, but why not tell the caller?
Yes agreed, I'll change this.
>
> > + return;
> > +
> > + rc = scandir(conf_dir, &namelist, filter_conf, alphasort);
> > + if (rc == -1) {
> > + if (errno != ENOENT)
> > + err(*ctx, "scandir for configs failed: %s\n",
> > + strerror(errno));
> > + return;
> > + }
> > +
> > + while (rc--) {
> > + if (value.len)
> > + strbuf_addstr(&value, " ");
> > + strbuf_addstr(&value, conf_dir);
> > + strbuf_addstr(&value, "/");
> > + strbuf_addstr(&value, namelist[rc]->d_name);
> > + free(namelist[rc]);
> > + }
>
> Any reason this needs to be converted to a space separated list? What
> if there are spaces in the path name?
>
> This could just cache the scandir result in the ctx directly and skip
> the strbuf usage.
Hm I hadn't looked too closely at this before, but makes sense, and
good point on paths with spaces. I'll fix.
>
> > + (*ctx)->configs = strbuf_detach(&value, NULL);
> > + free(namelist);
> > +}
> > +
> > +NDCTL_EXPORT const char *ndctl_get_configs_dir(struct ndctl_ctx *ctx)
> > +{
> > + if (ctx == NULL)
> > + return NULL;
> > + return ctx->configs;
> > +}
> > +
> > /**
> > * ndctl_new - instantiate a new library context
> > * @ctx: context to establish
> > @@ -331,6 +384,7 @@ NDCTL_EXPORT int ndctl_new(struct ndctl_ctx **ctx)
> > c->daxctl_ctx = daxctl_ctx;
> >
> > return 0;
> > +
>
> Unnecessary newline.
>
> > err_ctx:
> > daxctl_unref(daxctl_ctx);
> > err_daxctl:
> > diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> > index 87d07b7..883a56f 100644
> > --- a/ndctl/libndctl.h
> > +++ b/ndctl/libndctl.h
> > @@ -92,6 +92,8 @@ int ndctl_get_log_priority(struct ndctl_ctx *ctx);
> > void ndctl_set_log_priority(struct ndctl_ctx *ctx, int priority);
> > void ndctl_set_userdata(struct ndctl_ctx *ctx, void *userdata);
> > void *ndctl_get_userdata(struct ndctl_ctx *ctx);
> > +void ndctl_set_configs_dir(struct ndctl_ctx **ctx, char *conf_dir);
> > +const char *ndctl_get_configs_dir(struct ndctl_ctx *ctx);
> >
> > enum ndctl_persistence_domain {
> > PERSISTENCE_NONE = 0,
> > diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
> > index 31d2c5e..0f908db 100644
> > --- a/ndctl/ndctl.c
> > +++ b/ndctl/ndctl.c
> > @@ -125,6 +125,7 @@ int main(int argc, const char **argv)
> > rc = ndctl_new(&ctx);
> > if (rc)
> > goto out;
> > + ndctl_set_configs_dir(&ctx, NDCTL_CONF_DIR);
>
> I expect this to happen by default in ndctl_new().
>
> In fact, ndctl_new() might want to grow a config path argument to
> override the default.
Happening by default in ndctl_new sounds good.
However since ndctl_new() is an exported API, we can't just add a new
argument to it, can we? Do we expect callers to want custom paths,
outside of what a distro may set via NDCTL_CONF_DIR during build?
>
> > main_handle_internal_command(argc, argv, ctx, commands,
> > ARRAY_SIZE(commands), PROG_NDCTL);
> > ndctl_unref(ctx);
> > diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> > index a63b1e0..1caa031 100644
> > --- a/ndctl/Makefile.am
> > +++ b/ndctl/Makefile.am
> > @@ -7,8 +7,9 @@ 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_monitorconf)"' >>$@
> > $(AM_V_GEN) echo '#define NDCTL_KEYS_DIR "$(ndctl_keysdir)"' >>$@
> > + $(AM_V_GEN) echo '#define NDCTL_CONF_DIR "$(ndctl_confdir)"' >>$@
> >
> > ndctl_SOURCES = ndctl.c \
> > builtin.h \
> > @@ -73,7 +74,7 @@ ndctl_SOURCES += ../test/libndctl.c \
> > test.c
> > endif
> >
> > -monitor_configdir = $(ndctl_monitorconfdir)
> > +monitor_configdir = $(ndctl_confdir)
> > monitor_config_DATA = $(ndctl_monitorconf)
> >
> > if ENABLE_SYSTEMD_UNITS
> > diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
> > index e15bb22..f741c44 100644
> > --- a/ndctl/lib/Makefile.am
> > +++ b/ndctl/lib/Makefile.am
> > @@ -14,6 +14,10 @@ libndctl_la_SOURCES =\
> > ../../util/log.h \
> > ../../util/sysfs.c \
> > ../../util/sysfs.h \
> > + ../../util/strbuf.h \
> > + ../../util/strbuf.c \
> > + ../../util/wrapper.c \
> > + ../../util/usage.c \
>
> Seems this new utility code linking is not necessary per the above
> recommendation to just cache the scandir results in the ctx.
Yep makes sense.
>
> > ../../util/fletcher.h \
> > dimm.c \
> > inject.c \
> > diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> > index 58afb74..6e1b510 100644
> > --- a/ndctl/lib/libndctl.sym
> > +++ b/ndctl/lib/libndctl.sym
> > @@ -454,4 +454,6 @@ LIBNDCTL_25 {
> >
> > LIBNDCTL_26 {
> > ndctl_bus_nfit_translate_spa;
> > + ndctl_set_configs_dir;
> > + ndctl_get_configs_dir;
> > } LIBNDCTL_25;
> > --
> > 2.33.1
> >
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 03/12] ndctl: make ndctl support configuration files
2021-12-08 0:04 ` Verma, Vishal L
@ 2021-12-08 0:19 ` Dan Williams
0 siblings, 0 replies; 26+ messages in thread
From: Dan Williams @ 2021-12-08 0:19 UTC (permalink / raw)
To: Verma, Vishal L; +Cc: nvdimm, qi.fuli, Hu, Fenghua, qi.fuli
On Tue, Dec 7, 2021 at 4:05 PM Verma, Vishal L <vishal.l.verma@intel.com> wrote:
>
> On Tue, 2021-12-07 at 14:51 -0800, Dan Williams wrote:
> > On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
> > >
> > > From: QI Fuli <qi.fuli@fujitsu.com>
> > >
> > > Add ndctl_configs to ndctl_ctx for supporting ndctl global configuration
> > > files. All files with .conf suffix under {sysconfdir}/ndctl can be
> >
> > I expect this should be named a ".d" directory to match expectations
> > of when a directory supports multiple config-file snippets. Either
> > "{sysconfdir}/ndctl/ndctl.d", "{sysconfdir}/ndctl/conf.d",
> > ""{sysconfdir}/ndctl.d" would be ok with me. "{sysconfdir}/ndctl" is
> > still needed for the security keys.
>
> ".d" sounds good - I prefer "{sysconfdir}/ndctl.d" or even something
> like "{sysconfdir}/ndctl.conf.d" to make it explicit that this is for
> config files.
Sure, that works too. I note that modprobe.d is full of ".conf" files,
but I see no harm in including ".conf" in the directory name. Since
yum.repos.d is full of ".repo" files as a counter example in the
naming scheme.
[..]
> >
> > I expect this to happen by default in ndctl_new().
> >
> > In fact, ndctl_new() might want to grow a config path argument to
> > override the default.
>
> Happening by default in ndctl_new sounds good.
> However since ndctl_new() is an exported API, we can't just add a new
> argument to it, can we?
Oh, yeah, it would need to be a new ndctl_new(). No worries we can
cross that bridge later.
> Do we expect callers to want custom paths,
> outside of what a distro may set via NDCTL_CONF_DIR during build?
I was thinking it would be like mdadm where you can override the
location of the configuration file(s) on the command line. We can
always add that later, no need for feature creep right now.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 04/12] ndctl, config: add the default ndctl configuration file
2021-12-06 22:28 ` [ndctl PATCH v2 04/12] ndctl, config: add the default ndctl configuration file Vishal Verma
@ 2021-12-08 16:00 ` Dan Williams
0 siblings, 0 replies; 26+ messages in thread
From: Dan Williams @ 2021-12-08 16:00 UTC (permalink / raw)
To: Vishal Verma; +Cc: Linux NVDIMM, QI Fuli, Hu, Fenghua, QI Fuli
On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> From: QI Fuli <qi.fuli@fujitsu.com>
>
> Install ndctl/ndctl.conf as the default ndctl configuration file.
>
> Link: https://lore.kernel.org/r/20210824095106.104808-5-qi.fuli@fujitsu.com
> Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Couple small fixups, otherwise:
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> ---
> configure.ac | 2 ++
> ndctl/Makefile.am | 4 +++-
> ndctl/ndctl.conf | 56 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 61 insertions(+), 1 deletion(-)
> create mode 100644 ndctl/ndctl.conf
>
> diff --git a/configure.ac b/configure.ac
> index 42a66e1..9e1c6db 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -172,8 +172,10 @@ AC_SUBST([systemd_unitdir])
> AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
>
> ndctl_confdir=${sysconfdir}/ndctl
> +ndctl_conf=ndctl.conf
> ndctl_monitorconf=monitor.conf
> AC_SUBST([ndctl_confdir])
> +AC_SUBST([ndctl_conf])
> AC_SUBST([ndctl_monitorconf])
>
> daxctl_modprobe_datadir=${datadir}/daxctl
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index 1caa031..fceb3ab 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -43,7 +43,7 @@ keys_configdir = $(ndctl_keysdir)
> keys_config_DATA = $(ndctl_keysreadme)
> endif
>
> -EXTRA_DIST += keys.readme monitor.conf ndctl-monitor.service
> +EXTRA_DIST += keys.readme monitor.conf ndctl-monitor.service ndctl.conf
>
> if ENABLE_DESTRUCTIVE
> ndctl_SOURCES += ../test/blk_namespaces.c \
> @@ -74,6 +74,8 @@ ndctl_SOURCES += ../test/libndctl.c \
> test.c
> endif
>
> +ndctl_configdir = $(ndctl_confdir)
> +ndctl_config_DATA = $(ndctl_conf)
> monitor_configdir = $(ndctl_confdir)
> monitor_config_DATA = $(ndctl_monitorconf)
>
> diff --git a/ndctl/ndctl.conf b/ndctl/ndctl.conf
> new file mode 100644
> index 0000000..04d322d
> --- /dev/null
> +++ b/ndctl/ndctl.conf
> @@ -0,0 +1,56 @@
> +# This is the default ndctl configuration file. It contains the
> +# configuration directives that give ndctl instructions.
> +# Ndctl supports multiple configuration files. All files with the
> +# .conf suffix under {sysconfdir}/ndctl can be regarded as ndctl
Looks good, just the fixup to rename this to {sysconfdir}/ndctl.conf.d
...does the install process replace {sysconfdir} with /etc?
Meson makes file substitutions based on config fairly painless, so
this could wait to be fixed until after that conversion. Another
future item would be to get "man ndctl.conf" to give an overview of
ndctl configuration options.
> +# configuration files.
> +
> +# In this file, lines starting with a hash (#) are comments.
> +# The configurations should be in a [section] and follow <key> = <value>
> +# style. Multiple space-separated values are allowed, but except the
> +# following characters: : ? / \ % " ' $ & ! * { } [ ] ( ) = < > @
> +
> +[core]
> +# The values in [core] section work for all ndctl sub commands.
> +# dimm = all
> +# bus = all
> +# region = all
> +# namespace = all
> +
> +[monitor]
> +# The values in [monitor] section work for ndctl monitor.
> +# You can change the configuration of ndctl monitor by editing this
> +# file or use [--config-file=<file>] option to override this one.
> +# The changed value will work after restart ndctl monitor service.
> +
> +# 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.
> +# dimm = all
> +
> +# The objects to monitor are filtered via its parent bus by setting key "bus".
> +# If this value is different from the value of [--bus=<value>] option,
> +# both of the values will work.
> +# bus = all
> +
> +# The objects to monitor are filtered via region by setting key "region".
> +# If this value is different from the value of [--region=<value>] option,
> +# both of the values will work.
> +# region = all
> +
> +# The objects to monitor are filtered via namespace by setting key "namespace".
> +# If this value is different from the value of [--namespace=<value>] option,
> +# both of the values will work.
> +# namespace = all
> +
> +# The DIMM events to monitor are filtered via event type by setting key
> +# "dimm-event". If this value is different from the value of
> +# [--dimm-event=<value>] option, both of the values will work.
> +# dimm-event = all
> +
> +# Users can choose to output the notifications to syslog (log=syslog),
> +# to standard output (log=standard) or to write into a special file (log=<file>)
> +# by setting key "log". If this value is in conflict with the value of
> +# [--log=<value>] option, this value will be ignored.
> +# Note: Setting value to "standard" or relative path for <file> will not work
> +# when running moniotr as a daemon.
s/moniotr/monitor/
> +# log = /var/log/ndctl/monitor.log
> --
> 2.33.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 05/12] ndctl, monitor: refator monitor for supporting multiple config files
2021-12-06 22:28 ` [ndctl PATCH v2 05/12] ndctl, monitor: refator monitor for supporting multiple config files Vishal Verma
@ 2021-12-08 16:00 ` Dan Williams
0 siblings, 0 replies; 26+ messages in thread
From: Dan Williams @ 2021-12-08 16:00 UTC (permalink / raw)
To: Vishal Verma; +Cc: Linux NVDIMM, QI Fuli, Hu, Fenghua, QI Fuli
On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> From: QI Fuli <qi.fuli@fujitsu.com>
>
> Refactor ndctl monitor by using parse-configs helper to support multiple
> configuration files.
>
> Link: https://lore.kernel.org/r/20210824095106.104808-6-qi.fuli@fujitsu.com
> Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Look ok to me, perhaps a global s/configs/config_path/, but otherwise:
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Documentation/ndctl/ndctl-monitor.txt | 8 +--
> ndctl/monitor.c | 73 ++++++++++++++-------------
> Documentation/ndctl/Makefile.am | 2 +-
> 3 files changed, 44 insertions(+), 39 deletions(-)
>
> diff --git a/Documentation/ndctl/ndctl-monitor.txt b/Documentation/ndctl/ndctl-monitor.txt
> index dbc9070..8c8c35b 100644
> --- a/Documentation/ndctl/ndctl-monitor.txt
> +++ b/Documentation/ndctl/ndctl-monitor.txt
> @@ -21,8 +21,8 @@ objects and dumping the json format notifications to syslog, standard
> output or a logfile.
>
> The objects to monitor and smart events to notify can be selected by
> -setting options and/or the configuration file at
> -{ndctl_monitorconfdir}/{ndctl_monitorconf}
> +setting options and/or configuration files with .conf suffix under
> +{ndctl_confdir}
>
> Both, the values in configuration file and in options will work. If
> there is a conflict, the values in options will override the values in
> @@ -81,8 +81,8 @@ will not work if "--daemon" is specified.
>
> -c::
> --config-file=::
> - Provide the config file to use. This overrides the default config
> - typically found in {ndctl_monitorconfdir}
> + Provide the config file(s) to use. This overrides the default config
> + typically found in {ndctl_confdir}
>
> --daemon::
> Run a monitor as a daemon.
> diff --git a/ndctl/monitor.c b/ndctl/monitor.c
> index ca36179..6bf3160 100644
> --- a/ndctl/monitor.c
> +++ b/ndctl/monitor.c
> @@ -10,6 +10,7 @@
> #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>
> @@ -28,7 +29,7 @@
>
> static struct monitor {
> const char *log;
> - const char *config_file;
> + const char *configs;
> const char *dimm_event;
> FILE *log_file;
> bool daemon;
> @@ -463,7 +464,7 @@ out:
> return rc;
> }
>
> -static void parse_config(const char **arg, char *key, char *val, char *ident)
> +static void set_monitor_conf(const char **arg, char *key, char *val, char *ident)
> {
> struct strbuf value = STRBUF_INIT;
> size_t arg_len = *arg ? strlen(*arg) : 0;
> @@ -479,39 +480,25 @@ static void parse_config(const char **arg, char *key, char *val, char *ident)
> *arg = strbuf_detach(&value, NULL);
> }
>
> -static int read_config_file(struct ndctl_ctx *ctx, struct monitor *_monitor,
> - struct util_filter_params *_param)
> +static int parse_monitor_config(const struct config *configs,
> + const char *config_file)
> {
> 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;
> - }
> + char *buf = NULL, *seek, *value;
>
> buf = malloc(BUF_SIZE);
> if (!buf) {
> fail("malloc read config-file buf error\n");
> - rc = -ENOMEM;
> - goto out;
> + return -ENOMEM;
> }
> 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;
> - }
> + err(&monitor, "%s cannot be opened\n", config_file);
> + rc = -errno;
> goto out;
> }
>
> @@ -554,19 +541,18 @@ static int read_config_file(struct ndctl_ctx *ctx, struct monitor *_monitor,
> 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);
> + set_monitor_conf(¶m.bus, "bus", value, seek);
> + set_monitor_conf(¶m.dimm, "dimm", value, seek);
> + set_monitor_conf(¶m.region, "region", value, seek);
> + set_monitor_conf(¶m.namespace, "namespace", value, seek);
> + set_monitor_conf(&monitor.dimm_event, "dimm-event", value, seek);
>
> - if (!_monitor->log)
> - parse_config(&_monitor->log, "log", value, seek);
> + if (!monitor.log)
> + set_monitor_conf(&monitor.log, "log", value, seek);
> }
> fclose(f);
> out:
> free(buf);
> - free(config_file);
> return rc;
> }
>
> @@ -585,8 +571,8 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
> OPT_FILENAME('l', "log", &monitor.log,
> "<file> | syslog | standard",
> "where to output the monitor's notification"),
> - OPT_FILENAME('c', "config-file", &monitor.config_file,
> - "config-file", "override the default config"),
> + OPT_STRING('c', "config-file", &monitor.configs,
> + "config-file", "override default configs"),
> OPT_BOOLEAN('\0', "daemon", &monitor.daemon,
> "run ndctl monitor as a daemon"),
> OPT_BOOLEAN('u', "human", &monitor.human,
> @@ -601,7 +587,20 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
> "ndctl monitor [<options>]",
> NULL
> };
> - const char *prefix = "./";
> + const struct config configs[] = {
> + CONF_MONITOR(NDCTL_CONF_FILE, parse_monitor_config),
> + CONF_STR("core:bus", ¶m.bus, NULL),
> + CONF_STR("core:region", ¶m.region, NULL),
> + CONF_STR("core:dimm", ¶m.dimm, NULL),
> + CONF_STR("core:namespace", ¶m.namespace, NULL),
> + CONF_STR("monitor:bus", ¶m.bus, NULL),
> + CONF_STR("monitor:region", ¶m.region, NULL),
> + CONF_STR("monitor:dimm", ¶m.dimm, NULL),
> + CONF_STR("monitor:namespace", ¶m.namespace, NULL),
> + CONF_STR("monitor:dimm-event", &monitor.dimm_event, NULL),
> + CONF_END(),
> + };
> + const char *prefix = "./", *ndctl_configs;
> struct util_filter_ctx fctx = { 0 };
> struct monitor_filter_arg mfa = { 0 };
> int i, rc;
> @@ -621,7 +620,13 @@ 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, ¶m);
> + ndctl_configs = ndctl_get_configs_dir(ctx);
> + if (monitor.configs)
> + rc = parse_configs_prefix(monitor.configs, prefix, configs);
> + else if (ndctl_configs)
> + rc = parse_configs_prefix(ndctl_configs, prefix, configs);
> + else
> + rc = 0;
> if (rc)
> goto out;
>
> diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
> index f0d5b21..37855cc 100644
> --- a/Documentation/ndctl/Makefile.am
> +++ b/Documentation/ndctl/Makefile.am
> @@ -59,7 +59,7 @@ CLEANFILES = $(man1_MANS)
> .ONESHELL:
> attrs.adoc: $(srcdir)/Makefile.am
> $(AM_V_GEN) cat <<- EOF >$@
> - :ndctl_monitorconfdir: $(ndctl_monitorconfdir)
> + :ndctl_confdir: $(ndctl_confdir)
> :ndctl_monitorconf: $(ndctl_monitorconf)
> :ndctl_keysdir: $(ndctl_keysdir)
> EOF
> --
> 2.33.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 06/12] ndctl: Update ndctl.spec.in for 'ndctl.conf'
2021-12-06 22:28 ` [ndctl PATCH v2 06/12] ndctl: Update ndctl.spec.in for 'ndctl.conf' Vishal Verma
@ 2021-12-08 21:47 ` Dan Williams
0 siblings, 0 replies; 26+ messages in thread
From: Dan Williams @ 2021-12-08 21:47 UTC (permalink / raw)
To: Vishal Verma; +Cc: Linux NVDIMM, QI Fuli, Hu, Fenghua, QI Fuli
On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> The new config system introduces and installs a sample config file
> called ndctl.conf. Update the RPM spec to include this in the %files
> section for ndctl.
Modulo new ndctl.conf.d directory name:
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>
> Cc: QI Fuli <qi.fuli@fujitsu.com>
> Reviewed-by: QI Fuli <qi.fuli@jp.fujitsu.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
> ndctl.spec.in | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/ndctl.spec.in b/ndctl.spec.in
> index 0563b2d..07c36ec 100644
> --- a/ndctl.spec.in
> +++ b/ndctl.spec.in
> @@ -118,6 +118,7 @@ make check
> %{_sysconfdir}/modprobe.d/nvdimm-security.conf
>
> %config(noreplace) %{_sysconfdir}/ndctl/monitor.conf
> +%config(noreplace) %{_sysconfdir}/ndctl/ndctl.conf
>
> %files -n daxctl
> %defattr(-,root,root)
> --
> 2.33.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 07/12] daxctl: Documentation updates for persistent reconfiguration
2021-12-06 22:28 ` [ndctl PATCH v2 07/12] daxctl: Documentation updates for persistent reconfiguration Vishal Verma
@ 2021-12-08 21:51 ` Dan Williams
0 siblings, 0 replies; 26+ messages in thread
From: Dan Williams @ 2021-12-08 21:51 UTC (permalink / raw)
To: Vishal Verma; +Cc: Linux NVDIMM, QI Fuli, Hu, Fenghua
On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Add a man page update describing how daxctl-reconfigure-device(1) can
> be used for persistent reconfiguration of a daxctl device using a
> config file.
Looks good, although this makes me think we need to immediately
proceed to make an attempt at populating a /dev/dax/by-path that works
across both NVDIMM dax devices and soft-reserved dax devices.
You can add:
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ndctl PATCH v2 08/12] util/parse-config: refactor filter_conf_files into util/
2021-12-06 22:28 ` [ndctl PATCH v2 08/12] util/parse-config: refactor filter_conf_files into util/ Vishal Verma
@ 2021-12-08 21:52 ` Dan Williams
0 siblings, 0 replies; 26+ messages in thread
From: Dan Williams @ 2021-12-08 21:52 UTC (permalink / raw)
To: Vishal Verma; +Cc: Linux NVDIMM, QI Fuli, Hu, Fenghua, QI Fuli
On Mon, Dec 6, 2021 at 2:28 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Move filter_conf() into util/parse-configs.c as filter_conf_files() so
> that it can be reused by the config parser in daxctl.
>
> Cc: QI Fuli <qi.fuli@fujitsu.com>
> Reviewed-by: QI Fuli <qi.fuli@jp.fujitsu.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Looks good.
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2021-12-08 21:53 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-06 22:28 [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 01/12] ndctl, util: add iniparser helper Vishal Verma
2021-12-07 18:24 ` Dan Williams
2021-12-07 21:50 ` Verma, Vishal L
2021-12-06 22:28 ` [ndctl PATCH v2 02/12] ndctl, util: add parse-configs helper Vishal Verma
2021-12-07 21:58 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 03/12] ndctl: make ndctl support configuration files Vishal Verma
2021-12-07 22:51 ` Dan Williams
2021-12-08 0:04 ` Verma, Vishal L
2021-12-08 0:19 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 04/12] ndctl, config: add the default ndctl configuration file Vishal Verma
2021-12-08 16:00 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 05/12] ndctl, monitor: refator monitor for supporting multiple config files Vishal Verma
2021-12-08 16:00 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 06/12] ndctl: Update ndctl.spec.in for 'ndctl.conf' Vishal Verma
2021-12-08 21:47 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 07/12] daxctl: Documentation updates for persistent reconfiguration Vishal Verma
2021-12-08 21:51 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 08/12] util/parse-config: refactor filter_conf_files into util/ Vishal Verma
2021-12-08 21:52 ` Dan Williams
2021-12-06 22:28 ` [ndctl PATCH v2 09/12] daxctl: add basic config parsing support Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 10/12] util/parse-configs: add a key/value search helper Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 11/12] daxctl/device.c: add an option for getting params from a config file Vishal Verma
2021-12-06 22:28 ` [ndctl PATCH v2 12/12] daxctl: add systemd service and udev rule for automatic reconfiguration Vishal Verma
2021-12-06 23:10 ` [ndctl PATCH v2 00/12] Policy based reconfiguration for daxctl Verma, Vishal L
2021-12-07 11:24 ` qi.fuli
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.