* [PATCH v3 0/2] Improve user experience with xfs_fsr
@ 2026-03-31 12:26 cem
2026-03-31 12:26 ` [PATCH v3 1/2] fsr: package function should check for negative errors cem
2026-03-31 12:26 ` [PATCH v3 2/2] fsr: always print error messages from xfrog_defragrange() cem
0 siblings, 2 replies; 6+ messages in thread
From: cem @ 2026-03-31 12:26 UTC (permalink / raw)
To: aalbersh; +Cc: linux-xfs, djwong
From: Carlos Maiolino <cem@kernel.org>
This is the V3 of this series,
this fixes an issue based on an user report via IRC in January.
Patch 1 implement changes suggested by Darric, while patch 2 has no
change other than being rebased on top of patch 1
Carlos Maiolino (2):
fsr: package function should check for negative errors
fsr: always print error messages from xfrog_defragrange()
fsr/xfs_fsr.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] fsr: package function should check for negative errors
2026-03-31 12:26 [PATCH v3 0/2] Improve user experience with xfs_fsr cem
@ 2026-03-31 12:26 ` cem
2026-03-31 16:08 ` Darrick J. Wong
2026-03-31 12:26 ` [PATCH v3 2/2] fsr: always print error messages from xfrog_defragrange() cem
1 sibling, 1 reply; 6+ messages in thread
From: cem @ 2026-03-31 12:26 UTC (permalink / raw)
To: aalbersh; +Cc: linux-xfs, djwong
From: Carlos Maiolino <cem@kernel.org>
xfrog_defragrange as most other functions from libfrog return
a negative error value, while xfs_fsr's packfile(), expects
a positive error value.
Whenever xfrog_defragrange fails, the switch case always falls into the
default clausule, making the error message pointless.
Fix this by inverting xfrog_defragrange() return value call.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V3:
Just change xfrog_defragrange() return signal.
V2:
Change packfile() to check for negative error values
instead of changing the return sign from xfrog_defragrange().
fsr/xfs_fsr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 8845ff172fcb..390f7c5276df 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -1459,7 +1459,7 @@ packfile(
}
/* Swap the extents */
- error = xfrog_defragrange(file_fd->fd, &xdf);
+ error = -xfrog_defragrange(file_fd->fd, &xdf);
switch (error) {
case 0:
break;
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] fsr: always print error messages from xfrog_defragrange()
2026-03-31 12:26 [PATCH v3 0/2] Improve user experience with xfs_fsr cem
2026-03-31 12:26 ` [PATCH v3 1/2] fsr: package function should check for negative errors cem
@ 2026-03-31 12:26 ` cem
2026-03-31 16:09 ` Darrick J. Wong
1 sibling, 1 reply; 6+ messages in thread
From: cem @ 2026-03-31 12:26 UTC (permalink / raw)
To: aalbersh; +Cc: linux-xfs, djwong
From: Carlos Maiolino <cem@kernel.org>
Error messages from xfrog_defragrange() are only printed when
verbose/debug flags are used.
We had reports from users complaining it's hard to find out the error
messages lost in the middle of dozens of other informational messages.
Change packfile() behavior to print errors independently of verbose or
debug flags.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V3: Rebase on top of patch 1 changes
V2: Update commit description
fsr/xfs_fsr.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 390f7c5276df..24617f55e329 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -1464,19 +1464,14 @@ packfile(
case 0:
break;
case ENOTSUP:
- if (vflag || dflag)
- fsrprintf(_("%s: file type not supported\n"), fname);
break;
case EFAULT:
/* The file has changed since we started the copy */
- if (vflag || dflag)
- fsrprintf(_("%s: file modified defrag aborted\n"),
- fname);
+ fsrprintf(_("%s: file modified defrag aborted\n"), fname);
break;
case EBUSY:
/* Timestamp has changed or mmap'ed file */
- if (vflag || dflag)
- fsrprintf(_("%s: file busy\n"), fname);
+ fsrprintf(_("%s: file busy\n"), fname);
break;
default:
fsrprintf(_("XFS_IOC_SWAPEXT failed: %s: %s\n"),
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] fsr: package function should check for negative errors
2026-03-31 12:26 ` [PATCH v3 1/2] fsr: package function should check for negative errors cem
@ 2026-03-31 16:08 ` Darrick J. Wong
0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2026-03-31 16:08 UTC (permalink / raw)
To: cem; +Cc: aalbersh, linux-xfs
On Tue, Mar 31, 2026 at 02:26:30PM +0200, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
>
> xfrog_defragrange as most other functions from libfrog return
> a negative error value, while xfs_fsr's packfile(), expects
> a positive error value.
>
> Whenever xfrog_defragrange fails, the switch case always falls into the
> default clausule, making the error message pointless.
> Fix this by inverting xfrog_defragrange() return value call.
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
>
> ---
> V3:
> Just change xfrog_defragrange() return signal.
Looks good to me, thanks for fixing this :)
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
>
> V2:
> Change packfile() to check for negative error values
> instead of changing the return sign from xfrog_defragrange().
>
> fsr/xfs_fsr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index 8845ff172fcb..390f7c5276df 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -1459,7 +1459,7 @@ packfile(
> }
>
> /* Swap the extents */
> - error = xfrog_defragrange(file_fd->fd, &xdf);
> + error = -xfrog_defragrange(file_fd->fd, &xdf);
> switch (error) {
> case 0:
> break;
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] fsr: always print error messages from xfrog_defragrange()
2026-03-31 12:26 ` [PATCH v3 2/2] fsr: always print error messages from xfrog_defragrange() cem
@ 2026-03-31 16:09 ` Darrick J. Wong
2026-04-07 11:09 ` Carlos Maiolino
0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2026-03-31 16:09 UTC (permalink / raw)
To: cem; +Cc: aalbersh, linux-xfs
On Tue, Mar 31, 2026 at 02:26:31PM +0200, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
>
> Error messages from xfrog_defragrange() are only printed when
> verbose/debug flags are used.
>
> We had reports from users complaining it's hard to find out the error
> messages lost in the middle of dozens of other informational messages.
>
> Change packfile() behavior to print errors independently of verbose or
> debug flags.
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
>
> ---
> V3: Rebase on top of patch 1 changes
> V2: Update commit description
>
> fsr/xfs_fsr.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index 390f7c5276df..24617f55e329 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -1464,19 +1464,14 @@ packfile(
> case 0:
> break;
> case ENOTSUP:
> - if (vflag || dflag)
> - fsrprintf(_("%s: file type not supported\n"), fname);
Err.. did you mean to delete the fsrprintf call here as well?
--D
> break;
> case EFAULT:
> /* The file has changed since we started the copy */
> - if (vflag || dflag)
> - fsrprintf(_("%s: file modified defrag aborted\n"),
> - fname);
> + fsrprintf(_("%s: file modified defrag aborted\n"), fname);
> break;
> case EBUSY:
> /* Timestamp has changed or mmap'ed file */
> - if (vflag || dflag)
> - fsrprintf(_("%s: file busy\n"), fname);
> + fsrprintf(_("%s: file busy\n"), fname);
> break;
> default:
> fsrprintf(_("XFS_IOC_SWAPEXT failed: %s: %s\n"),
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] fsr: always print error messages from xfrog_defragrange()
2026-03-31 16:09 ` Darrick J. Wong
@ 2026-04-07 11:09 ` Carlos Maiolino
0 siblings, 0 replies; 6+ messages in thread
From: Carlos Maiolino @ 2026-04-07 11:09 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: aalbersh, linux-xfs
On Tue, Mar 31, 2026 at 09:09:37AM -0700, Darrick J. Wong wrote:
> On Tue, Mar 31, 2026 at 02:26:31PM +0200, cem@kernel.org wrote:
> > From: Carlos Maiolino <cem@kernel.org>
> >
> > Error messages from xfrog_defragrange() are only printed when
> > verbose/debug flags are used.
> >
> > We had reports from users complaining it's hard to find out the error
> > messages lost in the middle of dozens of other informational messages.
> >
> > Change packfile() behavior to print errors independently of verbose or
> > debug flags.
> >
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> >
> > ---
> > V3: Rebase on top of patch 1 changes
> > V2: Update commit description
> >
> > fsr/xfs_fsr.c | 9 ++-------
> > 1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> > index 390f7c5276df..24617f55e329 100644
> > --- a/fsr/xfs_fsr.c
> > +++ b/fsr/xfs_fsr.c
> > @@ -1464,19 +1464,14 @@ packfile(
> > case 0:
> > break;
> > case ENOTSUP:
> > - if (vflag || dflag)
> > - fsrprintf(_("%s: file type not supported\n"), fname);
>
> Err.. did you mean to delete the fsrprintf call here as well?
No, I probably should have messed it up when rebasing on top of patch 1.
I'll fix it.
>
> --D
>
> > break;
> > case EFAULT:
> > /* The file has changed since we started the copy */
> > - if (vflag || dflag)
> > - fsrprintf(_("%s: file modified defrag aborted\n"),
> > - fname);
> > + fsrprintf(_("%s: file modified defrag aborted\n"), fname);
> > break;
> > case EBUSY:
> > /* Timestamp has changed or mmap'ed file */
> > - if (vflag || dflag)
> > - fsrprintf(_("%s: file busy\n"), fname);
> > + fsrprintf(_("%s: file busy\n"), fname);
> > break;
> > default:
> > fsrprintf(_("XFS_IOC_SWAPEXT failed: %s: %s\n"),
> > --
> > 2.53.0
> >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-07 11:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-31 12:26 [PATCH v3 0/2] Improve user experience with xfs_fsr cem
2026-03-31 12:26 ` [PATCH v3 1/2] fsr: package function should check for negative errors cem
2026-03-31 16:08 ` Darrick J. Wong
2026-03-31 12:26 ` [PATCH v3 2/2] fsr: always print error messages from xfrog_defragrange() cem
2026-03-31 16:09 ` Darrick J. Wong
2026-04-07 11:09 ` Carlos Maiolino
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox