nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [ndctl PATCH] ndctl, test: Fix dax.sh return code
@ 2018-07-04 21:04 Dan Williams
  2018-07-06 20:26 ` Verma, Vishal L
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Williams @ 2018-07-04 21:04 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: linux-nvdimm

Commit 11b349b5236e "ndctl, test: Disable poison tests for now"
inadvertently prevented the dax.sh test from ever succeeding. Revert the
change to return $rc instead of 0.

Fixes: 11b349b5236e ("ndctl, test: Disable poison tests for now")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/dax.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/dax.sh b/test/dax.sh
index 30fe16721935..2a82281edb35 100755
--- a/test/dax.sh
+++ b/test/dax.sh
@@ -90,4 +90,4 @@ json=$($NDCTL create-namespace -m raw -f -e $dev)
 eval $(echo $json | sed -e "$json2var")
 [ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1
 
-exit $rc
+exit 0

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [ndctl PATCH] ndctl, test: Fix dax.sh return code
  2018-07-04 21:04 [ndctl PATCH] ndctl, test: Fix dax.sh return code Dan Williams
@ 2018-07-06 20:26 ` Verma, Vishal L
  2018-07-06 20:30   ` Dan Williams
  0 siblings, 1 reply; 6+ messages in thread
From: Verma, Vishal L @ 2018-07-06 20:26 UTC (permalink / raw)
  To: Williams, Dan J; +Cc: linux-nvdimm


On Wed, 2018-07-04 at 14:04 -0700, Dan Williams wrote:
> Commit 11b349b5236e "ndctl, test: Disable poison tests for now"
> inadvertently prevented the dax.sh test from ever succeeding. Revert
> the
> change to return $rc instead of 0.
> 
> Fixes: 11b349b5236e ("ndctl, test: Disable poison tests for now")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  test/dax.sh |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/dax.sh b/test/dax.sh
> index 30fe16721935..2a82281edb35 100755
> --- a/test/dax.sh
> +++ b/test/dax.sh
> @@ -90,4 +90,4 @@ json=$($NDCTL create-namespace -m raw -f -e $dev)
>  eval $(echo $json | sed -e "$json2var")
>  [ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1
>  
> -exit $rc
> +exit 0

I'm not sure I understand why this will cause it to always fail. rc is
set in run_test, and that takes care of erroring out when the test
returns anything other than 0 or 77. In case of 0 or 77, it falls
through. So in the end here, we do want 'exit $rc' so we propagate the
77 correctly. Am I missing something?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [ndctl PATCH] ndctl, test: Fix dax.sh return code
  2018-07-06 20:26 ` Verma, Vishal L
@ 2018-07-06 20:30   ` Dan Williams
  2018-07-06 20:32     ` Verma, Vishal L
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Williams @ 2018-07-06 20:30 UTC (permalink / raw)
  To: Verma, Vishal L; +Cc: linux-nvdimm

On Fri, Jul 6, 2018 at 1:26 PM, Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> On Wed, 2018-07-04 at 14:04 -0700, Dan Williams wrote:
>> Commit 11b349b5236e "ndctl, test: Disable poison tests for now"
>> inadvertently prevented the dax.sh test from ever succeeding. Revert
>> the
>> change to return $rc instead of 0.
>>
>> Fixes: 11b349b5236e ("ndctl, test: Disable poison tests for now")
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>>  test/dax.sh |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/test/dax.sh b/test/dax.sh
>> index 30fe16721935..2a82281edb35 100755
>> --- a/test/dax.sh
>> +++ b/test/dax.sh
>> @@ -90,4 +90,4 @@ json=$($NDCTL create-namespace -m raw -f -e $dev)
>>  eval $(echo $json | sed -e "$json2var")
>>  [ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1
>>
>> -exit $rc
>> +exit 0
>
> I'm not sure I understand why this will cause it to always fail. rc is
> set in run_test, and that takes care of erroring out when the test
> returns anything other than 0 or 77. In case of 0 or 77, it falls
> through. So in the end here, we do want 'exit $rc' so we propagate the
> 77 correctly. Am I missing something?

rc=1

https://github.com/pmem/ndctl/blob/master/test/dax.sh#L47
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [ndctl PATCH] ndctl, test: Fix dax.sh return code
  2018-07-06 20:30   ` Dan Williams
@ 2018-07-06 20:32     ` Verma, Vishal L
  2018-07-06 20:52       ` Dan Williams
  0 siblings, 1 reply; 6+ messages in thread
From: Verma, Vishal L @ 2018-07-06 20:32 UTC (permalink / raw)
  To: Williams, Dan J; +Cc: linux-nvdimm


On Fri, 2018-07-06 at 13:30 -0700, Dan Williams wrote:
> On Fri, Jul 6, 2018 at 1:26 PM, Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
> > 
> > On Wed, 2018-07-04 at 14:04 -0700, Dan Williams wrote:
> > > Commit 11b349b5236e "ndctl, test: Disable poison tests for now"
> > > inadvertently prevented the dax.sh test from ever succeeding.
> > > Revert
> > > the
> > > change to return $rc instead of 0.
> > > 
> > > Fixes: 11b349b5236e ("ndctl, test: Disable poison tests for now")
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > ---
> > >  test/dax.sh |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/test/dax.sh b/test/dax.sh
> > > index 30fe16721935..2a82281edb35 100755
> > > --- a/test/dax.sh
> > > +++ b/test/dax.sh
> > > @@ -90,4 +90,4 @@ json=$($NDCTL create-namespace -m raw -f -e
> > > $dev)
> > >  eval $(echo $json | sed -e "$json2var")
> > >  [ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1
> > > 
> > > -exit $rc
> > > +exit 0
> > 
> > I'm not sure I understand why this will cause it to always fail. rc
> > is
> > set in run_test, and that takes care of erroring out when the test
> > returns anything other than 0 or 77. In case of 0 or 77, it falls
> > through. So in the end here, we do want 'exit $rc' so we propagate
> > the
> > 77 correctly. Am I missing something?
> 
> rc=1
> 
> https://github.com/pmem/ndctl/blob/master/test/dax.sh#L47

Yes, but that gets overriden by:
https://github.com/pmem/ndctl/blob/master/test/dax.sh#L33
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [ndctl PATCH] ndctl, test: Fix dax.sh return code
  2018-07-06 20:32     ` Verma, Vishal L
@ 2018-07-06 20:52       ` Dan Williams
  2018-07-11 18:44         ` Vishal Verma
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Williams @ 2018-07-06 20:52 UTC (permalink / raw)
  To: Verma, Vishal L; +Cc: linux-nvdimm

On Fri, Jul 6, 2018 at 1:32 PM, Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> On Fri, 2018-07-06 at 13:30 -0700, Dan Williams wrote:
>> On Fri, Jul 6, 2018 at 1:26 PM, Verma, Vishal L
>> <vishal.l.verma@intel.com> wrote:
>> >
>> > On Wed, 2018-07-04 at 14:04 -0700, Dan Williams wrote:
>> > > Commit 11b349b5236e "ndctl, test: Disable poison tests for now"
>> > > inadvertently prevented the dax.sh test from ever succeeding.
>> > > Revert
>> > > the
>> > > change to return $rc instead of 0.
>> > >
>> > > Fixes: 11b349b5236e ("ndctl, test: Disable poison tests for now")
>> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> > > ---
>> > >  test/dax.sh |    2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/test/dax.sh b/test/dax.sh
>> > > index 30fe16721935..2a82281edb35 100755
>> > > --- a/test/dax.sh
>> > > +++ b/test/dax.sh
>> > > @@ -90,4 +90,4 @@ json=$($NDCTL create-namespace -m raw -f -e
>> > > $dev)
>> > >  eval $(echo $json | sed -e "$json2var")
>> > >  [ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1
>> > >
>> > > -exit $rc
>> > > +exit 0
>> >
>> > I'm not sure I understand why this will cause it to always fail. rc
>> > is
>> > set in run_test, and that takes care of erroring out when the test
>> > returns anything other than 0 or 77. In case of 0 or 77, it falls
>> > through. So in the end here, we do want 'exit $rc' so we propagate
>> > the
>> > 77 correctly. Am I missing something?
>>
>> rc=1
>>
>> https://github.com/pmem/ndctl/blob/master/test/dax.sh#L47
>
> Yes, but that gets overriden by:
> https://github.com/pmem/ndctl/blob/master/test/dax.sh#L33

Hmm not sure. I instrumented dax-pmd.c and the shell script to dump
the rc value:

    test_dax_poison: rc: 0
    rc: 1
    FAIL dax.sh (exit status: 1)

diff --git a/test/dax-poison.c b/test/dax-poison.c
index a25bf0b17d61..3cf2c108ca81 100644
--- a/test/dax-poison.c
+++ b/test/dax-poison.c
@@ -145,7 +145,9 @@ clear_error:
        x = *(volatile unsigned *) addr + align / 2;
        rc = 0;

+       fprintf(stderr, "%s: rc: %d\n", __func__, rc);
 out:
+       fprintf(stderr, "%s: rc: %d\n", __func__, rc);
        if (addr != MAP_FAILED)
                munmap(addr, 2 * align);
        free(buf);
diff --git a/test/dax.sh b/test/dax.sh
index 2a82281edb35..016fea5bb0bf 100755
--- a/test/dax.sh
+++ b/test/dax.sh
@@ -90,4 +90,5 @@ json=$($NDCTL create-namespace -m raw -f -e $dev)
 eval $(echo $json | sed -e "$json2var")
 [ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1

-exit 0
+echo "rc: $rc"
+exit $rc
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [ndctl PATCH] ndctl, test: Fix dax.sh return code
  2018-07-06 20:52       ` Dan Williams
@ 2018-07-11 18:44         ` Vishal Verma
  0 siblings, 0 replies; 6+ messages in thread
From: Vishal Verma @ 2018-07-11 18:44 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-nvdimm

On 07/06, Dan Williams wrote:
> On Fri, Jul 6, 2018 at 1:32 PM, Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
> >

[..]

> > Yes, but that gets overriden by:
> > https://github.com/pmem/ndctl/blob/master/test/dax.sh#L33
> 
> Hmm not sure. I instrumented dax-pmd.c and the shell script to dump
> the rc value:
> 
>     test_dax_poison: rc: 0
>     rc: 1
>     FAIL dax.sh (exit status: 1)
> 
> diff --git a/test/dax-poison.c b/test/dax-poison.c
> index a25bf0b17d61..3cf2c108ca81 100644
> --- a/test/dax-poison.c
> +++ b/test/dax-poison.c
> @@ -145,7 +145,9 @@ clear_error:
>         x = *(volatile unsigned *) addr + align / 2;
>         rc = 0;
> 
> +       fprintf(stderr, "%s: rc: %d\n", __func__, rc);
>  out:
> +       fprintf(stderr, "%s: rc: %d\n", __func__, rc);
>         if (addr != MAP_FAILED)
>                 munmap(addr, 2 * align);
>         free(buf);
> diff --git a/test/dax.sh b/test/dax.sh
> index 2a82281edb35..016fea5bb0bf 100755
> --- a/test/dax.sh
> +++ b/test/dax.sh
> @@ -90,4 +90,5 @@ json=$($NDCTL create-namespace -m raw -f -e $dev)
>  eval $(echo $json | sed -e "$json2var")
>  [ $mode != "fsdax" ] && echo "fail: $LINENO" &&  exit 1
> 
> -exit 0
> +echo "rc: $rc"
> +exit $rc

I went with the following since the original patch could potentially
lose an rc=77 status.

8<----


>From 5f6e14c827c068ad892b4451844b4ad2135f2696 Mon Sep 17 00:00:00 2001
From: Vishal Verma <vishal.l.verma@intel.com>
Date: Fri, 6 Jul 2018 15:55:43 -0600
Subject: [ndctl PATCH] ndctl, test: Fix dax.sh return code

Commit 11b349b5236e "ndctl, test: Disable poison tests for now"
inadvertently prevented the dax.sh test from ever succeeding. Make sure
the test function actually overrides rc every time instead of only when
there is an error so we don't propagate the initial rc=1 all the way to
the end.

Fixes: 11b349b5236e ("ndctl, test: Disable poison tests for now")
Based-on-patch-by: Dan Williams <dan.j.williams@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 test/dax.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/dax.sh b/test/dax.sh
index 30fe167..b63d563 100755
--- a/test/dax.sh
+++ b/test/dax.sh
@@ -29,6 +29,7 @@ err() {
 }
 
 run_test() {
+	rc=0
 	if ! ./dax-pmd $MNT/$FILE; then
 		rc=$?
 		if [ $rc -ne 77 -a $rc -ne 0 ]; then
-- 
2.14.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-11 18:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-04 21:04 [ndctl PATCH] ndctl, test: Fix dax.sh return code Dan Williams
2018-07-06 20:26 ` Verma, Vishal L
2018-07-06 20:30   ` Dan Williams
2018-07-06 20:32     ` Verma, Vishal L
2018-07-06 20:52       ` Dan Williams
2018-07-11 18:44         ` Vishal Verma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).