Re: [PATCH ndctl RESEND 1/2] ndctl/test: Add destroy region test

[Ira Weiny <ira.weiny@intel.com>]

Verma, Vishal L wrote:

[snip]

> > > > +# Find a memory device to create regions on to test the destroy
> > > > +readarray -t mems < <("$CXL" list -b cxl_test -M | jq -r '.[].memdev')
> > > > +for mem in ${mems[@]}; do
> > > > +        ramsize=$($CXL list -m $mem | jq -r '.[].ram_size')
> > > > +        if [ "$ramsize" == "null" ]; then
> > > > +                continue
> > > > +        fi
> > > > +        decoder=$($CXL list -b cxl_test -D -d root -m "$mem" |
> > > > +                  jq -r ".[] |
> > > > +                  select(.volatile_capable == true) |
> > > > +                  select(.nr_targets == 1) |
> > > > +                  select(.size >= ${ramsize}) |
> > > > +                  .decoder")
> > > > +        if [[ $decoder ]]; then
> > > > +               check_destroy_ram $mem $decoder
> > > > +               check_destroy_devdax $mem $decoder
> > > > +                break
> > > > +        fi
> > > > +done
> > > 
> > > Does this need to check results of the region disable & destroy?
> > > 
> > > Did the regression this is after leave a trace in the dmesg log,
> > > so checking that is all that's needed?
> > > 
> > 
> > The regression causes
> > 
> >         check_destroy_devdax()
> >                 $CXL disable-region $region
> > 
> > to fail.  That command failure will exit with an error which causes the
> > test script to exit with that error as well.
> > 
> > At least that is what happened when I used this to test the fix.  I'll
> > defer to Vishal if there is a more explicit or better way to check for
> > that cxl-cli command to fail.
> > 
> Correct, the set -e will cause the script to abort with an error exit
> code whenever a command fails.
> 
> I do wonder if we need this new test - with Dave's patch here[1],

I'm not sure.

> destroy-region and disable-region both use the same helper that
> performs the libdaxctl checks.
> 
> cxl-create-region.sh already has flows that create a region and then
> destroy it. Those should now cover this case as well yeah?

I thought it would have but I don't think it covers the case where the dax
device is not system ram (the default when creating a region).

Ira