Re: [RFC PATCH v4] ndctl: monitor: add ndctl monitor daemon

["Qi, Fuli" <qi.fuli@jp.fujitsu.com>]

>> diff --git a/ndctl/ndctl-monitor.service b/ndctl/ndctl-monitor.service
>> new file mode 100644
>> index 0000000..2f1417b
>> --- /dev/null
>> +++ b/ndctl/ndctl-monitor.service
>> @@ -0,0 +1,7 @@
>> +[Unit]
>> +Description=Ndctl Monitor Daemon
>> +
>> +[Service]
>> +Type=forking
>> +ExecStart=/usr/bin/ndctl monitor -f
>> +ExecStop=/usr/bin/kill ${MAINPID}
>> diff --git a/ndctl/ndctl-monitor@.service b/ndctl/ndctl-monitor@.service
>> new file mode 100644
>> index 0000000..91c85d9
>> --- /dev/null
>> +++ b/ndctl/ndctl-monitor@.service
>> @@ -0,0 +1,9 @@
>> +[Unit]
>> +Description=Ndctl Monitor Daemon
>> +
>> +[Service]
>> +Type=forking
>> +EnvironmentFile=/etc/sysconfig/ndctl/%i
>> +ExecStart=/usr/bin/ndctl monitor $OPTIONS
>> +ExecStop=/usr/bin/kill ${MAINPID}
>> +ExecStopPost=/usr/bin/rm /etc/sysconfig/ndctl/%i
>> diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
>> index d3c6db1..8938621 100644
>> --- a/ndctl/ndctl.c
>> +++ b/ndctl/ndctl.c
>> @@ -86,6 +86,7 @@ static struct cmd_struct commands[] = {
>>          { "inject-error", cmd_inject_error },
>>          { "update-firmware", cmd_update_firmware },
>>          { "inject-smart", cmd_inject_smart },
>> +       { "monitor", cmd_monitor },
>>          { "list", cmd_list },
>>          { "help", cmd_help },
>>          #ifdef ENABLE_TEST
>> diff --git a/util/filter.c b/util/filter.c
>> index b0b7fdf..fba2197 100644
>> --- a/util/filter.c
>> +++ b/util/filter.c
>> @@ -315,7 +315,7 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
>>                                  || !util_bus_filter_by_namespace(bus, param->namespace))
>>                          continue;
>>
>> -               if (!fctx->filter_bus(bus, fctx))
>> +               if (fctx->filter_bus && !fctx->filter_bus(bus, fctx))
>>                          continue;
>>
>>                  ndctl_dimm_foreach(bus, dimm) {
>> @@ -345,7 +345,8 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
>>                          if (type && ndctl_region_get_type(region) != type)
>>                                  continue;
>>
>> -                       if (!fctx->filter_region(region, fctx))
>> +                       if (fctx->filter_region &&
>> +                                       !fctx->filter_region(region, fctx))
> No, I don't agree with these change. The filter_bus() and
> filter_region() callbacks are mandatory because, like list, you need
> to be prepared to handle dimms underneath the bus, and namespaces
> underneath a region.

The filter_bus() and filter_region() callbacks are mandatory in ndctl list,
but not in ndctl monitor.
Current design only monitors the events coming from DIMMs, does not monitor
events coming from buses yet. When monitoring the events coming from DIMMs,
we just need to put the DIMMs which match filters into a linklist using
filter_dimm(). No additional actions will be taken on buses. It is the 
similar
case as in ndctl list, when the command does not contain [--buses] option,
filter_bus() will always return true.


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