Re: [LTP] [PATCH v2] tst_mkfs: print short hint when mkfs fails in device busy

[Petr Vorel <pvorel@suse.cz>]

> Petr Vorel <pvorel@suse.cz> wrote:

> > > diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c
> > > index 736324f04..dfec06a21 100644
> > > --- a/lib/tst_mkfs.c
> > > +++ b/lib/tst_mkfs.c
> > > @@ -107,6 +107,10 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
> > >                       "%s not found in $PATH", mkfs);
> > >       break;
> > >       default:
> > > +             tst_resm_(file, lineno, TWARN,
> > > +                     "Check if mkfs failed with the (loop) device busy. "
> > > +                     "Background probing (e.g., udisks2) can cause this. \n"
> > When 2 lines are needed, could it be without trailing space?
> >                         "Background probing (e.g., udisks2) can cause this.\n"

> > nit: I don't like LTP messages are split on more lines, but this is indeed too
> > long.

> +1

> > > +                     "Consider temporarily stopping udisks2 during the test.");

> > We already have suggestion about some process running in tst_umount()
> > in lib/tst_device.c:

> >                         tst_resm(TINFO, "Likely gvfsd-trash is probing newly "
> >                                  "mounted fs, kill it to speed up tests.");

> This is a good example, thanks!!

> (The debugging work exhausted my energy this week, and I felt like my
> brain was stuck there, unable to write a simple printed message.)

Understand :).

> So what about:

>  tst_resm_(file, lineno, TWARN,
>     "mkfs may have failed because the device is busy (e.g., udisks2 probing). "
>     "Consider disabling background probing services.");

+1

> > Would you agree to have a library function which would parse
> > /proc/*/comm to check if particular process is running?
> > (I would do it as a separate effort).

> Yes, that would be helpful to confirm some special damon is running,
> if something you were mean:

> if (tst_proc_comm_running("udisksd", NULL) ||
>     tst_proc_comm_running("gvfsd-trash", NULL)) {
>     tst_resm_(file, lineno, TINFO,
>               "Likely udisksd/gvfsd-trash is probing newly mounted fs
> in backgroud... ");
> }

Yes, something like this. It can be done as a separate effort to not block this
simple patch.

Kind regards,
Petr

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