[LTP] [PATCH 1/2] tst_device: Print info about acquiring device

[LTP] [PATCH 1/2] tst_device: Print info about acquiring device

[Petr Vorel]

Print how much space is being acquired. Also print hint on ENOSPC.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/tst_device.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/tst_device.c b/lib/tst_device.c
index d659b54cf..635b39d08 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -289,8 +289,10 @@ const char *tst_acquire_loop_device(unsigned int size, const char *filename)
 {
 	unsigned int acq_dev_size = size ? size : DEV_SIZE_MB;
 
+	tst_resm(TINFO, "Acquiring loop device %u MB", acq_dev_size);
 	if (tst_prealloc_file(filename, 1024 * 1024, acq_dev_size)) {
-		tst_resm(TWARN | TERRNO, "Failed to create %s", filename);
+		tst_resm(TWARN | TERRNO, "Failed to create %s%s", filename,
+			 errno == ENOSPC ? " (not enough space in $TMPDIR?)" : "");
 		return NULL;
 	}
 
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/2] tst_test: Don't print hints if failed to acquire

[Petr Vorel]

It does not make much sense to print hints when it failed to acquire the
device. It can be misleading.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/tst_test.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 8c212c983..834acda0e 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -88,6 +88,9 @@ int TST_ERR;
 int TST_PASS;
 long TST_RET;
 
+static struct tst_device tdev;
+struct tst_device *tst_device;
+
 static void do_cleanup(void);
 static void do_exit(int ret) __attribute__ ((noreturn));
 
@@ -883,7 +886,9 @@ static void do_exit(int ret)
 
 		if (results->broken) {
 			ret |= TBROK;
-			print_failure_hints();
+
+			if (!(tst_test->needs_device && !tdev.dev))
+				print_failure_hints();
 		}
 
 		fprintf(stderr, "\nSummary:\n");
@@ -965,9 +970,6 @@ static const char *get_tid(char *argv[])
 	return argv[0];
 }
 
-static struct tst_device tdev;
-struct tst_device *tst_device;
-
 static void assert_test_fn(void)
 {
 	int cnt = 0;
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] tst_test: Don't print hints if failed to acquire

[Cyril Hrubis]

Hi!
This is a great catch but I do not think that it should be limited to
the device init. We should distable the hints during the whole library
setup and only enable them once we actually start running the test.

I guess that we need:

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 190e8da2a..47d0ecf7d 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -61,6 +61,7 @@ static int mntpoint_mounted;
 static int ovl_mounted;
 static struct timespec tst_start_time; /* valid only for test pid */
 static int tdebug;
+static int show_failure_hints;

 struct results {
        int passed;
@@ -883,7 +884,8 @@ static void do_exit(int ret)

                if (results->broken) {
                        ret |= TBROK;
-                       print_failure_hints();
+                       if (show_failure_hints)
+                               print_failure_hints();
                }

                fprintf(stderr, "\nSummary:\n");
@@ -1740,6 +1742,8 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
        if (tst_test->test_variants)
                test_variants = tst_test->test_variants;

+       show_failure_hints = 1;
+
        for (tst_variant = 0; tst_variant < test_variants; tst_variant++) {
                if (tst_test->all_filesystems)
                        ret |= run_tcases_per_fs();



-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] tst_test: Don't print hints if failed to acquire

[Petr Vorel]

Hi Cyril,

> Hi!
> This is a great catch but I do not think that it should be limited to
> the device init. We should distable the hints during the whole library
> setup and only enable them once we actually start running the test.

+1. I suspected that, but did not have enough time to explore. Thanks for
raising this.

> I guess that we need:

I'll try to test it, hopefully next week. Or feel free to send your patch
and just put me in some tag (e.g.  Co-developed-by:, but not that important).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] tst_device: Print info about acquiring device

[Andrea Cervesato via ltp]

Hi!

Good idea

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

On 5/27/24 22:28, Petr Vorel wrote:
> Print how much space is being acquired. Also print hint on ENOSPC.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   lib/tst_device.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index d659b54cf..635b39d08 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -289,8 +289,10 @@ const char *tst_acquire_loop_device(unsigned int size, const char *filename)
>   {
>   	unsigned int acq_dev_size = size ? size : DEV_SIZE_MB;
>   
> +	tst_resm(TINFO, "Acquiring loop device %u MB", acq_dev_size);
>   	if (tst_prealloc_file(filename, 1024 * 1024, acq_dev_size)) {
> -		tst_resm(TWARN | TERRNO, "Failed to create %s", filename);
> +		tst_resm(TWARN | TERRNO, "Failed to create %s%s", filename,
> +			 errno == ENOSPC ? " (not enough space in $TMPDIR?)" : "");
>   		return NULL;
>   	}
>   

Andrea


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] tst_device: Print info about acquiring device

[Wei Gao via ltp]

On Tue, May 28, 2024 at 08:03:14AM +0200, Andrea Cervesato via ltp wrote:
> Hi!
> 
> Good idea
> 
> Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>
  Reviewed-by: Wei Gao <wegao@suse.com>
> 
> On 5/27/24 22:28, Petr Vorel wrote:
> > Print how much space is being acquired. Also print hint on ENOSPC.
> > 
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> >   lib/tst_device.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/tst_device.c b/lib/tst_device.c
> > index d659b54cf..635b39d08 100644
> > --- a/lib/tst_device.c
> > +++ b/lib/tst_device.c
> > @@ -289,8 +289,10 @@ const char *tst_acquire_loop_device(unsigned int size, const char *filename)
> >   {
> >   	unsigned int acq_dev_size = size ? size : DEV_SIZE_MB;
> > +	tst_resm(TINFO, "Acquiring loop device %u MB", acq_dev_size);
> >   	if (tst_prealloc_file(filename, 1024 * 1024, acq_dev_size)) {
> > -		tst_resm(TWARN | TERRNO, "Failed to create %s", filename);
> > +		tst_resm(TWARN | TERRNO, "Failed to create %s%s", filename,
> > +			 errno == ENOSPC ? " (not enough space in $TMPDIR?)" : "");
> >   		return NULL;
> >   	}
> 
> Andrea
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] tst_device: Print info about acquiring device

[Cyril Hrubis]

Hi!
> +	tst_resm(TINFO, "Acquiring loop device %u MB", acq_dev_size);
                         ^
			 Maybe "Preallocating device size=%uMB" ?

Since we are at the file preallocation step at this point in the code.

>  	if (tst_prealloc_file(filename, 1024 * 1024, acq_dev_size)) {
> -		tst_resm(TWARN | TERRNO, "Failed to create %s", filename);
> +		tst_resm(TWARN | TERRNO, "Failed to create %s%s", filename,
> +			 errno == ENOSPC ? " (not enough space in $TMPDIR?)" : "");

I guess that we could print the $TMPDIR value here as well with:

		tst_resm(TWARN | TERRNO, "Failed to create %s", filename);

		if (errno == ENOSPC)
			tst_resm(TWARN, "Not enough space at '%s'?", tst_get_tmpdir())


This leaks the memory though, that's because the tst_get_tmpdir() is ugly
function from the old library...

>  		return NULL;
>  	}
>  
> -- 
> 2.43.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp