* [PATCH v2 0/2] Improve user experience with xfs_fsr @ 2026-03-17 16:07 cem 2026-03-17 16:07 ` [PATCH v2 1/2] fsr: package function should check for negative errors cem 2026-03-17 16:07 ` [PATCH v2 2/2] fsr: always print error messages from xfrog_defragrange() cem 0 siblings, 2 replies; 5+ messages in thread From: cem @ 2026-03-17 16:07 UTC (permalink / raw) To: aalbersh; +Cc: linux-xfs, djwong From: Carlos Maiolino <cem@kernel.org> This is the V2 of this series (my apologies for the delay), based on an user report via IRC in January. V2 specific changes are detailed on each patch, but mostly patch 1 was changed. Patch 2 only the description Carlos Maiolino (2): fsr: package function should check for negative errors fsr: always print error messages from xfrog_defragrange() fsr/xfs_fsr.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) -- 2.53.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] fsr: package function should check for negative errors 2026-03-17 16:07 [PATCH v2 0/2] Improve user experience with xfs_fsr cem @ 2026-03-17 16:07 ` cem 2026-03-17 22:12 ` Darrick J. Wong 2026-03-17 16:07 ` [PATCH v2 2/2] fsr: always print error messages from xfrog_defragrange() cem 1 sibling, 1 reply; 5+ messages in thread From: cem @ 2026-03-17 16:07 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. Update packfile() to switch between negative errors. Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> --- V2: Change packfile() to check for negative error values instead of changing the return sign from xfrog_defragrange(). fsr/xfs_fsr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c index 8845ff172fcb..aa2a10839314 100644 --- a/fsr/xfs_fsr.c +++ b/fsr/xfs_fsr.c @@ -1463,17 +1463,17 @@ packfile( switch (error) { case 0: break; - case ENOTSUP: + case -ENOTSUP: if (vflag || dflag) fsrprintf(_("%s: file type not supported\n"), fname); break; - case EFAULT: + case -EFAULT: /* The file has changed since we started the copy */ if (vflag || dflag) fsrprintf(_("%s: file modified defrag aborted\n"), fname); break; - case EBUSY: + case -EBUSY: /* Timestamp has changed or mmap'ed file */ if (vflag || dflag) fsrprintf(_("%s: file busy\n"), fname); -- 2.53.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] fsr: package function should check for negative errors 2026-03-17 16:07 ` [PATCH v2 1/2] fsr: package function should check for negative errors cem @ 2026-03-17 22:12 ` Darrick J. Wong 0 siblings, 0 replies; 5+ messages in thread From: Darrick J. Wong @ 2026-03-17 22:12 UTC (permalink / raw) To: cem; +Cc: aalbersh, linux-xfs On Tue, Mar 17, 2026 at 05:07:39PM +0100, 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. > > Update packfile() to switch between negative errors. > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > --- > V2: > Change packfile() to check for negative error values > instead of changing the return sign from xfrog_defragrange(). > > fsr/xfs_fsr.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c > index 8845ff172fcb..aa2a10839314 100644 > --- a/fsr/xfs_fsr.c > +++ b/fsr/xfs_fsr.c > @@ -1463,17 +1463,17 @@ packfile( > switch (error) { > case 0: > break; > - case ENOTSUP: > + case -ENOTSUP: I would have just made the call site invert the sign: error = -xfrog_defragrange(...); because you wouldn't have to change the case statements, and... > if (vflag || dflag) > fsrprintf(_("%s: file type not supported\n"), fname); > break; > - case EFAULT: > + case -EFAULT: > /* The file has changed since we started the copy */ > if (vflag || dflag) > fsrprintf(_("%s: file modified defrag aborted\n"), > fname); > break; > - case EBUSY: > + case -EBUSY: > /* Timestamp has changed or mmap'ed file */ > if (vflag || dflag) > fsrprintf(_("%s: file busy\n"), fname); ...the default case below is still broken due to the strerror() call: default: fsrprintf(_("XFS_IOC_SWAPEXT failed: %s: %s\n"), fname, strerror(error)); https://lore.kernel.org/linux-xfs/20260120172039.GO15551@frogsfrogsfrogs/ --D ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] fsr: always print error messages from xfrog_defragrange() 2026-03-17 16:07 [PATCH v2 0/2] Improve user experience with xfs_fsr cem 2026-03-17 16:07 ` [PATCH v2 1/2] fsr: package function should check for negative errors cem @ 2026-03-17 16:07 ` cem 2026-03-17 22:12 ` Darrick J. Wong 1 sibling, 1 reply; 5+ messages in thread From: cem @ 2026-03-17 16:07 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> --- V2: Update commit description fsr/xfs_fsr.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c index aa2a10839314..151f8ba42380 100644 --- a/fsr/xfs_fsr.c +++ b/fsr/xfs_fsr.c @@ -1464,19 +1464,15 @@ packfile( case 0: break; case -ENOTSUP: - if (vflag || dflag) - fsrprintf(_("%s: file type not supported\n"), fname); + 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] 5+ messages in thread
* Re: [PATCH v2 2/2] fsr: always print error messages from xfrog_defragrange() 2026-03-17 16:07 ` [PATCH v2 2/2] fsr: always print error messages from xfrog_defragrange() cem @ 2026-03-17 22:12 ` Darrick J. Wong 0 siblings, 0 replies; 5+ messages in thread From: Darrick J. Wong @ 2026-03-17 22:12 UTC (permalink / raw) To: cem; +Cc: aalbersh, linux-xfs On Tue, Mar 17, 2026 at 05:07:40PM +0100, 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> Still Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> https://lore.kernel.org/linux-xfs/20260120172355.GP15551@frogsfrogsfrogs/ --D > --- > V2: Update commit description > > fsr/xfs_fsr.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c > index aa2a10839314..151f8ba42380 100644 > --- a/fsr/xfs_fsr.c > +++ b/fsr/xfs_fsr.c > @@ -1464,19 +1464,15 @@ packfile( > case 0: > break; > case -ENOTSUP: > - if (vflag || dflag) > - fsrprintf(_("%s: file type not supported\n"), fname); > + 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 [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-17 22:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-17 16:07 [PATCH v2 0/2] Improve user experience with xfs_fsr cem 2026-03-17 16:07 ` [PATCH v2 1/2] fsr: package function should check for negative errors cem 2026-03-17 22:12 ` Darrick J. Wong 2026-03-17 16:07 ` [PATCH v2 2/2] fsr: always print error messages from xfrog_defragrange() cem 2026-03-17 22:12 ` Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox