* [PATCH v5 0/4] ndctl, monitor: add ndctl monitor daemon
@ 2018-04-26 11:30 QI Fuli
2018-04-26 11:30 ` [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type QI Fuli
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: QI Fuli @ 2018-04-26 11:30 UTC (permalink / raw)
To: linux-nvdimm
This is the v5 patch for ndctl monitor daemon, a tiny daemon to monitor the
smart events of nvdimm DIMMs. Users can run a monitor as a one-shot command
or a daemon in background by using the [--daemon] option. DIMMs to monitor
can be selected by [--dimm] [--bus] [--region] [--namespace] options,
these options support multiple space-seperated arguments.
When a smart event fires, monitor daemon will log the notifications which
including dimm health status to syslog or a logfile by setting
[--logfile=<file|syslog>] option. monitor also can output the notifications
to stderr when it run as one-shot command by setting [--logfile=<stderr>].
The notifications follow json format and can be consumed by log collectors
like Fluentd. Users can change the configuration of monitor by editing
the default configuration file /etc/ndctl/monitor.conf or by using
[--config-file=<file>] option to override the default one.
Users can start a monitor daemon by the following command:
# ndctl monitor --dimm nmem1 --logfile /var/log/ndctl/monitor.log --daemon
Also, a monitor daemon can be started by systemd:
# systemctl start ndctl-monitor.service
In this case, monitor daemon follows the default configuration file
/etc/ndctl/monitor.conf.
Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
Change log since v4:
- Adding OPTION_FILENAME to make sure filename is correct
- Adding configuration file
- Adding [--config-file] option to override the default cofiguration file
- Making some options support multiple space-seperated arguments
- Making systemctl enable ndctl-monitor.service command work
- Making systemctl restart ndctl-monitor.service command work
- Making the dirtctory of systemd unit file to be configurable
- Changing log_file() and log_syslog to logreport()
- Changing date format in log to nanoseconds since epoch
- Changing select() to epoll()
- Adding filter_bus() and filter_region()
Change log since v3:
- Removing create-monitor, show-monitor, list-monitor, destroy-monitor
- Adding [--daemon] option to run ndctl monitor as a daemon
- Using systemd to manage ndctl monitor daemon
- Replacing filter_monitor_dimm() with filter_dimm()
Change log since v2:
- Changing the interface of daemon to the ndctl command line
- Changing the name of daemon form "nvdimmd" to "monitor"
- Removing the config file, unit_file, nvdimmd dir
- Removing nvdimmd_test program
Change log since v1:
- Adding a config file(/etc/nvdimmd/nvdimmd.conf)
- Using struct log_ctx instead of syslog()
- Using log_syslog() to save the notify messages to syslog
- Using log_file() to save the notify messages to special file
- Adding LOG_NOTICE level to log_priority
- Using automake instead of Makefile
- Adding a new util file(nvdimmd/util.c) including helper functions
needed for nvdimm daemon
- Adding nvdimmd_test program
QI Fuli (4):
ndctl, util: add OPTION_FILENAME to parse_opt_type
ndctl, monitor: add ndctl monitor daemon
ndctl, monitor: add default configuration file
ndctl, monitor: add the unit file of systemd for ndctl-monitor service
Makefile.am | 3 +-
autogen.sh | 3 +-
builtin.h | 1 +
configure.ac | 22 ++
ndctl/Makefile.am | 12 +-
ndctl/monitor.c | 460 ++++++++++++++++++++++++++++++++++++
ndctl/monitor.conf | 37 +++
ndctl/ndctl-monitor.service | 7 +
ndctl/ndctl.c | 1 +
util/abspath.c | 28 +++
util/help.c | 5 -
util/parse-options.c | 47 +++-
util/parse-options.h | 11 +-
util/util.h | 7 +
14 files changed, 628 insertions(+), 16 deletions(-)
create mode 100644 ndctl/monitor.c
create mode 100644 ndctl/monitor.conf
create mode 100644 ndctl/ndctl-monitor.service
create mode 100644 util/abspath.c
--
2.17.0.140.g0b0cc9f86
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type
2018-04-26 11:30 [PATCH v5 0/4] ndctl, monitor: add ndctl monitor daemon QI Fuli
@ 2018-04-26 11:30 ` QI Fuli
2018-05-04 3:25 ` Dan Williams
2018-04-26 11:30 ` [PATCH v5 2/4] ndctl, monitor: add ndctl monitor daemon QI Fuli
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: QI Fuli @ 2018-04-26 11:30 UTC (permalink / raw)
To: linux-nvdimm
This patch borrows the OPTION_FILENAME from git to ndctl to make sure
filename is correct. Some related refactoring is also included:
- adds parse_options_prefix() interface
- moves is_absolute from util/help.c to util/util.c
- adds a new file util/abspath.c
Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
---
Makefile.am | 3 ++-
util/abspath.c | 28 ++++++++++++++++++++++++++
util/help.c | 5 -----
util/parse-options.c | 47 ++++++++++++++++++++++++++++++++++++++------
util/parse-options.h | 11 +++++++++--
util/util.h | 7 +++++++
6 files changed, 87 insertions(+), 14 deletions(-)
create mode 100644 util/abspath.c
diff --git a/Makefile.am b/Makefile.am
index b538b1f..e0c463a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -69,6 +69,7 @@ libutil_a_SOURCES = \
util/strbuf.c \
util/wrapper.c \
util/filter.c \
- util/bitmap.c
+ util/bitmap.c \
+ util/abspath.c
nobase_include_HEADERS = daxctl/libdaxctl.h
diff --git a/util/abspath.c b/util/abspath.c
new file mode 100644
index 0000000..fdf4090
--- /dev/null
+++ b/util/abspath.c
@@ -0,0 +1,28 @@
+/* originally copied from git */
+
+#include <util/util.h>
+#include <util/strbuf.h>
+
+char *prefix_filename(const char *pfx, const char *arg)
+{
+ struct strbuf path = STRBUF_INIT;
+ size_t pfx_len = pfx ? strlen(pfx) : 0;
+
+ if (!pfx_len)
+ ;
+ else if (is_absolute_path(arg))
+ pfx_len = 0;
+ else
+ strbuf_add(&path, pfx, pfx_len);
+
+ strbuf_addstr(&path, arg);
+ return strbuf_detach(&path, NULL);
+}
+
+void fix_filename(const char *prefix, const char **file)
+{
+ if (!file || !*file || !prefix || is_absolute_path(*file)
+ || !strcmp("-", *file))
+ return;
+ *file = prefix_filename(prefix, *file);
+}
diff --git a/util/help.c b/util/help.c
index 8b8f951..2d57fa1 100644
--- a/util/help.c
+++ b/util/help.c
@@ -89,11 +89,6 @@ static char *cmd_to_page(const char *cmd, char **page, const char *util_name)
return *page;
}
-static int is_absolute_path(const char *path)
-{
- return path[0] == '/';
-}
-
static const char *system_path(const char *path)
{
static const char *prefix = PREFIX;
diff --git a/util/parse-options.c b/util/parse-options.c
index 751c091..c781174 100644
--- a/util/parse-options.c
+++ b/util/parse-options.c
@@ -55,6 +55,7 @@ static int get_value(struct parse_opt_ctx_t *p,
{
const char *s, *arg = NULL;
const int unset = flags & OPT_UNSET;
+ int err;
if (unset && p->opt)
return opterror(opt, "takes no value", flags);
@@ -77,6 +78,7 @@ static int get_value(struct parse_opt_ctx_t *p,
case OPTION_ARGUMENT:
case OPTION_GROUP:
case OPTION_STRING:
+ case OPTION_FILENAME:
case OPTION_INTEGER:
case OPTION_UINTEGER:
case OPTION_LONG:
@@ -121,6 +123,19 @@ static int get_value(struct parse_opt_ctx_t *p,
return get_arg(p, opt, flags, (const char **)opt->value);
return 0;
+ case OPTION_FILENAME:
+ err = 0;
+ if (unset)
+ *(const char **)opt->value = NULL;
+ else if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
+ *(const char **)opt->value = (const char *)opt->defval;
+ else
+ err = get_arg(p, opt, flags, (const char **)opt->value);
+
+ if (!err)
+ fix_filename(p->prefix, (const char **)opt->value);
+ return err;
+
case OPTION_CALLBACK:
if (unset)
return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
@@ -339,13 +354,14 @@ static void check_typos(const char *arg, const struct option *options)
}
}
-void parse_options_start(struct parse_opt_ctx_t *ctx,
- int argc, const char **argv, int flags)
+void parse_options_start(struct parse_opt_ctx_t *ctx, int argc,
+ const char **argv, const char *prefix, int flags)
{
memset(ctx, 0, sizeof(*ctx));
ctx->argc = argc - 1;
ctx->argv = argv + 1;
ctx->out = argv;
+ ctx->prefix = prefix;
ctx->cpidx = ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
ctx->flags = flags;
if ((flags & PARSE_OPT_KEEP_UNKNOWN) &&
@@ -453,8 +469,10 @@ int parse_options_end(struct parse_opt_ctx_t *ctx)
return ctx->cpidx + ctx->argc;
}
-int parse_options_subcommand(int argc, const char **argv, const struct option *options,
- const char *const subcommands[], const char *usagestr[], int flags)
+static int parse_options_subcommand_prefix(int argc, const char **argv,
+ const char *prefix, const struct option *options,
+ const char *const subcommands[],
+ const char *usagestr[], int flags)
{
struct parse_opt_ctx_t ctx;
@@ -474,7 +492,7 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
strbuf_release(&buf);
}
- parse_options_start(&ctx, argc, argv, flags);
+ parse_options_start(&ctx, argc, argv, prefix, flags);
switch (parse_options_step(&ctx, options, usagestr)) {
case PARSE_OPT_HELP:
exit(129);
@@ -503,10 +521,26 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
return parse_options_end(&ctx);
}
+int parse_options_subcommand(int argc, const char **argv,
+ const struct option *options, const char *const subcommands[],
+ const char *usagestr[], int flags)
+{
+ return parse_options_subcommand_prefix(argc, argv, NULL, options,
+ subcommands, usagestr, flags);
+}
+
+int parse_options_prefix(int argc, const char **argv, const char *prefix,
+ const struct option *options,
+ const char * const usagestr[], int flags)
+{
+ return parse_options_subcommand_prefix(argc, argv, prefix, options,
+ NULL, (const char **) usagestr, flags);
+}
+
int parse_options(int argc, const char **argv, const struct option *options,
const char * const usagestr[], int flags)
{
- return parse_options_subcommand(argc, argv, options, NULL,
+ return parse_options_subcommand_prefix(argc, argv, NULL, options, NULL,
(const char **) usagestr, flags);
}
@@ -557,6 +591,7 @@ static void print_option_help(const struct option *opts, int full)
if (opts->flags & PARSE_OPT_NOARG)
break;
/* FALLTHROUGH */
+ case OPTION_FILENAME:
case OPTION_STRING:
if (opts->argh) {
if (opts->flags & PARSE_OPT_OPTARG)
diff --git a/util/parse-options.h b/util/parse-options.h
index 6fd6b24..fc5015a 100644
--- a/util/parse-options.h
+++ b/util/parse-options.h
@@ -38,6 +38,7 @@ enum parse_opt_type {
OPTION_CALLBACK,
OPTION_U64,
OPTION_UINTEGER,
+ OPTION_FILENAME,
};
enum parse_opt_flags {
@@ -135,6 +136,7 @@ struct option {
#define OPT_LONG(s, l, v, h) { .type = OPTION_LONG, .short_name = (s), .long_name = (l), .value = check_vtype(v, long *), .help = (h) }
#define OPT_U64(s, l, v, h) { .type = OPTION_U64, .short_name = (s), .long_name = (l), .value = check_vtype(v, u64 *), .help = (h) }
#define OPT_STRING(s, l, v, a, h) { .type = OPTION_STRING, .short_name = (s), .long_name = (l), .value = check_vtype(v, const char **), (a), .help = (h) }
+#define OPT_FILENAME(s, l, v, a, h) { .type = OPTION_FILENAME, .short_name = (s), .long_name = (l), .value = check_vtype(v, const char **), (a), .help = (h) }
#define OPT_DATE(s, l, v, h) \
{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), .argh = "time", .help = (h), .callback = parse_opt_approxidate_cb }
#define OPT_CALLBACK(s, l, v, a, h, f) \
@@ -156,6 +158,10 @@ extern int parse_options(int argc, const char **argv,
const struct option *options,
const char * const usagestr[], int flags);
+extern int parse_options_prefix(int argc, const char **argv,
+ const char *prefix, const struct option *options,
+ const char * const usagestr[], int flags);
+
extern int parse_options_subcommand(int argc, const char **argv,
const struct option *options,
const char *const subcommands[],
@@ -185,6 +191,7 @@ struct parse_opt_ctx_t {
int argc, cpidx;
const char *opt;
int flags;
+ const char *prefix;
};
extern int parse_options_usage(const char * const *usagestr,
@@ -192,8 +199,8 @@ extern int parse_options_usage(const char * const *usagestr,
const char *optstr,
bool short_opt);
-extern void parse_options_start(struct parse_opt_ctx_t *ctx,
- int argc, const char **argv, int flags);
+extern void parse_options_start(struct parse_opt_ctx_t *ctx, int argc,
+ const char **argv, const char *prefix, int flags);
extern int parse_options_step(struct parse_opt_ctx_t *ctx,
const struct option *options,
diff --git a/util/util.h b/util/util.h
index 162aade..001707e 100644
--- a/util/util.h
+++ b/util/util.h
@@ -79,6 +79,11 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
return strncmp(str, prefix, len) ? NULL : str + len;
}
+static inline int is_absolute_path(const char *path)
+{
+ return path[0] == '/';
+}
+
void usage(const char *err) NORETURN;
void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
@@ -87,5 +92,7 @@ void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);
char *xstrdup(const char *str);
void *xrealloc(void *ptr, size_t size);
int prefixcmp(const char *str, const char *prefix);
+char *prefix_filename(const char *pfx, const char *arg);
+void fix_filename(const char *prefix, const char **file);
#endif /* __UTIL_H__ */
--
2.17.0.140.g0b0cc9f86
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 2/4] ndctl, monitor: add ndctl monitor daemon
2018-04-26 11:30 [PATCH v5 0/4] ndctl, monitor: add ndctl monitor daemon QI Fuli
2018-04-26 11:30 ` [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type QI Fuli
@ 2018-04-26 11:30 ` QI Fuli
2018-04-26 11:30 ` [PATCH v5 3/4] ndctl, monitor: add default configuration file QI Fuli
2018-04-26 11:30 ` [PATCH v5 4/4] ndctl, monitor: add the unit file of systemd for ndctl-monitor service QI Fuli
3 siblings, 0 replies; 9+ messages in thread
From: QI Fuli @ 2018-04-26 11:30 UTC (permalink / raw)
To: linux-nvdimm
This patch adds the body file of ndctl monitor daemon.
Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
---
builtin.h | 1 +
ndctl/Makefile.am | 3 +-
ndctl/monitor.c | 460 ++++++++++++++++++++++++++++++++++++++++++++++
ndctl/ndctl.c | 1 +
4 files changed, 464 insertions(+), 1 deletion(-)
create mode 100644 ndctl/monitor.c
diff --git a/builtin.h b/builtin.h
index d3cc723..675a6ce 100644
--- a/builtin.h
+++ b/builtin.h
@@ -39,6 +39,7 @@ int cmd_inject_error(int argc, const char **argv, void *ctx);
int cmd_wait_scrub(int argc, const char **argv, void *ctx);
int cmd_start_scrub(int argc, const char **argv, void *ctx);
int cmd_list(int argc, const char **argv, void *ctx);
+int cmd_monitor(int argc, const char **argv, void *ctx);
#ifdef ENABLE_TEST
int cmd_test(int argc, const char **argv, void *ctx);
#endif
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index d22a379..7dbf223 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -16,7 +16,8 @@ ndctl_SOURCES = ndctl.c \
util/json-smart.c \
util/json-firmware.c \
inject-error.c \
- inject-smart.c
+ inject-smart.c \
+ monitor.c
if ENABLE_DESTRUCTIVE
ndctl_SOURCES += ../test/blk_namespaces.c \
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
new file mode 100644
index 0000000..ab6e701
--- /dev/null
+++ b/ndctl/monitor.c
@@ -0,0 +1,460 @@
+/*
+ * Copyright(c) 2018, FUJITSU LIMITED. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Lesser General Public License,
+ * version 2.1, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for
+ * more details.
+ */
+
+#include <stdio.h>
+#include <json-c/json.h>
+#include <libgen.h>
+#include <dirent.h>
+#include <util/log.h>
+#include <util/json.h>
+#include <util/filter.h>
+#include <util/util.h>
+#include <util/parse-options.h>
+#include <util/strbuf.h>
+#include <ndctl/lib/private.h>
+#include <ndctl/libndctl.h>
+#include <sys/stat.h>
+#include <sys/epoll.h>
+#define BUF_SIZE 2048
+
+static enum log_destination {
+ LOG_DESTINATION_STDERR = 1,
+ LOG_DESTINATION_SYSLOG = 2,
+ LOG_DESTINATION_FILE = 3,
+} log_destination = LOG_DESTINATION_SYSLOG;
+
+static struct {
+ const char *logfile;
+ const char *config_file;
+ bool daemon;
+} monitor;
+
+struct monitor_dimm_node {
+ struct ndctl_dimm *dimm;
+ int health_eventfd;
+ struct list_node list;
+};
+
+struct monitor_filter_arg {
+ struct list_head mdimm;
+ int maxfd_dimm;
+ int num_dimm;
+ unsigned long flags;
+};
+
+struct util_filter_params param;
+
+static int did_fail;
+
+#define fail(fmt, ...) \
+do { \
+ did_fail = 1; \
+ err(ctx, "ndctl-%s:%s:%d: " fmt, \
+ VERSION, __func__, __LINE__, ##__VA_ARGS__); \
+} while (0)
+
+static bool is_dir(char *filepath)
+{
+ DIR *dir = opendir(filepath);
+ if (dir) {
+ closedir(dir);
+ return true;
+ }
+ return false;
+}
+
+static int recur_mkdir(char *filepath, mode_t mode)
+{
+ char *p;
+ char *buf = (char *)malloc(strlen(filepath) + 4);
+
+ strcpy(buf, filepath);
+ for (p = strchr(buf + 1, '/'); p; p = strchr(p + 1, '/')) {
+ *p = '\0';
+ if (!is_dir(buf)) {
+ if (mkdir(buf, mode) < 0) {
+ free(buf);
+ return -1;
+ }
+ }
+ *p = '/';
+ }
+
+ free(buf);
+ return 0;
+}
+
+static void set_value(const char **arg, char *val)
+{
+ struct strbuf value = STRBUF_INIT;
+ size_t arg_len = *arg ? strlen(*arg) : 0;
+
+ if (arg_len) {
+ strbuf_add(&value, *arg, arg_len);
+ strbuf_addstr(&value, " ");
+ }
+ strbuf_addstr(&value, val);
+ *arg = strbuf_detach(&value, NULL);
+}
+
+static void logreport(struct ndctl_ctx *ctx, int priority, const char *file,
+ int line, const char *fn, const char *format, va_list args)
+{
+ char *log_dir;
+ char *buf = (char *)malloc(BUF_SIZE);
+ vsnprintf(buf, BUF_SIZE, format, args);
+
+ switch (log_destination) {
+ case LOG_DESTINATION_STDERR:
+ fprintf(stderr, "%s\n", buf);
+ goto end;
+
+ case LOG_DESTINATION_SYSLOG:
+ syslog(priority, "%s\n", buf);
+ goto end;
+
+ case LOG_DESTINATION_FILE:
+ log_dir = dirname(strdup(monitor.logfile));
+ if (!is_dir(log_dir) && recur_mkdir(log_dir, 0744) != 0)
+ goto log_file_err;
+ FILE *f = fopen(monitor.logfile, "a+");
+ if (!f)
+ goto log_file_err;
+ fprintf(f, "%s\n", buf);
+ fclose(f);
+ free(log_dir);
+ goto end;
+
+ log_file_err:
+ log_destination = LOG_DESTINATION_SYSLOG;
+ fail("open logfile %s failed\n%s", monitor.logfile, buf);
+ free(log_dir);
+ }
+end:
+ free(buf);
+ return;
+}
+
+static void notify_json_msg(struct ndctl_ctx *ctx, struct ndctl_dimm *dimm)
+{
+ struct json_object *jmsg, *jdatetime, *jpid, *jdimm, *jhealth;
+ struct timespec ts;
+ char datetime[32];
+
+ jmsg = json_object_new_object();
+ if (!jmsg) {
+ fail("\n");
+ return;
+ }
+
+ clock_gettime(CLOCK_REALTIME, &ts);
+ sprintf(datetime, "%10ld.%09ld", ts.tv_sec, ts.tv_nsec);
+ jdatetime = json_object_new_string(datetime);
+ if (!jdatetime) {
+ fail("\n");
+ return;
+ }
+ json_object_object_add(jmsg, "datetime", jdatetime);
+
+ jpid = json_object_new_int((int)getpid());
+ if (!jpid) {
+ fail("\n");
+ return;
+ }
+ json_object_object_add(jmsg, "pid", jpid);
+
+ jdimm = util_dimm_to_json(dimm, 0);
+ if (!dimm) {
+ fail("\n");
+ return;
+ }
+ json_object_object_add(jmsg, "dimm", jdimm);
+
+ jhealth = util_dimm_health_to_json(dimm);
+ if (!jhealth) {
+ fail("\n");
+ return;
+ }
+ json_object_object_add(jdimm, "health", jhealth);
+
+ notice(ctx, "%s",
+ json_object_to_json_string_ext(jmsg, JSON_C_TO_STRING_PLAIN));
+}
+
+static bool filter_region(struct ndctl_region *region,
+ struct util_filter_ctx *ctx)
+{
+ return true;
+}
+
+static void filter_dimm(struct ndctl_dimm *dimm, struct util_filter_ctx *ctx)
+{
+ struct monitor_filter_arg *mfa = (struct monitor_filter_arg *)ctx->arg;
+
+ if (!ndctl_dimm_is_cmd_supported(dimm, ND_CMD_SMART_THRESHOLD))
+ return;
+
+ struct monitor_dimm_node *mdn = malloc(sizeof(*mdn));
+ mdn->dimm = dimm;
+ int fd = ndctl_dimm_get_health_eventfd(dimm);
+ mdn->health_eventfd = fd;
+ list_add_tail(&mfa->mdimm, &mdn->list);
+ if (fd > mfa->maxfd_dimm)
+ mfa->maxfd_dimm = fd;
+ mfa->num_dimm++;
+}
+
+static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
+{
+ return true;
+}
+
+static int monitor_dimm_event(struct ndctl_ctx *ctx,
+ struct monitor_filter_arg *mfa)
+{
+ struct epoll_event ev, events[mfa->num_dimm];
+ struct ndctl_dimm **dimms;
+ int nfds, epollfd;
+ struct monitor_dimm_node *mdn;
+ char buf;
+
+ dimms = calloc(mfa->maxfd_dimm + 1, sizeof(struct ndctl_dimm *));
+ if (!dimms) {
+ fail("\n");
+ goto out;
+ }
+
+ epollfd = epoll_create1(0);
+ if (epollfd == -1) {
+ err(ctx, "epoll_create1 error\n");
+ goto out;
+ }
+ list_for_each(&mfa->mdimm, mdn, list) {
+ memset(&ev, 0, sizeof(ev));
+ pread(mdn->health_eventfd, &buf, 1, 0);
+ ev.data.fd = mdn->health_eventfd;
+ if (epoll_ctl(epollfd, EPOLL_CTL_ADD,
+ mdn->health_eventfd, &ev) != 0) {
+ err(ctx, "epoll_ctl error\n");
+ goto out;
+ }
+ dimms[mdn->health_eventfd] = mdn->dimm;
+ }
+
+ while(1){
+ nfds = epoll_wait(epollfd, events, mfa->num_dimm, -1);
+ if (nfds <= 0) {
+ err(ctx, "epoll_wait error\n");
+ goto out;
+ }
+ for (int i = 0; i < nfds; i++) {
+ memset(&ev, 0, sizeof(ev));
+ ev = events[i];
+ notify_json_msg(ctx, dimms[ev.data.fd]);
+ pread(ev.data.fd, &buf, 1, 0);
+ }
+ if (did_fail)
+ goto out;
+ }
+ return 0;
+out:
+ free(dimms);
+ return 1;
+}
+
+static int read_config_file(struct ndctl_ctx *ctx, const char *prefix)
+{
+ FILE *f;
+ int line_nr = 0;
+ char buf[BUF_SIZE];
+ char *config_file = "/etc/ndctl/monitor.conf";
+
+ if (monitor.config_file)
+ f = fopen(monitor.config_file, "r");
+ else
+ f = fopen(config_file, "r");
+
+ if (f == NULL) {
+ error("config-file: %s cannot be opened\n", config_file);
+ return -1;
+ }
+
+ while (fgets(buf, BUF_SIZE, f)) {
+ size_t len;
+ char *key;
+ char *val;
+
+ line_nr++;
+
+ /* find key */
+ key = buf;
+ while (isspace(key[0]))
+ key++;
+
+ /* comment or empty line */
+ if (key[0] == '#' || key[0] == '\0')
+ continue;
+
+ /* split key/value */
+ val = strchr(key, '=');
+ if (!val) {
+ err(ctx, "missing <key>=<value> in '%s'[%i], skip line\n",
+ config_file, line_nr);
+ continue;
+ }
+
+ val[0] = '\0';
+ val++;
+
+ /* find value */
+ while (isspace(val[0]))
+ val++;
+
+ /* terminate key */
+ len = strlen(key);
+ if (len == 0)
+ continue;
+ while (isspace(key[len-1]))
+ len--;
+ key[len] = '\0';
+
+ /*terminate value */
+ len = strlen(val);
+ if (len == 0)
+ continue;
+ while (isspace(val[len-1]))
+ len--;
+ val[len] = '\0';
+
+ if (len == 0)
+ continue;
+
+ /* unquote */
+ if (val[0] == '"' || val[0] == '\'') {
+ if (val[len-1] != val[0]) {
+ err(ctx, "inconsistent quoting in '%s'[%i], skip line\n",
+ config_file, line_nr);
+ continue;
+ }
+ val[len-1] = '\0';
+ val++;
+ }
+
+ if (strcmp(key, "bus") == 0) {
+ set_value((const char **)¶m.bus, val);
+ continue;
+ }
+ if (strcmp(key, "dimm") == 0) {
+ set_value((const char **)¶m.dimm, val);
+ continue;
+ }
+ if (strcmp(key, "region") == 0) {
+ set_value((const char **)¶m.region, val);
+ continue;
+ }
+ if (strcmp(key, "namespace") == 0) {
+ set_value((const char **)¶m.namespace, val);
+ continue;
+ }
+ if (strcmp(key, "logfile") == 0) {
+ if (monitor.logfile)
+ continue;
+ set_value((const char **)&monitor.logfile, val);
+ fix_filename(prefix, (const char **)&monitor.logfile);
+ }
+ }
+ fclose(f);
+ return 0;
+}
+
+int cmd_monitor(int argc, const char **argv, void *ctx)
+{
+ const struct option options[] = {
+ OPT_STRING('b', "bus", ¶m.bus, "bus-id", "filter by bus"),
+ OPT_STRING('r', "region", ¶m.region, "region-id",
+ "filter by region"),
+ OPT_STRING('d', "dimm", ¶m.dimm, "dimm-id",
+ "filter by dimm"),
+ OPT_STRING('n', "namespace", ¶m.namespace,
+ "namespace-id", "filter by namespace id"),
+ OPT_FILENAME('l', "logfile", &monitor.logfile, "file|syslog|stderr",
+ "the place to output the monitor's notification"),
+ OPT_FILENAME('c',"config-file", &monitor.config_file, "config-file",
+ "use file to override the default configuration"),
+ OPT_BOOLEAN('D',"daemon", &monitor.daemon,
+ "run ndctl monitor as a daemon"),
+ OPT_END(),
+ };
+ const char * const u[] = {
+ "ndctl monitor [<options>]",
+ NULL
+ };
+ const char *prefix = "./";
+ struct util_filter_ctx fctx = { 0 };
+ struct monitor_filter_arg mfa = { 0 };
+
+ argc = parse_options_prefix(argc, argv, prefix, options, u, 0);
+ for (int i = 0; i < argc; i++) {
+ error("unknown parameter \"%s\"\n", argv[i]);
+ }
+ if (argc)
+ usage_with_options(u, options);
+
+ ndctl_set_log_fn((struct ndctl_ctx *)ctx, logreport);
+ ndctl_set_log_priority((struct ndctl_ctx *)ctx, LOG_INFO);
+
+ if (read_config_file((struct ndctl_ctx *)ctx, prefix))
+ goto out;
+
+ if (monitor.logfile) {
+ if (strcmp(monitor.logfile, "./stderr") == 0)
+ log_destination = LOG_DESTINATION_STDERR;
+ else if (strcmp(monitor.logfile, "./syslog") == 0)
+ log_destination = LOG_DESTINATION_SYSLOG;
+ else
+ log_destination = LOG_DESTINATION_FILE;
+ }
+
+ if (monitor.daemon) {
+ if (daemon(0, 0) != 0) {
+ err((struct ndctl_ctx *)ctx, "daemon start failed\n");
+ goto out;
+ }
+ info((struct ndctl_ctx *)ctx, "ndctl monitor daemon started\n");
+ }
+
+ fctx.filter_bus = filter_bus;
+ fctx.filter_dimm = filter_dimm;
+ fctx.filter_region = filter_region;
+ fctx.filter_namespace = NULL;
+ fctx.arg = &mfa;
+ list_head_init(&mfa.mdimm);
+ mfa.num_dimm = 0;
+ mfa.maxfd_dimm = -1;
+ mfa.flags = 0;
+
+ if (util_filter_walk(ctx, &fctx, ¶m))
+ goto out;
+
+ if (!mfa.num_dimm) {
+ err((struct ndctl_ctx *)ctx, "no dimms can be monitored\n");
+ goto out;
+ }
+
+ if (monitor_dimm_event(ctx, &mfa))
+ goto out;
+
+ return 0;
+out:
+ return 1;
+}
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 7daadeb..73dabfa 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -89,6 +89,7 @@ static struct cmd_struct commands[] = {
{ "wait-scrub", cmd_wait_scrub },
{ "start-scrub", cmd_start_scrub },
{ "list", cmd_list },
+ { "monitor", cmd_monitor},
{ "help", cmd_help },
#ifdef ENABLE_TEST
{ "test", cmd_test },
--
2.17.0.140.g0b0cc9f86
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 3/4] ndctl, monitor: add default configuration file
2018-04-26 11:30 [PATCH v5 0/4] ndctl, monitor: add ndctl monitor daemon QI Fuli
2018-04-26 11:30 ` [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type QI Fuli
2018-04-26 11:30 ` [PATCH v5 2/4] ndctl, monitor: add ndctl monitor daemon QI Fuli
@ 2018-04-26 11:30 ` QI Fuli
2018-04-26 11:30 ` [PATCH v5 4/4] ndctl, monitor: add the unit file of systemd for ndctl-monitor service QI Fuli
3 siblings, 0 replies; 9+ messages in thread
From: QI Fuli @ 2018-04-26 11:30 UTC (permalink / raw)
To: linux-nvdimm
This patch adds the default configuration file of ndctl monitor.
Users can change the configuration of ndctl monitor by editing this
file or by using [--config-file=<file>] option to override this one.
The monitor started by systemd follows the configuration in this file.
Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
---
ndctl/Makefile.am | 5 +++++
ndctl/monitor.conf | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)
create mode 100644 ndctl/monitor.conf
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 7dbf223..ae3d894 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -42,3 +42,8 @@ ndctl_SOURCES += ../test/libndctl.c \
../test/multi-pmem.c \
../test/core.c
endif
+
+monitor_config_file = monitor.conf
+monitor_configdir = /etc/ndctl/
+monitor_config_DATA = $(monitor_config_file)
+EXTRA_DIST += $(monitor_config_file)
diff --git a/ndctl/monitor.conf b/ndctl/monitor.conf
new file mode 100644
index 0000000..13ad7b0
--- /dev/null
+++ b/ndctl/monitor.conf
@@ -0,0 +1,37 @@
+# This is the main ndctl monitor configuration file. It contains the
+# configuration directives that give ndctl monitor instructions.
+# You can change the configuration of ndctl monitor by editing this
+# file or by using [--config-file=<file>] option to override this one.
+# The changed value will work after restart ndctl monitor service.
+
+# In this file, lines starting with a hash (#) are comments.
+# The configurations shold follow <key> = <value> style.
+# Multiple space-seperated values are allowed, but except the following
+# charecters: : # ? / \ % " '
+
+# The dimms 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 dimms 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 dimms to monitor are filtered via region by setting key "region".
+# If this value is different from the value of [--namespace=<value>] option,
+# both of the values will work.
+# region = all
+
+# The dimms 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
+
+# Users can choose to output the notifications to syslog (logfile=syslog),
+# stderr (logfile=stderr) or to write into a special file (logfile=<file>)
+# by setting key "logfile".
+# If this value is in conflict with the value of [--logfile=<value>] option,
+# this value will be ignored.
+# logfile = /var/log/ndctl/monitor.log
--
2.17.0.140.g0b0cc9f86
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 4/4] ndctl, monitor: add the unit file of systemd for ndctl-monitor service
2018-04-26 11:30 [PATCH v5 0/4] ndctl, monitor: add ndctl monitor daemon QI Fuli
` (2 preceding siblings ...)
2018-04-26 11:30 ` [PATCH v5 3/4] ndctl, monitor: add default configuration file QI Fuli
@ 2018-04-26 11:30 ` QI Fuli
2018-05-01 15:07 ` Masayoshi Mizuma
3 siblings, 1 reply; 9+ messages in thread
From: QI Fuli @ 2018-04-26 11:30 UTC (permalink / raw)
To: linux-nvdimm
This patch adds the systemd unit file for ndctl-monitor service.
The systemd unit directory can be configured by setting environment
variable "--with-systemd-unit-dir[=DIR]".
Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
---
autogen.sh | 3 ++-
configure.ac | 22 ++++++++++++++++++++++
ndctl/Makefile.am | 4 ++++
ndctl/ndctl-monitor.service | 7 +++++++
4 files changed, 35 insertions(+), 1 deletion(-)
create mode 100644 ndctl/ndctl-monitor.service
diff --git a/autogen.sh b/autogen.sh
index a23cf53..b226c7a 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -17,7 +17,8 @@ libdir() {
args="--prefix=/usr \
--sysconfdir=/etc \
---libdir=$(libdir /usr/lib)"
+--libdir=$(libdir /usr/lib) \
+--with-systemd-unit-dir"
echo
echo "----------------------------------------------------------------"
diff --git a/configure.ac b/configure.ac
index cddad16..60c6537 100644
--- a/configure.ac
+++ b/configure.ac
@@ -135,6 +135,27 @@ AC_CHECK_FUNCS([ \
secure_getenv\
])
+PKG_PROG_PKG_CONFIG
+AC_ARG_WITH([systemd-unit-dir],
+ AS_HELP_STRING([--with-systemd-unit-dir[=DIR]],
+ [Directory for systemd service files]),
+ [],
+ [with_systemd_unit_dir=yes])
+
+if test "x$with_systemd_unit_dir" = "xyes"; then
+ def_systemd_unit_dir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)
+ if test "x$def_systemd_unit_dir" = "x"; then
+ AC_MSG_ERROR([systemd support requested but pkg-config unable to query systemd package])
+ with_systemd_unit_dir=no
+ else
+ with_systemd_unit_dir="$def_systemd_unit_dir"
+ fi
+fi
+
+AS_IF([test "x$with_systemd_unit_dir" != "xno"],
+ [AC_SUBST([SYSTEMD_UNIT_DIR], [$with_systemd_unit_dir])])
+AM_CONDITIONAL([ENABLE_SYSTEMD_UNIT_DIR], [test "x$with_systemd_unit_dir" != "xno"])
+
my_CFLAGS="\
-Wall \
-Wchar-subscripts \
@@ -172,6 +193,7 @@ AC_MSG_RESULT([
sysconfdir: ${sysconfdir}
libdir: ${libdir}
includedir: ${includedir}
+ systemd-unit-dir: ${SYSTEMD_UNIT_DIR}
compiler: ${CC}
cflags: ${CFLAGS}
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index ae3d894..b3053d0 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -47,3 +47,7 @@ monitor_config_file = monitor.conf
monitor_configdir = /etc/ndctl/
monitor_config_DATA = $(monitor_config_file)
EXTRA_DIST += $(monitor_config_file)
+
+if ENABLE_SYSTEMD_UNIT_DIR
+systemd_unitDATA = ndctl-monitor.service
+endif
diff --git a/ndctl/ndctl-monitor.service b/ndctl/ndctl-monitor.service
new file mode 100644
index 0000000..44f9326
--- /dev/null
+++ b/ndctl/ndctl-monitor.service
@@ -0,0 +1,7 @@
+[Unit]
+Description=Ndctl Monitor Daemon
+
+[Service]
+Type=forking
+ExecStart=/usr/bin/ndctl monitor --daemon
+ExecStop=/bin/kill ${MAINPID}
--
2.17.0.140.g0b0cc9f86
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] ndctl, monitor: add the unit file of systemd for ndctl-monitor service
2018-04-26 11:30 ` [PATCH v5 4/4] ndctl, monitor: add the unit file of systemd for ndctl-monitor service QI Fuli
@ 2018-05-01 15:07 ` Masayoshi Mizuma
2018-05-07 1:47 ` Qi, Fuli
0 siblings, 1 reply; 9+ messages in thread
From: Masayoshi Mizuma @ 2018-05-01 15:07 UTC (permalink / raw)
To: qi.fuli, linux-nvdimm
Hello QI,
I think the systemd service is nice, however, it seems that it does
not work fine... because ndctl-monitor.service was not installed
to systemd dir.
On 04/26/2018 07:30 AM, QI Fuli wrote:
> This patch adds the systemd unit file for ndctl-monitor service.
> The systemd unit directory can be configured by setting environment
> variable "--with-systemd-unit-dir[=DIR]".
>
> Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
>
> ---
> autogen.sh | 3 ++-
> configure.ac | 22 ++++++++++++++++++++++
> ndctl/Makefile.am | 4 ++++
> ndctl/ndctl-monitor.service | 7 +++++++
> 4 files changed, 35 insertions(+), 1 deletion(-)
> create mode 100644 ndctl/ndctl-monitor.service
>
> diff --git a/autogen.sh b/autogen.sh
> index a23cf53..b226c7a 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -17,7 +17,8 @@ libdir() {
>
> args="--prefix=/usr \
> --sysconfdir=/etc \
> ---libdir=$(libdir /usr/lib)"
> +--libdir=$(libdir /usr/lib) \
> +--with-systemd-unit-dir"
>
> echo
> echo "----------------------------------------------------------------"
> diff --git a/configure.ac b/configure.ac
> index cddad16..60c6537 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -135,6 +135,27 @@ AC_CHECK_FUNCS([ \
> secure_getenv\
> ])
>
> +PKG_PROG_PKG_CONFIG
> +AC_ARG_WITH([systemd-unit-dir],
> + AS_HELP_STRING([--with-systemd-unit-dir[=DIR]],
> + [Directory for systemd service files]),
> + [],
> + [with_systemd_unit_dir=yes])
> +
> +if test "x$with_systemd_unit_dir" = "xyes"; then
> + def_systemd_unit_dir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)
> + if test "x$def_systemd_unit_dir" = "x"; then
> + AC_MSG_ERROR([systemd support requested but pkg-config unable to query systemd package])
> + with_systemd_unit_dir=no
> + else
> + with_systemd_unit_dir="$def_systemd_unit_dir"
> + fi
> +fi
> +
> +AS_IF([test "x$with_systemd_unit_dir" != "xno"],
> + [AC_SUBST([SYSTEMD_UNIT_DIR], [$with_systemd_unit_dir])])
> +AM_CONDITIONAL([ENABLE_SYSTEMD_UNIT_DIR], [test "x$with_systemd_unit_dir" != "xno"])
> +
> my_CFLAGS="\
> -Wall \
> -Wchar-subscripts \
> @@ -172,6 +193,7 @@ AC_MSG_RESULT([
> sysconfdir: ${sysconfdir}
> libdir: ${libdir}
> includedir: ${includedir}
> + systemd-unit-dir: ${SYSTEMD_UNIT_DIR}
>
> compiler: ${CC}
> cflags: ${CFLAGS}
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index ae3d894..b3053d0 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -47,3 +47,7 @@ monitor_config_file = monitor.conf
> monitor_configdir = /etc/ndctl/
> monitor_config_DATA = $(monitor_config_file)
> EXTRA_DIST += $(monitor_config_file)
> +
> +if ENABLE_SYSTEMD_UNIT_DIR
> +systemd_unitDATA = ndctl-monitor.service
> +endif
I'm not familiar with autoconf syntax, but I think the 'systemd_unitDATA'
may be wrong... *_DATA is correct, isn't it?
And, your autoconf code is a little different from the example of autoconf
guide for systemd [1]. It might be a good idea to use the example code simply
because it makes easy to find the systemd code for someone who try to
add systemd service in the future.
[1] https://www.freedesktop.org/software/systemd/man/daemon.html
> diff --git a/ndctl/ndctl-monitor.service b/ndctl/ndctl-monitor.service
> new file mode 100644
> index 0000000..44f9326
> --- /dev/null
> +++ b/ndctl/ndctl-monitor.service
> @@ -0,0 +1,7 @@
> +[Unit]
> +Description=Ndctl Monitor Daemon
> +
> +[Service]
> +Type=forking
> +ExecStart=/usr/bin/ndctl monitor --daemon
> +ExecStop=/bin/kill ${MAINPID}
>
How about the following change?
---
autogen.sh | 3 ++-
configure.ac | 15 +++++++++++++++
ndctl/Makefile.am | 4 ++++
ndctl/ndctl-monitor.service | 7 +++++++
4 files changed, 28 insertions(+), 1 deletion(-)
create mode 100644 ndctl/ndctl-monitor.service
diff --git a/autogen.sh b/autogen.sh
index a23cf53..bc58002 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -17,7 +17,8 @@ libdir() {
args="--prefix=/usr \
--sysconfdir=/etc \
---libdir=$(libdir /usr/lib)"
+--libdir=$(libdir /usr/lib) \
+--with-systemdsystemunitdir"
echo
echo "----------------------------------------------------------------"
diff --git a/configure.ac b/configure.ac
index 3eaac32..6fe054f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -122,6 +122,20 @@ AC_CHECK_FUNCS([ \
secure_getenv\
])
+AC_ARG_WITH([systemdsystemunitdir],
+ [AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files])],,
+ [with_systemdsystemunitdir=auto])
+AS_IF([test "x$with_systemdsystemunitdir" = "xyes" -o "x$with_systemdsystemunitdir" = "xauto"],
+ [def_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)
+ AS_IF([test "x$def_systemdsystemunitdir" = "x"],
+ [AS_IF([test "x$with_systemdsystemunitdir" = "xyes"],
+ [AC_MSG_ERROR([systemd support requested but pkg-config unable to query systemd package])])
+ with_systemdsystemunitdir=no],
+ [with_systemdsystemunitdir="$def_systemdsystemunitdir"])])
+AS_IF([test "x$with_systemdsystemunitdir" != "xno"],
+ [AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])])
+AM_CONDITIONAL([HAVE_SYSTEMD], [test "x$with_systemdsystemunitdir" != "xno"])
+
my_CFLAGS="\
-Wall \
-Wchar-subscripts \
@@ -159,6 +173,7 @@ AC_MSG_RESULT([
sysconfdir: ${sysconfdir}
libdir: ${libdir}
includedir: ${includedir}
+ systemdunitdir: ${systemdsystemunitdir}
compiler: ${CC}
cflags: ${CFLAGS}
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index ae3d894..6ee4726 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -47,3 +47,7 @@ monitor_config_file = monitor.conf
monitor_configdir = /etc/ndctl/
monitor_config_DATA = $(monitor_config_file)
EXTRA_DIST += $(monitor_config_file)
+
+if HAVE_SYSTEMD
+systemdsystemunit_DATA = ndctl-monitor.service
+endif
diff --git a/ndctl/ndctl-monitor.service b/ndctl/ndctl-monitor.service
new file mode 100644
index 0000000..44f9326
--- /dev/null
+++ b/ndctl/ndctl-monitor.service
@@ -0,0 +1,7 @@
+[Unit]
+Description=Ndctl Monitor Daemon
+
+[Service]
+Type=forking
+ExecStart=/usr/bin/ndctl monitor --daemon
+ExecStop=/bin/kill ${MAINPID}
--
1.8.3.1
- Masa
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type
2018-04-26 11:30 ` [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type QI Fuli
@ 2018-05-04 3:25 ` Dan Williams
2018-05-07 1:50 ` Qi, Fuli
0 siblings, 1 reply; 9+ messages in thread
From: Dan Williams @ 2018-05-04 3:25 UTC (permalink / raw)
To: QI Fuli; +Cc: linux-nvdimm
On Thu, Apr 26, 2018 at 4:30 AM, QI Fuli <qi.fuli@jp.fujitsu.com> wrote:
> This patch borrows the OPTION_FILENAME from git to ndctl to make sure
> filename is correct. Some related refactoring is also included:
> - adds parse_options_prefix() interface
> - moves is_absolute from util/help.c to util/util.c
> - adds a new file util/abspath.c
>
> Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
>
[..]
> diff --git a/util/abspath.c b/util/abspath.c
> new file mode 100644
> index 0000000..fdf4090
> --- /dev/null
> +++ b/util/abspath.c
> @@ -0,0 +1,28 @@
> +/* originally copied from git */
Lets say the source file name that it came from in and get and add the license.
/* SPDX-License-Identifier: GPL-2.0 */
/* originally copied from git/abspath.c */
Otherwise, this patch looks good to me.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v5 4/4] ndctl, monitor: add the unit file of systemd for ndctl-monitor service
2018-05-01 15:07 ` Masayoshi Mizuma
@ 2018-05-07 1:47 ` Qi, Fuli
0 siblings, 0 replies; 9+ messages in thread
From: Qi, Fuli @ 2018-05-07 1:47 UTC (permalink / raw)
To: 'Masayoshi Mizuma', linux-nvdimm
> -----Original Message-----
> From: Masayoshi Mizuma [mailto:msys.mizuma@gmail.com]
> Sent: Wednesday, May 2, 2018 12:07 AM
> To: Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>; linux-nvdimm@lists.01.org
> Subject: Re: [PATCH v5 4/4] ndctl, monitor: add the unit file of systemd for
> ndctl-monitor service
>
> Hello QI,
>
> I think the systemd service is nice, however, it seems that it does
> not work fine... because ndctl-monitor.service was not installed
> to systemd dir.
>
> On 04/26/2018 07:30 AM, QI Fuli wrote:
> > This patch adds the systemd unit file for ndctl-monitor service.
> > The systemd unit directory can be configured by setting environment
> > variable "--with-systemd-unit-dir[=DIR]".
> >
> > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
> >
> > ---
> > autogen.sh | 3 ++-
> > configure.ac | 22 ++++++++++++++++++++++
> > ndctl/Makefile.am | 4 ++++
> > ndctl/ndctl-monitor.service | 7 +++++++
> > 4 files changed, 35 insertions(+), 1 deletion(-)
> > create mode 100644 ndctl/ndctl-monitor.service
> >
> > diff --git a/autogen.sh b/autogen.sh
> > index a23cf53..b226c7a 100755
> > --- a/autogen.sh
> > +++ b/autogen.sh
> > @@ -17,7 +17,8 @@ libdir() {
> >
> > args="--prefix=/usr \
> > --sysconfdir=/etc \
> > ---libdir=$(libdir /usr/lib)"
> > +--libdir=$(libdir /usr/lib) \
> > +--with-systemd-unit-dir"
> >
> > echo
> > echo "----------------------------------------------------------------"
> > diff --git a/configure.ac b/configure.ac
> > index cddad16..60c6537 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -135,6 +135,27 @@ AC_CHECK_FUNCS([ \
> > secure_getenv\
> > ])
> >
> > +PKG_PROG_PKG_CONFIG
> > +AC_ARG_WITH([systemd-unit-dir],
> > + AS_HELP_STRING([--with-systemd-unit-dir[=DIR]],
> > + [Directory for systemd service files]),
> > + [],
> > + [with_systemd_unit_dir=yes])
> > +
> > +if test "x$with_systemd_unit_dir" = "xyes"; then
> > + def_systemd_unit_dir=$($PKG_CONFIG
> --variable=systemdsystemunitdir systemd)
> > + if test "x$def_systemd_unit_dir" = "x"; then
> > + AC_MSG_ERROR([systemd support requested but pkg-config
> unable to query systemd package])
> > + with_systemd_unit_dir=no
> > + else
> > + with_systemd_unit_dir="$def_systemd_unit_dir"
> > + fi
> > +fi
> > +
> > +AS_IF([test "x$with_systemd_unit_dir" != "xno"],
> > + [AC_SUBST([SYSTEMD_UNIT_DIR], [$with_systemd_unit_dir])])
> > +AM_CONDITIONAL([ENABLE_SYSTEMD_UNIT_DIR], [test
> "x$with_systemd_unit_dir" != "xno"])
> > +
> > my_CFLAGS="\
> > -Wall \
> > -Wchar-subscripts \
> > @@ -172,6 +193,7 @@ AC_MSG_RESULT([
> > sysconfdir: ${sysconfdir}
> > libdir: ${libdir}
> > includedir: ${includedir}
> > + systemd-unit-dir: ${SYSTEMD_UNIT_DIR}
> >
> > compiler: ${CC}
> > cflags: ${CFLAGS}
> > diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> > index ae3d894..b3053d0 100644
> > --- a/ndctl/Makefile.am
> > +++ b/ndctl/Makefile.am
> > @@ -47,3 +47,7 @@ monitor_config_file = monitor.conf
> > monitor_configdir = /etc/ndctl/
> > monitor_config_DATA = $(monitor_config_file)
> > EXTRA_DIST += $(monitor_config_file)
> > +
> > +if ENABLE_SYSTEMD_UNIT_DIR
> > +systemd_unitDATA = ndctl-monitor.service
> > +endif
>
> I'm not familiar with autoconf syntax, but I think the 'systemd_unitDATA'
> may be wrong... *_DATA is correct, isn't it?
Thank you for your comments.
Yes, this is a bug.
I will make a new version and fix it.
> And, your autoconf code is a little different from the example of autoconf
> guide for systemd [1]. It might be a good idea to use the example code simply
> because it makes easy to find the systemd code for someone who try to
> add systemd service in the future.
>
> [1] https://www.freedesktop.org/software/systemd/man/daemon.html
>
> > diff --git a/ndctl/ndctl-monitor.service b/ndctl/ndctl-monitor.service
> > new file mode 100644
> > index 0000000..44f9326
> > --- /dev/null
> > +++ b/ndctl/ndctl-monitor.service
> > @@ -0,0 +1,7 @@
> > +[Unit]
> > +Description=Ndctl Monitor Daemon
> > +
> > +[Service]
> > +Type=forking
> > +ExecStart=/usr/bin/ndctl monitor --daemon
> > +ExecStop=/bin/kill ${MAINPID}
> >
>
> How about the following change?
>
I did try to use the autoconf systemd example code, but I found the indent of generated code in configure was difficult to read.
Therefore, I didn’t use "AS_IF" macro.
Moreover, the values "auto" and "yes" have the same meaning in this case, I deleted "auto" and make the code easy to read.
Also, imitating the other items in configure.ac like "ENABLE_BASH_COMPLETION", I changed the conditional "HAVE_SYSTEMD" to "ENABLE_SYSTEMD_UNIT_DIR".
> ---
> autogen.sh | 3 ++-
> configure.ac | 15 +++++++++++++++
> ndctl/Makefile.am | 4 ++++
> ndctl/ndctl-monitor.service | 7 +++++++
> 4 files changed, 28 insertions(+), 1 deletion(-)
> create mode 100644 ndctl/ndctl-monitor.service
>
> diff --git a/autogen.sh b/autogen.sh
> index a23cf53..bc58002 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -17,7 +17,8 @@ libdir() {
>
> args="--prefix=/usr \
> --sysconfdir=/etc \
> ---libdir=$(libdir /usr/lib)"
> +--libdir=$(libdir /usr/lib) \
> +--with-systemdsystemunitdir"
>
> echo
> echo "----------------------------------------------------------------"
> diff --git a/configure.ac b/configure.ac
> index 3eaac32..6fe054f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -122,6 +122,20 @@ AC_CHECK_FUNCS([ \
> secure_getenv\
> ])
>
> +AC_ARG_WITH([systemdsystemunitdir],
> + [AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for
> systemd service files])],,
> + [with_systemdsystemunitdir=auto])
> +AS_IF([test "x$with_systemdsystemunitdir" = "xyes" -o
> "x$with_systemdsystemunitdir" = "xauto"],
> + [def_systemdsystemunitdir=$($PKG_CONFIG
> --variable=systemdsystemunitdir systemd)
> + AS_IF([test "x$def_systemdsystemunitdir" = "x"],
> + [AS_IF([test "x$with_systemdsystemunitdir" = "xyes"],
> + [AC_MSG_ERROR([systemd support requested but
> pkg-config unable to query systemd package])])
> + with_systemdsystemunitdir=no],
> +
> [with_systemdsystemunitdir="$def_systemdsystemunitdir"])])
> +AS_IF([test "x$with_systemdsystemunitdir" != "xno"],
> + [AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])])
> +AM_CONDITIONAL([HAVE_SYSTEMD], [test "x$with_systemdsystemunitdir" !=
> "xno"])
> +
> my_CFLAGS="\
> -Wall \
> -Wchar-subscripts \
> @@ -159,6 +173,7 @@ AC_MSG_RESULT([
> sysconfdir: ${sysconfdir}
> libdir: ${libdir}
> includedir: ${includedir}
> + systemdunitdir: ${systemdsystemunitdir}
>
> compiler: ${CC}
> cflags: ${CFLAGS}
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index ae3d894..6ee4726 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -47,3 +47,7 @@ monitor_config_file = monitor.conf
> monitor_configdir = /etc/ndctl/
> monitor_config_DATA = $(monitor_config_file)
> EXTRA_DIST += $(monitor_config_file)
> +
> +if HAVE_SYSTEMD
> +systemdsystemunit_DATA = ndctl-monitor.service
> +endif
> diff --git a/ndctl/ndctl-monitor.service b/ndctl/ndctl-monitor.service
> new file mode 100644
> index 0000000..44f9326
> --- /dev/null
> +++ b/ndctl/ndctl-monitor.service
> @@ -0,0 +1,7 @@
> +[Unit]
> +Description=Ndctl Monitor Daemon
> +
> +[Service]
> +Type=forking
> +ExecStart=/usr/bin/ndctl monitor --daemon
> +ExecStop=/bin/kill ${MAINPID}
> --
> 1.8.3.1
>
> - Masa
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type
2018-05-04 3:25 ` Dan Williams
@ 2018-05-07 1:50 ` Qi, Fuli
0 siblings, 0 replies; 9+ messages in thread
From: Qi, Fuli @ 2018-05-07 1:50 UTC (permalink / raw)
To: 'Dan Williams'; +Cc: linux-nvdimm
> -----Original Message-----
> From: Dan Williams [mailto:dan.j.williams@intel.com]
> Sent: Friday, May 4, 2018 12:26 PM
> To: Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>
> Cc: linux-nvdimm <linux-nvdimm@lists.01.org>
> Subject: Re: [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type
>
> On Thu, Apr 26, 2018 at 4:30 AM, QI Fuli <qi.fuli@jp.fujitsu.com> wrote:
> > This patch borrows the OPTION_FILENAME from git to ndctl to make sure
> > filename is correct. Some related refactoring is also included:
> > - adds parse_options_prefix() interface
> > - moves is_absolute from util/help.c to util/util.c
> > - adds a new file util/abspath.c
> >
> > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
> >
> [..]
> > diff --git a/util/abspath.c b/util/abspath.c new file mode 100644
> > index 0000000..fdf4090
> > --- /dev/null
> > +++ b/util/abspath.c
> > @@ -0,0 +1,28 @@
> > +/* originally copied from git */
>
> Lets say the source file name that it came from in and get and add the license.
>
> /* SPDX-License-Identifier: GPL-2.0 */
> /* originally copied from git/abspath.c */
>
> Otherwise, this patch looks good to me.
>
Thank you for your comment.
I will make a new version and add the license.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-05-07 1:51 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-26 11:30 [PATCH v5 0/4] ndctl, monitor: add ndctl monitor daemon QI Fuli
2018-04-26 11:30 ` [PATCH v5 1/4] ndctl, util: add OPTION_FILENAME to parse_opt_type QI Fuli
2018-05-04 3:25 ` Dan Williams
2018-05-07 1:50 ` Qi, Fuli
2018-04-26 11:30 ` [PATCH v5 2/4] ndctl, monitor: add ndctl monitor daemon QI Fuli
2018-04-26 11:30 ` [PATCH v5 3/4] ndctl, monitor: add default configuration file QI Fuli
2018-04-26 11:30 ` [PATCH v5 4/4] ndctl, monitor: add the unit file of systemd for ndctl-monitor service QI Fuli
2018-05-01 15:07 ` Masayoshi Mizuma
2018-05-07 1:47 ` Qi, Fuli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).