ltp.lists.linux.it archive mirror
 help / color / mirror / Atom feed
* [LTP] [PATCH v3] safe_mount: safe_umount: print debug info about the operation
@ 2023-06-15  1:24 Murphy Zhou
  2023-06-16  2:41 ` Li Wang
  2023-06-16  2:50 ` Li Wang
  0 siblings, 2 replies; 6+ messages in thread
From: Murphy Zhou @ 2023-06-15  1:24 UTC (permalink / raw)
  To: ltp

Make the source and the target to mount/umount visible. It's
good for debugging.

Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
---
v3:
  Apply suggestions from Li and Martin. Thanks very much!

 lib/safe_macros.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index af6dd0716..26f9136af 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -898,7 +898,16 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
 	       const void *data)
 {
 	int rval = -1;
+	char mpath[PATH_MAX];
 
+	if (realpath(target, mpath)) {
+		tst_resm_(file, lineno, TINFO,
+			"Mounting %s to %s fstyp=%s flags=%lx",
+			source, mpath, filesystemtype, mountflags);
+	} else {
+		tst_resm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Cannot resolve the absolute path of %s", target);
+	}
 	/*
 	 * Don't try using the kernel's NTFS driver when mounting NTFS, since
 	 * the kernel's NTFS driver doesn't have proper write support.
@@ -948,6 +957,14 @@ int safe_umount(const char *file, const int lineno, void (*cleanup_fn)(void),
 		const char *target)
 {
 	int rval;
+	char mpath[PATH_MAX];
+
+	if (realpath(target, mpath)) {
+		tst_resm_(file, lineno, TINFO, "Umounting %s", mpath);
+	} else {
+		tst_resm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"Cannot resolve the absolute path of %s", target);
+	}
 
 	rval = tst_umount(target);
 
-- 
2.31.1


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH v3] safe_mount: safe_umount: print debug info about the operation
  2023-06-15  1:24 [LTP] [PATCH v3] safe_mount: safe_umount: print debug info about the operation Murphy Zhou
@ 2023-06-16  2:41 ` Li Wang
  2023-06-16  2:50 ` Li Wang
  1 sibling, 0 replies; 6+ messages in thread
From: Li Wang @ 2023-06-16  2:41 UTC (permalink / raw)
  To: Murphy Zhou; +Cc: ltp

Patch applied, thanks!

-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH v3] safe_mount: safe_umount: print debug info about the operation
  2023-06-15  1:24 [LTP] [PATCH v3] safe_mount: safe_umount: print debug info about the operation Murphy Zhou
  2023-06-16  2:41 ` Li Wang
@ 2023-06-16  2:50 ` Li Wang
  2023-06-16  8:38   ` Martin Doucha
  1 sibling, 1 reply; 6+ messages in thread
From: Li Wang @ 2023-06-16  2:50 UTC (permalink / raw)
  To: Murphy Zhou; +Cc: ltp

On Thu, Jun 15, 2023 at 9:24 AM Murphy Zhou <jencce.kernel@gmail.com> wrote:

> Make the source and the target to mount/umount visible. It's
> good for debugging.
>
> Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> ---
> v3:
>   Apply suggestions from Li and Martin. Thanks very much!
>
>  lib/safe_macros.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> index af6dd0716..26f9136af 100644
> --- a/lib/safe_macros.c
> +++ b/lib/safe_macros.c
> @@ -898,7 +898,16 @@ int safe_mount(const char *file, const int lineno,
> void (*cleanup_fn)(void),
>                const void *data)
>  {
>         int rval = -1;
> +       char mpath[PATH_MAX];
>
> +       if (realpath(target, mpath)) {
> +               tst_resm_(file, lineno, TINFO,
> +                       "Mounting %s to %s fstyp=%s flags=%lx",
> +                       source, mpath, filesystemtype, mountflags);
> +       } else {
> +               tst_resm_(file, lineno, TBROK | TERRNO, cleanup_fn,
>

Forget to mention that, this should be tst_brkm_(.., TBROK | TERRNO, ...),
I manually fix it and pushed.



> +                       "Cannot resolve the absolute path of %s", target);
> +       }
>         /*
>          * Don't try using the kernel's NTFS driver when mounting NTFS,
> since
>          * the kernel's NTFS driver doesn't have proper write support.
> @@ -948,6 +957,14 @@ int safe_umount(const char *file, const int lineno,
> void (*cleanup_fn)(void),
>                 const char *target)
>  {
>         int rval;
> +       char mpath[PATH_MAX];
> +
> +       if (realpath(target, mpath)) {
> +               tst_resm_(file, lineno, TINFO, "Umounting %s", mpath);
> +       } else {
> +               tst_resm_(file, lineno, TBROK | TERRNO, cleanup_fn,
>

tst_brkm_()



> +                       "Cannot resolve the absolute path of %s", target);
> +       }
>
>         rval = tst_umount(target);
>
> --
> 2.31.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH v3] safe_mount: safe_umount: print debug info about the operation
  2023-06-16  2:50 ` Li Wang
@ 2023-06-16  8:38   ` Martin Doucha
  2023-06-16  9:23     ` Li Wang
  0 siblings, 1 reply; 6+ messages in thread
From: Martin Doucha @ 2023-06-16  8:38 UTC (permalink / raw)
  To: Li Wang, Murphy Zhou; +Cc: ltp

On 16. 06. 23 4:50, Li Wang wrote:
> On Thu, Jun 15, 2023 at 9:24 AM Murphy Zhou <jencce.kernel@gmail.com> wrote:
> 
>> Make the source and the target to mount/umount visible. It's
>> good for debugging.
>>
>> Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
>> ---
>> v3:
>>    Apply suggestions from Li and Martin. Thanks very much!
>>
>>   lib/safe_macros.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
>> index af6dd0716..26f9136af 100644
>> --- a/lib/safe_macros.c
>> +++ b/lib/safe_macros.c
>> @@ -898,7 +898,16 @@ int safe_mount(const char *file, const int lineno,
>> void (*cleanup_fn)(void),
>>                 const void *data)
>>   {
>>          int rval = -1;
>> +       char mpath[PATH_MAX];
>>
>> +       if (realpath(target, mpath)) {
>> +               tst_resm_(file, lineno, TINFO,
>> +                       "Mounting %s to %s fstyp=%s flags=%lx",
>> +                       source, mpath, filesystemtype, mountflags);
>> +       } else {
>> +               tst_resm_(file, lineno, TBROK | TERRNO, cleanup_fn,
>>
> 
> Forget to mention that, this should be tst_brkm_(.., TBROK | TERRNO, ...),
> I manually fix it and pushed.

I thought my review of the v2 patch was clear enough that both branches 
of this condition should call tst_resm_(... TINFO ...). There is no 
reason to fail the test here.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH v3] safe_mount: safe_umount: print debug info about the operation
  2023-06-16  8:38   ` Martin Doucha
@ 2023-06-16  9:23     ` Li Wang
  2023-06-16 12:27       ` Murphy Zhou
  0 siblings, 1 reply; 6+ messages in thread
From: Li Wang @ 2023-06-16  9:23 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

On Fri, Jun 16, 2023 at 4:38 PM Martin Doucha <mdoucha@suse.cz> wrote:

> On 16. 06. 23 4:50, Li Wang wrote:
> > On Thu, Jun 15, 2023 at 9:24 AM Murphy Zhou <jencce.kernel@gmail.com>
> wrote:
> >
> >> Make the source and the target to mount/umount visible. It's
> >> good for debugging.
> >>
> >> Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> >> ---
> >> v3:
> >>    Apply suggestions from Li and Martin. Thanks very much!
> >>
> >>   lib/safe_macros.c | 17 +++++++++++++++++
> >>   1 file changed, 17 insertions(+)
> >>
> >> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> >> index af6dd0716..26f9136af 100644
> >> --- a/lib/safe_macros.c
> >> +++ b/lib/safe_macros.c
> >> @@ -898,7 +898,16 @@ int safe_mount(const char *file, const int lineno,
> >> void (*cleanup_fn)(void),
> >>                 const void *data)
> >>   {
> >>          int rval = -1;
> >> +       char mpath[PATH_MAX];
> >>
> >> +       if (realpath(target, mpath)) {
> >> +               tst_resm_(file, lineno, TINFO,
> >> +                       "Mounting %s to %s fstyp=%s flags=%lx",
> >> +                       source, mpath, filesystemtype, mountflags);
> >> +       } else {
> >> +               tst_resm_(file, lineno, TBROK | TERRNO, cleanup_fn,
> >>
> >
> > Forget to mention that, this should be tst_brkm_(.., TBROK | TERRNO,
> ...),
> > I manually fix it and pushed.
>
> I thought my review of the v2 patch was clear enough that both branches
> of this condition should call tst_resm_(... TINFO ...). There is no
> reason to fail the test here.
>

Ah, sorry for overlooking that. I saw here that used tst_resm_( ...TBRK ...)
which is not the correct usage. So I guess he might want tst_brkm_(),
but anyway you are right. Let's fix it in a separate patch.


-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH v3] safe_mount: safe_umount: print debug info about the operation
  2023-06-16  9:23     ` Li Wang
@ 2023-06-16 12:27       ` Murphy Zhou
  0 siblings, 0 replies; 6+ messages in thread
From: Murphy Zhou @ 2023-06-16 12:27 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

My bad.. I should have changed it to TINFO|TERRNO.. Thanks all!

On Fri, Jun 16, 2023 at 5:23 PM Li Wang <liwang@redhat.com> wrote:
>
>
>
> On Fri, Jun 16, 2023 at 4:38 PM Martin Doucha <mdoucha@suse.cz> wrote:
>>
>> On 16. 06. 23 4:50, Li Wang wrote:
>> > On Thu, Jun 15, 2023 at 9:24 AM Murphy Zhou <jencce.kernel@gmail.com> wrote:
>> >
>> >> Make the source and the target to mount/umount visible. It's
>> >> good for debugging.
>> >>
>> >> Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
>> >> ---
>> >> v3:
>> >>    Apply suggestions from Li and Martin. Thanks very much!
>> >>
>> >>   lib/safe_macros.c | 17 +++++++++++++++++
>> >>   1 file changed, 17 insertions(+)
>> >>
>> >> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
>> >> index af6dd0716..26f9136af 100644
>> >> --- a/lib/safe_macros.c
>> >> +++ b/lib/safe_macros.c
>> >> @@ -898,7 +898,16 @@ int safe_mount(const char *file, const int lineno,
>> >> void (*cleanup_fn)(void),
>> >>                 const void *data)
>> >>   {
>> >>          int rval = -1;
>> >> +       char mpath[PATH_MAX];
>> >>
>> >> +       if (realpath(target, mpath)) {
>> >> +               tst_resm_(file, lineno, TINFO,
>> >> +                       "Mounting %s to %s fstyp=%s flags=%lx",
>> >> +                       source, mpath, filesystemtype, mountflags);
>> >> +       } else {
>> >> +               tst_resm_(file, lineno, TBROK | TERRNO, cleanup_fn,
>> >>
>> >
>> > Forget to mention that, this should be tst_brkm_(.., TBROK | TERRNO, ...),
>> > I manually fix it and pushed.
>>
>> I thought my review of the v2 patch was clear enough that both branches
>> of this condition should call tst_resm_(... TINFO ...). There is no
>> reason to fail the test here.
>
>
> Ah, sorry for overlooking that. I saw here that used tst_resm_( ...TBRK ...)
> which is not the correct usage. So I guess he might want tst_brkm_(),
> but anyway you are right. Let's fix it in a separate patch.
>
>
> --
> Regards,
> Li Wang

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-06-16 12:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-15  1:24 [LTP] [PATCH v3] safe_mount: safe_umount: print debug info about the operation Murphy Zhou
2023-06-16  2:41 ` Li Wang
2023-06-16  2:50 ` Li Wang
2023-06-16  8:38   ` Martin Doucha
2023-06-16  9:23     ` Li Wang
2023-06-16 12:27       ` Murphy Zhou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).