[PATCH 1/6] wic: Add workdir argument

[PATCH 1/6] wic: Add workdir argument

[Paul Barker]

This allows the path for the temporary workdir used by wic to be set
when running wic from bitbake or directly from the command line.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 scripts/lib/wic/plugins/imager/direct.py | 2 +-
 scripts/wic                              | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index 0ca67b77d5..b329568c7a 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -62,7 +62,7 @@ class DirectPlugin(ImagerPlugin):
 
         self.name = "%s-%s" % (os.path.splitext(os.path.basename(wks_file))[0],
                                strftime("%Y%m%d%H%M"))
-        self.workdir = tempfile.mkdtemp(dir=self.outdir, prefix='tmp.wic.')
+        self.workdir = options.workdir or tempfile.mkdtemp(dir=self.outdir, prefix='tmp.wic.')
         self._image = None
         self.ptable_format = self.ks.bootloader.ptable
         self.parts = self.ks.partitions
diff --git a/scripts/wic b/scripts/wic
index 24700f380f..a741aed364 100755
--- a/scripts/wic
+++ b/scripts/wic
@@ -312,6 +312,8 @@ def wic_init_parser_create(subparser):
 
     subparser.add_argument("-o", "--outdir", dest="outdir", default='.',
                       help="name of directory to create image in")
+    subparser.add_argument("-w", "--workdir",
+                      help="temporary workdir to use for intermediate files")
     subparser.add_argument("-e", "--image-name", dest="image_name",
                       help="name of the image to use the artifacts from "
                            "e.g. core-image-sato")
-- 
2.26.2

[PATCH 2/6] image_types_wic: Move wic working directory

[Paul Barker]

By default the wic working directory is placed under the output
directory. When invoking wic under bitbake, the wic output directory is
added to PSEUDO_PATHS_IGNORE to avoid issues with files being removed
from outside a pseudo environment (see oe-core commit ad8f5532ff).

However, wic will copy the rootfs directory into its working directory
if it needs to add or remove content before creating a filesystem image.
This copy of the rootfs directory must be tracked by pseudo in order to
keep the permissions correct in the resulting image. So we can't have
the wic working directory under a path in PSEUDO_PATHS_IGNORE unless
we like broken permissions.

To fix this the new '-w' argument to wic is used to move the working
directory away from the output directory.

Note that wic deletes the temporary working directory automatically
when it finishes creating an image so users won't normally see the
'tmp-wic' directory under WORKDIR.

Fixes [Yocto #14129]

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 meta/classes/image_types_wic.bbclass | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/classes/image_types_wic.bbclass b/meta/classes/image_types_wic.bbclass
index 286e0f5d54..a5ed780ceb 100644
--- a/meta/classes/image_types_wic.bbclass
+++ b/meta/classes/image_types_wic.bbclass
@@ -29,15 +29,16 @@ WIC_CREATE_EXTRA_ARGS ?= ""
 IMAGE_CMD_wic () {
 	out="${IMGDEPLOYDIR}/${IMAGE_NAME}"
 	build_wic="${WORKDIR}/build-wic"
+	tmp_wic="${WORKDIR}/tmp-wic"
 	wks="${WKS_FULL_PATH}"
 	if [ -z "$wks" ]; then
 		bbfatal "No kickstart files from WKS_FILES were found: ${WKS_FILES}. Please set WKS_FILE or WKS_FILES appropriately."
 	fi
-	BUILDDIR="${TOPDIR}" PSEUDO_UNLOAD=1 wic create "$wks" --vars "${STAGING_DIR}/${MACHINE}/imgdata/" -e "${IMAGE_BASENAME}" -o "$build_wic/" ${WIC_CREATE_EXTRA_ARGS}
+	BUILDDIR="${TOPDIR}" PSEUDO_UNLOAD=1 wic create "$wks" --vars "${STAGING_DIR}/${MACHINE}/imgdata/" -e "${IMAGE_BASENAME}" -o "$build_wic/" -w "$tmp_wic" ${WIC_CREATE_EXTRA_ARGS}
 	mv "$build_wic/$(basename "${wks%.wks}")"*.direct "$out${IMAGE_NAME_SUFFIX}.wic"
 }
 IMAGE_CMD_wic[vardepsexclude] = "WKS_FULL_PATH WKS_FILES TOPDIR"
-do_image_wic[cleandirs] = "${WORKDIR}/build-wic"
+do_image_wic[cleandirs] = "${WORKDIR}/build-wic ${WORKDIR}/tmp-wic"
 
 PSEUDO_IGNORE_PATHS .= ",${WORKDIR}/build-wic"
 
-- 
2.26.2

[PATCH 3/6] wic: Allow exec_native_cmd to run HOSTTOOLS

[Paul Barker]

This allows programs from HOSTTOOLS (e.g. 'install', 'rm', 'mv', etc) to
be more easily executed by wic. Without this change only programs from
an actual *-native recipe built by bitbake can be executed by wic.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 meta/classes/image_types_wic.bbclass | 2 +-
 scripts/lib/wic/misc.py              | 7 +++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/meta/classes/image_types_wic.bbclass b/meta/classes/image_types_wic.bbclass
index a5ed780ceb..48f015de9b 100644
--- a/meta/classes/image_types_wic.bbclass
+++ b/meta/classes/image_types_wic.bbclass
@@ -3,7 +3,7 @@
 WICVARS ?= "\
            BBLAYERS IMGDEPLOYDIR DEPLOY_DIR_IMAGE FAKEROOTCMD IMAGE_BASENAME IMAGE_EFI_BOOT_FILES IMAGE_BOOT_FILES \
            IMAGE_LINK_NAME IMAGE_ROOTFS INITRAMFS_FSTYPES INITRD INITRD_LIVE ISODIR RECIPE_SYSROOT_NATIVE \
-           ROOTFS_SIZE STAGING_DATADIR STAGING_DIR STAGING_LIBDIR TARGET_SYS \
+           ROOTFS_SIZE STAGING_DATADIR STAGING_DIR STAGING_LIBDIR TARGET_SYS HOSTTOOLS_DIR \
            KERNEL_IMAGETYPE MACHINE INITRAMFS_IMAGE INITRAMFS_IMAGE_BUNDLE INITRAMFS_LINK_NAME APPEND \
            ASSUME_PROVIDED PSEUDO_IGNORE_PATHS"
 
diff --git a/scripts/lib/wic/misc.py b/scripts/lib/wic/misc.py
index 4b08d649c6..75b219cd3f 100644
--- a/scripts/lib/wic/misc.py
+++ b/scripts/lib/wic/misc.py
@@ -138,9 +138,12 @@ def exec_native_cmd(cmd_and_args, native_sysroot, pseudo=""):
     if pseudo:
         cmd_and_args = pseudo + cmd_and_args
 
-    native_paths = "%s/sbin:%s/usr/sbin:%s/usr/bin:%s/bin" % \
+    hosttools_dir = get_bitbake_var("HOSTTOOLS_DIR")
+
+    native_paths = "%s/sbin:%s/usr/sbin:%s/usr/bin:%s/bin:%s" % \
                    (native_sysroot, native_sysroot,
-                    native_sysroot, native_sysroot)
+                    native_sysroot, native_sysroot,
+                    hosttools_dir)
 
     native_cmd_and_args = "export PATH=%s:$PATH;%s" % \
                    (native_paths, cmd_and_args)
-- 
2.26.2

[PATCH 4/6] wic: Update pseudo db when excluding content from rootfs

[Paul Barker]

To exclude content from the rootfs, wic makes a copy (using hardlinks if
possible) of the rootfs directory and associated pseudo db, then removes
files & directories as needed. However if these files and directories
are removed using the python functions os.remove and shutil.rmtree, the
copied pseudo db will not be updated correctly. For files copied from
the original rootfs, if hardlinks were used successfully when copying
the rootfs this should mean that the relevant inodes can't be reused and
so the risk of pseudo aborts should be avoided. However, this logic
doesn't apply for directories (as they can't be hardlinked) or for files
added via the '--include-path' argument (as they weren't present in the
original rootfs) and so there remains some risk of inodes being reused
and the pseudo db becoming corrupted.

To fix this, use the 'rm' command under pseudo when removing files &
directories from the copied rootfs to ensure that the copied pseudo db
is updated.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 scripts/lib/wic/plugins/source/rootfs.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index f1db83f8a1..afb1eb9202 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -199,17 +199,20 @@ class RootfsPlugin(SourcePlugin):
                 if not os.path.lexists(full_path):
                     continue
 
+                if new_pseudo:
+                    pseudo = cls.__get_pseudo(native_sysroot, new_rootfs, new_pseudo)
+                else:
+                    pseudo = None
                 if path.endswith(os.sep):
                     # Delete content only.
                     for entry in os.listdir(full_path):
                         full_entry = os.path.join(full_path, entry)
-                        if os.path.isdir(full_entry) and not os.path.islink(full_entry):
-                            shutil.rmtree(full_entry)
-                        else:
-                            os.remove(full_entry)
+                        rm_cmd = "rm -rf %s" % (full_entry)
+                        exec_native_cmd(rm_cmd, native_sysroot, pseudo)
                 else:
                     # Delete whole directory.
-                    shutil.rmtree(full_path)
+                    rm_cmd = "rm -rf %s" % (full_path)
+                    exec_native_cmd(rm_cmd, native_sysroot, pseudo)
 
         part.prepare_rootfs(cr_workdir, oe_builddir,
                             new_rootfs or part.rootfs_dir, native_sysroot,
-- 
2.26.2

[PATCH 5/6] wic: Copy rootfs dir if fstab needs updating

[Paul Barker]

By default, wic updates the /etc/fstab in the rootfs to include details
of additional partitions described in the selected wks file. If this
modification is performed in place, other tasks which create an image
file from the rootfs directory (e.g. do_image_tar and do_image_ext4)
will pick up the modified fstab file which would not be appropriate for
those images as they do not include the additional partitions described
in the wks file. wic does undo modifications to the fstab file once it
has finished creating the filesystem image, however this leaves open a
race condition if one of the other tasks reads the contents of the fstab
file from the rootfs directory between the point where wic modifies the
fstab file and the point where wic restores the files original content.

This could be solved by adding a lockfile for tasks which use the rootfs
directory to ensure that no other such task is reading the rootfs
directory while do_image_wic is running. This would serialize several
do_image_* tasks and result in slower builds, especially for large
images. Another drawback of this solution is that it is hard to
selectively optimise - adding lockfiles to do_image_* tasks would result
in these tasks always being serialized even if no fstab modification
will take place.

An alternative solution is to copy the rootfs directory when fstab needs
to be modified. The code to do this in wic already exists as it is
needed when including or excluding content in the rootfs. This still
results in an impact on build times but the copy uses hardlinks if
possible (so little data is actually copied) and we can make selective
optimisations to improve things. The rootfs copy will only take place if
fstab modification is required (or if it was already needed to include
or exclude rootfs content). We can also follow up with further
optimisations after this commit. So this second solution is chosen.

Fixes [Yocto #13994]

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 scripts/lib/wic/partition.py             |  5 +++-
 scripts/lib/wic/plugins/imager/direct.py | 36 ++++++++----------------
 scripts/lib/wic/plugins/source/rootfs.py | 17 +++++++++--
 3 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 286c7867cb..f59eceb23d 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.updated_fstab_path = None
 
         self.lineno = lineno
         self.source_file = ""
@@ -118,11 +119,13 @@ class Partition():
         return self.fixed_size if self.fixed_size else self.size
 
     def prepare(self, creator, cr_workdir, oe_builddir, rootfs_dir,
-                bootimg_dir, kernel_dir, native_sysroot):
+                bootimg_dir, kernel_dir, native_sysroot, updated_fstab_path):
         """
         Prepare content for individual partitions, depending on
         partition command parameters.
         """
+        self.updated_fstab_path = updated_fstab_path
+
         if not self.source:
             if not self.size and not self.fixed_size:
                 raise WicError("The %s partition has a size of zero. Please "
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index b329568c7a..d0f228567e 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -58,7 +58,7 @@ class DirectPlugin(ImagerPlugin):
         self.compressor = options.compressor
         self.bmap = options.bmap
         self.no_fstab_update = options.no_fstab_update
-        self.original_fstab = None
+        self.updated_fstab_path = None
 
         self.name = "%s-%s" % (os.path.splitext(os.path.basename(wks_file))[0],
                                strftime("%Y%m%d%H%M"))
@@ -90,11 +90,8 @@ class DirectPlugin(ImagerPlugin):
         finally:
             self.cleanup()
 
-    def _write_fstab(self, image_rootfs):
-        """overriden to generate fstab (temporarily) in rootfs. This is called
-        from _create, make sure it doesn't get called from
-        BaseImage.create()
-        """
+    def update_fstab(self, image_rootfs):
+        """Assume partition order same as in wks"""
         if not image_rootfs:
             return
 
@@ -104,18 +101,9 @@ class DirectPlugin(ImagerPlugin):
 
         with open(fstab_path) as fstab:
             fstab_lines = fstab.readlines()
-            self.original_fstab = fstab_lines.copy()
-
-        if self._update_fstab(fstab_lines, self.parts):
-            with open(fstab_path, "w") as fstab:
-                fstab.writelines(fstab_lines)
-        else:
-            self.original_fstab = None
 
-    def _update_fstab(self, fstab_lines, parts):
-        """Assume partition order same as in wks"""
         updated = False
-        for part in parts:
+        for part in self.parts:
             if not part.realnum or not part.mountpoint \
                or part.mountpoint == "/":
                 continue
@@ -144,7 +132,10 @@ class DirectPlugin(ImagerPlugin):
             fstab_lines.append(line)
             updated = True
 
-        return updated
+        if updated:
+            self.updated_fstab_path = os.path.join(self.workdir, "fstab")
+            with open(self.updated_fstab_path, "w") as f:
+                f.writelines(fstab_lines)
 
     def _full_path(self, path, name, extention):
         """ Construct full file path to a file we generate. """
@@ -160,7 +151,7 @@ class DirectPlugin(ImagerPlugin):
         a partitioned image.
         """
         if not self.no_fstab_update:
-            self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
+            self.update_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
 
         for part in self.parts:
             # get rootfs size from bitbake variable if it's not set in .ks file
@@ -273,12 +264,6 @@ class DirectPlugin(ImagerPlugin):
             if os.path.isfile(path):
                 shutil.move(path, os.path.join(self.outdir, fname))
 
-        #Restore original fstab
-        if self.original_fstab:
-            fstab_path = self.rootfs_dir.get("ROOTFS_DIR") + "/etc/fstab"
-            with open(fstab_path, "w") as fstab:
-                fstab.writelines(self.original_fstab)
-
         # remove work directory
         shutil.rmtree(self.workdir, ignore_errors=True)
 
@@ -358,7 +343,8 @@ class PartitionedImage():
             # sizes before we can add them and do the layout.
             part.prepare(imager, imager.workdir, imager.oe_builddir,
                          imager.rootfs_dir, imager.bootimg_dir,
-                         imager.kernel_dir, imager.native_sysroot)
+                         imager.kernel_dir, imager.native_sysroot,
+                         imager.updated_fstab_path)
 
             # Converting kB to sectors for parted
             part.size_sec = part.disk_size * 1024 // self.sector_size
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index afb1eb9202..6fd415af5b 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -103,9 +103,9 @@ class RootfsPlugin(SourcePlugin):
         new_rootfs = None
         new_pseudo = None
         # Handle excluded paths.
-        if part.exclude_path or part.include_path or part.change_directory:
-            # We need a new rootfs directory we can delete files from. Copy to
-            # workdir.
+        if part.exclude_path or part.include_path or part.change_directory or part.updated_fstab_path:
+            # We need a new rootfs directory we can safely modify without
+            # interfering with other tasks. Copy to workdir.
             new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))
 
             if os.path.lexists(new_rootfs):
@@ -214,6 +214,17 @@ class RootfsPlugin(SourcePlugin):
                     rm_cmd = "rm -rf %s" % (full_path)
                     exec_native_cmd(rm_cmd, native_sysroot, pseudo)
 
+            has_fstab = os.path.exists(os.path.join(new_rootfs, "etc/fstab"))
+            if part.updated_fstab_path and has_fstab:
+                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 %s %s" % (part.updated_fstab_path, fstab_path)
+                if new_pseudo:
+                    pseudo = cls.__get_pseudo(native_sysroot, new_rootfs, new_pseudo)
+                else:
+                    pseudo = None
+                exec_native_cmd(install_cmd, native_sysroot, pseudo)
+
         part.prepare_rootfs(cr_workdir, oe_builddir,
                             new_rootfs or part.rootfs_dir, native_sysroot,
                             pseudo_dir = new_pseudo or pseudo_dir)
-- 
2.26.2

[PATCH 6/6] wic: Optimise fstab modification for ext2/3/4 and msdos partitions

[Paul Barker]

The fix for [Yocto #13994] required the rootfs directory to be copied
(using hardlinks if possible) when modifying the fstab file under wic.

We can optimise this copy away for filesystems where we have the tools
to modify the contents of the partition image after it is created. For
ext2/3/4 filesystems we have the debugfs tool and for msdos/vfat
filesystems we have the mcopy tool. So for any of these filesystems we
skip the modification of the fstab file in the rootfs directory (and
skip the associated copy unless it is otherwise necessary) and update
the contents of fstab directly in the partition image.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 scripts/lib/wic/partition.py             | 27 +++++++++++++++++++-----
 scripts/lib/wic/plugins/source/rootfs.py |  9 +++++---
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index f59eceb23d..e574f40c47 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -55,6 +55,8 @@ class Partition():
         self.fsuuid = args.fsuuid
         self.type = args.type
         self.updated_fstab_path = None
+        self.has_fstab = False
+        self.update_fstab_in_rootfs = False
 
         self.lineno = lineno
         self.source_file = ""
@@ -125,6 +127,8 @@ 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"):
+            self.update_fstab_in_rootfs = True
 
         if not self.source:
             if not self.size and not self.fixed_size:
@@ -250,7 +254,7 @@ class Partition():
 
         prefix = "ext" if self.fstype.startswith("ext") else self.fstype
         method = getattr(self, "prepare_rootfs_" + prefix)
-        method(rootfs, oe_builddir, rootfs_dir, native_sysroot, pseudo)
+        method(rootfs, cr_workdir, oe_builddir, rootfs_dir, native_sysroot, pseudo)
         self.source_file = rootfs
 
         # get the rootfs size in the right units for kickstart (kB)
@@ -258,7 +262,7 @@ class Partition():
         out = exec_cmd(du_cmd)
         self.size = int(out.split()[0])
 
-    def prepare_rootfs_ext(self, rootfs, oe_builddir, rootfs_dir,
+    def prepare_rootfs_ext(self, rootfs, cr_workdir, oe_builddir, rootfs_dir,
                            native_sysroot, pseudo):
         """
         Prepare content for an ext2/3/4 rootfs partition.
@@ -282,10 +286,19 @@ 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:
+            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)
 
-    def prepare_rootfs_btrfs(self, rootfs, oe_builddir, rootfs_dir,
+    def prepare_rootfs_btrfs(self, rootfs, cr_workdir, oe_builddir, rootfs_dir,
                              native_sysroot, pseudo):
         """
         Prepare content for a btrfs rootfs partition.
@@ -308,7 +321,7 @@ class Partition():
              self.mkfs_extraopts, self.fsuuid, rootfs)
         exec_native_cmd(mkfs_cmd, native_sysroot, pseudo=pseudo)
 
-    def prepare_rootfs_msdos(self, rootfs, oe_builddir, rootfs_dir,
+    def prepare_rootfs_msdos(self, rootfs, cr_workdir, oe_builddir, rootfs_dir,
                              native_sysroot, pseudo):
         """
         Prepare content for a msdos/vfat rootfs partition.
@@ -337,12 +350,16 @@ 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:
+            mcopy_cmd = "mcopy -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)
 
     prepare_rootfs_vfat = prepare_rootfs_msdos
 
-    def prepare_rootfs_squashfs(self, rootfs, oe_builddir, rootfs_dir,
+    def prepare_rootfs_squashfs(self, rootfs, cr_workdir, oe_builddir, rootfs_dir,
                                 native_sysroot, pseudo):
         """
         Prepare content for a squashfs rootfs partition.
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index 6fd415af5b..96d940a91d 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -94,6 +94,7 @@ class RootfsPlugin(SourcePlugin):
                                "it is not a valid path, exiting" % part.rootfs_dir)
 
         part.rootfs_dir = cls.__get_rootfs_dir(rootfs_dir)
+        part.has_fstab = os.path.exists(os.path.join(part.rootfs_dir, "etc/fstab"))
         pseudo_dir = os.path.join(part.rootfs_dir, "../pseudo")
         if not os.path.lexists(pseudo_dir):
             logger.warn("%s folder does not exist. "
@@ -103,7 +104,7 @@ class RootfsPlugin(SourcePlugin):
         new_rootfs = None
         new_pseudo = None
         # Handle excluded paths.
-        if part.exclude_path or part.include_path or part.change_directory or part.updated_fstab_path:
+        if part.exclude_path or part.include_path or part.change_directory or part.update_fstab_in_rootfs:
             # We need a new rootfs directory we can safely modify without
             # interfering with other tasks. Copy to workdir.
             new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))
@@ -214,8 +215,10 @@ class RootfsPlugin(SourcePlugin):
                     rm_cmd = "rm -rf %s" % (full_path)
                     exec_native_cmd(rm_cmd, native_sysroot, pseudo)
 
-            has_fstab = os.path.exists(os.path.join(new_rootfs, "etc/fstab"))
-            if part.updated_fstab_path and has_fstab:
+            # Update part.has_fstab here as fstab may have been added or
+            # removed by the above modifications.
+            part.has_fstab = os.path.exists(os.path.join(new_rootfs, "etc/fstab"))
+            if part.update_fstab_in_rootfs and part.has_fstab:
                 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 %s %s" % (part.updated_fstab_path, fstab_path)
-- 
2.26.2

Re: [OE-core] [PATCH 6/6] wic: Optimise fstab modification for ext2/3/4 and msdos partitions

[Alexander Kanavin]

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

Can there be a warning if the filesystem is not one of the supported ones?

Alex

On Mon, 11 Jan 2021 at 11:33, Paul Barker <pbarker@konsulko.com> wrote:

> The fix for [Yocto #13994] required the rootfs directory to be copied
> (using hardlinks if possible) when modifying the fstab file under wic.
>
> We can optimise this copy away for filesystems where we have the tools
> to modify the contents of the partition image after it is created. For
> ext2/3/4 filesystems we have the debugfs tool and for msdos/vfat
> filesystems we have the mcopy tool. So for any of these filesystems we
> skip the modification of the fstab file in the rootfs directory (and
> skip the associated copy unless it is otherwise necessary) and update
> the contents of fstab directly in the partition image.
>
> Signed-off-by: Paul Barker <pbarker@konsulko.com>
> ---
>  scripts/lib/wic/partition.py             | 27 +++++++++++++++++++-----
>  scripts/lib/wic/plugins/source/rootfs.py |  9 +++++---
>  2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> index f59eceb23d..e574f40c47 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -55,6 +55,8 @@ class Partition():
>          self.fsuuid = args.fsuuid
>          self.type = args.type
>          self.updated_fstab_path = None
> +        self.has_fstab = False
> +        self.update_fstab_in_rootfs = False
>
>          self.lineno = lineno
>          self.source_file = ""
> @@ -125,6 +127,8 @@ 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"):
> +            self.update_fstab_in_rootfs = True
>
>          if not self.source:
>              if not self.size and not self.fixed_size:
> @@ -250,7 +254,7 @@ class Partition():
>
>          prefix = "ext" if self.fstype.startswith("ext") else self.fstype
>          method = getattr(self, "prepare_rootfs_" + prefix)
> -        method(rootfs, oe_builddir, rootfs_dir, native_sysroot, pseudo)
> +        method(rootfs, cr_workdir, oe_builddir, rootfs_dir,
> native_sysroot, pseudo)
>          self.source_file = rootfs
>
>          # get the rootfs size in the right units for kickstart (kB)
> @@ -258,7 +262,7 @@ class Partition():
>          out = exec_cmd(du_cmd)
>          self.size = int(out.split()[0])
>
> -    def prepare_rootfs_ext(self, rootfs, oe_builddir, rootfs_dir,
> +    def prepare_rootfs_ext(self, rootfs, cr_workdir, oe_builddir,
> rootfs_dir,
>                             native_sysroot, pseudo):
>          """
>          Prepare content for an ext2/3/4 rootfs partition.
> @@ -282,10 +286,19 @@ 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:
> +            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)
>
> -    def prepare_rootfs_btrfs(self, rootfs, oe_builddir, rootfs_dir,
> +    def prepare_rootfs_btrfs(self, rootfs, cr_workdir, oe_builddir,
> rootfs_dir,
>                               native_sysroot, pseudo):
>          """
>          Prepare content for a btrfs rootfs partition.
> @@ -308,7 +321,7 @@ class Partition():
>               self.mkfs_extraopts, self.fsuuid, rootfs)
>          exec_native_cmd(mkfs_cmd, native_sysroot, pseudo=pseudo)
>
> -    def prepare_rootfs_msdos(self, rootfs, oe_builddir, rootfs_dir,
> +    def prepare_rootfs_msdos(self, rootfs, cr_workdir, oe_builddir,
> rootfs_dir,
>                               native_sysroot, pseudo):
>          """
>          Prepare content for a msdos/vfat rootfs partition.
> @@ -337,12 +350,16 @@ 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:
> +            mcopy_cmd = "mcopy -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)
>
>      prepare_rootfs_vfat = prepare_rootfs_msdos
>
> -    def prepare_rootfs_squashfs(self, rootfs, oe_builddir, rootfs_dir,
> +    def prepare_rootfs_squashfs(self, rootfs, cr_workdir, oe_builddir,
> rootfs_dir,
>                                  native_sysroot, pseudo):
>          """
>          Prepare content for a squashfs rootfs partition.
> diff --git a/scripts/lib/wic/plugins/source/rootfs.py
> b/scripts/lib/wic/plugins/source/rootfs.py
> index 6fd415af5b..96d940a91d 100644
> --- a/scripts/lib/wic/plugins/source/rootfs.py
> +++ b/scripts/lib/wic/plugins/source/rootfs.py
> @@ -94,6 +94,7 @@ class RootfsPlugin(SourcePlugin):
>                                 "it is not a valid path, exiting" %
> part.rootfs_dir)
>
>          part.rootfs_dir = cls.__get_rootfs_dir(rootfs_dir)
> +        part.has_fstab = os.path.exists(os.path.join(part.rootfs_dir,
> "etc/fstab"))
>          pseudo_dir = os.path.join(part.rootfs_dir, "../pseudo")
>          if not os.path.lexists(pseudo_dir):
>              logger.warn("%s folder does not exist. "
> @@ -103,7 +104,7 @@ class RootfsPlugin(SourcePlugin):
>          new_rootfs = None
>          new_pseudo = None
>          # Handle excluded paths.
> -        if part.exclude_path or part.include_path or
> part.change_directory or part.updated_fstab_path:
> +        if part.exclude_path or part.include_path or
> part.change_directory or part.update_fstab_in_rootfs:
>              # We need a new rootfs directory we can safely modify without
>              # interfering with other tasks. Copy to workdir.
>              new_rootfs = os.path.realpath(os.path.join(cr_workdir,
> "rootfs%d" % part.lineno))
> @@ -214,8 +215,10 @@ class RootfsPlugin(SourcePlugin):
>                      rm_cmd = "rm -rf %s" % (full_path)
>                      exec_native_cmd(rm_cmd, native_sysroot, pseudo)
>
> -            has_fstab = os.path.exists(os.path.join(new_rootfs,
> "etc/fstab"))
> -            if part.updated_fstab_path and has_fstab:
> +            # Update part.has_fstab here as fstab may have been added or
> +            # removed by the above modifications.
> +            part.has_fstab = os.path.exists(os.path.join(new_rootfs,
> "etc/fstab"))
> +            if part.update_fstab_in_rootfs and part.has_fstab:
>                  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 %s %s" %
> (part.updated_fstab_path, fstab_path)
> --
> 2.26.2
>
>
> 
>
>

[-- Attachment #2: Type: text/html, Size: 9049 bytes --]

Re: [OE-core] [PATCH 6/6] wic: Optimise fstab modification for ext2/3/4 and msdos partitions

[Alexander Kanavin]

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

Oh right nevermind - it's an optimization, not an actual fix which is
elsewhere.

Alex

On Mon, 11 Jan 2021 at 11:36, Alexander Kanavin via lists.openembedded.org
<alex.kanavin=gmail.com@lists.openembedded.org> wrote:

> Can there be a warning if the filesystem is not one of the supported ones?
>
> Alex
>
> On Mon, 11 Jan 2021 at 11:33, Paul Barker <pbarker@konsulko.com> wrote:
>
>> The fix for [Yocto #13994] required the rootfs directory to be copied
>> (using hardlinks if possible) when modifying the fstab file under wic.
>>
>> We can optimise this copy away for filesystems where we have the tools
>> to modify the contents of the partition image after it is created. For
>> ext2/3/4 filesystems we have the debugfs tool and for msdos/vfat
>> filesystems we have the mcopy tool. So for any of these filesystems we
>> skip the modification of the fstab file in the rootfs directory (and
>> skip the associated copy unless it is otherwise necessary) and update
>> the contents of fstab directly in the partition image.
>>
>> Signed-off-by: Paul Barker <pbarker@konsulko.com>
>> ---
>>  scripts/lib/wic/partition.py             | 27 +++++++++++++++++++-----
>>  scripts/lib/wic/plugins/source/rootfs.py |  9 +++++---
>>  2 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
>> index f59eceb23d..e574f40c47 100644
>> --- a/scripts/lib/wic/partition.py
>> +++ b/scripts/lib/wic/partition.py
>> @@ -55,6 +55,8 @@ class Partition():
>>          self.fsuuid = args.fsuuid
>>          self.type = args.type
>>          self.updated_fstab_path = None
>> +        self.has_fstab = False
>> +        self.update_fstab_in_rootfs = False
>>
>>          self.lineno = lineno
>>          self.source_file = ""
>> @@ -125,6 +127,8 @@ 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"):
>> +            self.update_fstab_in_rootfs = True
>>
>>          if not self.source:
>>              if not self.size and not self.fixed_size:
>> @@ -250,7 +254,7 @@ class Partition():
>>
>>          prefix = "ext" if self.fstype.startswith("ext") else self.fstype
>>          method = getattr(self, "prepare_rootfs_" + prefix)
>> -        method(rootfs, oe_builddir, rootfs_dir, native_sysroot, pseudo)
>> +        method(rootfs, cr_workdir, oe_builddir, rootfs_dir,
>> native_sysroot, pseudo)
>>          self.source_file = rootfs
>>
>>          # get the rootfs size in the right units for kickstart (kB)
>> @@ -258,7 +262,7 @@ class Partition():
>>          out = exec_cmd(du_cmd)
>>          self.size = int(out.split()[0])
>>
>> -    def prepare_rootfs_ext(self, rootfs, oe_builddir, rootfs_dir,
>> +    def prepare_rootfs_ext(self, rootfs, cr_workdir, oe_builddir,
>> rootfs_dir,
>>                             native_sysroot, pseudo):
>>          """
>>          Prepare content for an ext2/3/4 rootfs partition.
>> @@ -282,10 +286,19 @@ 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:
>> +            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)
>>
>> -    def prepare_rootfs_btrfs(self, rootfs, oe_builddir, rootfs_dir,
>> +    def prepare_rootfs_btrfs(self, rootfs, cr_workdir, oe_builddir,
>> rootfs_dir,
>>                               native_sysroot, pseudo):
>>          """
>>          Prepare content for a btrfs rootfs partition.
>> @@ -308,7 +321,7 @@ class Partition():
>>               self.mkfs_extraopts, self.fsuuid, rootfs)
>>          exec_native_cmd(mkfs_cmd, native_sysroot, pseudo=pseudo)
>>
>> -    def prepare_rootfs_msdos(self, rootfs, oe_builddir, rootfs_dir,
>> +    def prepare_rootfs_msdos(self, rootfs, cr_workdir, oe_builddir,
>> rootfs_dir,
>>                               native_sysroot, pseudo):
>>          """
>>          Prepare content for a msdos/vfat rootfs partition.
>> @@ -337,12 +350,16 @@ 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:
>> +            mcopy_cmd = "mcopy -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)
>>
>>      prepare_rootfs_vfat = prepare_rootfs_msdos
>>
>> -    def prepare_rootfs_squashfs(self, rootfs, oe_builddir, rootfs_dir,
>> +    def prepare_rootfs_squashfs(self, rootfs, cr_workdir, oe_builddir,
>> rootfs_dir,
>>                                  native_sysroot, pseudo):
>>          """
>>          Prepare content for a squashfs rootfs partition.
>> diff --git a/scripts/lib/wic/plugins/source/rootfs.py
>> b/scripts/lib/wic/plugins/source/rootfs.py
>> index 6fd415af5b..96d940a91d 100644
>> --- a/scripts/lib/wic/plugins/source/rootfs.py
>> +++ b/scripts/lib/wic/plugins/source/rootfs.py
>> @@ -94,6 +94,7 @@ class RootfsPlugin(SourcePlugin):
>>                                 "it is not a valid path, exiting" %
>> part.rootfs_dir)
>>
>>          part.rootfs_dir = cls.__get_rootfs_dir(rootfs_dir)
>> +        part.has_fstab = os.path.exists(os.path.join(part.rootfs_dir,
>> "etc/fstab"))
>>          pseudo_dir = os.path.join(part.rootfs_dir, "../pseudo")
>>          if not os.path.lexists(pseudo_dir):
>>              logger.warn("%s folder does not exist. "
>> @@ -103,7 +104,7 @@ class RootfsPlugin(SourcePlugin):
>>          new_rootfs = None
>>          new_pseudo = None
>>          # Handle excluded paths.
>> -        if part.exclude_path or part.include_path or
>> part.change_directory or part.updated_fstab_path:
>> +        if part.exclude_path or part.include_path or
>> part.change_directory or part.update_fstab_in_rootfs:
>>              # We need a new rootfs directory we can safely modify without
>>              # interfering with other tasks. Copy to workdir.
>>              new_rootfs = os.path.realpath(os.path.join(cr_workdir,
>> "rootfs%d" % part.lineno))
>> @@ -214,8 +215,10 @@ class RootfsPlugin(SourcePlugin):
>>                      rm_cmd = "rm -rf %s" % (full_path)
>>                      exec_native_cmd(rm_cmd, native_sysroot, pseudo)
>>
>> -            has_fstab = os.path.exists(os.path.join(new_rootfs,
>> "etc/fstab"))
>> -            if part.updated_fstab_path and has_fstab:
>> +            # Update part.has_fstab here as fstab may have been added or
>> +            # removed by the above modifications.
>> +            part.has_fstab = os.path.exists(os.path.join(new_rootfs,
>> "etc/fstab"))
>> +            if part.update_fstab_in_rootfs and part.has_fstab:
>>                  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 %s %s" %
>> (part.updated_fstab_path, fstab_path)
>> --
>> 2.26.2
>>
>>
>>
>>
>>
> 
>
>

[-- Attachment #2: Type: text/html, Size: 9681 bytes --]

Re: [OE-core] [PATCH 6/6] wic: Optimise fstab modification for ext2/3/4 and msdos partitions

[Paul Barker]

On Mon, 11 Jan 2021 at 10:37, Alexander Kanavin <alex.kanavin@gmail.com> wrote:
>
> Oh right nevermind - it's an optimization, not an actual fix which is elsewhere.

Yes, for other fstypes we just fallback on copying the rootfs
directory (using hardlinks if possible).

-- 
Paul Barker
Konsulko Group

Re: [OE-core] [PATCH 1/6] wic: Add workdir argument

[Lee Chee Yang]

You might want to update scripts/lib/wic/help.py with description for this new features too

>-----Original Message-----
>From: openembedded-core@lists.openembedded.org <openembedded-
>core@lists.openembedded.org> On Behalf Of Paul Barker
>Sent: Monday, 11 January, 2021 6:33 PM
>To: openembedded-core@lists.openembedded.org
>Cc: Paul Barker <pbarker@konsulko.com>
>Subject: [OE-core] [PATCH 1/6] wic: Add workdir argument
>
>This allows the path for the temporary workdir used by wic to be set when
>running wic from bitbake or directly from the command line.
>
>Signed-off-by: Paul Barker <pbarker@konsulko.com>
>---
> scripts/lib/wic/plugins/imager/direct.py | 2 +-
> scripts/wic                              | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/scripts/lib/wic/plugins/imager/direct.py
>b/scripts/lib/wic/plugins/imager/direct.py
>index 0ca67b77d5..b329568c7a 100644
>--- a/scripts/lib/wic/plugins/imager/direct.py
>+++ b/scripts/lib/wic/plugins/imager/direct.py
>@@ -62,7 +62,7 @@ class DirectPlugin(ImagerPlugin):
>
>         self.name = "%s-%s" % (os.path.splitext(os.path.basename(wks_file))[0],
>                                strftime("%Y%m%d%H%M"))
>-        self.workdir = tempfile.mkdtemp(dir=self.outdir, prefix='tmp.wic.')
>+        self.workdir = options.workdir or
>+ tempfile.mkdtemp(dir=self.outdir, prefix='tmp.wic.')
>         self._image = None
>         self.ptable_format = self.ks.bootloader.ptable
>         self.parts = self.ks.partitions diff --git a/scripts/wic b/scripts/wic index
>24700f380f..a741aed364 100755
>--- a/scripts/wic
>+++ b/scripts/wic
>@@ -312,6 +312,8 @@ def wic_init_parser_create(subparser):
>
>     subparser.add_argument("-o", "--outdir", dest="outdir", default='.',
>                       help="name of directory to create image in")
>+    subparser.add_argument("-w", "--workdir",
>+                      help="temporary workdir to use for intermediate
>+ files")
>     subparser.add_argument("-e", "--image-name", dest="image_name",
>                       help="name of the image to use the artifacts from "
>                            "e.g. core-image-sato")
>--
>2.26.2

Re: [OE-core] [PATCH 1/6] wic: Add workdir argument

[Paul Barker]

On Mon, 11 Jan 2021 at 14:17, Lee, Chee Yang <chee.yang.lee@intel.com> wrote:
>
> You might want to update scripts/lib/wic/help.py with description for this new features too

/me looks at help.py

That is really frustrating. The help output should be generated from
the argument parsers instead of carrying hand-written duplicate
information.

-- 
Paul Barker
Konsulko Group

Re: [OE-core] [PATCH 1/6] wic: Add workdir argument

[Paul Barker]

On Mon, 11 Jan 2021 at 14:22, Paul Barker <pbarker@konsulko.com> wrote:
>
> On Mon, 11 Jan 2021 at 14:17, Lee, Chee Yang <chee.yang.lee@intel.com> wrote:
> >
> > You might want to update scripts/lib/wic/help.py with description for this new features too
>
> /me looks at help.py
>
> That is really frustrating. The help output should be generated from
> the argument parsers instead of carrying hand-written duplicate
> information.

I'll send a follow up patch to add to the documentation in help.py.

-- 
Paul Barker
Konsulko Group