On Wed, Jan 16, 2019 at 06:31:51PM -0800, Bart Van Assche wrote:
> On 1/16/19 5:40 PM, Omar Sandoval wrote:
> > On Tue, Jan 15, 2019 at 08:40:41AM -0800, Bart Van Assche wrote:
> > > On Tue, 2019-01-01 at 19:13 -0800, Bart Van Assche wrote:
> > > > On 12/4/18 9:47 AM, Josef Bacik wrote:
> > > > > In order to test io.latency and other cgroup related things we need 
> > > > > some
> > > > > supporting helpers to setup and tear down cgroup2.  This adds support
> > > > > for checking that we can even configure cgroup2 things, set them up if
> > > > > need be, and then add the cleanup stuff to the main cleanup function 
> > > > > so
> > > > > everything is always in a clean state.
> > > > 
> > > > Is this the patch that went in as commit ae7daae7e35a ("blktests: add
> > > > cgroup2 infrastructure")? I think that commit introduced a regression.
> > > > With that patch applied the SRP tests fail as follows:
> > > > 
> > > > # ./check -q srp/001
> > > > srp/001 (Create and remove LUNs)
> > > >       runtime  4.067s  ...
> > > > common/cgroup: line 25: CGROUP2_DIR: unbound variable
> > > > 
> > > > Is this a known issue?
> > > 
> > > Hi Josef,
> > > 
> > > Had you noticed this e-mail?
> > > 
> > > Thanks,
> > > 
> > > Bart.
> > 
> > Hey, Bart, I just pushed a fix for this:
> > 
> > commit 8a274578e2895b9f0b66c09f3a8f63b5ff1293b2
> > Author: Omar Sandoval <[email protected]>
> > Date:   Wed Jan 16 17:34:19 2019 -0800
> > 
> >      cgroup: test if CGROUP2_DIR is set with -v instead of -n
> >      common/multipath-over-rdma does set -u, so -n "$CGROUP2_DIR" fails with
> >      an unbound variable error. Instead, use -v to test if the variable was
> >      set.
> >      Signed-off-by: Omar Sandoval <[email protected]>
> > 
> > diff --git a/common/cgroup b/common/cgroup
> > index 48e546f..554ebf7 100644
> > --- a/common/cgroup
> > +++ b/common/cgroup
> > @@ -22,7 +22,7 @@ _init_cgroup2()
> >   _exit_cgroup2()
> >   {
> > -   if [[ -n $CGROUP2_DIR ]]; then
> > +   if [[ -v CGROUP2_DIR ]]; then
> >             find "$CGROUP2_DIR" -type d -delete
> >             unset CGROUP2_DIR
> >     fi
> 
> That change looks good to me. Thanks!
> 

Hmm sorry Bart, I wasn't trying to ignore you, your email ended up in a folder
for some reason and I didn't see this until Omar sent his patch.  Thanks,

Josef

Reply via email to