[RFC,1/8] lib/tst_test.c: mntpoint implies tmpdir

If mntpoint is set in the test structure we create a directory hence it
should set needs_tmpdir flag so that we are sure we create directory in
the right place.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 lib/tst_test.c | 3 +++
 1 file changed, 3 insertions(+)

On 2018/04/05 22:01, Cyril Hrubis wrote:
> If mntpoint is set in the test structure we create a directory hence it
> should set needs_tmpdir flag so that we are sure we create directory in
> the right place.
>
> Signed-off-by: Cyril Hrubis<chrubis@suse.cz>
> ---
>   lib/tst_test.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 8be13327c..5e10460b0 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -743,6 +743,9 @@ static void do_setup(int argc, char *argv[])
>   	if (tst_test->all_filesystems)
>   		tst_test->needs_device = 1;
>
> +	if (tst_test->mntpoint)
> +		tst_test->needs_tmpdir = 1;
> +
Hi Cyril,

Setting mntpoint made needs_device flag true, and subsequent 
needs_tmpdir() returned true
and still created a directory which is used to be mounted.  So it seems 
unnecessary to set
needs_tmpdir flag.

Thanks,
Xiao Yang
>   	setup_ipc();
>
>   	if (needs_tmpdir()&&  !tst_tmpdir_created())

Hi!
> Setting mntpoint made needs_device flag true, and subsequent 
> needs_tmpdir() returned true
> and still created a directory which is used to be mounted.  So it seems 
> unnecessary to set
> needs_tmpdir flag.

Ah, right, we do have the condition up there, but it does not work for
needs_rofs for example that utilizes mntpoint as well.

So the correct patch should be:

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 8be13327c..05ba95e2e 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -638,6 +638,7 @@ static int needs_tmpdir(void)
 {
        return tst_test->needs_tmpdir ||
               tst_test->needs_device ||
+              tst_test->mntpoint ||
               tst_test->resource_files ||
               tst_test->needs_checkpoints;
 }

Which will ensure that the test library will not create files outside of
the test temporary directory.

Thanks for pointing it out!

On 2018/04/10 17:14, Cyril Hrubis wrote:
> Hi!
>> Setting mntpoint made needs_device flag true, and subsequent
>> needs_tmpdir() returned true
>> and still created a directory which is used to be mounted.  So it seems
>> unnecessary to set
>> needs_tmpdir flag.
> Ah, right, we do have the condition up there, but it does not work for
> needs_rofs for example that utilizes mntpoint as well.
>
> So the correct patch should be:
>
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 8be13327c..05ba95e2e 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -638,6 +638,7 @@ static int needs_tmpdir(void)
>   {
>          return tst_test->needs_tmpdir ||
>                 tst_test->needs_device ||
> +              tst_test->mntpoint ||
>                 tst_test->resource_files ||
>                 tst_test->needs_checkpoints;
>   }
>
> Which will ensure that the test library will not create files outside of
> the test temporary directory.
Hi Cyril,

For needs_rofs flag, this patch looks good to me.

Besides, can we repalce needs_device with mount_device in needs_tmpdir() ?
I think just needs_device flag needn't create a temporary directory(e.g. 
ioctl06).

Thanks,
Xiao Yang
> Thanks for pointing it out!
>

Hi!
> > Ah, right, we do have the condition up there, but it does not work for
> > needs_rofs for example that utilizes mntpoint as well.
> >
> > So the correct patch should be:
> >
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 8be13327c..05ba95e2e 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -638,6 +638,7 @@ static int needs_tmpdir(void)
> >   {
> >          return tst_test->needs_tmpdir ||
> >                 tst_test->needs_device ||
> > +              tst_test->mntpoint ||
> >                 tst_test->resource_files ||
> >                 tst_test->needs_checkpoints;
> >   }
> >
> > Which will ensure that the test library will not create files outside of
> > the test temporary directory.
> Hi Cyril,
> 
> For needs_rofs flag, this patch looks good to me.
> 
> Besides, can we repalce needs_device with mount_device in needs_tmpdir() ?
> I think just needs_device flag needn't create a temporary directory(e.g. 
> ioctl06).

Actually we can't since unless user passes path to a real device via the
LTP_DEV env variable the loop device is backed by a temporary file that
has to be created somewhere. See the tst_device.c and the
tst_acquire_device__() function.