Re: [Qemu-block] [PATCH 5/6] tests: add iotests helpers for dealing with TLS certificates

[Eric Blake]

On 11/19/18 5:04 AM, Max Reitz wrote:

> > > > +tls_dir="${TEST_DIR}/tls"
> > > > +
> > > > +function tls_x509_cleanup()
> > > > +{
> > > > +    rm -f ${tls_dir}/*.pem
> > > > +    rm -f ${tls_dir}/*/*.pem
> > > > +    rmdir ${tls_dir}/*
> > > > +    rmdir ${tls_dir}
> > >
> > > Why not just:
> > > rm -rf $tls_dir
> >
> > Yeah, I guess we could do that for simplicity
> >
> > > Also, the quoting is a bit inconsistent. if ${TEST_DIR} can contain spaces,
> > > then all uses of ${tls_dir} need to be in "".
> >
> > Hmm, yes.
>
> Which by the way is a very good reason *not* to blindly use "rm -r".

If we ever revive Jeff Cody's patches to let each test run with its own 
temporary directory, then we don't need this function at all.  With that 
in place, you either run the testsuite in debug mode (all temporary 
files preserved) or in normal mode (./check itself does rm -rf on the 
temporary directory).

> So far we only seem to have one instance of "rm -r" in the iotests (and
> that is on three files, so I don't even know why that has -r), and I'm
> glad about that.

But until we have the global implementation of per-test temporary 
directories with cleanup relegated to the testsuite driver, I'm fine 
following your preference of explicit deletion of specific files rather 
than recursive deletion of the tree, even if it is more lines of code.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org