* [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