mkfs.ubifs: make failure to list extended attributes quiet and non-fatal

Message ID 20171020192807.25761-1-plroskin@gmail.com
State New
Delegated to: Richard Weinberger
Headers show
Series
  • mkfs.ubifs: make failure to list extended attributes quiet and non-fatal
Related show

Commit Message

Pavel Roskin Oct. 20, 2017, 7:28 p.m.
mkfs.ubifs treats all errors returned by llistxattr() except ENOENT as
fatal. In particular, if the host system doesn't support extended
attributes, mkfs.ubifs would fail. It happens even without any flags that
would explicitly request extended attribute support.

As a result, it's impossible to build an OpenEmbedded based UBI image on
a system without extended attribute support if the xattr feature is
enabled in DISTRO_FEATURES.

Take a clue from mkfs.jffs2, which treats llistxattr() errors as
non-fatal and doesn't report them by default. Report the error at the
debug level 1 and higher.

The second llistxattr() error remains fatal, as it indicates unexpected
behavior.

Don't use goto before the buffer is allocated. Don't initialize buf with
NULL, it should not be used before the xmalloc() call.

Signed-off-by: Pavel Roskin <plroskin@gmail.com>
---
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

Comments

David Oberhollenzer Oct. 23, 2017, 9:58 p.m. | #1
Hi,

On 10/20/2017 09:28 PM, Pavel Roskin wrote:
> mkfs.ubifs treats all errors returned by llistxattr() except ENOENT as
> fatal. In particular, if the host system doesn't support extended
> attributes, mkfs.ubifs would fail. It happens even without any flags that
> would explicitly request extended attribute support.
> 
Would it also work if we simply extended the check you patched to
test for ENOTSUP in addition?

Thanks,

David
Pavel Roskin Oct. 23, 2017, 11:17 p.m. | #2
Hello David,

On Mon, Oct 23, 2017 at 2:58 PM, David Oberhollenzer
<david.oberhollenzer@sigma-star.at> wrote:
> Hi,
>
> On 10/20/2017 09:28 PM, Pavel Roskin wrote:
>> mkfs.ubifs treats all errors returned by llistxattr() except ENOENT as
>> fatal. In particular, if the host system doesn't support extended
>> attributes, mkfs.ubifs would fail. It happens even without any flags that
>> would explicitly request extended attribute support.
>>
> Would it also work if we simply extended the check you patched to
> test for ENOTSUP in addition?

Yes, that would work. I checked the OpenEmbedded build with the new
patch, it passes. The error is called EOPNOTSUPP. I can submit the
patch if that's the approach you prefer.

Patch

diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
index d432dfe..e390b5a 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
@@ -1122,22 +1122,17 @@  static int inode_add_xattr(struct ubifs_ino_node *host_ino,
 {
 	int ret;
 	struct qstr nm;
-	void *buf = NULL;
+	void *buf;
 	ssize_t len;
 	ssize_t pos = 0;
 
 	len = llistxattr(path_name, NULL, 0);
-	if (len < 0) {
-		if (errno == ENOENT)
-			return 0;
-
-		sys_err_msg("llistxattr failed on %s", path_name);
-
-		return len;
-	}
+	if (len < 0)
+		dbg_msg(1, "llistxattr('%s') = %d : %s\n", path_name,
+			errno, strerror(errno));
 
-	if (len == 0)
-		goto noxattr;
+	if (len <= 0)
+		return 0;
 
 	buf = xmalloc(len);
 
@@ -1191,10 +1186,6 @@  static int inode_add_xattr(struct ubifs_ino_node *host_ino,
 			goto out_free;
 	}
 
-noxattr:
-	free(buf);
-	return 0;
-
 out_free:
 	free(buf);