From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgwkm03.jp.fujitsu.com (mgwkm03.jp.fujitsu.com [202.219.69.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6042C2097FAA4 for ; Thu, 12 Jul 2018 18:44:21 -0700 (PDT) Received: from g01jpfmpwkw02.exch.g01.fujitsu.local (g01jpfmpwkw02.exch.g01.fujitsu.local [10.0.193.56]) by kw-mxoi1.gw.nic.fujitsu.com (Postfix) with ESMTP id 1DF40AC010D for ; Fri, 13 Jul 2018 10:44:12 +0900 (JST) From: "Qi, Fuli" Subject: RE: [ndctl PATCH v11 5/5] ndctl, test: add a new unit test for monitor Date: Fri, 13 Jul 2018 01:44:10 +0000 Message-ID: <0DEDF3B159719A448A49EF0E7B11E3223DA7F89A@g01jpexmbkw24> References: <20180711030012.9186-1-qi.fuli@jp.fujitsu.com> <20180711030012.9186-6-qi.fuli@jp.fujitsu.com> <1531425717.7574.63.camel@intel.com> In-Reply-To: <1531425717.7574.63.camel@intel.com> Content-Language: en-US MIME-Version: 1.0 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: "'Verma, Vishal L'" , "linux-nvdimm@lists.01.org" , "msys.mizuma@gmail.com" List-ID: > On Thu, 2018-07-12 at 15:51 -0400, Masayoshi Mizuma wrote: > > Hi Qi, > > > > Nice work! Let me ask some comments. > > > > On 07/10/2018 11:00 PM, QI Fuli wrote: > > [...] > > > diff --git a/test/monitor.sh b/test/monitor.sh new file mode 100755 > > > index 0000000..43cb11f > > > --- /dev/null > > > +++ b/test/monitor.sh > > > @@ -0,0 +1,177 @@ > > > +#!/bin/bash -Ex > > > + > > > +# SPDX-License-Identifier: GPL-2.0 > > > +# Copyright(c) 2018, FUJITSU LIMITED. All rights reserved. > > > + > > > +rc=77 > > > +logfile="" > > > +conf_file="" > > > +monitor_dimms="" > > > +monitor_regions="" > > > +monitor_namespace="" > > > +monitor_pid=65536 > > > + > > > +. ./common > > > + > > > +trap 'err $LINENO' ERR > > > + > > > +check_min_kver "4.15" || do_skip "kernel $KVER may not support monitor service" > > > + > > > +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 } > > > + > > > +start_monitor() > > > +{ > > > + logfile=$(mktemp) > > > + $NDCTL monitor -l $logfile $1 & > > > + monitor_pid=$! > > > + truncate --size 0 $logfile #remove startup log > > > + sync; sleep 3 > > > > Should this sync be moved to before the truncate? > > > > > +} > > > + > > > +get_monitor_dimm() > > > +{ > > > + jlist=$(./list-smart-dimm -b $smart_supported_bus $1) > > > + monitor_dimms=$(jq '.[]."dev"?, ."dev"?' <<<$jlist | sort | uniq | > > > +xargs) } > > > + > > > +call_notify() > > > +{ > > > + ./smart-notify $smart_supported_bus > > > + sync; sleep 3 > > > +} > > > + > > > +inject_smart() > > > +{ > > > + $NDCTL inject-smart $monitor_dimms $1 > > > + sync; sleep 3 > > > +} > > > + > > > +check_result() > > > +{ > > > + jlog=$(cat $logfile) > > > + notify_dimms=$(jq ."dimm"."dev" <<<$jlog | sort | uniq | xargs) > > > + [[ $monitor_dimms == $notify_dimms ]] } > > > + > > > +stop_monitor() > > > +{ > > > + kill $monitor_pid > > > + rm $logfile > > > +} > > > + > > > +create_conf_file() > > > +{ > > > + conf_file=$(mktemp) > > > + echo "dimm = $1" > $conf_file > > > +} > > > + > > > +test_filter_dimm() > > > +{ > > > + smart_supported_bus=$NFIT_TEST_BUS0 > > > + monitor_dimms=$(./list-smart-dimm -b $smart_supported_bus | jq -r .[0].dev) > > > + if [ -z $monitor_dimms ]; then > > > + smart_supported_bus=$NFIT_TEST_BUS1 > > > + monitor_dimms=$(./list-smart-dimm -b $smart_supported_bus | jq > -r .[0].dev) > > > + fi > > > + start_monitor "-d $monitor_dimms" > > > + call_notify > > > + check_result > > > + stop_monitor > > > +} > > > > I think the global variable "smart_supported_bus" configuration should > > be separated from this function. > > Like as: > > > > set_smart_supported_bus() > > { > > smart_supported_bus=$NFIT_TEST_BUS0 > > monitor_dimms=$(./list-smart-dimm -b $smart_supported_bus | jq > -r .[0].dev) > > if [ -z $monitor_dimms ]; then > > smart_supported_bus=$NFIT_TEST_BUS1 > > fi > > } > > > > > + > > > +test_filter_bus() > > > +{ > > > + monitor_dimms="" > > > + get_monitor_dimm > > > + start_monitor "-b $smart_supported_bus" > > > + call_notify > > > + check_result > > > + stop_monitor > > > +} > > > + > > > +test_filter_region() > > > +{ > > > + monitor_dimms="" > > > + monitor_region="" > > > + count=$($NDCTL list -R -b $smart_supported_bus | jq -r .[].dev | wc -l) > > > + i=0 > > > + while [ $i -lt $count ]; do > > > + monitor_region=$($NDCTL list -R -b $smart_supported_bus | jq > -r .[$i].dev) > > > + get_monitor_dimm "-r $monitor_region" > > > + [ ! -z $monitor_dimms ] && break > > > + i=$[$i+1] > > > > i=$(( $i + 1 )) is better. $[...] is not described in the man page of bash... > > .. and within the $(( )), you can forego the '$' for variables. So it > becomes: i=$((i + 1)) > Ok, I will fix it. Thanks Vishal. Thanks Masa. QI > > > > > + done > > > + start_monitor "-r $monitor_region" > > > + call_notify > > > + check_result > > > + stop_monitor > > > +} > > > + > > > +test_filter_namespace() > > > +{ > > > + monitor_dimms="" > > > + init > > > + monitor_namespace=$($NDCTL create-namespace -b $smart_supported_bus | jq > -r .dev) > > > + get_monitor_dimm "-n $monitor_namespace" > > > + start_monitor "-n $monitor_namespace" > > > + call_notify > > > + check_result > > > + stop_monitor > > > + $NDCTL destroy-namespace $monitor_namespace -f } > > > + > > > +test_conf_file() > > > +{ > > > + create_conf_file "$monitor_dimms" > > > + start_monitor "-c $conf_file" > > > + call_notify > > > + check_result > > > + stop_monitor > > > + rm $conf_file > > > +} > > > + > > > +test_filter_dimmevent() > > > +{ > > > + monitor_dimms="$(echo $monitor_dimms | awk '{print $1}')" > > > + > > > + start_monitor "-d $monitor_dimms -D dimm-unclean-shutdown" > > > + inject_smart "-U" > > > + check_result > > > + stop_monitor > > > + > > > + inject_value=$($NDCTL list -H -d $monitor_dimms | jq > -r ."health"."spares_threshold") > > > + inject_value=$[$inject_value-1] > > > > Same as above. > > > > > + start_monitor "-d $monitor_dimms -D dimm-spares-remaining" > > > + inject_smart "-s $inject_value" > > > + check_result > > > + stop_monitor > > > + > > > + inject_value=$($NDCTL list -H -d $monitor_dimms | jq > -r ."health"."temperature_threshold") > > > + inject_value=$[$inject_value+1] > > > > Same as above. > > > > Thanks, > > Masa > > > > > + start_monitor "-d $monitor_dimms -D dimm-media-temperature" > > > + inject_smart "-s $inject_value" > > > + check_result > > > + stop_monitor > > > +} > > > + > > > +do_tests() > > > +{ > > > + test_filter_dimm > > > + test_filter_bus > > > + test_filter_region > > > + test_filter_namespace > > > + test_conf_file > > > + test_filter_dimmevent > > > +} > > > + > > > +modprobe nfit_test > > > +rc=1 > > > +init > > > +do_tests > > > +_cleanup > > > +exit 0 > > > > > _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm