Bug 1372939 - xfs/300 fails with xfsprogs-4.5.0-6.el7; not built against libattr
Summary: xfs/300 fails with xfsprogs-4.5.0-6.el7; not built against libattr
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: xfsprogs
Version: 7.3
Hardware: All
OS: Unspecified
unspecified
unspecified
Target Milestone: rc
: ---
Assignee: Eric Sandeen
QA Contact: Zorro Lang
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-09-04 02:01 UTC by Eric Sandeen
Modified: 2016-11-04 06:24 UTC (History)
8 users (show)

Fixed In Version: xfsprogs-4.5.0-7.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-11-04 06:24:48 UTC
Target Upstream Version:


Attachments (Terms of Use)
Patch for rhel-7.3 (1.39 KB, patch)
2016-09-06 19:25 UTC, Yaakov Selkowitz
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2016:2431 0 normal SHIPPED_LIVE xfsprogs bug fix and enhancement update 2016-11-03 14:00:51 UTC

Description Eric Sandeen 2016-09-04 02:01:30 UTC
xfstest xfs/300 tests xfs_fsr

With the stock build:

# rpm -qif `which xfs_fsr`
Name        : xfsprogs
Version     : 4.5.0
Release     : 6.el7
Architecture: aarch64
Install Date: Sat 03 Sep 2016 09:57:43 PM EDT
Group       : System Environment/Base
Size        : 4402142
License     : GPL+ and LGPLv2+
Signature   : RSA/SHA256, Wed 10 Aug 2016 11:11:30 AM EDT, Key ID 938a80caf21541eb
Source RPM  : xfsprogs-4.5.0-6.el7.src.rpm
Build Date  : Tue 09 Aug 2016 02:21:35 PM EDT
Build Host  : arm64-017.build.eng.bos.redhat.com
Relocations : (not relocatable)
Packager    : Red Hat, Inc. <http://bugzilla.redhat.com/bugzilla>
Vendor      : Red Hat, Inc.
URL         : http://oss.sgi.com/projects/xfs/
Summary     : Utilities for managing the XFS filesystem

It fails:

# ./check xfs/300
...
xfs/300 1s ... - output mismatch (see /root/sandeen/xfstests-dev/results//xfs/300.out.bad)
    --- tests/xfs/300.out	2016-09-03 21:49:47.529941074 -0400
    +++ /root/sandeen/xfstests-dev/results//xfs/300.out.bad	2016-09-03 21:58:45.696283777 -0400
    @@ -1,3 +1,3 @@
     QA output created by 300
     SCRATCH_MNT/300.test
    -extents before:7 after:1 DONE SCRATCH_MNT/300.test
    +XFS_IOC_SWAPEXT failed: SCRATCH_MNT/300.test: Invalid argument
    ...
    (Run 'diff -u tests/xfs/300.out /root/sandeen/xfstests-dev/results//xfs/300.out.bad'  to see the entire diff)
Ran: xfs/300
Failures: xfs/300
Failed 1 of 1 tests

If I rebuild with:
# rpm -q gcc
gcc-4.8.5-9.el7.aarch64

# rpm -e xfsprogs
# wget http://download-node-02.eng.bos.redhat.com/brewroot/packages/xfsprogs/4.5.0/6.el7/src/xfsprogs-4.5.0-6.el7.src.rpm
# rpmbuild --rebuild xfsprogs-4.5.0-6.el7.src.rpm 
# rpm -ivh /root/rpmbuild/RPMS/aarch64/xfsprogs-4.5.0-6.el7.aarch64.rpm

#./check xfs/300
xfs/300 1s ... 1s
Ran: xfs/300
Passed all 1 tests

Comment 2 Zorro Lang 2016-09-04 10:22:26 UTC
Thanks for find this, Eric. We can't find this problem due to another known issue.

When auto test with beaker, xfs/300 get below output:
---
QA output created by 300
---

When I run xfs/300 manually, I got below output:
---
QA output created by 300
SCRATCH_MNT/300.test
XFS_IOC_SWAPEXT failed: SCRATCH_MNT/300.test: Invalid argument
---

Both of them are unexpected.

Thanks,
Zorro

Comment 3 Eric Sandeen 2016-09-06 18:20:22 UTC
I may need to dig into this more; I did a scratch build of xfsprogs:

https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=11707699

which I think used the new gcc, and that build still fails.

Comment 4 Eric Sandeen 2016-09-06 18:43:59 UTC
This fails on all arches because we somehow lost a dependency on libattr, and fsr now #ifdefs away calls if configure didn't find the attr headers.

commit c14c7b795c0bdb92d925bab44e8239867b2883c6
Author: Jan Tulak <jtulak>
Date:   Wed Oct 14 10:58:03 2015 +1100

    build: Add autoconf check for fsetxattr call

This is a regression, and needs to be fixed for all arches.

Comment 5 Yaakov Selkowitz 2016-09-06 19:04:25 UTC
Link to the commit:

http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/cmds/xfsprogs.git;a=commitdiff;h=c14c7b795c0bdb92d925bab44e8239867b2883c6;hp=7f7fd8ac1f5cc0d3af7c2f050ac070a321581de5

Supposed rationale:

> OS X has fsetxattr() in another header and with different arguments.
> For now, check for the Linux variant and if not available, skip
> the code using the call.

So the configure test looks for attr/xattr.h (from libattr-devel) to determine if it's on Linux, but then uses sys/xattr.h in the sources.  Honestly, this looks bogus to me; why not just use #ifndef __APPLE__ around that code?

Looks like we need BuildRequires: libattr-devel for attr/xattr.h in the configure test.  Nothing appears to link with libattr, so glibc's implementation should still be used.

Comment 6 Eric Sandeen 2016-09-06 19:09:27 UTC
(In reply to Yaakov Selkowitz from comment #5)

Yep, not a big fan of that change - I hadn't looked at the two different headers, ugh.

> Looks like we need BuildRequires: libattr-devel for attr/xattr.h in the
> configure test.  Nothing appears to link with libattr, so glibc's
> implementation should still be used.

Yes, that's the simplest/fastest fix for now I think.

I'll look harder at the commit and why we're looking at 2 different headers, thanks!

-Eric

Comment 7 Eric Sandeen 2016-09-06 19:19:22 UTC
This does probably fix it as well, w/o the new dependency:

diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
index 7a847e9..55946ab 100644
--- a/m4/package_libcdev.m4
+++ b/m4/package_libcdev.m4
@@ -239,7 +239,7 @@ AC_DEFUN([AC_HAVE_FSETXATTR],
        have_fsetxattr=yes,
        [],
        [#include <sys/types.h>
-        #include <attr/xattr.h>]
+        #include <sys/xattr.h>]
        )
     AC_SUBST(have_fsetxattr)
   ])

Comment 8 Yaakov Selkowitz 2016-09-06 19:20:28 UTC
(In reply to Eric Sandeen from comment #6)
> (In reply to Yaakov Selkowitz from comment #5)
> 
> Yep, not a big fan of that change - I hadn't looked at the two different
> headers, ugh.
> 
> Yes, that's the simplest/fastest fix for now I think.

Scratch build in progress.

> I'll look harder at the commit and why we're looking at 2 different headers,
> thanks!

IIUC, both Darwin[1] and Linux have <sys/xattr.h> but libattr is not available on both and it provides <attr/xattr.h>, so someone thought this was a "smart" way of detecting Linux vs. Darwin fsetxattr.

IMHO the entire commit is incorrect; it should really be reverted, and the #ifdef HAVE_FSETXATTR block conditional on #ifndef __APPLE__ instead.  Alternatively, a more complicated configure test can be written to determine how many arguments fsetxattr accepts, but I'm not sure if any other systems have the extra argument.

Comment 9 Yaakov Selkowitz 2016-09-06 19:21:28 UTC
(In reply to Eric Sandeen from comment #7)
> This does probably fix it as well, w/o the new dependency:
> 
>         [#include <sys/types.h>
> -        #include <attr/xattr.h>]
> +        #include <sys/xattr.h>]

Darwin has <sys/xattr.h> too, hence that wouldn't help.

Comment 10 Yaakov Selkowitz 2016-09-06 19:25:03 UTC
Created attachment 1198407 [details]
Patch for rhel-7.3

Successful scratch build:

https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=11709316

Most notably, in build.log:

checking whether fsetxattr is declared... yes

and libattr.so.1 does NOT show up as a dependency.

Comment 14 errata-xmlrpc 2016-11-04 06:24:48 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2016-2431.html


Note You need to log in before you can comment on or make changes to this bug.