diff mbox series

[1/1] Fstab: Fix xattrs not being maintained on fstab file when using wic fstab update funtionalities

Message ID 20251017111210.501333-2-dani.barra25@gmail.com
State New
Headers show
Series [1/1] Fstab: Fix xattrs not being maintained on fstab file when using wic fstab update funtionalities | expand

Commit Message

Daniel Andrade Oct. 17, 2025, 11:04 a.m. UTC 
From: Daniel Andrade <dani.barra25@gmail.com>

The wic fstab generation capability was removing the xattrs of the /etc/fstab, possibly rendering the produced image unusable in some case (e.g.: SELinux enforced on a read-only rootfs).
The file appears as unlabeled no matter the fstype chosen.
To bypass this, some late procedures done on ext* and msdos fstype have been removed and all the fstab logic was passed to rootfs.py.
On this one, the idea is to still use the install command, but in this case, executed outside the pseudo environment. After the file as sucessfully been place, the idea was to check again for inconsistencies
in the database (now the fstab inode is different) and force it to rebuild with the correct inode, ensure fstab inode is correctly set on all the tables, including the xattrs one.

Signed-off-by: Daniel Andrade <dani.barra25@gmail.com>
---
 scripts/lib/wic/partition.py             | 15 +--------------
 scripts/lib/wic/plugins/source/rootfs.py |  8 ++++----
 2 files changed, 5 insertions(+), 18 deletions(-)

Comments

Mathieu Dubois-Briand Oct. 19, 2025, 10:02 a.m. UTC | #1 
On Fri Oct 17, 2025 at 1:04 PM CEST, dani.barra25 wrote:
> From: Daniel Andrade <dani.barra25@gmail.com>
>
> The wic fstab generation capability was removing the xattrs of the /etc/fstab, possibly rendering the produced image unusable in some case (e.g.: SELinux enforced on a read-only rootfs).
> The file appears as unlabeled no matter the fstype chosen.
> To bypass this, some late procedures done on ext* and msdos fstype have been removed and all the fstab logic was passed to rootfs.py.
> On this one, the idea is to still use the install command, but in this case, executed outside the pseudo environment. After the file as sucessfully been place, the idea was to check again for inconsistencies
> in the database (now the fstab inode is different) and force it to rebuild with the correct inode, ensure fstab inode is correctly set on all the tables, including the xattrs one.
>
> Signed-off-by: Daniel Andrade <dani.barra25@gmail.com>
> ---

Hi Daniel,

Thanks for the new version. It looks like this one is now breaking
wic.Wic.test_exclude_path and wic.Wic.test_include_path tests:

2025-10-19 07:55:18,010 - oe-selftest - INFO - wic.Wic.test_exclude_path (subunit.RemotedTestCase)
2025-10-19 07:55:18,011 - oe-selftest - INFO -  ... FAIL
...
AssertionError: Command 'wic create temp.wks -e core-image-minimal -o /srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/wic-tmp' returned non-zero exit status 1:
...
ERROR: _exec_cmd: export PATH=/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/sbin:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/usr/sbin:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/usr/bin:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/usr/bin/x86_64-poky-linux:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/bin:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/hosttools:$PATH;export PSEUDO_PREFIX=/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/usr;export PSEUDO_LOCALSTATEDIR=/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/wic-tmp/tmp.wic.dhlnwwia/pseudo1;export PSEUDO_PASSWD=/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/wic-tmp/tmp.wic.dhlnwwia/rootfs1;export PSEUDO_NOSYMLINKEXP=1;/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/sysroots-components/x86_64/pseudo-native/usr/bin/pseudo  -B  returned '1' instead of 0
output: PRAGMA journal_mode = OFF;: database is locked
PRAGMA synchronous = OFF;: database is locked
Failed: database is locked
Error getting 'logs.db' database.
PRAGMA journal_mode = OFF;: database is locked
PRAGMA synchronous = OFF;: database is locked
Failed: database is locked
Error getting 'files.db' database.
error during load from disk: database is locked
pdb_files: database error.
Couldn't start file list, can't scan.
db cleanup for server shutdown, 07:55:17.956
memory-to-file backup complete, 07:55:17.956.
db cleanup finished, 07:55:17.956
considering table files
considering table xattrs
considering table migrations

https://autobuilder.yoctoproject.org/valkyrie/#/builders/35/builds/2574

Can you have a look at these errors?

Thanks,
Mathieu
Daniel Andrade Oct. 19, 2025, 3:41 p.m. UTC | #2 
Hi Mathieu,

I did not notice that it was failing other tests. It seemed like the error
was caused due to a lock file. To fix it, before issuing the repair of the
database, I first shutdown any open connection using -S. Doing this I saw
that the 3 tests passed and fstab was correctly labeled.
The other thing I noticed was the different timestamps, but as I said a
while ago, it was caused due to using SOURCE_DATE_EPOCH,

REPRODUCIBLE_TIMESTAMP_ROOTFS, and SOURCE_DATE_EPOCH_FALLBACK. This should
also be reviewed since it might compromise build repeatability.

In any case, it was not the topic of the patch, I just wanted to state this
in case someone checked the contents of the produced image.

Best Regards,
Daniel

A domingo, 19/10/2025, 11:02, Mathieu Dubois-Briand <
mathieu.dubois-briand@bootlin.com> escreveu:

> On Fri Oct 17, 2025 at 1:04 PM CEST, dani.barra25 wrote:
> > From: Daniel Andrade <dani.barra25@gmail.com>
> >
> > The wic fstab generation capability was removing the xattrs of the
> /etc/fstab, possibly rendering the produced image unusable in some case
> (e.g.: SELinux enforced on a read-only rootfs).
> > The file appears as unlabeled no matter the fstype chosen.
> > To bypass this, some late procedures done on ext* and msdos fstype have
> been removed and all the fstab logic was passed to rootfs.py.
> > On this one, the idea is to still use the install command, but in this
> case, executed outside the pseudo environment. After the file as
> sucessfully been place, the idea was to check again for inconsistencies
> > in the database (now the fstab inode is different) and force it to
> rebuild with the correct inode, ensure fstab inode is correctly set on all
> the tables, including the xattrs one.
> >
> > Signed-off-by: Daniel Andrade <dani.barra25@gmail.com>
> > ---
>
> Hi Daniel,
>
> Thanks for the new version. It looks like this one is now breaking
> wic.Wic.test_exclude_path and wic.Wic.test_include_path tests:
>
> 2025-10-19 07:55:18,010 - oe-selftest - INFO - wic.Wic.test_exclude_path
> (subunit.RemotedTestCase)
> 2025-10-19 07:55:18,011 - oe-selftest - INFO -  ... FAIL
> ...
> AssertionError: Command 'wic create temp.wks -e core-image-minimal -o
> /srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/wic-tmp'
> returned non-zero exit status 1:
> ...
> ERROR: _exec_cmd: export
> PATH=/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/sbin:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/usr/sbin:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/usr/bin:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/usr/bin/x86_64-poky-linux:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/bin:/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/hosttools:$PATH;export
> PSEUDO_PREFIX=/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/work/x86-64-v3-poky-linux/wic-tools/1.0/recipe-sysroot-native/usr;export
> PSEUDO_LOCALSTATEDIR=/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/wic-tmp/tmp.wic.dhlnwwia/pseudo1;export
> PSEUDO_PASSWD=/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/wic-tmp/tmp.wic.dhlnwwia/rootfs1;export
> PSEUDO_NOSYMLINKEXP=1;/srv/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-3133304/tmp/sysroots-components/x86_64/pseudo-native/usr/bin/pseudo
> -B  returned '1' instead of 0
> output: PRAGMA journal_mode = OFF;: database is locked
> PRAGMA synchronous = OFF;: database is locked
> Failed: database is locked
> Error getting 'logs.db' database.
> PRAGMA journal_mode = OFF;: database is locked
> PRAGMA synchronous = OFF;: database is locked
> Failed: database is locked
> Error getting 'files.db' database.
> error during load from disk: database is locked
> pdb_files: database error.
> Couldn't start file list, can't scan.
> db cleanup for server shutdown, 07:55:17.956
> memory-to-file backup complete, 07:55:17.956.
> db cleanup finished, 07:55:17.956
> considering table files
> considering table xattrs
> considering table migrations
>
> https://autobuilder.yoctoproject.org/valkyrie/#/builders/35/builds/2574
>
> Can you have a look at these errors?
>
> Thanks,
> Mathieu
>
> --
> Mathieu Dubois-Briand, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
>
diff mbox series

Patch

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index bf2c34d594..82d754835c 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -131,7 +131,7 @@  class Partition():
         partition command parameters.
         """
         self.updated_fstab_path = updated_fstab_path
-        if self.updated_fstab_path and not (self.fstype.startswith("ext") or self.fstype == "msdos"):
+        if self.updated_fstab_path:
             self.update_fstab_in_rootfs = True
 
         if not self.source:
@@ -295,15 +295,6 @@  class Partition():
             (self.fstype, extraopts, rootfs, label_str, self.fsuuid, rootfs_dir)
         exec_native_cmd(mkfs_cmd, native_sysroot, pseudo=pseudo)
 
-        if self.updated_fstab_path and self.has_fstab and not self.no_fstab_update:
-            debugfs_script_path = os.path.join(cr_workdir, "debugfs_script")
-            with open(debugfs_script_path, "w") as f:
-                f.write("cd etc\n")
-                f.write("rm fstab\n")
-                f.write("write %s fstab\n" % (self.updated_fstab_path))
-            debugfs_cmd = "debugfs -w -f %s %s" % (debugfs_script_path, rootfs)
-            exec_native_cmd(debugfs_cmd, native_sysroot)
-
         mkfs_cmd = "fsck.%s -pvfD %s" % (self.fstype, rootfs)
         exec_native_cmd(mkfs_cmd, native_sysroot, pseudo=pseudo)
 
@@ -400,10 +391,6 @@  class Partition():
         mcopy_cmd = "mcopy -i %s -s %s/* ::/" % (rootfs, rootfs_dir)
         exec_native_cmd(mcopy_cmd, native_sysroot)
 
-        if self.updated_fstab_path and self.has_fstab and not self.no_fstab_update:
-            mcopy_cmd = "mcopy -m -i %s %s ::/etc/fstab" % (rootfs, self.updated_fstab_path)
-            exec_native_cmd(mcopy_cmd, native_sysroot)
-
         chmod_cmd = "chmod 644 %s" % rootfs
         exec_cmd(chmod_cmd)
 
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index e29f3a4c2f..c0d9a28b91 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -225,11 +225,11 @@  class RootfsPlugin(SourcePlugin):
                 fstab_path = os.path.join(new_rootfs, "etc/fstab")
                 # Assume that fstab should always be owned by root with fixed permissions
                 install_cmd = "install -m 0644 -p %s %s" % (part.updated_fstab_path, fstab_path)
+                exec_native_cmd(install_cmd, native_sysroot)
                 if new_pseudo:
-                    pseudo = cls.__get_pseudo(native_sysroot, new_rootfs, new_pseudo)
-                else:
-                    pseudo = None
-                exec_native_cmd(install_cmd, native_sysroot, pseudo)
+                    pseudo_cmd = "%s -B " % (cls.__get_pseudo(native_sysroot, new_rootfs, new_pseudo))
+                    exec_native_cmd(pseudo_cmd, native_sysroot)
+
 
         part.prepare_rootfs(cr_workdir, oe_builddir,
                             new_rootfs or part.rootfs_dir, native_sysroot,