[OE-core][PATCH] 1/2] wic: Add --no-fstab-update part option

[OE-core][PATCH] 1/2] wic: Add --no-fstab-update part option

[Daniel Gomez]

When embedding a rootfs image (e.g. 'rootfs-dir') as a partition we might
want to keep the stock fstab for that image. In such case, use
this option to not update the fstab and use the stock one instead.

This option allows to specify which partitions get the fstab updated
and which get the stock fstab.

The option matches the argument you can pass to wic itself where the
same action is performed but for all the partitions.

Example:
    part /export --source rootfs --rootfs-dir=hockeycam-image --fstype=ext4
--label export --align 1024 --no-fstab-update

    part / --source rootfs --fstype=ext4 --label rootfs --align 1024

Signed-off-by: Daniel Gomez <daniel@qtec.com>
---
 scripts/lib/wic/help.py      | 3 +++
 scripts/lib/wic/ksparser.py  | 1 +
 scripts/lib/wic/partition.py | 5 +++--
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index 991e5094bb..5c3d278d4e 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -991,6 +991,9 @@ DESCRIPTION
                              multiple partitions and we want to keep the right
                              permissions and usernames in all the partitions.
 
+         --no-fstab-update: This option is specific to wic. It does not update the
+                            '/etc/fstab' stock file for the given partition.
+
          --extra-space: This option is specific to wic. It adds extra
                         space after the space filled by the content
                         of the partition. The final size can go
diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index 7a4cc83af5..0df9eb0d05 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -185,6 +185,7 @@ class KickStart():
         part.add_argument('--use-uuid', action='store_true')
         part.add_argument('--uuid')
         part.add_argument('--fsuuid')
+        part.add_argument('--no-fstab-update', action='store_true')
 
         bootloader = subparsers.add_parser('bootloader')
         bootloader.add_argument('--append')
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index e0b2c5bdf2..ab304f1b2a 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -54,6 +54,7 @@ class Partition():
         self.uuid = args.uuid
         self.fsuuid = args.fsuuid
         self.type = args.type
+        self.no_fstab_update = args.no_fstab_update
         self.updated_fstab_path = None
         self.has_fstab = False
         self.update_fstab_in_rootfs = False
@@ -286,7 +287,7 @@ 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:
+        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")
@@ -350,7 +351,7 @@ 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:
+        if self.updated_fstab_path and self.has_fstab and not self.no_fstab_update:
             mcopy_cmd = "mcopy -i %s %s ::/etc/fstab" % (rootfs, self.updated_fstab_path)
             exec_native_cmd(mcopy_cmd, native_sysroot)
 
-- 
2.30.2

[OE-core][PATCH] 2/2] oeqa: wic: Add tests for --no-fstab-update

[Daniel Gomez]

Add tests for the --no-fstab-update wic part command.

Signed-off-by: Daniel Gomez <daniel@qtec.com>
---
 meta/lib/oeqa/selftest/cases/wic.py | 56 +++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/wic.py b/meta/lib/oeqa/selftest/cases/wic.py
index 2efbe514c1..a58360851a 100644
--- a/meta/lib/oeqa/selftest/cases/wic.py
+++ b/meta/lib/oeqa/selftest/cases/wic.py
@@ -11,6 +11,7 @@
 import os
 import sys
 import unittest
+import hashlib
 
 from glob import glob
 from shutil import rmtree, copy
@@ -686,6 +687,61 @@ part /etc --source rootfs --fstype=ext4 --change-directory=etc
                                       % (wks_file, self.resultdir), ignore_status=True).status)
         os.remove(wks_file)
 
+    def test_no_fstab_update(self):
+        """Test --no-fstab-update wks option."""
+
+        oldpath = os.environ['PATH']
+        os.environ['PATH'] = get_bb_var("PATH", "wic-tools")
+
+        # Get stock fstab from base-files recipe
+        bitbake('base-files')
+        bf_fstab = os.path.join(get_bb_var('WORKDIR', 'base-files'),'image/etc/fstab')
+        bf_fstab_md5sum = runCmd('md5sum %s 2>/dev/null' % bf_fstab).output.split(" ")[0]
+
+        try:
+            no_fstab_update_path = os.path.join(self.resultdir, 'test-no-fstab-update')
+            os.makedirs(no_fstab_update_path)
+            wks_file = os.path.join(no_fstab_update_path, 'temp.wks')
+            with open(wks_file, 'w') as wks:
+                wks.writelines(['part / --source rootfs --fstype=ext4 --label rootfs\n',
+                                'part /mnt/p2 --source rootfs --rootfs-dir=core-image-minimal ',
+                                '--fstype=ext4 --label p2 --no-fstab-update\n'])
+            runCmd("wic create %s -e core-image-minimal -o %s" \
+                                       % (wks_file, self.resultdir))
+
+            part_fstab_md5sum = []
+            for i in range(1, 3):
+                part = glob(os.path.join(self.resultdir, 'temp-*.direct.p') + str(i))[0]
+                part_fstab = runCmd("debugfs -R 'cat etc/fstab' %s 2>/dev/null" % (part))
+                part_fstab_md5sum.append(hashlib.md5((part_fstab.output + "\n\n").encode('utf-8')).hexdigest())
+
+            # '/etc/fstab' in partition 2 should contain the same stock fstab file at base-file recipe.
+            self.assertEqual(bf_fstab_md5sum, part_fstab_md5sum[1])
+
+            # '/etc/fstab' in partition 1 should contain an updated fstab file.
+            self.assertNotEqual(bf_fstab_md5sum, part_fstab_md5sum[0])
+
+        finally:
+            os.environ['PATH'] = oldpath
+
+    def test_no_fstab_update_errors(self):
+        """Test --no-fstab-update wks option error handling."""
+        wks_file = 'temp.wks'
+
+        # Absolute argument.
+        with open(wks_file, 'w') as wks:
+            wks.write("part / --source rootfs --fstype=ext4 --no-fstab-update /etc")
+        self.assertNotEqual(0, runCmd("wic create %s -e core-image-minimal -o %s" \
+                                      % (wks_file, self.resultdir), ignore_status=True).status)
+        os.remove(wks_file)
+
+        # Argument pointing to parent directory.
+        with open(wks_file, 'w') as wks:
+            wks.write("part / --source rootfs --fstype=ext4 --no-fstab-update ././..")
+        self.assertNotEqual(0, runCmd("wic create %s -e core-image-minimal -o %s" \
+                                      % (wks_file, self.resultdir), ignore_status=True).status)
+        os.remove(wks_file)
+
 class Wic2(WicTestCase):
 
     def test_bmap_short(self):
-- 
2.30.2

Re: [OE-core][PATCH] 1/2] wic: Add --no-fstab-update part option

[Paul Barker]

[-- Attachment #1: Type: text/plain, Size: 4195 bytes --]

On Tue, 10 Aug 2021 15:06:02 +0200
Daniel Gomez <daniel@qtec.com> wrote:

> When embedding a rootfs image (e.g. 'rootfs-dir') as a partition we might
> want to keep the stock fstab for that image. In such case, use
> this option to not update the fstab and use the stock one instead.
> 
> This option allows to specify which partitions get the fstab updated
> and which get the stock fstab.
> 
> The option matches the argument you can pass to wic itself where the
> same action is performed but for all the partitions.
> 
> Example:
>     part /export --source rootfs --rootfs-dir=hockeycam-image --fstype=ext4
> --label export --align 1024 --no-fstab-update
> 
>     part / --source rootfs --fstype=ext4 --label rootfs --align 1024
> 
> Signed-off-by: Daniel Gomez <daniel@qtec.com>
> ---
>  scripts/lib/wic/help.py      | 3 +++
>  scripts/lib/wic/ksparser.py  | 1 +
>  scripts/lib/wic/partition.py | 5 +++--
>  3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
> index 991e5094bb..5c3d278d4e 100644
> --- a/scripts/lib/wic/help.py
> +++ b/scripts/lib/wic/help.py
> @@ -991,6 +991,9 @@ DESCRIPTION
>                               multiple partitions and we want to keep the right
>                               permissions and usernames in all the partitions.
>  
> +         --no-fstab-update: This option is specific to wic. It does not update the
> +                            '/etc/fstab' stock file for the given partition.
> +
>           --extra-space: This option is specific to wic. It adds extra
>                          space after the space filled by the content
>                          of the partition. The final size can go
> diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
> index 7a4cc83af5..0df9eb0d05 100644
> --- a/scripts/lib/wic/ksparser.py
> +++ b/scripts/lib/wic/ksparser.py
> @@ -185,6 +185,7 @@ class KickStart():
>          part.add_argument('--use-uuid', action='store_true')
>          part.add_argument('--uuid')
>          part.add_argument('--fsuuid')
> +        part.add_argument('--no-fstab-update', action='store_true')
>  
>          bootloader = subparsers.add_parser('bootloader')
>          bootloader.add_argument('--append')
> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> index e0b2c5bdf2..ab304f1b2a 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -54,6 +54,7 @@ class Partition():
>          self.uuid = args.uuid
>          self.fsuuid = args.fsuuid
>          self.type = args.type
> +        self.no_fstab_update = args.no_fstab_update
>          self.updated_fstab_path = None
>          self.has_fstab = False
>          self.update_fstab_in_rootfs = False
> @@ -286,7 +287,7 @@ 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:
> +        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")
> @@ -350,7 +351,7 @@ 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:
> +        if self.updated_fstab_path and self.has_fstab and not self.no_fstab_update:
>              mcopy_cmd = "mcopy -i %s %s ::/etc/fstab" % (rootfs, self.updated_fstab_path)
>              exec_native_cmd(mcopy_cmd, native_sysroot)
>  

This looks like the right approach. However, you're still missing the
handling for filesystems other than ext2/3/4 & msdos. See
http://git.yoctoproject.org/cgit/cgit.cgi/poky/tree/scripts/lib/wic/plugins/source/rootfs.py#n218
and grep to see if there are any other uses of updated_fstab_path.

Thanks,

-- 
Paul Barker
https://pbarker.dev/

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

Re: [OE-core][PATCH] 1/2] wic: Add --no-fstab-update part option

[Daniel Gomez]

Hi Paul,

On Tue, 10 Aug 2021 at 15:25, Paul Barker <paul@pbarker.dev> wrote:
>
> On Tue, 10 Aug 2021 15:06:02 +0200
> Daniel Gomez <daniel@qtec.com> wrote:
>
> > When embedding a rootfs image (e.g. 'rootfs-dir') as a partition we might
> > want to keep the stock fstab for that image. In such case, use
> > this option to not update the fstab and use the stock one instead.
> >
> > This option allows to specify which partitions get the fstab updated
> > and which get the stock fstab.
> >
> > The option matches the argument you can pass to wic itself where the
> > same action is performed but for all the partitions.
> >
> > Example:
> >     part /export --source rootfs --rootfs-dir=hockeycam-image --fstype=ext4
> > --label export --align 1024 --no-fstab-update
> >
> >     part / --source rootfs --fstype=ext4 --label rootfs --align 1024
> >
> > Signed-off-by: Daniel Gomez <daniel@qtec.com>
> > ---
> >  scripts/lib/wic/help.py      | 3 +++
> >  scripts/lib/wic/ksparser.py  | 1 +
> >  scripts/lib/wic/partition.py | 5 +++--
> >  3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
> > index 991e5094bb..5c3d278d4e 100644
> > --- a/scripts/lib/wic/help.py
> > +++ b/scripts/lib/wic/help.py
> > @@ -991,6 +991,9 @@ DESCRIPTION
> >                               multiple partitions and we want to keep the right
> >                               permissions and usernames in all the partitions.
> >
> > +         --no-fstab-update: This option is specific to wic. It does not update the
> > +                            '/etc/fstab' stock file for the given partition.
> > +
> >           --extra-space: This option is specific to wic. It adds extra
> >                          space after the space filled by the content
> >                          of the partition. The final size can go
> > diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
> > index 7a4cc83af5..0df9eb0d05 100644
> > --- a/scripts/lib/wic/ksparser.py
> > +++ b/scripts/lib/wic/ksparser.py
> > @@ -185,6 +185,7 @@ class KickStart():
> >          part.add_argument('--use-uuid', action='store_true')
> >          part.add_argument('--uuid')
> >          part.add_argument('--fsuuid')
> > +        part.add_argument('--no-fstab-update', action='store_true')
> >
> >          bootloader = subparsers.add_parser('bootloader')
> >          bootloader.add_argument('--append')
> > diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> > index e0b2c5bdf2..ab304f1b2a 100644
> > --- a/scripts/lib/wic/partition.py
> > +++ b/scripts/lib/wic/partition.py
> > @@ -54,6 +54,7 @@ class Partition():
> >          self.uuid = args.uuid
> >          self.fsuuid = args.fsuuid
> >          self.type = args.type
> > +        self.no_fstab_update = args.no_fstab_update
> >          self.updated_fstab_path = None
> >          self.has_fstab = False
> >          self.update_fstab_in_rootfs = False
> > @@ -286,7 +287,7 @@ 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:
> > +        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")
> > @@ -350,7 +351,7 @@ 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:
> > +        if self.updated_fstab_path and self.has_fstab and not self.no_fstab_update:
> >              mcopy_cmd = "mcopy -i %s %s ::/etc/fstab" % (rootfs, self.updated_fstab_path)
> >              exec_native_cmd(mcopy_cmd, native_sysroot)
> >
>
> This looks like the right approach. However, you're still missing the
> handling for filesystems other than ext2/3/4 & msdos. See
> http://git.yoctoproject.org/cgit/cgit.cgi/poky/tree/scripts/lib/wic/plugins/source/rootfs.py#n218
> and grep to see if there are any other uses of updated_fstab_path.

Thanks for your quick reply. Sorry, I wasn't sure about that part of
the code so thanks for mentioning it again.
There is this line:

        install_cmd = "install -m 0644 %s %s" %
(part.updated_fstab_path, fstab_path)

Which is always replace/updated (for msdos/ext2/3/4) in partition.py with:

        if self.updated_fstab_path and self.has_fstab and not
self.no_fstab_update

I will then include the  'no_fstab_update' partition option inside
rootfs.py so we make sure the partition for the rest of the formats
won't get the fstab updated file.

I'll send a v2 patch.

>
> Thanks,
>
> --
> Paul Barker
> https://pbarker.dev/