| Message ID | 20180705112304.2656-1-qi.fuli@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Qi, I tried this test after I applied your latest monitor patches [1]. I found some points. Could you check the following? [1] https://lists.01.org/pipermail/linux-nvdimm/2018-June/016562.html On 07/05/2018 07:23 AM, QI Fuli wrote: > Add a new unit test to test the follow options of the monitor command. > --dimm > --bus > --region > --namespace > --logfile > --config-file > > Based-on-patch-by: Yasunori Goto <y-goto@jp.fujitsu.com> > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > --- > test/Makefile.am | 3 +- > test/monitor.sh | 115 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 117 insertions(+), 1 deletion(-) > create mode 100755 test/monitor.sh > > diff --git a/test/Makefile.am b/test/Makefile.am > index cd451e9..8c76462 100644 > --- a/test/Makefile.am > +++ b/test/Makefile.am > @@ -21,7 +21,8 @@ TESTS =\ > btt-pad-compat.sh \ > firmware-update.sh \ > ack-shutdown-count-set \ > - rescan-partitions.sh > + rescan-partitions.sh \ > + monitor.sh > > check_PROGRAMS =\ > libndctl \ > diff --git a/test/monitor.sh b/test/monitor.sh > new file mode 100755 > index 0000000..4f113c8 > --- /dev/null > +++ b/test/monitor.sh > @@ -0,0 +1,115 @@ > +#!/bin/bash -Ex > + > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright(c) 2018, FUJITSU LIMITED. All rights reserved. > + > +rc=77 > +logfile="" > +conf_file="" > +monitor_pid=65536 > +FILTER_DIMM="nmem1" > +FILTER_BUS="ndbus1" Is this ndbus0? because the dimms under nfit_test.1 are not supported smart. # ./test/smart-notify nfit_test.1 nmem5: no smart command support nmem4: no smart command support libdaxctl: daxctl_unref: context 0x20d41e0 released libndctl: ndctl_unref: context 0x20d4310 released # > +FILTER_BUS_MAP_DIMM="nmem0:nmem1:nmem2:nmem3" How about getting the dimms by ndctl list command instead of hard-cording? > +FILTER_REGION="region1" > +FILTER_REGION_MAP_DIMM="nmem0:nmem1" Is this "nmem0:nmem1:nmem2:nmem3"? Or why don't you getting the dimms by ndctl list command instead of hard-cording? # ./ndctl/ndctl list -r region1 -D [ { "dev":"nmem1", "id":"cdab-0a-07e0-feffffff", "handle":1, "phys_id":1 }, { "dev":"nmem3", "id":"cdab-0a-07e0-fefeffff", "handle":257, "phys_id":3 }, { "dev":"nmem0", "id":"cdab-0a-07e0-ffffffff", "handle":0, "phys_id":0 }, { "dev":"nmem2", "id":"cdab-0a-07e0-fffeffff", "handle":256, "phys_id":2 } ] # > +FILTER_NAMESPACE="namespace1.0" > +FILTER_NAMESPACE_MAP_DIMM="nmem0:nmem1" Same as above. > +CONF_FILE_SET_DIMM="nmem1:nmem3" > + > +. ./common > + > +trap 'err $LINENO' ERR > + > +check_min_kver "4.15" || do_skip "kernel $KVER may not support monitor service" > + > +start_monitor() > +{ > + logfile=$(mktemp) > + $NDCTL monitor -l $logfile $1 & > + monitor_pid=$! > + truncate --size 0 $logfile #remove startup log > +} > + > +call_notify() > +{ > + ./smart-notify $NFIT_TEST_BUS0 I think monitor daemon needs some time for writing out the log. So why don't we have some sleep time? Like as: sync; sleep 3 > +} > + > +check_result() > +{ > + json=$(cat $logfile) > + dimms=$(jq ."dimm"."dev" <<<$json | sort | uniq | sed -e ':loop; N; $!b loop; s/\n/:/g' | sed 's/\"//g') > + [[ $1 == $dimms ]] > +} > + > +stop_monitor() > +{ > + kill $monitor_pid > + rm $logfile > +} > + > +create_conf_file() > +{ > + conf_file=$(mktemp) > + echo 'dimm=nmem1 nmem3' > $conf_file > + cat $conf_file > +} > + > +test_filter_dimm() > +{ > + start_monitor "-d $FILTER_DIMM" > + call_notify > + check_result "$FILTER_DIMM" > + stop_monitor > +} > + > +test_filter_bus() > +{ > + start_monitor "-b $FILTER_BUS" > + call_notify > + check_result $FILTER_BUS_MAP_DIMM > + stop_monitor > +} > + > +test_filter_region() > +{ > + start_monitor "-r $FILTER_REGION" > + call_notify > + check_result $FILTER_REGION_MAP_DIMM > + stop_monitor > +} > + > +test_filter_namespace() > +{ > + $NDCTL create-namespace -r region1 -n $FILTER_NAMESPACE > + start_monitor "-n $FILTER_NAMESPACE" > + call_notify > + check_result $FILTER_NAMESPACE_MAP_DIMM > + stop_monitor > + $NDCTL destroy-namespace $FILTER_NAMESPACE -f > +} > + > +test_conf_file() > +{ > + create_conf_file > + start_monitor "-c $conf_file" > + call_notify > + check_result $CONF_FILE_SET_DIMM > + stop_monitor > + rm $conf_file > +} > + > +do_tests() > +{ > + test_filter_dimm > + test_filter_bus > + test_filter_region > + test_filter_namespace > + test_conf_file > +} > + > +modprobe nfit_test > +rc=1 Why don't you add initialize the region before testing? like as follows. init() { $NDCTL disable-region -b $NFIT_TEST_BUS0 all $NDCTL zero-labels -b $NFIT_TEST_BUS0 all $NDCTL enable-region -b $NFIT_TEST_BUS0 all } init Thanks, Masa > +do_tests > +_cleanup > +exit 0 >
On Thu, Jul 5, 2018 at 2:06 PM, Masayoshi Mizuma <msys.mizuma@gmail.com> wrote: > Hi Qi, > > I tried this test after I applied your latest monitor patches [1]. > I found some points. Could you check the following? > > [1] https://lists.01.org/pipermail/linux-nvdimm/2018-June/016562.html > > On 07/05/2018 07:23 AM, QI Fuli wrote: [..] > Is this "nmem0:nmem1:nmem2:nmem3"? > Or why don't you getting the dimms by ndctl list command instead of hard-cording? Yes, this is needed. The test fails for me because I have an e820 defined bus and a typical ACPI NFIT bus along with the nfit_test buses on my test system.
> -----Original Message----- > From: Dan Williams [mailto:dan.j.williams@intel.com] > Sent: Friday, July 6, 2018 6:26 AM > To: Masayoshi Mizuma <msys.mizuma@gmail.com> > Cc: Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>; linux-nvdimm > <linux-nvdimm@lists.01.org> > Subject: Re: [ndctl PATCH] ndctl, test: add a new unit test for monitor > > On Thu, Jul 5, 2018 at 2:06 PM, Masayoshi Mizuma <msys.mizuma@gmail.com> wrote: > > Hi Qi, > > > > I tried this test after I applied your latest monitor patches [1]. > > I found some points. Could you check the following? > > > > [1] https://lists.01.org/pipermail/linux-nvdimm/2018-June/016562.html > > > > On 07/05/2018 07:23 AM, QI Fuli wrote: > [..] > > Is this "nmem0:nmem1:nmem2:nmem3"? > > Or why don't you getting the dimms by ndctl list command instead of hard-cording? > > Yes, this is needed. The test fails for me because I have an e820 defined bus and > a typical ACPI NFIT bus along with the nfit_test buses on my test system. Masa, Dan, Thank you very much for your comments. I will fix it in next version. QI
diff --git a/test/Makefile.am b/test/Makefile.am index cd451e9..8c76462 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -21,7 +21,8 @@ TESTS =\ btt-pad-compat.sh \ firmware-update.sh \ ack-shutdown-count-set \ - rescan-partitions.sh + rescan-partitions.sh \ + monitor.sh check_PROGRAMS =\ libndctl \ diff --git a/test/monitor.sh b/test/monitor.sh new file mode 100755 index 0000000..4f113c8 --- /dev/null +++ b/test/monitor.sh @@ -0,0 +1,115 @@ +#!/bin/bash -Ex + +# SPDX-License-Identifier: GPL-2.0 +# Copyright(c) 2018, FUJITSU LIMITED. All rights reserved. + +rc=77 +logfile="" +conf_file="" +monitor_pid=65536 +FILTER_DIMM="nmem1" +FILTER_BUS="ndbus1" +FILTER_BUS_MAP_DIMM="nmem0:nmem1:nmem2:nmem3" +FILTER_REGION="region1" +FILTER_REGION_MAP_DIMM="nmem0:nmem1" +FILTER_NAMESPACE="namespace1.0" +FILTER_NAMESPACE_MAP_DIMM="nmem0:nmem1" +CONF_FILE_SET_DIMM="nmem1:nmem3" + +. ./common + +trap 'err $LINENO' ERR + +check_min_kver "4.15" || do_skip "kernel $KVER may not support monitor service" + +start_monitor() +{ + logfile=$(mktemp) + $NDCTL monitor -l $logfile $1 & + monitor_pid=$! + truncate --size 0 $logfile #remove startup log +} + +call_notify() +{ + ./smart-notify $NFIT_TEST_BUS0 +} + +check_result() +{ + json=$(cat $logfile) + dimms=$(jq ."dimm"."dev" <<<$json | sort | uniq | sed -e ':loop; N; $!b loop; s/\n/:/g' | sed 's/\"//g') + [[ $1 == $dimms ]] +} + +stop_monitor() +{ + kill $monitor_pid + rm $logfile +} + +create_conf_file() +{ + conf_file=$(mktemp) + echo 'dimm=nmem1 nmem3' > $conf_file + cat $conf_file +} + +test_filter_dimm() +{ + start_monitor "-d $FILTER_DIMM" + call_notify + check_result "$FILTER_DIMM" + stop_monitor +} + +test_filter_bus() +{ + start_monitor "-b $FILTER_BUS" + call_notify + check_result $FILTER_BUS_MAP_DIMM + stop_monitor +} + +test_filter_region() +{ + start_monitor "-r $FILTER_REGION" + call_notify + check_result $FILTER_REGION_MAP_DIMM + stop_monitor +} + +test_filter_namespace() +{ + $NDCTL create-namespace -r region1 -n $FILTER_NAMESPACE + start_monitor "-n $FILTER_NAMESPACE" + call_notify + check_result $FILTER_NAMESPACE_MAP_DIMM + stop_monitor + $NDCTL destroy-namespace $FILTER_NAMESPACE -f +} + +test_conf_file() +{ + create_conf_file + start_monitor "-c $conf_file" + call_notify + check_result $CONF_FILE_SET_DIMM + stop_monitor + rm $conf_file +} + +do_tests() +{ + test_filter_dimm + test_filter_bus + test_filter_region + test_filter_namespace + test_conf_file +} + +modprobe nfit_test +rc=1 +do_tests +_cleanup +exit 0
Add a new unit test to test the follow options of the monitor command. --dimm --bus --region --namespace --logfile --config-file Based-on-patch-by: Yasunori Goto <y-goto@jp.fujitsu.com> Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> --- test/Makefile.am | 3 +- test/monitor.sh | 115 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100755 test/monitor.sh