[RFC patch 1/4]ndctl: nvdimmd: notify/monitor the feathers of over threshold event

[RFC patch 1/4]ndctl: nvdimmd: notify/monitor the feathers of over threshold event

[Qi, Fuli]

Libnvdimmd.c provides functions which are used by nvdimm daemon, and
currently it just supports for logging. 
Libnvdimmd.h is a head file of libnvdimmd.c.
Since I do not use automake, I defined gentenv.h to compile instead of it temporarily.
So I suppose more good way is necessary.

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

---
nvdimmd/Makefile     |   7 +++
nvdimmd/getenv.h     |   1 +
nvdimmd/libnvdimmd.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++
nvdimmd/libnvdimmd.h |  31 +++++++++++
4 files changed, 180 insertions(+)
diff --git a/nvdimmd/Makefile b/nvdimmd/Makefile
new file mode 100644
index 0000000..a20a747
--- /dev/null
+++ b/nvdimmd/Makefile
@@ -0,0 +1,7 @@
+CC		= gcc
+IDIR		= -I../ -I../ndctl
+
+libnvdimmd.o:	libnvdimmd.c
+			$(CC) -o libnvdimmd.o $(IDIR) -c libnvdimmd.c
+clean:
+			rm -rf *.o
diff --git a/nvdimmd/getenv.h b/nvdimmd/getenv.h
new file mode 100644
index 0000000..45747b4
--- /dev/null
+++ b/nvdimmd/getenv.h
@@ -0,0 +1 @@
+#define HAVE_SECURE_GETENV 1
diff --git a/nvdimmd/libnvdimmd.c b/nvdimmd/libnvdimmd.c
new file mode 100644
index 0000000..89ff701
--- /dev/null
+++ b/nvdimmd/libnvdimmd.c
@@ -0,0 +1,141 @@
+/*
+ * Copyright (c) 2017, 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.
+ */
+
+/*
+ * This program is used to provide nvdimm daemon necessary functions.
+ */
+
+#include "getenv.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <syslog.h>
+#include <ndctl/libndctl.h>
+#include <ndctl/lib/libndctl-private.h>
+#include "libnvdimmd.h"
+#define BUF_SIZE 4096
+
+static int get_health_info(threshold_dimm *t_dimm)
+{
+	struct ndctl_cmd *cmd;
+	int rc;
+	unsigned int flags;
+	char *msg = "nvdimm warning: dimm over threshold notify";
+	char *err_msg;
+
+	cmd = ndctl_dimm_cmd_new_smart(t_dimm->dimm);
+	if (!cmd) {
+		err_msg = "failed to prepare command to get health info";
+		syslog(LOG_WARNING, "%s [%s], %s\n", msg, t_dimm->devname, err_msg);
+		return -1;
+	}
+
+	rc = ndctl_cmd_submit(cmd);
+	if (rc || ndctl_cmd_get_firmware_status(cmd)) {
+		err_msg = "failed to submit command to get health info";
+		syslog(LOG_WARNING, "%s [%s], %s\n", msg, t_dimm->devname, err_msg);
+		ndctl_cmd_unref(cmd);
+		return -1;
+	}
+
+	flags = ndctl_cmd_smart_get_flags(cmd);
+	if (flags & ND_SMART_HEALTH_VALID) {
+		unsigned int health = ndctl_cmd_smart_get_health(cmd);
+		if (health & ND_SMART_FATAL_HEALTH)
+			t_dimm->health_state = "fatal";
+		else if (health & ND_SMART_CRITICAL_HEALTH)
+			t_dimm->health_state = "critical";
+		else if (health & ND_SMART_NON_CRITICAL_HEALTH)
+			t_dimm->health_state = "non-critical";
+		else
+			t_dimm->health_state = "ok";
+	} else {
+		t_dimm->health_state = "failed to get data";
+	}
+	if (flags & ND_SMART_SPARES_VALID)
+		t_dimm->spares = ndctl_cmd_smart_get_spares(cmd);
+	else
+		t_dimm->spares = -1;
+
+	ndctl_cmd_unref(cmd);
+	return 0;
+}
+
+int log_notify(threshold_dimm *t_dimm, int count_dimm, fd_set fds, int count_select)
+{
+	int log_notify = 0;
+	char *msg = "nvdimm warning: dimm over threshold notify";
+
+	for (int i = 0; i < count_dimm; i++) {
+		if (log_notify >= count_select)
+			break;
+
+		if (!FD_ISSET(t_dimm[i].health_eventfd, &fds))
+			continue;
+
+		log_notify++;
+		if (get_health_info(&t_dimm[i]))
+			continue;
+
+		if (t_dimm[i].spares == -1) {
+			syslog(LOG_WARNING,
+				"%s [%s]\nhealth_state: %s\n"
+				"spares_percentage: failed to get data\n",
+				msg, t_dimm[i].devname, t_dimm[i].health_state);
+			continue;
+		}
+		syslog(LOG_WARNING,"%s [%s]\nhealth_state: %s\nspares_percentage: %d\n",
+			msg, t_dimm[i].devname, t_dimm[i].health_state, t_dimm[i].spares);
+	}
+	return log_notify;
+}
+
+static struct ndctl_dimm *is_supported_threshold_notify(struct ndctl_dimm *dimm)
+{
+	if (ndctl_dimm_is_cmd_supported(dimm, ND_CMD_SMART_THRESHOLD))
+		return dimm;
+	return NULL;
+}
+
+int
+get_threshold_dimm(struct ndctl_ctx *ctx, threshold_dimm *t_dimm, fd_set *fds, int *maxfd)
+{
+	struct ndctl_bus *bus;
+	struct ndctl_dimm *dimm;
+	char buf[BUF_SIZE];
+	int fd, count_dimm = 0;
+
+	ndctl_bus_foreach(ctx, bus) {
+		ndctl_dimm_foreach(bus, dimm) {
+
+			if (!is_supported_threshold_notify(dimm))
+				continue;
+			t_dimm[count_dimm].dimm = dimm;
+			t_dimm[count_dimm].devname = ndctl_dimm_get_devname(dimm);
+			fd = ndctl_dimm_get_health_eventfd(dimm);
+			read(fd, buf, sizeof(buf));
+			t_dimm[count_dimm].health_eventfd = fd;
+
+			if (fds)
+				FD_SET(fd, fds);
+			if (maxfd) {
+				if (*maxfd < fd)
+					*maxfd = fd;
+			}
+			count_dimm++;
+		}
+	}
+	return count_dimm;
+}
diff --git a/nvdimmd/libnvdimmd.h b/nvdimmd/libnvdimmd.h
new file mode 100644
index 0000000..78e5871
--- /dev/null
+++ b/nvdimmd/libnvdimmd.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2017, 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.
+ */
+
+#ifndef _LIBNVDIMMD_H_
+#define _LIBBVDIMMD_H_
+
+#include <stdio.h>
+#define NUM_MAX_DIMM 1024
+
+typedef struct {
+	struct ndctl_dimm *dimm;
+	const char *devname;
+	int health_eventfd;
+	int spares;
+	char *health_state;
+} threshold_dimm;
+
+int log_notify(threshold_dimm *t_dimm, int count_dimm, fd_set fds, int count_select);
+int get_threshold_dimm(struct ndctl_ctx *ctx, threshold_dimm *t_dimm, fd_set *fds, int *maxfd);
+
+#endif

--
QI Fuli <qi.fuli@jp.fujitsu.com>


_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC patch 1/4]ndctl: nvdimmd: notify/monitor the feathers of over threshold event

[Dan Williams]

On Thu, Aug 31, 2017 at 6:45 PM, Qi, Fuli <qi.fuli@jp.fujitsu.com> wrote:
> Libnvdimmd.c provides functions which are used by nvdimm daemon, and
> currently it just supports for logging.
> Libnvdimmd.h is a head file of libnvdimmd.c.
> Since I do not use automake, I defined gentenv.h to compile instead of it temporarily.
> So I suppose more good way is necessary.
>
> Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
>
> ---
> nvdimmd/Makefile     |   7 +++
> nvdimmd/getenv.h     |   1 +
> nvdimmd/libnvdimmd.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++
> nvdimmd/libnvdimmd.h |  31 +++++++++++
> 4 files changed, 180 insertions(+)
> diff --git a/nvdimmd/Makefile b/nvdimmd/Makefile
> new file mode 100644
> index 0000000..a20a747
> --- /dev/null
> +++ b/nvdimmd/Makefile
> @@ -0,0 +1,7 @@
> +CC             = gcc
> +IDIR           = -I../ -I../ndctl
> +
> +libnvdimmd.o:  libnvdimmd.c
> +                       $(CC) -o libnvdimmd.o $(IDIR) -c libnvdimmd.c
> +clean:
> +                       rm -rf *.o
> diff --git a/nvdimmd/getenv.h b/nvdimmd/getenv.h
> new file mode 100644
> index 0000000..45747b4
> --- /dev/null
> +++ b/nvdimmd/getenv.h
> @@ -0,0 +1 @@
> +#define HAVE_SECURE_GETENV 1
> diff --git a/nvdimmd/libnvdimmd.c b/nvdimmd/libnvdimmd.c
> new file mode 100644
> index 0000000..89ff701
> --- /dev/null
> +++ b/nvdimmd/libnvdimmd.c

This doesn't need to be a separate library, just roll it into nvdimmd directly.

> @@ -0,0 +1,141 @@
> +/*
> + * Copyright (c) 2017, 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.
> + */
> +
> +/*
> + * This program is used to provide nvdimm daemon necessary functions.
> + */
> +
> +#include "getenv.h"
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <syslog.h>
> +#include <ndctl/libndctl.h>
> +#include <ndctl/lib/libndctl-private.h>
> +#include "libnvdimmd.h"
> +#define BUF_SIZE 4096
> +
> +static int get_health_info(threshold_dimm *t_dimm)
> +{
> +       struct ndctl_cmd *cmd;
> +       int rc;
> +       unsigned int flags;
> +       char *msg = "nvdimm warning: dimm over threshold notify";
> +       char *err_msg;
> +
> +       cmd = ndctl_dimm_cmd_new_smart(t_dimm->dimm);
> +       if (!cmd) {
> +               err_msg = "failed to prepare command to get health info";
> +               syslog(LOG_WARNING, "%s [%s], %s\n", msg, t_dimm->devname, err_msg);
> +               return -1;

Let's use the same logging scheme as libndctl where we have 'struct
log_ctx' and the the log function can be set to a custom routine. That
way we can redirect log messages to syslog or structured json output
simply by changing log_ctx.log_fn.

> +       }
> +
> +       rc = ndctl_cmd_submit(cmd);
> +       if (rc || ndctl_cmd_get_firmware_status(cmd)) {
> +               err_msg = "failed to submit command to get health info";
> +               syslog(LOG_WARNING, "%s [%s], %s\n", msg, t_dimm->devname, err_msg);
> +               ndctl_cmd_unref(cmd);
> +               return -1;
> +       }
> +
> +       flags = ndctl_cmd_smart_get_flags(cmd);
> +       if (flags & ND_SMART_HEALTH_VALID) {
> +               unsigned int health = ndctl_cmd_smart_get_health(cmd);
> +               if (health & ND_SMART_FATAL_HEALTH)
> +                       t_dimm->health_state = "fatal";
> +               else if (health & ND_SMART_CRITICAL_HEALTH)
> +                       t_dimm->health_state = "critical";
> +               else if (health & ND_SMART_NON_CRITICAL_HEALTH)
> +                       t_dimm->health_state = "non-critical";
> +               else
> +                       t_dimm->health_state = "ok";
> +       } else {
> +               t_dimm->health_state = "failed to get data";
> +       }
> +       if (flags & ND_SMART_SPARES_VALID)
> +               t_dimm->spares = ndctl_cmd_smart_get_spares(cmd);
> +       else
> +               t_dimm->spares = -1;
> +
> +       ndctl_cmd_unref(cmd);
> +       return 0;
> +}
> +
> +int log_notify(threshold_dimm *t_dimm, int count_dimm, fd_set fds, int count_select)
> +{
> +       int log_notify = 0;
> +       char *msg = "nvdimm warning: dimm over threshold notify";
> +
> +       for (int i = 0; i < count_dimm; i++) {
> +               if (log_notify >= count_select)
> +                       break;
> +
> +               if (!FD_ISSET(t_dimm[i].health_eventfd, &fds))
> +                       continue;
> +
> +               log_notify++;
> +               if (get_health_info(&t_dimm[i]))
> +                       continue;
> +
> +               if (t_dimm[i].spares == -1) {
> +                       syslog(LOG_WARNING,
> +                               "%s [%s]\nhealth_state: %s\n"
> +                               "spares_percentage: failed to get data\n",
> +                               msg, t_dimm[i].devname, t_dimm[i].health_state);
> +                       continue;
> +               }
> +               syslog(LOG_WARNING,"%s [%s]\nhealth_state: %s\nspares_percentage: %d\n",
> +                       msg, t_dimm[i].devname, t_dimm[i].health_state, t_dimm[i].spares);
> +       }
> +       return log_notify;
> +}
> +
> +static struct ndctl_dimm *is_supported_threshold_notify(struct ndctl_dimm *dimm)
> +{
> +       if (ndctl_dimm_is_cmd_supported(dimm, ND_CMD_SMART_THRESHOLD))
> +               return dimm;
> +       return NULL;
> +}
> +
> +int
> +get_threshold_dimm(struct ndctl_ctx *ctx, threshold_dimm *t_dimm, fd_set *fds, int *maxfd)
> +{
> +       struct ndctl_bus *bus;
> +       struct ndctl_dimm *dimm;
> +       char buf[BUF_SIZE];
> +       int fd, count_dimm = 0;
> +
> +       ndctl_bus_foreach(ctx, bus) {
> +               ndctl_dimm_foreach(bus, dimm) {
> +
> +                       if (!is_supported_threshold_notify(dimm))
> +                               continue;
> +                       t_dimm[count_dimm].dimm = dimm;
> +                       t_dimm[count_dimm].devname = ndctl_dimm_get_devname(dimm);
> +                       fd = ndctl_dimm_get_health_eventfd(dimm);
> +                       read(fd, buf, sizeof(buf));
> +                       t_dimm[count_dimm].health_eventfd = fd;
> +
> +                       if (fds)
> +                               FD_SET(fd, fds);
> +                       if (maxfd) {
> +                               if (*maxfd < fd)
> +                                       *maxfd = fd;
> +                       }
> +                       count_dimm++;
> +               }
> +       }
> +       return count_dimm;
> +}
> diff --git a/nvdimmd/libnvdimmd.h b/nvdimmd/libnvdimmd.h
> new file mode 100644
> index 0000000..78e5871
> --- /dev/null
> +++ b/nvdimmd/libnvdimmd.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (c) 2017, 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.
> + */
> +
> +#ifndef _LIBNVDIMMD_H_
> +#define _LIBBVDIMMD_H_

Extra 'B', but no need to fix since we can delete this header and
merge this file with nvdimmd.c.

> +
> +#include <stdio.h>
> +#define NUM_MAX_DIMM 1024
> +
> +typedef struct {
> +       struct ndctl_dimm *dimm;
> +       const char *devname;
> +       int health_eventfd;
> +       int spares;
> +       char *health_state;
> +} threshold_dimm;

ndctl tries to follow Linux kernel coding style as much as possible,
so please type out the full struct name instead of using a typedef.
More background can be found here:

    https://www.kernel.org/doc/html/v4.10/process/coding-style.html#typedefs

> +int log_notify(threshold_dimm *t_dimm, int count_dimm, fd_set fds, int count_select);
> +int get_threshold_dimm(struct ndctl_ctx *ctx, threshold_dimm *t_dimm, fd_set *fds, int *maxfd);
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC patch 1/4]ndctl: nvdimmd: notify/monitor the feathers of over threshold event

[Dan Williams]

On Sat, Sep 23, 2017 at 10:17 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Thu, Aug 31, 2017 at 6:45 PM, Qi, Fuli <qi.fuli@jp.fujitsu.com> wrote:
>> Libnvdimmd.c provides functions which are used by nvdimm daemon, and
>> currently it just supports for logging.
>> Libnvdimmd.h is a head file of libnvdimmd.c.
>> Since I do not use automake, I defined gentenv.h to compile instead of it temporarily.
>> So I suppose more good way is necessary.
>>
>> Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
>>
>> ---
>> nvdimmd/Makefile     |   7 +++
>> nvdimmd/getenv.h     |   1 +
>> nvdimmd/libnvdimmd.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> nvdimmd/libnvdimmd.h |  31 +++++++++++
>> 4 files changed, 180 insertions(+)
>> diff --git a/nvdimmd/Makefile b/nvdimmd/Makefile
>> new file mode 100644
>> index 0000000..a20a747
>> --- /dev/null
>> +++ b/nvdimmd/Makefile
>> @@ -0,0 +1,7 @@
>> +CC             = gcc
>> +IDIR           = -I../ -I../ndctl
>> +
>> +libnvdimmd.o:  libnvdimmd.c
>> +                       $(CC) -o libnvdimmd.o $(IDIR) -c libnvdimmd.c
>> +clean:
>> +                       rm -rf *.o
>> diff --git a/nvdimmd/getenv.h b/nvdimmd/getenv.h
>> new file mode 100644
>> index 0000000..45747b4
>> --- /dev/null
>> +++ b/nvdimmd/getenv.h
>> @@ -0,0 +1 @@
>> +#define HAVE_SECURE_GETENV 1
>> diff --git a/nvdimmd/libnvdimmd.c b/nvdimmd/libnvdimmd.c
>> new file mode 100644
>> index 0000000..89ff701
>> --- /dev/null
>> +++ b/nvdimmd/libnvdimmd.c
>
> This doesn't need to be a separate library, just roll it into nvdimmd directly.
>
>> @@ -0,0 +1,141 @@
>> +/*
>> + * Copyright (c) 2017, 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.
>> + */
>> +
>> +/*
>> + * This program is used to provide nvdimm daemon necessary functions.
>> + */
>> +
>> +#include "getenv.h"
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include <string.h>
>> +#include <sys/stat.h>
>> +#include <syslog.h>
>> +#include <ndctl/libndctl.h>
>> +#include <ndctl/lib/libndctl-private.h>
>> +#include "libnvdimmd.h"
>> +#define BUF_SIZE 4096
>> +
>> +static int get_health_info(threshold_dimm *t_dimm)
>> +{
>> +       struct ndctl_cmd *cmd;
>> +       int rc;
>> +       unsigned int flags;
>> +       char *msg = "nvdimm warning: dimm over threshold notify";
>> +       char *err_msg;
>> +
>> +       cmd = ndctl_dimm_cmd_new_smart(t_dimm->dimm);
>> +       if (!cmd) {
>> +               err_msg = "failed to prepare command to get health info";
>> +               syslog(LOG_WARNING, "%s [%s], %s\n", msg, t_dimm->devname, err_msg);
>> +               return -1;
>
> Let's use the same logging scheme as libndctl where we have 'struct
> log_ctx' and the the log function can be set to a custom routine. That
> way we can redirect log messages to syslog or structured json output
> simply by changing log_ctx.log_fn.

Thinking further about this, using log_ctx is probably not sufficient
either. We'll also need metadata with each event message, see the
extra fields that the systemd journal tracks for inspiration:

    https://www.freedesktop.org/software/systemd/man/systemd.journal-fields.html
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC patch 1/4]ndctl: nvdimmd: notify/monitor the feathers of over threshold event

[Qi, Fuli]

>> Let's use the same logging scheme as libndctl where we have 'struct
>> log_ctx' and the the log function can be set to a custom routine. That
>> way we can redirect log messages to syslog or structured json output
>> simply by changing log_ctx.log_fn.
> Thinking further about this, using log_ctx is probably not sufficient
> either. We'll also need metadata with each event message, see the
> extra fields that the systemd journal tracks for inspiration:
>
>      https://www.freedesktop.org/software/systemd/man/systemd.journal-fields.html

When I was trying to replace syslog with libndctl logging scheme,
I found that libndctl logging scheme only outputs to console.
However there is no console in a nvdimm daemon. Users can't catch
event messages from console, we should make the outputs to files or
syslog instead.

The follow are what I want to do.
1) add a new function log_event() into libndctl logging scheme
As you mentioned, using log_ctx may not be sufficient, and in order to
make adding metadata with each event message conveniently in the future,
I am thinking about adding log_event() into libndctl logging scheme.
The log_event() selects destination (file or syslog) of output depends on
configuration, and outputs messages to the destination.

2) add LOG_NOTICE into log_priority
Since there are only LOG_INFO, LOG_ERROR and LOG_DEBUG in log_priority now,
I am also thinking about adding LOG_NOTICE into log_priority for event messages.
I think the LOG_INFO level is too low for the event messages of nvdimm status,
and its event is not up to LOG_ERROR level.

Here is the summary of log_event() that I can think of for now.

  #define notice(x, arg...) log_notice(&(x)->ctx, ## arg)
  #define log_notice(ctx, arg...) log_cond(ctx, LOG_NOTICE, ##arg)
  
  //make the notice_out() to save event messages to output file
  #define notice_out(x, arg...) log_notice_out(&(x)->ctx, ## arg)
  #define log_notice_out(ctx, arg...) log_event(ctx, LOG_NOTICE, __FILE__, __LINE__, __FUNCTION__, ## arg)
  
  void log_event(struct log_ctx *ctx, int priority, const char *file, int line,
		const char *fn, const char *format, ...)
	// check config items
	if(output_path){
		if(output_format = json)
			// output json format to output_path
		else
			// output text format to output_path
	} else
		printf();

If you agree with this idea, I will implement it asap.
Or if I had any misunderstanding, please kindly let me know.

QI Fuli


_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC patch 1/4]ndctl: nvdimmd: notify/monitor the feathers of over threshold event

[Dan Williams]

On Thu, Oct 12, 2017 at 7:23 PM, Qi, Fuli <qi.fuli@jp.fujitsu.com> wrote:
>>>
>>> Let's use the same logging scheme as libndctl where we have 'struct
>>> log_ctx' and the the log function can be set to a custom routine. That
>>> way we can redirect log messages to syslog or structured json output
>>> simply by changing log_ctx.log_fn.
>>
>> Thinking further about this, using log_ctx is probably not sufficient
>> either. We'll also need metadata with each event message, see the
>> extra fields that the systemd journal tracks for inspiration:
>>
>>      https://www.freedesktop.org/software/systemd/man/systemd.journal-fields.html
>
>
> When I was trying to replace syslog with libndctl logging scheme,
> I found that libndctl logging scheme only outputs to console.
> However there is no console in a nvdimm daemon. Users can't catch
> event messages from console, we should make the outputs to files or
> syslog instead.
>
> The follow are what I want to do.
> 1) add a new function log_event() into libndctl logging scheme
> As you mentioned, using log_ctx may not be sufficient, and in order to
> make adding metadata with each event message conveniently in the future,
> I am thinking about adding log_event() into libndctl logging scheme.
> The log_event() selects destination (file or syslog) of output depends on
> configuration, and outputs messages to the destination.
>
> 2) add LOG_NOTICE into log_priority
> Since there are only LOG_INFO, LOG_ERROR and LOG_DEBUG in log_priority now,
> I am also thinking about adding LOG_NOTICE into log_priority for event messages.
> I think the LOG_INFO level is too low for the event messages of nvdimm status,
> and its event is not up to LOG_ERROR level.
>
> Here is the summary of log_event() that I can think of for now.
>
>  #define notice(x, arg...) log_notice(&(x)->ctx, ## arg)
>  #define log_notice(ctx, arg...) log_cond(ctx, LOG_NOTICE, ##arg)
>   //make the notice_out() to save event messages to output file
>  #define notice_out(x, arg...) log_notice_out(&(x)->ctx, ## arg)
>  #define log_notice_out(ctx, arg...) log_event(ctx, LOG_NOTICE, __FILE__, __LINE__, __FUNCTION__, ## arg)
>   void log_event(struct log_ctx *ctx, int priority, const char *file, int line,
>                 const char *fn, const char *format, ...)
>         // check config items
>         if(output_path){
>                 if(output_format = json)
>                         // output json format to output_path
>                 else
>                         // output text format to output_path
>         } else
>                 printf();
>
> If you agree with this idea, I will implement it asap.
> Or if I had any misunderstanding, please kindly let me know.

Yes, I think this looks on the right track. My question is whether we
need a device name for the source of the log event rather than
function name and line number. Events can be associated with the bus,
dimms, regions, or namespace.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC patch 1/4]ndctl: nvdimmd: notify/monitor the feathers of over threshold event

[Qi, Fuli]

On 2017/10/18 16:22, Dan Williams wrote:
> On Thu, Oct 12, 2017 at 7:23 PM, Qi, Fuli <qi.fuli@jp.fujitsu.com> wrote:
>>>> Let's use the same logging scheme as libndctl where we have 'struct
>>>> log_ctx' and the the log function can be set to a custom routine. That
>>>> way we can redirect log messages to syslog or structured json output
>>>> simply by changing log_ctx.log_fn.
>>> Thinking further about this, using log_ctx is probably not sufficient
>>> either. We'll also need metadata with each event message, see the
>>> extra fields that the systemd journal tracks for inspiration:
>>>
>>>       https://www.freedesktop.org/software/systemd/man/systemd.journal-fields.html
>>
>> When I was trying to replace syslog with libndctl logging scheme,
>> I found that libndctl logging scheme only outputs to console.
>> However there is no console in a nvdimm daemon. Users can't catch
>> event messages from console, we should make the outputs to files or
>> syslog instead.
>>
>> The follow are what I want to do.
>> 1) add a new function log_event() into libndctl logging scheme
>> As you mentioned, using log_ctx may not be sufficient, and in order to
>> make adding metadata with each event message conveniently in the future,
>> I am thinking about adding log_event() into libndctl logging scheme.
>> The log_event() selects destination (file or syslog) of output depends on
>> configuration, and outputs messages to the destination.
>>
>> 2) add LOG_NOTICE into log_priority
>> Since there are only LOG_INFO, LOG_ERROR and LOG_DEBUG in log_priority now,
>> I am also thinking about adding LOG_NOTICE into log_priority for event messages.
>> I think the LOG_INFO level is too low for the event messages of nvdimm status,
>> and its event is not up to LOG_ERROR level.
>>
>> Here is the summary of log_event() that I can think of for now.
>>
>>   #define notice(x, arg...) log_notice(&(x)->ctx, ## arg)
>>   #define log_notice(ctx, arg...) log_cond(ctx, LOG_NOTICE, ##arg)
>>    //make the notice_out() to save event messages to output file
>>   #define notice_out(x, arg...) log_notice_out(&(x)->ctx, ## arg)
>>   #define log_notice_out(ctx, arg...) log_event(ctx, LOG_NOTICE, __FILE__, __LINE__, __FUNCTION__, ## arg)
>>    void log_event(struct log_ctx *ctx, int priority, const char *file, int line,
>>                  const char *fn, const char *format, ...)
>>          // check config items
>>          if(output_path){
>>                  if(output_format = json)
>>                          // output json format to output_path
>>                  else
>>                          // output text format to output_path
>>          } else
>>                  printf();
>>
>> If you agree with this idea, I will implement it asap.
>> Or if I had any misunderstanding, please kindly let me know.
> Yes, I think this looks on the right track. My question is whether we
> need a device name for the source of the log event rather than
> function name and line number. Events can be associated with the bus,
> dimms, regions, or namespace.
>
Sorry, I just referred to the original do_log(). As your mentioned,
I will change the parameters with a device name and think more
about the event's information what should be added.

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm