* [ndctl PATCH] ndctl, test: add a unit test for partition rescanning @ 2018-03-05 23:56 Vishal Verma 2018-03-06 21:34 ` Dan Williams 0 siblings, 1 reply; 7+ messages in thread From: Vishal Verma @ 2018-03-05 23:56 UTC (permalink / raw) To: linux-nvdimm; +Cc: Dariusz Dokupil pre-4.16 kernels had a bug where BTT partitions wouldn't come up on driver probe because we were adding a zero-sized disk. Add a unit test that creates partitions, and cycles the namespace to ensure the partitions are automatically brought up. This performs the test for raw, memory, and sector modes. Reported-by: Dariusz Dokupil <dariusz.dokupil@intel.com> Cc: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- test/Makefile.am | 3 +- test/rescan-partitions.sh | 106 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100755 test/rescan-partitions.sh diff --git a/test/Makefile.am b/test/Makefile.am index 749055c..496a663 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -20,7 +20,8 @@ TESTS =\ hugetlb \ btt-pad-compat.sh \ firmware-update.sh \ - ack-shutdown-count-set + ack-shutdown-count-set \ + rescan-partitions.sh check_PROGRAMS =\ libndctl \ diff --git a/test/rescan-partitions.sh b/test/rescan-partitions.sh new file mode 100755 index 0000000..6dd289b --- /dev/null +++ b/test/rescan-partitions.sh @@ -0,0 +1,106 @@ +#!/bin/bash -Ex +# SPDX-License-Identifier: GPL-2.0 +# Copyright(c) 2018 Intel Corporation. All rights reserved. + +[ -f "../ndctl/ndctl" ] && [ -x "../ndctl/ndctl" ] && ndctl="../ndctl/ndctl" +[ -f "./ndctl/ndctl" ] && [ -x "./ndctl/ndctl" ] && ndctl="./ndctl/ndctl" +[ -z "$ndctl" ] && echo "Couldn't find an ndctl binary" && exit 1 +bus="nfit_test.0" +json2var="s/[{}\",]//g; s/:/=/g" +dev="" +size="" +blockdev="" +rc=77 + +trap 'err $LINENO' ERR + +# sample json: +#{ +# "dev":"namespace5.0", +# "mode":"sector", +# "size":"60.00 MiB (62.92 MB)", +# "uuid":"f1baa71a-d165-4da4-bb6a-083a2b0e6469", +# "blockdev":"pmem5s", +#} + +# $1: Line number +# $2: exit code +err() +{ + [ -n "$2" ] && rc="$2" + echo "test/rescan-partitions.sh: failed at line $1" + exit "$rc" +} + +check_min_kver() +{ + local ver="$1" + : "${KVER:=$(uname -r)}" + + [ -n "$ver" ] || return 1 + [[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | head -1)" ]] +} +check_min_kver "4.16" || { echo "kernel $KVER may not contain fixes for partition rescanning"; exit "$rc"; } + +check_prereq() +{ + if ! command -v "$1" >/dev/null; then + echo "missing '$1', skipping.." + exit "$rc" + fi +} +check_prereq "parted" +check_prereq "blockdev" + +reset() +{ + $ndctl disable-region -b "$bus" all + $ndctl zero-labels -b "$bus" all + $ndctl enable-region -b "$bus" all +} + +test_mode() +{ + local mode="$1" + + # create namespace + json=$($ndctl create-namespace -b "$bus" -t pmem -m "$mode") + eval "$(echo "$json" | sed -e "$json2var")" + [ -n "$dev" ] || err "$LINENO" 2 + [ -n "$size" ] || err "$LINENO" 2 + [ -n "$blockdev" ] || err "$LINENO" 2 + [ $size -gt 0 ] || err "$LINENO" 2 + + # create partition + parted --script /dev/$blockdev mklabel gpt mkpart primary 1MiB 10MiB + + # verify it is created + sleep 1 + blockdev --rereadpt /dev/$blockdev + sleep 1 + partdev="$(grep -Eo "${blockdev}.+" /proc/partitions)" + test -b /dev/$partdev + + # cycle the namespace, and verify the partition is read + # without needing to do a blockdev --rereadpt + $ndctl disable-namespace $dev + $ndctl enable-namespace $dev + if [ -b /dev/$partdev ]; then + echo "mode: $mode - partition read successful" + else + echo "mode: $mode - partition read failed" + err "$LINENO" 1 + fi + + $ndctl disable-namespace $dev + $ndctl destroy-namespace $dev +} + +rc=1 +reset +test_mode "raw" +test_mode "memory" +test_mode "sector" +reset + +exit 0 -- 2.14.3 _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [ndctl PATCH] ndctl, test: add a unit test for partition rescanning 2018-03-05 23:56 [ndctl PATCH] ndctl, test: add a unit test for partition rescanning Vishal Verma @ 2018-03-06 21:34 ` Dan Williams 2018-03-06 21:41 ` Verma, Vishal L 0 siblings, 1 reply; 7+ messages in thread From: Dan Williams @ 2018-03-06 21:34 UTC (permalink / raw) To: Vishal Verma; +Cc: Dariusz Dokupil, linux-nvdimm On Mon, Mar 5, 2018 at 3:56 PM, Vishal Verma <vishal.l.verma@intel.com> wrote: > pre-4.16 kernels had a bug where BTT partitions wouldn't come up on > driver probe because we were adding a zero-sized disk. Add a unit test > that creates partitions, and cycles the namespace to ensure the > partitions are automatically brought up. This performs the test for raw, > memory, and sector modes. > > Reported-by: Dariusz Dokupil <dariusz.dokupil@intel.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > test/Makefile.am | 3 +- > test/rescan-partitions.sh | 106 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 108 insertions(+), 1 deletion(-) > create mode 100755 test/rescan-partitions.sh > > diff --git a/test/Makefile.am b/test/Makefile.am > index 749055c..496a663 100644 > --- a/test/Makefile.am > +++ b/test/Makefile.am > @@ -20,7 +20,8 @@ TESTS =\ > hugetlb \ > btt-pad-compat.sh \ > firmware-update.sh \ > - ack-shutdown-count-set > + ack-shutdown-count-set \ > + rescan-partitions.sh > > check_PROGRAMS =\ > libndctl \ > diff --git a/test/rescan-partitions.sh b/test/rescan-partitions.sh > new file mode 100755 > index 0000000..6dd289b > --- /dev/null > +++ b/test/rescan-partitions.sh > @@ -0,0 +1,106 @@ > +#!/bin/bash -Ex > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright(c) 2018 Intel Corporation. All rights reserved. > + > +[ -f "../ndctl/ndctl" ] && [ -x "../ndctl/ndctl" ] && ndctl="../ndctl/ndctl" > +[ -f "./ndctl/ndctl" ] && [ -x "./ndctl/ndctl" ] && ndctl="./ndctl/ndctl" > +[ -z "$ndctl" ] && echo "Couldn't find an ndctl binary" && exit 1 > +bus="nfit_test.0" > +json2var="s/[{}\",]//g; s/:/=/g" > +dev="" > +size="" > +blockdev="" > +rc=77 > + > +trap 'err $LINENO' ERR > + > +# sample json: > +#{ > +# "dev":"namespace5.0", > +# "mode":"sector", > +# "size":"60.00 MiB (62.92 MB)", > +# "uuid":"f1baa71a-d165-4da4-bb6a-083a2b0e6469", > +# "blockdev":"pmem5s", > +#} > + > +# $1: Line number > +# $2: exit code > +err() > +{ > + [ -n "$2" ] && rc="$2" > + echo "test/rescan-partitions.sh: failed at line $1" > + exit "$rc" > +} > + > +check_min_kver() > +{ > + local ver="$1" > + : "${KVER:=$(uname -r)}" > + > + [ -n "$ver" ] || return 1 > + [[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | head -1)" ]] > +} > +check_min_kver "4.16" || { echo "kernel $KVER may not contain fixes for partition rescanning"; exit "$rc"; } I have a proposal for kernel version checks going forward. How about tests that fail the kernel version check return SKIP / PASS, and kernels that pass the kernel version check return FAIL / PASS for the test. This way we don't bail out early on backport kernels that could otherwise pass the test. Thoughts? _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [ndctl PATCH] ndctl, test: add a unit test for partition rescanning 2018-03-06 21:34 ` Dan Williams @ 2018-03-06 21:41 ` Verma, Vishal L 2018-03-06 21:47 ` Dan Williams 0 siblings, 1 reply; 7+ messages in thread From: Verma, Vishal L @ 2018-03-06 21:41 UTC (permalink / raw) To: Williams, Dan J; +Cc: Dokupil, Dariusz, linux-nvdimm@lists.01.org On Tue, 2018-03-06 at 13:34 -0800, Dan Williams wrote: > On Mon, Mar 5, 2018 at 3:56 PM, Vishal Verma <vishal.l.verma@intel.co > m> wrote: > > pre-4.16 kernels had a bug where BTT partitions wouldn't come up on > > driver probe because we were adding a zero-sized disk. Add a unit > > test > > that creates partitions, and cycles the namespace to ensure the > > partitions are automatically brought up. This performs the test for > > raw, > > memory, and sector modes. > > > > Reported-by: Dariusz Dokupil <dariusz.dokupil@intel.com> > > Cc: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > --- > > test/Makefile.am | 3 +- > > test/rescan-partitions.sh | 106 > > ++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 108 insertions(+), 1 deletion(-) > > create mode 100755 test/rescan-partitions.sh > > > > diff --git a/test/Makefile.am b/test/Makefile.am > > index 749055c..496a663 100644 > > --- a/test/Makefile.am > > +++ b/test/Makefile.am > > @@ -20,7 +20,8 @@ TESTS =\ > > hugetlb \ > > btt-pad-compat.sh \ > > firmware-update.sh \ > > - ack-shutdown-count-set > > + ack-shutdown-count-set \ > > + rescan-partitions.sh > > > > check_PROGRAMS =\ > > libndctl \ > > diff --git a/test/rescan-partitions.sh b/test/rescan-partitions.sh > > new file mode 100755 > > index 0000000..6dd289b > > --- /dev/null > > +++ b/test/rescan-partitions.sh > > @@ -0,0 +1,106 @@ > > +#!/bin/bash -Ex > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright(c) 2018 Intel Corporation. All rights reserved. > > + > > +[ -f "../ndctl/ndctl" ] && [ -x "../ndctl/ndctl" ] && > > ndctl="../ndctl/ndctl" > > +[ -f "./ndctl/ndctl" ] && [ -x "./ndctl/ndctl" ] && > > ndctl="./ndctl/ndctl" > > +[ -z "$ndctl" ] && echo "Couldn't find an ndctl binary" && exit 1 > > +bus="nfit_test.0" > > +json2var="s/[{}\",]//g; s/:/=/g" > > +dev="" > > +size="" > > +blockdev="" > > +rc=77 > > + > > +trap 'err $LINENO' ERR > > + > > +# sample json: > > +#{ > > +# "dev":"namespace5.0", > > +# "mode":"sector", > > +# "size":"60.00 MiB (62.92 MB)", > > +# "uuid":"f1baa71a-d165-4da4-bb6a-083a2b0e6469", > > +# "blockdev":"pmem5s", > > +#} > > + > > +# $1: Line number > > +# $2: exit code > > +err() > > +{ > > + [ -n "$2" ] && rc="$2" > > + echo "test/rescan-partitions.sh: failed at line $1" > > + exit "$rc" > > +} > > + > > +check_min_kver() > > +{ > > + local ver="$1" > > + : "${KVER:=$(uname -r)}" > > + > > + [ -n "$ver" ] || return 1 > > + [[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | head -1)" > > ]] > > +} > > +check_min_kver "4.16" || { echo "kernel $KVER may not contain > > fixes for partition rescanning"; exit "$rc"; } > > I have a proposal for kernel version checks going forward. How about > tests that fail the kernel version check return SKIP / PASS, and > kernels that pass the kernel version check return FAIL / PASS for the > test. This way we don't bail out early on backport kernels that could > otherwise pass the test. > > Thoughts? So do you mean, for example, if a kernel version check fails, don't skip immediately. Instead continue to run the test, and if something actually fails, then return with a SKIP. And conversely, for a kernel version that passes the included check, no other condition can return a SKIP? (What about something like missing packages?) _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [ndctl PATCH] ndctl, test: add a unit test for partition rescanning 2018-03-06 21:41 ` Verma, Vishal L @ 2018-03-06 21:47 ` Dan Williams 2018-03-06 21:49 ` Verma, Vishal L 0 siblings, 1 reply; 7+ messages in thread From: Dan Williams @ 2018-03-06 21:47 UTC (permalink / raw) To: Verma, Vishal L; +Cc: Dokupil, Dariusz, linux-nvdimm@lists.01.org On Tue, Mar 6, 2018 at 1:41 PM, Verma, Vishal L <vishal.l.verma@intel.com> wrote: > > On Tue, 2018-03-06 at 13:34 -0800, Dan Williams wrote: >> On Mon, Mar 5, 2018 at 3:56 PM, Vishal Verma <vishal.l.verma@intel.co >> m> wrote: >> > pre-4.16 kernels had a bug where BTT partitions wouldn't come up on >> > driver probe because we were adding a zero-sized disk. Add a unit >> > test >> > that creates partitions, and cycles the namespace to ensure the >> > partitions are automatically brought up. This performs the test for >> > raw, >> > memory, and sector modes. >> > >> > Reported-by: Dariusz Dokupil <dariusz.dokupil@intel.com> >> > Cc: Dan Williams <dan.j.williams@intel.com> >> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> >> > --- >> > test/Makefile.am | 3 +- >> > test/rescan-partitions.sh | 106 >> > ++++++++++++++++++++++++++++++++++++++++++++++ >> > 2 files changed, 108 insertions(+), 1 deletion(-) >> > create mode 100755 test/rescan-partitions.sh >> > >> > diff --git a/test/Makefile.am b/test/Makefile.am >> > index 749055c..496a663 100644 >> > --- a/test/Makefile.am >> > +++ b/test/Makefile.am >> > @@ -20,7 +20,8 @@ TESTS =\ >> > hugetlb \ >> > btt-pad-compat.sh \ >> > firmware-update.sh \ >> > - ack-shutdown-count-set >> > + ack-shutdown-count-set \ >> > + rescan-partitions.sh >> > >> > check_PROGRAMS =\ >> > libndctl \ >> > diff --git a/test/rescan-partitions.sh b/test/rescan-partitions.sh >> > new file mode 100755 >> > index 0000000..6dd289b >> > --- /dev/null >> > +++ b/test/rescan-partitions.sh >> > @@ -0,0 +1,106 @@ >> > +#!/bin/bash -Ex >> > +# SPDX-License-Identifier: GPL-2.0 >> > +# Copyright(c) 2018 Intel Corporation. All rights reserved. >> > + >> > +[ -f "../ndctl/ndctl" ] && [ -x "../ndctl/ndctl" ] && >> > ndctl="../ndctl/ndctl" >> > +[ -f "./ndctl/ndctl" ] && [ -x "./ndctl/ndctl" ] && >> > ndctl="./ndctl/ndctl" >> > +[ -z "$ndctl" ] && echo "Couldn't find an ndctl binary" && exit 1 >> > +bus="nfit_test.0" >> > +json2var="s/[{}\",]//g; s/:/=/g" >> > +dev="" >> > +size="" >> > +blockdev="" >> > +rc=77 >> > + >> > +trap 'err $LINENO' ERR >> > + >> > +# sample json: >> > +#{ >> > +# "dev":"namespace5.0", >> > +# "mode":"sector", >> > +# "size":"60.00 MiB (62.92 MB)", >> > +# "uuid":"f1baa71a-d165-4da4-bb6a-083a2b0e6469", >> > +# "blockdev":"pmem5s", >> > +#} >> > + >> > +# $1: Line number >> > +# $2: exit code >> > +err() >> > +{ >> > + [ -n "$2" ] && rc="$2" >> > + echo "test/rescan-partitions.sh: failed at line $1" >> > + exit "$rc" >> > +} >> > + >> > +check_min_kver() >> > +{ >> > + local ver="$1" >> > + : "${KVER:=$(uname -r)}" >> > + >> > + [ -n "$ver" ] || return 1 >> > + [[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | head -1)" >> > ]] >> > +} >> > +check_min_kver "4.16" || { echo "kernel $KVER may not contain >> > fixes for partition rescanning"; exit "$rc"; } >> >> I have a proposal for kernel version checks going forward. How about >> tests that fail the kernel version check return SKIP / PASS, and >> kernels that pass the kernel version check return FAIL / PASS for the >> test. This way we don't bail out early on backport kernels that could >> otherwise pass the test. >> >> Thoughts? > > So do you mean, for example, if a kernel version check fails, don't > skip immediately. Instead continue to run the test, and if something > actually fails, then return with a SKIP. > > And conversely, for a kernel version that passes the included check, no > other condition can return a SKIP? (What about something like missing > packages?) Yeah, how about an unmet dependency whether it is kernel version or support package results in a SKIP / PASS result, but if all dependencies are met the test becomes FAIL / PASS? _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [ndctl PATCH] ndctl, test: add a unit test for partition rescanning 2018-03-06 21:47 ` Dan Williams @ 2018-03-06 21:49 ` Verma, Vishal L 2018-03-06 22:04 ` Dan Williams 0 siblings, 1 reply; 7+ messages in thread From: Verma, Vishal L @ 2018-03-06 21:49 UTC (permalink / raw) To: Williams, Dan J; +Cc: Dokupil, Dariusz, linux-nvdimm@lists.01.org On Tue, 2018-03-06 at 13:47 -0800, Dan Williams wrote: > On Tue, Mar 6, 2018 at 1:41 PM, Verma, Vishal L > <vishal.l.verma@intel.com> wrote: > > > > On Tue, 2018-03-06 at 13:34 -0800, Dan Williams wrote: > > > On Mon, Mar 5, 2018 at 3:56 PM, Vishal Verma <vishal.l.verma@inte > > > l.co > > > m> wrote: > > > > pre-4.16 kernels had a bug where BTT partitions wouldn't come > > > > up on > > > > driver probe because we were adding a zero-sized disk. Add a > > > > unit > > > > test > > > > that creates partitions, and cycles the namespace to ensure the > > > > partitions are automatically brought up. This performs the test > > > > for > > > > raw, > > > > memory, and sector modes. > > > > > > > > Reported-by: Dariusz Dokupil <dariusz.dokupil@intel.com> > > > > Cc: Dan Williams <dan.j.williams@intel.com> > > > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > > > --- > > > > test/Makefile.am | 3 +- > > > > test/rescan-partitions.sh | 106 > > > > ++++++++++++++++++++++++++++++++++++++++++++++ > > > > 2 files changed, 108 insertions(+), 1 deletion(-) > > > > create mode 100755 test/rescan-partitions.sh > > > > > > > > diff --git a/test/Makefile.am b/test/Makefile.am > > > > index 749055c..496a663 100644 > > > > --- a/test/Makefile.am > > > > +++ b/test/Makefile.am > > > > @@ -20,7 +20,8 @@ TESTS =\ > > > > hugetlb \ > > > > btt-pad-compat.sh \ > > > > firmware-update.sh \ > > > > - ack-shutdown-count-set > > > > + ack-shutdown-count-set \ > > > > + rescan-partitions.sh > > > > > > > > check_PROGRAMS =\ > > > > libndctl \ > > > > diff --git a/test/rescan-partitions.sh b/test/rescan- > > > > partitions.sh > > > > new file mode 100755 > > > > index 0000000..6dd289b > > > > --- /dev/null > > > > +++ b/test/rescan-partitions.sh > > > > @@ -0,0 +1,106 @@ > > > > +#!/bin/bash -Ex > > > > +# SPDX-License-Identifier: GPL-2.0 > > > > +# Copyright(c) 2018 Intel Corporation. All rights reserved. > > > > + > > > > +[ -f "../ndctl/ndctl" ] && [ -x "../ndctl/ndctl" ] && > > > > ndctl="../ndctl/ndctl" > > > > +[ -f "./ndctl/ndctl" ] && [ -x "./ndctl/ndctl" ] && > > > > ndctl="./ndctl/ndctl" > > > > +[ -z "$ndctl" ] && echo "Couldn't find an ndctl binary" && > > > > exit 1 > > > > +bus="nfit_test.0" > > > > +json2var="s/[{}\",]//g; s/:/=/g" > > > > +dev="" > > > > +size="" > > > > +blockdev="" > > > > +rc=77 > > > > + > > > > +trap 'err $LINENO' ERR > > > > + > > > > +# sample json: > > > > +#{ > > > > +# "dev":"namespace5.0", > > > > +# "mode":"sector", > > > > +# "size":"60.00 MiB (62.92 MB)", > > > > +# "uuid":"f1baa71a-d165-4da4-bb6a-083a2b0e6469", > > > > +# "blockdev":"pmem5s", > > > > +#} > > > > + > > > > +# $1: Line number > > > > +# $2: exit code > > > > +err() > > > > +{ > > > > + [ -n "$2" ] && rc="$2" > > > > + echo "test/rescan-partitions.sh: failed at line $1" > > > > + exit "$rc" > > > > +} > > > > + > > > > +check_min_kver() > > > > +{ > > > > + local ver="$1" > > > > + : "${KVER:=$(uname -r)}" > > > > + > > > > + [ -n "$ver" ] || return 1 > > > > + [[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | head > > > > -1)" > > > > ]] > > > > +} > > > > +check_min_kver "4.16" || { echo "kernel $KVER may not contain > > > > fixes for partition rescanning"; exit "$rc"; } > > > > > > I have a proposal for kernel version checks going forward. How > > > about > > > tests that fail the kernel version check return SKIP / PASS, and > > > kernels that pass the kernel version check return FAIL / PASS for > > > the > > > test. This way we don't bail out early on backport kernels that > > > could > > > otherwise pass the test. > > > > > > Thoughts? > > > > So do you mean, for example, if a kernel version check fails, don't > > skip immediately. Instead continue to run the test, and if > > something > > actually fails, then return with a SKIP. > > > > And conversely, for a kernel version that passes the included > > check, no > > other condition can return a SKIP? (What about something like > > missing > > packages?) > > Yeah, how about an unmet dependency whether it is kernel version or > support package results in a SKIP / PASS result, but if all > dependencies are met the test becomes FAIL / PASS? Yeah that makes sense to me. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [ndctl PATCH] ndctl, test: add a unit test for partition rescanning 2018-03-06 21:49 ` Verma, Vishal L @ 2018-03-06 22:04 ` Dan Williams 2018-03-06 22:07 ` Verma, Vishal L 0 siblings, 1 reply; 7+ messages in thread From: Dan Williams @ 2018-03-06 22:04 UTC (permalink / raw) To: Verma, Vishal L; +Cc: Dokupil, Dariusz, linux-nvdimm@lists.01.org On Tue, Mar 6, 2018 at 1:49 PM, Verma, Vishal L <vishal.l.verma@intel.com> wrote: > > On Tue, 2018-03-06 at 13:47 -0800, Dan Williams wrote: >> On Tue, Mar 6, 2018 at 1:41 PM, Verma, Vishal L >> <vishal.l.verma@intel.com> wrote: >> > >> > On Tue, 2018-03-06 at 13:34 -0800, Dan Williams wrote: >> > > On Mon, Mar 5, 2018 at 3:56 PM, Vishal Verma <vishal.l.verma@inte >> > > l.co >> > > m> wrote: >> > > > pre-4.16 kernels had a bug where BTT partitions wouldn't come >> > > > up on >> > > > driver probe because we were adding a zero-sized disk. Add a >> > > > unit >> > > > test >> > > > that creates partitions, and cycles the namespace to ensure the >> > > > partitions are automatically brought up. This performs the test >> > > > for >> > > > raw, >> > > > memory, and sector modes. >> > > > >> > > > Reported-by: Dariusz Dokupil <dariusz.dokupil@intel.com> >> > > > Cc: Dan Williams <dan.j.williams@intel.com> >> > > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> >> > > > --- >> > > > test/Makefile.am | 3 +- >> > > > test/rescan-partitions.sh | 106 >> > > > ++++++++++++++++++++++++++++++++++++++++++++++ >> > > > 2 files changed, 108 insertions(+), 1 deletion(-) >> > > > create mode 100755 test/rescan-partitions.sh >> > > > >> > > > diff --git a/test/Makefile.am b/test/Makefile.am >> > > > index 749055c..496a663 100644 >> > > > --- a/test/Makefile.am >> > > > +++ b/test/Makefile.am >> > > > @@ -20,7 +20,8 @@ TESTS =\ >> > > > hugetlb \ >> > > > btt-pad-compat.sh \ >> > > > firmware-update.sh \ >> > > > - ack-shutdown-count-set >> > > > + ack-shutdown-count-set \ >> > > > + rescan-partitions.sh >> > > > >> > > > check_PROGRAMS =\ >> > > > libndctl \ >> > > > diff --git a/test/rescan-partitions.sh b/test/rescan- >> > > > partitions.sh >> > > > new file mode 100755 >> > > > index 0000000..6dd289b >> > > > --- /dev/null >> > > > +++ b/test/rescan-partitions.sh >> > > > @@ -0,0 +1,106 @@ >> > > > +#!/bin/bash -Ex >> > > > +# SPDX-License-Identifier: GPL-2.0 >> > > > +# Copyright(c) 2018 Intel Corporation. All rights reserved. >> > > > + >> > > > +[ -f "../ndctl/ndctl" ] && [ -x "../ndctl/ndctl" ] && >> > > > ndctl="../ndctl/ndctl" >> > > > +[ -f "./ndctl/ndctl" ] && [ -x "./ndctl/ndctl" ] && >> > > > ndctl="./ndctl/ndctl" >> > > > +[ -z "$ndctl" ] && echo "Couldn't find an ndctl binary" && >> > > > exit 1 >> > > > +bus="nfit_test.0" >> > > > +json2var="s/[{}\",]//g; s/:/=/g" >> > > > +dev="" >> > > > +size="" >> > > > +blockdev="" >> > > > +rc=77 >> > > > + >> > > > +trap 'err $LINENO' ERR >> > > > + >> > > > +# sample json: >> > > > +#{ >> > > > +# "dev":"namespace5.0", >> > > > +# "mode":"sector", >> > > > +# "size":"60.00 MiB (62.92 MB)", >> > > > +# "uuid":"f1baa71a-d165-4da4-bb6a-083a2b0e6469", >> > > > +# "blockdev":"pmem5s", >> > > > +#} >> > > > + >> > > > +# $1: Line number >> > > > +# $2: exit code >> > > > +err() >> > > > +{ >> > > > + [ -n "$2" ] && rc="$2" >> > > > + echo "test/rescan-partitions.sh: failed at line $1" >> > > > + exit "$rc" >> > > > +} >> > > > + >> > > > +check_min_kver() >> > > > +{ >> > > > + local ver="$1" >> > > > + : "${KVER:=$(uname -r)}" >> > > > + >> > > > + [ -n "$ver" ] || return 1 >> > > > + [[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | head >> > > > -1)" >> > > > ]] >> > > > +} >> > > > +check_min_kver "4.16" || { echo "kernel $KVER may not contain >> > > > fixes for partition rescanning"; exit "$rc"; } >> > > >> > > I have a proposal for kernel version checks going forward. How >> > > about >> > > tests that fail the kernel version check return SKIP / PASS, and >> > > kernels that pass the kernel version check return FAIL / PASS for >> > > the >> > > test. This way we don't bail out early on backport kernels that >> > > could >> > > otherwise pass the test. >> > > >> > > Thoughts? >> > >> > So do you mean, for example, if a kernel version check fails, don't >> > skip immediately. Instead continue to run the test, and if >> > something >> > actually fails, then return with a SKIP. >> > >> > And conversely, for a kernel version that passes the included >> > check, no >> > other condition can return a SKIP? (What about something like >> > missing >> > packages?) >> >> Yeah, how about an unmet dependency whether it is kernel version or >> support package results in a SKIP / PASS result, but if all >> dependencies are met the test becomes FAIL / PASS? > > Yeah that makes sense to me. I also think it's past time that we made check_min_kver() a function in separate source file that all the shell tests import by sourcing. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [ndctl PATCH] ndctl, test: add a unit test for partition rescanning 2018-03-06 22:04 ` Dan Williams @ 2018-03-06 22:07 ` Verma, Vishal L 0 siblings, 0 replies; 7+ messages in thread From: Verma, Vishal L @ 2018-03-06 22:07 UTC (permalink / raw) To: Williams, Dan J; +Cc: Dokupil, Dariusz, linux-nvdimm@lists.01.org On Tue, 2018-03-06 at 14:04 -0800, Dan Williams wrote: > On Tue, Mar 6, 2018 at 1:49 PM, Verma, Vishal L > <vishal.l.verma@intel.com> wrote: > > > > On Tue, 2018-03-06 at 13:47 -0800, Dan Williams wrote: > > > On Tue, Mar 6, 2018 at 1:41 PM, Verma, Vishal L > > > <vishal.l.verma@intel.com> wrote: > > > > > > > > On Tue, 2018-03-06 at 13:34 -0800, Dan Williams wrote: > > > > > On Mon, Mar 5, 2018 at 3:56 PM, Vishal Verma <vishal.l.verma@ > > > > > inte > > > > > l.co > > > > > m> wrote: > > > > > > pre-4.16 kernels had a bug where BTT partitions wouldn't > > > > > > come > > > > > > up on > > > > > > driver probe because we were adding a zero-sized disk. Add > > > > > > a > > > > > > unit > > > > > > test > > > > > > that creates partitions, and cycles the namespace to ensure > > > > > > the > > > > > > partitions are automatically brought up. This performs the > > > > > > test > > > > > > for > > > > > > raw, > > > > > > memory, and sector modes. > > > > > > > > > > > > Reported-by: Dariusz Dokupil <dariusz.dokupil@intel.com> > > > > > > Cc: Dan Williams <dan.j.williams@intel.com> > > > > > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > > > > > --- > > > > > > test/Makefile.am | 3 +- > > > > > > test/rescan-partitions.sh | 106 > > > > > > ++++++++++++++++++++++++++++++++++++++++++++++ > > > > > > 2 files changed, 108 insertions(+), 1 deletion(-) > > > > > > create mode 100755 test/rescan-partitions.sh > > > > > > > > > > > > diff --git a/test/Makefile.am b/test/Makefile.am > > > > > > index 749055c..496a663 100644 > > > > > > --- a/test/Makefile.am > > > > > > +++ b/test/Makefile.am > > > > > > @@ -20,7 +20,8 @@ TESTS =\ > > > > > > hugetlb \ > > > > > > btt-pad-compat.sh \ > > > > > > firmware-update.sh \ > > > > > > - ack-shutdown-count-set > > > > > > + ack-shutdown-count-set \ > > > > > > + rescan-partitions.sh > > > > > > > > > > > > check_PROGRAMS =\ > > > > > > libndctl \ > > > > > > diff --git a/test/rescan-partitions.sh b/test/rescan- > > > > > > partitions.sh > > > > > > new file mode 100755 > > > > > > index 0000000..6dd289b > > > > > > --- /dev/null > > > > > > +++ b/test/rescan-partitions.sh > > > > > > @@ -0,0 +1,106 @@ > > > > > > +#!/bin/bash -Ex > > > > > > +# SPDX-License-Identifier: GPL-2.0 > > > > > > +# Copyright(c) 2018 Intel Corporation. All rights > > > > > > reserved. > > > > > > + > > > > > > +[ -f "../ndctl/ndctl" ] && [ -x "../ndctl/ndctl" ] && > > > > > > ndctl="../ndctl/ndctl" > > > > > > +[ -f "./ndctl/ndctl" ] && [ -x "./ndctl/ndctl" ] && > > > > > > ndctl="./ndctl/ndctl" > > > > > > +[ -z "$ndctl" ] && echo "Couldn't find an ndctl binary" && > > > > > > exit 1 > > > > > > +bus="nfit_test.0" > > > > > > +json2var="s/[{}\",]//g; s/:/=/g" > > > > > > +dev="" > > > > > > +size="" > > > > > > +blockdev="" > > > > > > +rc=77 > > > > > > + > > > > > > +trap 'err $LINENO' ERR > > > > > > + > > > > > > +# sample json: > > > > > > +#{ > > > > > > +# "dev":"namespace5.0", > > > > > > +# "mode":"sector", > > > > > > +# "size":"60.00 MiB (62.92 MB)", > > > > > > +# "uuid":"f1baa71a-d165-4da4-bb6a-083a2b0e6469", > > > > > > +# "blockdev":"pmem5s", > > > > > > +#} > > > > > > + > > > > > > +# $1: Line number > > > > > > +# $2: exit code > > > > > > +err() > > > > > > +{ > > > > > > + [ -n "$2" ] && rc="$2" > > > > > > + echo "test/rescan-partitions.sh: failed at line $1" > > > > > > + exit "$rc" > > > > > > +} > > > > > > + > > > > > > +check_min_kver() > > > > > > +{ > > > > > > + local ver="$1" > > > > > > + : "${KVER:=$(uname -r)}" > > > > > > + > > > > > > + [ -n "$ver" ] || return 1 > > > > > > + [[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | > > > > > > head > > > > > > -1)" > > > > > > ]] > > > > > > +} > > > > > > +check_min_kver "4.16" || { echo "kernel $KVER may not > > > > > > contain > > > > > > fixes for partition rescanning"; exit "$rc"; } > > > > > > > > > > I have a proposal for kernel version checks going forward. > > > > > How > > > > > about > > > > > tests that fail the kernel version check return SKIP / PASS, > > > > > and > > > > > kernels that pass the kernel version check return FAIL / PASS > > > > > for > > > > > the > > > > > test. This way we don't bail out early on backport kernels > > > > > that > > > > > could > > > > > otherwise pass the test. > > > > > > > > > > Thoughts? > > > > > > > > So do you mean, for example, if a kernel version check fails, > > > > don't > > > > skip immediately. Instead continue to run the test, and if > > > > something > > > > actually fails, then return with a SKIP. > > > > > > > > And conversely, for a kernel version that passes the included > > > > check, no > > > > other condition can return a SKIP? (What about something like > > > > missing > > > > packages?) > > > > > > Yeah, how about an unmet dependency whether it is kernel version > > > or > > > support package results in a SKIP / PASS result, but if all > > > dependencies are met the test becomes FAIL / PASS? > > > > Yeah that makes sense to me. > > I also think it's past time that we made check_min_kver() a function > in separate source file that all the shell tests import by sourcing. There are definitely a fair number of boilerplate things that we've been copying around from test to test. I can take a shot at a cleanup. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-06 22:01 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-05 23:56 [ndctl PATCH] ndctl, test: add a unit test for partition rescanning Vishal Verma 2018-03-06 21:34 ` Dan Williams 2018-03-06 21:41 ` Verma, Vishal L 2018-03-06 21:47 ` Dan Williams 2018-03-06 21:49 ` Verma, Vishal L 2018-03-06 22:04 ` Dan Williams 2018-03-06 22:07 ` Verma, Vishal L
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.