* [PATCH 1/5] xfsrestore: remove unused variable strctxp
2026-02-24 7:17 [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG= Donald Douwsma
@ 2026-02-24 7:17 ` Donald Douwsma
2026-02-24 7:17 ` [PATCH 2/5] annotate variables only used for assert Donald Douwsma
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Donald Douwsma @ 2026-02-24 7:17 UTC (permalink / raw)
To: linux-xfs; +Cc: Donald Douwsma
Reviewing xfsdump warnings showed
content.c: In function ‘restore_extattr’:
content.c:8635:27: warning: unused variable ‘strctxp’ [-Wunused-variable]
8635 | stream_context_t *strctxp = (stream_context_t *)drivep->d_strmcontextp;
| ^~~~~~~
strctxp is never used in restore_extattr() remove it.
Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
---
restore/content.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/restore/content.c b/restore/content.c
index 7ec3a4d..b916e39 100644
--- a/restore/content.c
+++ b/restore/content.c
@@ -8632,7 +8632,6 @@ restore_extattr(drive_t *drivep,
{
drive_ops_t *dop = drivep->d_opsp;
extattrhdr_t *ahdrp = (extattrhdr_t *)get_extattrbuf(drivep->d_index);
- stream_context_t *strctxp = (stream_context_t *)drivep->d_strmcontextp;
bstat_t *bstatp = &fhdrp->fh_stat;
bool_t isfilerestored = BOOL_FALSE;
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/5] annotate variables only used for assert
2026-02-24 7:17 [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG= Donald Douwsma
2026-02-24 7:17 ` [PATCH 1/5] xfsrestore: remove unused variable strctxp Donald Douwsma
@ 2026-02-24 7:17 ` Donald Douwsma
2026-02-24 7:17 ` [PATCH 3/5] xfsrestore: include TREE_DEBUG all builds Donald Douwsma
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Donald Douwsma @ 2026-02-24 7:17 UTC (permalink / raw)
To: linux-xfs; +Cc: Donald Douwsma
When building with NDEBUG defined we see warnings on variables only used
for assert(3).
warning: variable ‘rval’ set but not used [-Wunused-but-set-variable]
Annotate these with __attribute__((unused)) and remove any REFERENCED
comment style annotations that no longer work.
Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
---
common/drive_minrmt.c | 5 ++---
common/drive_scsitape.c | 5 ++---
common/drive_simple.c | 9 +++------
common/main.c | 6 ++----
common/mlog.c | 6 ++----
common/qlock.c | 18 ++++++++----------
common/ring.c | 2 +-
dump/content.c | 8 +++-----
restore/content.c | 30 ++++++++++--------------------
restore/dirattr.c | 3 +--
restore/inomap.c | 9 +++------
restore/tree.c | 36 ++++++++++++------------------------
restore/win.c | 3 +--
13 files changed, 50 insertions(+), 90 deletions(-)
diff --git a/common/drive_minrmt.c b/common/drive_minrmt.c
index 0cbf235..ced2b21 100644
--- a/common/drive_minrmt.c
+++ b/common/drive_minrmt.c
@@ -880,8 +880,7 @@ static void
do_return_read_buf(drive_t *drivep, char *bufp, size_t retcnt)
{
drive_context_t *contextp = (drive_context_t *)drivep->d_contextp;
- /* REFERENCED */
- size_t ownedcnt;
+ __attribute__((unused)) size_t ownedcnt;
mlog(MLOG_DEBUG | MLOG_DRIVE,
"rmt drive op: return read buf: sz %d (0x%x)\n",
@@ -1230,7 +1229,7 @@ do_seek_mark(drive_t *drivep, drive_mark_t *markp)
assert(wantedoffset - currentoffset < (off64_t)tape_recsz);
wantedcnt = (size_t)(wantedoffset - currentoffset);
if (contextp->dc_recp) {
- uint32_t recoff;
+ __attribute__((unused)) uint32_t recoff;
#ifdef DEBUG
rec_hdr_t *rechdrp = (rec_hdr_t *)contextp->dc_recp;
#endif
diff --git a/common/drive_scsitape.c b/common/drive_scsitape.c
index 878a5d7..e50bbf9 100644
--- a/common/drive_scsitape.c
+++ b/common/drive_scsitape.c
@@ -993,8 +993,7 @@ static void
do_return_read_buf(drive_t *drivep, char *bufp, size_t retcnt)
{
drive_context_t *contextp = (drive_context_t *)drivep->d_contextp;
- /* REFERENCED */
- size_t ownedcnt;
+ __attribute__((unused)) size_t ownedcnt;
mlog(MLOG_DEBUG | MLOG_DRIVE,
"drive op: return read buf: sz %d (0x%x)\n",
@@ -1343,7 +1342,7 @@ do_seek_mark(drive_t *drivep, drive_mark_t *markp)
assert(wantedoffset - currentoffset < (off64_t)tape_recsz);
wantedcnt = (size_t)(wantedoffset - currentoffset);
if (contextp->dc_recp) {
- uint32_t recoff;
+ __attribute__((unused)) uint32_t recoff;
#ifdef DEBUG
rec_hdr_t *rechdrp = (rec_hdr_t *)contextp->dc_recp;
#endif
diff --git a/common/drive_simple.c b/common/drive_simple.c
index 5c3ed4b..a085df0 100644
--- a/common/drive_simple.c
+++ b/common/drive_simple.c
@@ -682,8 +682,7 @@ static void
do_return_read_buf(drive_t *drivep, char *retp, size_t retcnt)
{
drive_context_t *contextp = (drive_context_t *)drivep->d_contextp;
- /* REFERENCED */
- size_t ownedcnt;
+ __attribute__((unused)) size_t ownedcnt;
mlog(MLOG_NITTY | MLOG_DRIVE,
"drive_simple return_read_buf( returning %u )\n",
@@ -745,8 +744,7 @@ do_seek_mark(drive_t *drivep, drive_mark_t *markp)
off64_t mark = *(off64_t *)markp;
off64_t nextoff;
off64_t strmoff;
- /* REFERENCED */
- int nread;
+ __attribute__((unused)) int nread;
off64_t nreadneeded64;
int nreadneeded;
int rval;
@@ -1058,8 +1056,7 @@ do_set_mark(drive_t *drivep,
global_hdr_t *gwhdrp = drivep->d_gwritehdrp;
drive_hdr_t *dwhdrp = drivep->d_writehdrp;
off64_t newoff;
- /* REFERENCED */
- int nwritten;
+ __attribute__((unused)) int nwritten;
/* assert the header has been flushed
*/
diff --git a/common/main.c b/common/main.c
index 6141ffb..c064fca 100644
--- a/common/main.c
+++ b/common/main.c
@@ -162,8 +162,7 @@ main(int argc, char *argv[])
int prbcld_xc = EXIT_NORMAL;
int xc;
bool_t ok;
- /* REFERENCED */
- int rval;
+ __attribute__((unused)) int rval;
int err;
/* sanity checks
@@ -2050,8 +2049,7 @@ set_rlimits(size64_t *vmszp)
#ifdef RESTORE
size64_t vmsz;
#endif /* RESTORE */
- /* REFERENCED */
- int rval;
+ __attribute__((unused)) int rval;
assert(minstacksz <= maxstacksz);
diff --git a/common/mlog.c b/common/mlog.c
index 7f8640b..e5915b0 100644
--- a/common/mlog.c
+++ b/common/mlog.c
@@ -669,8 +669,7 @@ rv_t
mlog_get_hint(void)
{
stream_state_t states[] = { S_RUNNING };
- /* REFERENCED */
- bool_t ok;
+ __attribute__((unused)) bool_t ok;
rv_t hint;
if (pthread_equal(pthread_self(), parenttid))
@@ -727,8 +726,7 @@ mlog_exit_flush(void)
int streamix;
int exit_code;
rv_t exit_return, exit_hint;
- /* REFERENCED */
- bool_t ok;
+ __attribute__((unused)) bool_t ok;
ok = stream_get_exit_status(tids[i],
states,
diff --git a/common/qlock.c b/common/qlock.c
index 81a0e0b..bc0f11b 100644
--- a/common/qlock.c
+++ b/common/qlock.c
@@ -103,8 +103,7 @@ qlock_lock(qlockh_t qlockh)
{
qlock_t *qlockp = (qlock_t *)qlockh;
pthread_t tid;
- /* REFERENCED */
- int rval;
+ __attribute__((unused)) int rval;
/* get the caller's tid
*/
@@ -146,8 +145,7 @@ void
qlock_unlock(qlockh_t qlockh)
{
qlock_t *qlockp = (qlock_t *)qlockh;
- /* REFERENCED */
- int rval;
+ __attribute__((unused)) int rval;
/* verify lock is held by this thread
*/
@@ -167,7 +165,7 @@ qsemh_t
qsem_alloc(ix_t cnt)
{
sem_t *semp;
- int rval;
+ __attribute__((unused)) int rval;
/* allocate a semaphore
*/
@@ -186,7 +184,7 @@ void
qsem_free(qsemh_t qsemh)
{
sem_t *semp = (sem_t *)qsemh;
- int rval;
+ __attribute__((unused)) int rval;
/* destroy the mutex and condition
*/
@@ -202,7 +200,7 @@ void
qsemP(qsemh_t qsemh)
{
sem_t *semp = (sem_t *)qsemh;
- int rval;
+ __attribute__((unused)) int rval;
/* "P" the semaphore
*/
@@ -214,7 +212,7 @@ void
qsemV(qsemh_t qsemh)
{
sem_t *semp = (sem_t *)qsemh;
- int rval;
+ __attribute__((unused)) int rval;
/* "V" the semaphore
*/
@@ -227,7 +225,7 @@ qsemPwouldblock(qsemh_t qsemh)
{
sem_t *semp = (sem_t *)qsemh;
int count;
- int rval;
+ __attribute__((unused)) int rval;
rval = sem_getvalue(semp, &count);
assert(!rval);
@@ -240,7 +238,7 @@ qsemPavail(qsemh_t qsemh)
{
sem_t *semp = (sem_t *)qsemh;
int count;
- int rval;
+ __attribute__((unused)) int rval;
rval = sem_getvalue(semp, &count);
assert(!rval);
diff --git a/common/ring.c b/common/ring.c
index 7c6b499..ba15dfe 100644
--- a/common/ring.c
+++ b/common/ring.c
@@ -45,7 +45,7 @@ ring_create(size_t ringlen,
void *clientctxp,
int *rvalp)
{
- bool_t ok;
+ __attribute__((unused)) bool_t ok;
ring_t *ringp;
size_t mix;
diff --git a/dump/content.c b/dump/content.c
index 6462267..62a0f3f 100644
--- a/dump/content.c
+++ b/dump/content.c
@@ -1082,8 +1082,7 @@ content_init(int argc,
* values not dumped.
*/
if (inv_idbt != INV_TOKEN_NULL) {
- /* REFERENCED */
- bool_t ok1;
+ __attribute__((unused)) bool_t ok1;
ok = inv_lastsession_level_equalto(&fsid,
inv_idbt,
(u_char_t)sc_level,
@@ -1968,7 +1967,7 @@ create_inv_session(
ix_t subtreecnt,
size_t strmix)
{
- int rval;
+ __attribute__((unused)) int rval;
char *qmntpnt;
char *qfsdevice;
@@ -5504,8 +5503,7 @@ write_pad(drive_t *drivep, size_t sz)
static void
inv_cleanup(void)
{
- /* REFERENCED */
- bool_t ok;
+ __attribute__((unused)) bool_t ok;
if (sc_inv_stmtokenp && sc_contextp) {
size_t strmix;
diff --git a/restore/content.c b/restore/content.c
index b916e39..b91e5f0 100644
--- a/restore/content.c
+++ b/restore/content.c
@@ -3246,8 +3246,7 @@ eatdirdump(drive_t *drivep,
for (;;) {
register direnthdr_t *dhdrp =
(direnthdr_t *)direntbuf;
- /* REFERENCED */
- register size_t namelen;
+ __attribute__((unused)) size_t namelen;
rv = read_dirent(drivep,
dhdrp,
@@ -3696,8 +3695,7 @@ wipepersstate(void)
}
while ((direntp = readdir64(dirp)) != 0) {
- /* REFERENCED */
- int len;
+ __attribute__((unused)) int len;
if (!strcmp(direntp->d_name, ".")) {
continue;
}
@@ -4924,8 +4922,7 @@ pi_allocdesc(dh_t *deschp)
ix_t newdescpgcnt = olddescpgcnt + DAU;
ix_t descppg = pgsz / PERS_DESCSZ;
ix_t descix;
- /* REFERENCED */
- int rval;
+ __attribute__((unused)) int rval;
/* first unmap if any existing descriptors
*/
@@ -8029,8 +8026,7 @@ read_filehdr(drive_t *drivep, filehdr_t *fhdrp, bool_t fhcs)
{
bstat_t *bstatp = &fhdrp->fh_stat;
drive_ops_t *dop = drivep->d_opsp;
- /* REFERENCED */
- int nread;
+ __attribute__((unused)) int nread;
int rval;
filehdr_t tmpfh;
@@ -8088,8 +8084,7 @@ static rv_t
read_extenthdr(drive_t *drivep, extenthdr_t *ehdrp, bool_t ehcs)
{
drive_ops_t *dop = drivep->d_opsp;
- /* REFERENCED */
- int nread;
+ __attribute__((unused)) int nread;
int rval;
extenthdr_t tmpeh;
@@ -8151,8 +8146,7 @@ read_dirent(drive_t *drivep,
{
global_hdr_t *grhdrp = drivep->d_greadhdrp;
drive_ops_t *dop = drivep->d_opsp;
- /* REFERENCED */
- int nread;
+ __attribute__((unused)) int nread;
int rval;
direnthdr_t tmpdh;
char *namep; // beginning of name following the direnthdr_t
@@ -8273,8 +8267,7 @@ static rv_t
read_extattrhdr(drive_t *drivep, extattrhdr_t *ahdrp, bool_t ahcs)
{
drive_ops_t *dop = drivep->d_opsp;
- /* REFERENCED */
- int nread;
+ __attribute__((unused)) int nread;
int rval;
extattrhdr_t tmpah;
@@ -8345,8 +8338,7 @@ static rv_t
discard_padding(size_t sz, drive_t *drivep)
{
drive_ops_t *dop = drivep->d_opsp;
- /* REFERENCED */
- int nread;
+ __attribute__((unused)) int nread;
int rval;
nread = read_buf(0,
@@ -8644,8 +8636,7 @@ restore_extattr(drive_t *drivep,
*/
for (;;) {
size_t recsz;
- /* REFERENCED */
- int nread;
+ __attribute__((unused)) int nread;
int rval;
rv_t rv;
@@ -9185,8 +9176,7 @@ static void
pi_show(char *introstring)
{
char strbuf[100];
- /* REFERENCED */
- int strbuflen;
+ __attribute__((unused)) int strbuflen;
fold_t fold;
if (mlog_level_ss[MLOG_SS_MEDIA] < MLOG_NITTY + 1) {
diff --git a/restore/dirattr.c b/restore/dirattr.c
index 1f44cc0..b50384f 100644
--- a/restore/dirattr.c
+++ b/restore/dirattr.c
@@ -332,8 +332,7 @@ dirattr_init(char *hkdir, bool_t resume, uint64_t dircnt)
void
dirattr_cleanup(void)
{
- /* REFERENCED */
- int rval;
+ __attribute__((unused)) int rval;
if (!dtp) {
return;
diff --git a/restore/inomap.c b/restore/inomap.c
index 13e1247..39c5ca0 100644
--- a/restore/inomap.c
+++ b/restore/inomap.c
@@ -191,11 +191,9 @@ inomap_restore_pers(drive_t *drivep,
hnk_t *pershnkp;
hnk_t *tmphnkp;
int fd;
- /* REFERENCED */
- int nread;
+ __attribute__((unused)) int nread;
int rval;
- /* REFERENCED */
- int rval1;
+ __attribute__((unused)) int rval1;
int i;
bool_t ok;
@@ -311,8 +309,7 @@ inomap_discard(drive_t *drivep, content_inode_hdr_t *scrhdrp)
{
drive_ops_t *dop = drivep->d_opsp;
uint64_t tmphnkcnt;
- /* REFERENCED */
- int nread;
+ __attribute__((unused)) int nread;
int rval;
/* get inomap info from media hdr
diff --git a/restore/tree.c b/restore/tree.c
index 4707fdc..7e79ef2 100644
--- a/restore/tree.c
+++ b/restore/tree.c
@@ -910,8 +910,7 @@ tree_addent(nh_t parh, xfs_ino_t ino, gen_t gen, char *name, size_t namelen)
if (hardp->n_flags & NF_ISDIR) {
nh_t renameh;
node_t *renamep;
- /* REFERENCED */
- int namebuflen;
+ __attribute__((unused)) int namebuflen;
hardp->n_flags |= NF_REFED;
if (hardp->n_parh == persp->p_orphh) {
@@ -1425,8 +1424,7 @@ noref_elim_recurse(nh_t parh,
path2,
_("tmp dir rename dst"));
if (!ok) {
- /* REFERENCED */
- nrh_t dummynrh;
+ __attribute__((unused)) nrh_t dummynrh;
dummynrh = disown(cldh);
assert(dummynrh == NRH_NULL);
adopt(parh, cldh, nrh);
@@ -1439,8 +1437,7 @@ noref_elim_recurse(nh_t parh,
path2);
rval = rename(path1, path2);
if (rval) {
- /* REFERENCED */
- nrh_t dummynrh;
+ __attribute__((unused)) nrh_t dummynrh;
mlog(MLOG_NORMAL | MLOG_WARNING, _(
"unable to rename dir %s "
"to dir %s: %s\n"),
@@ -1543,8 +1540,7 @@ noref_elim_recurse(nh_t parh,
path2,
_("tmp nondir rename dst"));
if (!ok) {
- /* REFERENCED */
- nrh_t dummynrh;
+ __attribute__((unused)) nrh_t dummynrh;
dummynrh = disown(cldh);
assert(dummynrh == NRH_NULL);
adopt(parh, cldh, nrh);
@@ -1557,8 +1553,7 @@ noref_elim_recurse(nh_t parh,
path2);
rval = rename(path1, path2);
if (rval) {
- /* REFERENCED */
- nrh_t dummynrh;
+ __attribute__((unused)) nrh_t dummynrh;
mlog(MLOG_NORMAL | MLOG_WARNING, _(
"unable to rename nondir %s "
"to nondir %s: %s\n"),
@@ -1577,8 +1572,7 @@ noref_elim_recurse(nh_t parh,
Node_unmap(cldh, &cldp);
}
if (canunlinkpr) {
- /* REFERENCED */
- nrh_t nrh;
+ __attribute__((unused)) nrh_t nrh;
assert(!mustorphpr);
if (isrealpr) {
@@ -1699,8 +1693,7 @@ rename_dirs(nh_t cldh,
{
while (cldh != NH_NULL) {
node_t *cldp;
- /* REFERENCED */
- nh_t parh;
+ __attribute__((unused)) nh_t parh;
bool_t isdirpr;
nh_t renameh;
bool_t isrenamepr;
@@ -1720,8 +1713,7 @@ rename_dirs(nh_t cldh,
if (isrenamepr) {
node_t *renamep;
int rval;
- /* REFERENCED */
- nrh_t dummynrh;
+ __attribute__((unused)) nrh_t dummynrh;
bool_t ok;
renamep = Node_map(renameh);
@@ -2870,8 +2862,7 @@ tsi_cmd_pwd_recurse(void *ctxp,
{
node_t *np;
register nh_t parh;
- /* REFERENCED */
- register int namelen;
+ __attribute__((unused)) int namelen;
nrh_t nrh;
assert(nh != NH_NULL);
@@ -2956,8 +2947,7 @@ tsi_cmd_ls(void *ctxp,
ino = cldp->n_ino;
Node_unmap(cldh, &cldp);
if (cldh != persp->p_orphh) {
- /* REFERENCED */
- int namelen;
+ __attribute__((unused)) int namelen;
namelen = namreg_get(nrh,
tranp->t_inter.i_name,
sizeof(tranp->t_inter.i_name));
@@ -3370,8 +3360,7 @@ tsi_walkpath(char *arg, nh_t rooth, nh_t cwdh,
node_t *sibp;
nh_t nextsibh;
nrh_t nrh;
- /* REFERENCED */
- int siblen;
+ __attribute__((unused)) int siblen;
sibp = Node_map(sibh);
nrh = sibp->n_nrh;
@@ -3907,8 +3896,7 @@ link_matchh(nh_t hardh, nh_t parh, char *name)
nh_t nexth;
np = Node_map(hardh);
if (np->n_parh == parh) {
- /* REFERENCED */
- int namelen;
+ __attribute__((unused)) int namelen;
namelen = namreg_get(np->n_nrh,
tranp->t_namebuf,
sizeof(tranp->t_namebuf));
diff --git a/restore/win.c b/restore/win.c
index 5d40592..64dae1a 100644
--- a/restore/win.c
+++ b/restore/win.c
@@ -246,8 +246,7 @@ win_map(segix_t segix, void **pp)
assert(winp);
tranp->t_wincnt++;
} else if (tranp->t_lruheadp) {
- /* REFERENCED */
- int rval;
+ __attribute__((unused)) int rval;
#ifdef TREE_DEBUG
mlog(MLOG_DEBUG | MLOG_TREE | MLOG_NOLOCK,
"win_map(): get head from lru freelist & unmap\n");
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/5] xfsrestore: include TREE_DEBUG all builds
2026-02-24 7:17 [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG= Donald Douwsma
2026-02-24 7:17 ` [PATCH 1/5] xfsrestore: remove unused variable strctxp Donald Douwsma
2026-02-24 7:17 ` [PATCH 2/5] annotate variables only used for assert Donald Douwsma
@ 2026-02-24 7:17 ` Donald Douwsma
2026-02-24 7:17 ` [PATCH 4/5] xfsrestore: remove failing assert from noref_elim_recurse Donald Douwsma
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Donald Douwsma @ 2026-02-24 7:17 UTC (permalink / raw)
To: linux-xfs; +Cc: Donald Douwsma
TREE_DEBUG guards additional tree debugging code, making it unavailable
for troubleshooting production issues in the field. Remove the define
and move the logging to level debug or nitty.
xfsrestore -v tree=debug -f dump /tmp/testdir
Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
---
restore/tree.c | 43 +++++++++++++++----------------------------
restore/win.c | 21 ++++++---------------
2 files changed, 21 insertions(+), 43 deletions(-)
diff --git a/restore/tree.c b/restore/tree.c
index 7e79ef2..bf89c6a 100644
--- a/restore/tree.c
+++ b/restore/tree.c
@@ -1342,13 +1342,13 @@ noref_elim_recurse(nh_t parh,
grandcldh = cldp->n_cldh;
nextcldh = cldp->n_sibh;
-#ifdef TREE_DEBUG
- ok = Node2path(cldh, path1, _("noref debug"));
- mlog(MLOG_TRACE | MLOG_TREE,
- "noref: %s: isdir = %d, isreal = %d, isref = %d, "
- "isrenamedir = %d\n",
- path1, isdirpr, isrealpr, isrefpr, isrenamepr);
-#endif
+ if (mlog_level_ss[MLOG_SS_TREE] == MLOG_DEBUG) {
+ ok = Node2path(cldh, path1, _("noref debug"));
+ mlog(MLOG_DEBUG | MLOG_TREE,
+ "noref: %s: isdir = %d, isreal = %d, isref = %d, "
+ "isrenamedir = %d\n",
+ path1, isdirpr, isrealpr, isrefpr, isrenamepr);
+ }
Node_unmap(cldh, &cldp);
@@ -3616,11 +3616,9 @@ adopt(nh_t parh, nh_t cldh, nrh_t nrh)
node_t *cldp;
node_t *sibp;
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE,
+ mlog(MLOG_NITTY | MLOG_TREE,
"adopt(parent=%llu,child=%llu,node=%llu): \n",
parh, cldh, nrh);
-#endif
/* fix up our child - put at front of child list */
cldp = Node_map(cldh);
@@ -3921,10 +3919,8 @@ link_in(nh_t nh)
gen_t gen;
nh_t hardh;
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE,
+ mlog(MLOG_NITTY | MLOG_TREE,
"link_in(%llu): map in node\n", nh);
-#endif
/* map in the node to read ino and gen
*/
@@ -3940,18 +3936,14 @@ link_in(nh_t nh)
* of hard link (lnk) list.
*/
if (hardh == NH_NULL) {
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE,
+ mlog(MLOG_NITTY | MLOG_TREE,
"link_in(): hash node in for ino %llu\n", ino);
-#endif
hash_in(nh);
} else {
nh_t prevh = hardh;
node_t *prevp = Node_map(prevh);
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE,
+ mlog(MLOG_NITTY | MLOG_TREE,
"link_in(): put at end of hash list\n");
-#endif
while (prevp->n_lnkh != NH_NULL) {
nh_t nexth = prevp->n_lnkh;
Node_unmap(prevh, &prevp);
@@ -3967,10 +3959,8 @@ link_in(nh_t nh)
*/
np->n_lnkh = NH_NULL;
Node_unmap(nh, &np);
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE,
+ mlog(MLOG_NITTY | MLOG_TREE,
"link_in(%llu): UNmap in node\n", nh);
-#endif
}
static void
@@ -4362,11 +4352,9 @@ hash_find(xfs_ino_t ino, gen_t gen)
return NH_NULL;
}
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE,
+ mlog(MLOG_NITTY | MLOG_TREE,
"hash_find(%llu,%u): traversing hash link list\n",
ino, gen);
-#endif
/* walk the list until found.
*/
@@ -4382,10 +4370,9 @@ hash_find(xfs_ino_t ino, gen_t gen)
}
Node_unmap(nh, &np);
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE,
+ mlog(MLOG_NITTY | MLOG_TREE,
"hash_find(): return nh = %llu\n", nh);
-#endif
+
return nh;
}
diff --git a/restore/win.c b/restore/win.c
index 64dae1a..8540a98 100644
--- a/restore/win.c
+++ b/restore/win.c
@@ -185,10 +185,9 @@ win_map(segix_t segix, void **pp)
CRITICAL_BEGIN();
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE | MLOG_NOLOCK,
+ mlog(MLOG_NITTY | MLOG_TREE | MLOG_NOLOCK,
"win_map(segix=%u,addr=%p)\n", segix, pp);
-#endif
+
/* resize the array if necessary */
if (segix >= tranp->t_segmaplen)
win_segmap_resize(segix);
@@ -198,10 +197,8 @@ win_map(segix_t segix, void **pp)
*/
winp = tranp->t_segmap[segix];
if (winp) {
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE | MLOG_NOLOCK,
+ mlog(MLOG_NITTY | MLOG_TREE | MLOG_NOLOCK,
"win_map(): requested segment already mapped\n");
-#endif
if (winp->w_refcnt == 0) {
assert(tranp->t_lruheadp);
assert(tranp->t_lrutailp);
@@ -238,19 +235,15 @@ win_map(segix_t segix, void **pp)
* otherwise reuse any descriptor on the LRU list.
*/
if (tranp->t_wincnt < tranp->t_winmax) {
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE | MLOG_NOLOCK,
+ mlog(MLOG_NITTY | MLOG_TREE | MLOG_NOLOCK,
"win_map(): create a new window\n");
-#endif
winp = (win_t *)calloc(1, sizeof(win_t));
assert(winp);
tranp->t_wincnt++;
} else if (tranp->t_lruheadp) {
__attribute__((unused)) int rval;
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE | MLOG_NOLOCK,
+ mlog(MLOG_NITTY | MLOG_TREE | MLOG_NOLOCK,
"win_map(): get head from lru freelist & unmap\n");
-#endif
assert(tranp->t_lrutailp);
winp = tranp->t_lruheadp;
tranp->t_lruheadp = winp->w_nextp;
@@ -284,11 +277,9 @@ win_map(segix_t segix, void **pp)
OFF64MAX - segoff - (off64_t)tranp->t_segsz + 1ll);
assert(!(tranp->t_segsz % pgsz));
assert(!((tranp->t_firstoff + segoff) % (off64_t)pgsz));
-#ifdef TREE_DEBUG
- mlog(MLOG_DEBUG | MLOG_TREE | MLOG_NOLOCK,
+ mlog(MLOG_NITTY | MLOG_TREE | MLOG_NOLOCK,
"win_map(): mmap segment at %lld, size = %llu\n",
(off64_t)(tranp->t_firstoff + segoff), tranp->t_segsz);
-#endif
tranp->t_winmmaps++;
winp->w_p = mmap_autogrow(
tranp->t_segsz,
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/5] xfsrestore: remove failing assert from noref_elim_recurse
2026-02-24 7:17 [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG= Donald Douwsma
` (2 preceding siblings ...)
2026-02-24 7:17 ` [PATCH 3/5] xfsrestore: include TREE_DEBUG all builds Donald Douwsma
@ 2026-02-24 7:17 ` Donald Douwsma
2026-03-23 3:01 ` Donald Douwsma
2026-02-24 7:17 ` [PATCH 5/5] xfsrestore: assert suppression workaround Donald Douwsma
2026-03-13 13:12 ` [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG= Andrey Albershteyn
5 siblings, 1 reply; 9+ messages in thread
From: Donald Douwsma @ 2026-02-24 7:17 UTC (permalink / raw)
To: linux-xfs; +Cc: Donald Douwsma
We are getting reports from the field where cumulative restores are
aborting due due to a failing assertion.
xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed
This appears to be caused by a rename within the tree between restore
levels, or a combination of modifications occurring during dump.
Fixing it will likely require changes in noref_elim_recurse and
tree_post, to ensure elements of the tree are created for these edge
conditions. Given the state of xfsdump this is a bit risky for
maintenance streams.
While current builds have assert(3) active, remove this one allowing
xfsrestore to continue, warning on failing renames for directory nodes
that haven't been created. Once the full tree is built referenced
directories will end up in the orphanage allowing for user recovery.
Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
---
restore/tree.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/restore/tree.c b/restore/tree.c
index bf89c6a..81666ac 100644
--- a/restore/tree.c
+++ b/restore/tree.c
@@ -1404,12 +1404,13 @@ noref_elim_recurse(nh_t parh,
Node_free(&cldh);
}
+ /* Process renames for directories that have been created.
+ */
if (isrenamepr) {
nrh_t nrh;
node_t *renamep;
assert(isrefpr);
- assert(isrealpr);
ok = Node2path(cldh,
path1,
_("tmp dir rename src"));
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 4/5] xfsrestore: remove failing assert from noref_elim_recurse
2026-02-24 7:17 ` [PATCH 4/5] xfsrestore: remove failing assert from noref_elim_recurse Donald Douwsma
@ 2026-03-23 3:01 ` Donald Douwsma
0 siblings, 0 replies; 9+ messages in thread
From: Donald Douwsma @ 2026-03-23 3:01 UTC (permalink / raw)
To: linux-xfs
On 24/2/26 18:17, Donald Douwsma wrote:
> We are getting reports from the field where cumulative restores are
> aborting due due to a failing assertion.
>
> xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed
>
> This appears to be caused by a rename within the tree between restore
> levels, or a combination of modifications occurring during dump.
> Fixing it will likely require changes in noref_elim_recurse and
> tree_post, to ensure elements of the tree are created for these edge
> conditions. Given the state of xfsdump this is a bit risky for
> maintenance streams.
With some more digging it looks like this is triggered during subtree
restores. A cumulative restore started by excluding a specific subtree
with -s doesn't cause subsequent restores to prune the non subtree
sections from the restore.
This leads to the assert firing when in noref_elim_recurse when its
asked to handle a rename for a directory not created due to the
subtree restriction.
mkdir -p /mnt/scratch/dir1/dir2/dir3
mkdir -p /mnt/scratch/restore_me/dirA/dirB/dirC
xfsdump -L label -M media -l0 -f /tmp/l0.dump /mnt/scratch
mv /mnt/scratch/dir1/dir2 /mnt/scratch/dir10
xfsdump -L label -M media -l2 -f /tmp/l2.dump /mnt/scratch
dir=$(mktemp -d /mnt/scratch/dir_XXX)
xfsrestore -r -f /tmp/l0.dump -s restore_me $dir
xfsrestore -r -f /tmp/l2.dump $dir
## xfsrestore: tree.c:1369: noref_elim_recurse: Assertion `isrealpr' failed.
The subtree filter state was saved in the state file
[root@rhel8 ~]# hexdump -C /mnt/scratch/dir_ILX/xfsrestorehousekeepingdir/state | grep restore_me
00005010 72 65 73 74 6f 72 65 5f 6d 65 00 00 00 00 00 00 |restore_me......|
[root@rhel8 ~]#
> While current builds have assert(3) active, remove this one allowing
> xfsrestore to continue, warning on failing renames for directory nodes
> that haven't been created. Once the full tree is built referenced
> directories will end up in the orphanage allowing for user recovery.
This didn't end up in the orphanage, which seems to suggest that the
subtree option had some affect.
[root@rhel8 xfsdump-dev]# restore/xfsrestore -r -f /tmp/l2.2.dump $dir
restore/xfsrestore: using file dump (drive_simple) strategy
restore/xfsrestore: version 3.2.0 (dump format 3.0) - type ^C for status and control
restore/xfsrestore: searching media for dump
restore/xfsrestore: examining media file 0
restore/xfsrestore: dump description:
restore/xfsrestore: hostname: rhel8
restore/xfsrestore: mount point: /mnt/scratch
restore/xfsrestore: volume: /dev/vdc
restore/xfsrestore: session time: Mon Mar 23 13:25:57 2026
restore/xfsrestore: level: 2
restore/xfsrestore: session label: "label"
restore/xfsrestore: media label: "media"
restore/xfsrestore: file system id: cb6c9beb-45a7-42b4-b125-502ed787e4fb
restore/xfsrestore: session id: 3832f932-2b4f-4f87-841d-f071e4cb3a1c
restore/xfsrestore: media id: 58d8ce69-90a7-4164-901d-4a64a0f9ba69
restore/xfsrestore: using online session inventory
restore/xfsrestore: searching media for directory dump
restore/xfsrestore: reading directories
restore/xfsrestore: 16 directories and 21 entries processed
restore/xfsrestore: directory post-processing
restore/xfsrestore: WARNING: unable to rename dir dir1/dir2 to dir orphanage/67109056.572736669: No such file or directory
restore/xfsrestore: restore complete: 0 seconds elapsed
restore/xfsrestore: Restore Summary:
restore/xfsrestore: stream 0 /tmp/l2.2.dump OK (success)
restore/xfsrestore: Restore Status: SUCCESS
[root@rhel8 xfsdump-dev]#
[root@rhel8 xfsdump-dev]# find $dir
/mnt/scratch/dir_P67
/mnt/scratch/dir_P67/xfsrestorehousekeepingdir
/mnt/scratch/dir_P67/xfsrestorehousekeepingdir/state
/mnt/scratch/dir_P67/xfsrestorehousekeepingdir/namreg
/mnt/scratch/dir_P67/xfsrestorehousekeepingdir/tree
/mnt/scratch/dir_P67/xfsrestorehousekeepingdir/dirextattr
/mnt/scratch/dir_P67/xfsrestorehousekeepingdir/dirattr
/mnt/scratch/dir_P67/restore_me
/mnt/scratch/dir_P67/restore_me/dirA
/mnt/scratch/dir_P67/restore_me/dirA/dirB
/mnt/scratch/dir_P67/restore_me/dirA/dirB/dirC
/mnt/scratch/dir_P67/restore_me/dirD
[root@rhel8 xfsdump-dev]#
>
> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
> ---
> restore/tree.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/restore/tree.c b/restore/tree.c
> index bf89c6a..81666ac 100644
> --- a/restore/tree.c
> +++ b/restore/tree.c
> @@ -1404,12 +1404,13 @@ noref_elim_recurse(nh_t parh,
> Node_free(&cldh);
> }
>
> + /* Process renames for directories that have been created.
> + */
> if (isrenamepr) {
> nrh_t nrh;
> node_t *renamep;
>
> assert(isrefpr);
> - assert(isrealpr);
> ok = Node2path(cldh,
> path1,
> _("tmp dir rename src"));
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 5/5] xfsrestore: assert suppression workaround
2026-02-24 7:17 [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG= Donald Douwsma
` (3 preceding siblings ...)
2026-02-24 7:17 ` [PATCH 4/5] xfsrestore: remove failing assert from noref_elim_recurse Donald Douwsma
@ 2026-02-24 7:17 ` Donald Douwsma
2026-03-13 13:12 ` [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG= Andrey Albershteyn
5 siblings, 0 replies; 9+ messages in thread
From: Donald Douwsma @ 2026-02-24 7:17 UTC (permalink / raw)
To: linux-xfs; +Cc: Donald Douwsma
Allow users to workaround assertions encountered during a restore by
suppressing them using 'xfsrestore -z ...`.
This is not the right approach for ongoing upstream development, but it
may be useful as a workaround for downstream maintenance releases built
without NDEBUG, even then I'm hesitant.
Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
---
include/config.h.in | 4 ++++
man/man8/xfsrestore.8 | 4 ++++
restore/Makefile | 6 ++++--
restore/content.c | 5 ++++-
restore/getopt.h | 4 ++--
restore/restore_assert.c | 39 +++++++++++++++++++++++++++++++++++++++
restore/restore_assert.h | 18 ++++++++++++++++++
7 files changed, 75 insertions(+), 5 deletions(-)
create mode 100644 restore/restore_assert.c
create mode 100644 restore/restore_assert.h
diff --git a/include/config.h.in b/include/config.h.in
index 3b35d83..9cc6628 100644
--- a/include/config.h.in
+++ b/include/config.h.in
@@ -50,4 +50,8 @@ typedef unsigned short umode_t;
#define NBBY 8
#endif
+#if !defined(NDEBUG) && defined(RESTORE)
+#include "../restore/restore_assert.h"
+#endif
+
#endif /* __CONFIG_H__ */
diff --git a/man/man8/xfsrestore.8 b/man/man8/xfsrestore.8
index df7dde0..84e1b12 100644
--- a/man/man8/xfsrestore.8
+++ b/man/man8/xfsrestore.8
@@ -254,6 +254,10 @@ the dump without this option.
In the cumulative mode, this option is required only for a base (level 0)
dump. You no longer need this option for level 1+ dumps.
.TP 5
+.B \-z
+If xfsrestore fails due to an assertion this option can be used to ignore it for
+the purpose of debugging or to recover data from a problematic archive.
+.TP 5
\f3\-v\f1 \f2verbosity\f1
.\" set inter-paragraph distance to 0
.PD 0
diff --git a/restore/Makefile b/restore/Makefile
index ac3f8c8..5a5ee62 100644
--- a/restore/Makefile
+++ b/restore/Makefile
@@ -76,7 +76,8 @@ LOCALS = \
namreg.c \
node.c \
tree.c \
- win.c
+ win.c \
+ restore_assert.c
LOCALINCL = \
bag.h \
@@ -87,7 +88,8 @@ LOCALINCL = \
namreg.h \
node.h \
tree.h \
- win.h
+ win.h \
+ restore_assert.h
LTCOMMAND = xfsrestore
diff --git a/restore/content.c b/restore/content.c
index b91e5f0..71de87a 100644
--- a/restore/content.c
+++ b/restore/content.c
@@ -864,7 +864,7 @@ bool_t restore_rootdir_permissions;
bool_t need_fixrootdir;
char *media_change_alert_program = NULL;
size_t perssz;
-
+int workaround_assert;
/* definition of locally defined static variables *****************************/
@@ -1191,6 +1191,9 @@ content_init(int argc, char *argv[], size64_t vmsz)
case GETOPT_FIXROOTDIR:
need_fixrootdir = BOOL_TRUE;
break;
+ case GETOPT_WORKAROUND:
+ workaround_assert = 1;
+ break;
}
}
diff --git a/restore/getopt.h b/restore/getopt.h
index b0c0c7d..4c4c2b2 100644
--- a/restore/getopt.h
+++ b/restore/getopt.h
@@ -26,7 +26,7 @@
* purpose is to contain that command string.
*/
-#define GETOPT_CMDSTRING "a:b:c:def:himn:op:qrs:tv:wxABCDEFG:H:I:JKL:M:NO:PQRS:TUVWX:Y:"
+#define GETOPT_CMDSTRING "a:b:c:def:himn:op:qrs:tv:wxzABCDEFG:H:I:JKL:M:NO:PQRS:TUVWX:Y:"
#define GETOPT_WORKSPACE 'a' /* workspace dir (content.c) */
#define GETOPT_BLOCKSIZE 'b' /* blocksize for rmt */
@@ -53,7 +53,7 @@
#define GETOPT_SMALLWINDOW 'w' /* use a small window for dir entries */
#define GETOPT_FIXROOTDIR 'x' /* try to fix rootdir due to bulkstat misuse */
/* 'y' */
-/* 'z' */
+#define GETOPT_WORKAROUND 'z' /* enable workaroundz */
#define GETOPT_NOEXTATTR 'A' /* do not restore ext. file attr. */
#define GETOPT_ROOTPERM 'B' /* restore ownership and permissions for root directory */
#define GETOPT_RECCHKSUM 'C' /* use record checksums */
diff --git a/restore/restore_assert.c b/restore/restore_assert.c
new file mode 100644
index 0000000..e2f3054
--- /dev/null
+++ b/restore/restore_assert.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Red Hat, Inc. All Rights Reserved.
+ */
+
+#include <stddef.h>
+#include <stdlib.h>
+
+#include "config.h"
+
+#include "mlog.h"
+
+/*
+ * Override glibc __assert_fail to allow users to optionally ignore assertions.
+ *
+ * We could optionally require a number of assertions to skip, allowing the user
+ * to consider each problem and continue only if a specified number is not
+ * exceeded
+ */
+
+extern int workaround_assert;
+
+void __xfsrestore_assert_fail(const char *__assertion, const char *__file,
+ unsigned int __line, const char *__function)
+{
+ static int assert_count;
+ assert_count++;
+
+ mlog(MLOG_ERROR|MLOG_NOLOCK, _("%s:%d: %s: Assertion %s failed\n"),
+ __file, __line, __function, __assertion);
+
+ if(!workaround_assert) {
+ mlog(MLOG_ERROR|MLOG_NOLOCK, _("Recovery may be possible using the `-z` option\n"));
+ abort();
+ }
+}
+
+
+
diff --git a/restore/restore_assert.h b/restore/restore_assert.h
new file mode 100644
index 0000000..8eb22e6
--- /dev/null
+++ b/restore/restore_assert.h
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Red Hat, Inc. All Rights Reserved.
+ */
+
+#ifndef _XFS_RESTORE_ASSERT_H_
+#define _XFS_RESTORE_ASSERT_H_
+
+#if defined(assert)
+#undef assert
+extern void __xfsrestore_assert_fail(const char *__assertion, const char *__file,
+ unsigned int __line, const char *__function);
+#define assert(expr) \
+ ((expr) ? (void)0 : \
+ __xfsrestore_assert_fail(#expr, __FILE__, __LINE__, __func__))
+#endif
+
+#endif /* _XFS_RESTORE_ASSERT_H_ */
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG=
2026-02-24 7:17 [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG= Donald Douwsma
` (4 preceding siblings ...)
2026-02-24 7:17 ` [PATCH 5/5] xfsrestore: assert suppression workaround Donald Douwsma
@ 2026-03-13 13:12 ` Andrey Albershteyn
2026-03-23 1:48 ` Donald Douwsma
5 siblings, 1 reply; 9+ messages in thread
From: Andrey Albershteyn @ 2026-03-13 13:12 UTC (permalink / raw)
To: Donald Douwsma; +Cc: linux-xfs
On 2026-02-24 18:17:07, Donald Douwsma wrote:
> tldr; Fedora builds with -DDEBUG, is that what folk want or expect?
>
> We see cases from the field where xfsrestore aborts on an assert(3).
> In each case there's a real problem that needs to be fixed, but these
> problems are preventing folk from restoring their data. With the asserts
> suppressed or removed xfsrestore usually does something sane enough, at
> worst leaving restored content in the orphanage where a user can find
> it.
>
> This was notably seen with the original xfsdump bind mount fix, now
> we're seeing asserts fire during cumulative restores in
> noref_elim_recurse.
>
> xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed
>
> This appears to be caused by a rename to within the tree between restore
> levels, or a combination of modifications during occurring during the
> dump. Fixing it will likely require changes in noref_elim_recurse, or
> tree_post, to ensure elements of the tree are created for these edge
> cases.
>
> But why are we seeing assert(3) active in Fedora based distro builds?
>
> xfsprogs and xfsdump m4/package_globals.m4 shows builds should default
> to DEBUG and xfsdump doc/INSTALL talks about how to turn off asserts and
> create an optimized build, but only Debian has its build rules in-tree,
> where they set DEBUG=-DNDEBUG.
>
> Building locally on Fedora or similar gets varied results with recent
> Fedora versions building as if DEBUG=-DNDEBUG was set within the realm
> of configure or autoconf, which may be misleading people.
>
> But for Fedora release builds we can see that they're building with
> DEBUG=-DDEBUG resulting in gcc called with -DDEBUG in the build logs.
>
> https://src.fedoraproject.org/rpms/xfsdump
> https://kojipkgs.fedoraproject.org//packages/xfsdump/3.2.0/4.fc44/data/logs/x86_64/build.log
>
> https://src.fedoraproject.org/rpms/xfsprogs
> https://kojipkgs.fedoraproject.org//packages/xfsprogs/6.18.0/2.fc44/data/logs/x86_64/build.log
>
> I don't think the intention was to have these build as DEBUG, but that
> seems to be the current result. But before I dig into this further can I
> confirm what people think this should do?
>
> The following series is illustrative of the changes to build xfsdump
> cleanly with DEBUG=-DNDEBUG, or the type of downstream workarounds
> required to help end users hitting problems for the status quo.
>
> Don
>
> Donald Douwsma (5):
> xfsrestore: remove unused variable strctxp
> annotate variables only used for assert
> xfsrestore: include TREE_DEBUG all builds
First 3 patches looks good to me to include, not sure about the
rest and the original use of the those asserts.
--
- Andrey
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG=
2026-03-13 13:12 ` [PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG= Andrey Albershteyn
@ 2026-03-23 1:48 ` Donald Douwsma
0 siblings, 0 replies; 9+ messages in thread
From: Donald Douwsma @ 2026-03-23 1:48 UTC (permalink / raw)
To: Andrey Albershteyn; +Cc: linux-xfs
On 14/3/26 00:12, Andrey Albershteyn wrote:
> On 2026-02-24 18:17:07, Donald Douwsma wrote:
>> tldr; Fedora builds with -DDEBUG, is that what folk want or expect?
>>
>> We see cases from the field where xfsrestore aborts on an assert(3).
>> In each case there's a real problem that needs to be fixed, but these
>> problems are preventing folk from restoring their data. With the asserts
>> suppressed or removed xfsrestore usually does something sane enough, at
>> worst leaving restored content in the orphanage where a user can find
>> it.
>>
>> This was notably seen with the original xfsdump bind mount fix, now
>> we're seeing asserts fire during cumulative restores in
>> noref_elim_recurse.
>>
>> xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed
>>
>> This appears to be caused by a rename to within the tree between restore
>> levels, or a combination of modifications during occurring during the
>> dump. Fixing it will likely require changes in noref_elim_recurse, or
>> tree_post, to ensure elements of the tree are created for these edge
>> cases.
>>
>> But why are we seeing assert(3) active in Fedora based distro builds?
>>
>> xfsprogs and xfsdump m4/package_globals.m4 shows builds should default
>> to DEBUG and xfsdump doc/INSTALL talks about how to turn off asserts and
>> create an optimized build, but only Debian has its build rules in-tree,
>> where they set DEBUG=-DNDEBUG.
>>
>> Building locally on Fedora or similar gets varied results with recent
>> Fedora versions building as if DEBUG=-DNDEBUG was set within the realm
>> of configure or autoconf, which may be misleading people.
>>
>> But for Fedora release builds we can see that they're building with
>> DEBUG=-DDEBUG resulting in gcc called with -DDEBUG in the build logs.
>>
>> https://src.fedoraproject.org/rpms/xfsdump
>> https://kojipkgs.fedoraproject.org//packages/xfsdump/3.2.0/4.fc44/data/logs/x86_64/build.log
>>
>> https://src.fedoraproject.org/rpms/xfsprogs
>> https://kojipkgs.fedoraproject.org//packages/xfsprogs/6.18.0/2.fc44/data/logs/x86_64/build.log
>>
>> I don't think the intention was to have these build as DEBUG, but that
>> seems to be the current result. But before I dig into this further can I
>> confirm what people think this should do?
>>
>> The following series is illustrative of the changes to build xfsdump
>> cleanly with DEBUG=-DNDEBUG, or the type of downstream workarounds
>> required to help end users hitting problems for the status quo.
>>
>> Don
>>
>> Donald Douwsma (5):
>> xfsrestore: remove unused variable strctxp
>> annotate variables only used for assert
>> xfsrestore: include TREE_DEBUG all builds
>
> First 3 patches looks good to me to include, not sure about the
> rest and the original use of the those asserts.
>
Thanks for looking over these.
I've done a bit more digging and have a reproducer for the recent
noref_elim_recurse assert, with what I know now the tree debug
messages may not be as useful.
Regarding the assert stuff, its a bit of a can of worms, the
documentation in the build system is misleading, when it was
written the idea seems to have been to have debug and release
builds, but Fedora/rpm distros didn't follow that.
Since then it looks like most packages have moved to assert(3),
being active for all builds. So perhaps its better to keep it
that way and remove some of the unused DEBUG options from the
docs and build scripts.
^ permalink raw reply [flat|nested] 9+ messages in thread