#11662 - wic should mount /boot
v5 cover #11662 - wic should mount /boot
Hi,
This patchset adds /boot to the /etc/fstab of root partition, making it mounted on boot. It also fixes reporting and testing issues caused by this change.
The patchset also fixes long standing bug: wic updated fstab inplace in rootfs directory. This causes other tasks working with rootfs directory to produce incorrect results or crash. This is fixed by hadlinking rootfs content to the temporary directory before updating fstab.
This approach caused do_image_tar to fail with the error "file changed as we read it" as hardlinking changes files ctime. In order to solve this we had to modify do_image_tar to ignore file changes.
Changes in v2: squashed patches by reviewer's request Changes in v3: unlink /etc/fstab in rootfs copy before updating it Changes in v4: used 'cp -a' instead of copyhardlinktree to avoid
do_image_tar failure due to changed ctime
Changes in v5: back to hardlinking. ignored tar exit code 1.
The following changes since commit b73f5e088a543775a2a94b60302f750edfffbd10:
wic-tools: add dependency to e2fsprogs-native (2017-07-27 16:07:26 +0300)
are available in the git repository at:
git://git.yoctoproject.org/poky-contrib ed/wip
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=ed/wip
Ed Bartosh (7):
image_types.bbclass: ignore tar exit code 1
wic: copy rootfs directory before changing fstab
wic: use absolute paths in rootfs plugin
wic: rootfs: fix rootfs path reporting
wic: rootfs: make copied rootfs unique
wic: add /boot mount point to fstab by default
oe-selftest: wic: fix test_quemu
meta/classes/image_types.bbclass | 3 ++- meta/lib/oeqa/selftest/cases/wic.py | 2 +- scripts/lib/wic/plugins/imager/direct.py | 26 ++++++++++++++++---------- scripts/lib/wic/plugins/source/rootfs.py | 16 +++++++--------- 4 files changed, 26 insertions(+), 21 deletions(-)
Signature · 1 line
2.1.4
PATCH v5 1/7 image_types.bbclass: ignore tar exit code 1
tar exists with 1 and produces warning "file changed as we read it" if content is changed while tar archives it. Even hardlinking content causes tar to fail this way as it changes file ctime.
Other tasks running in parallel with do_image_tar may need to hardlink rootfs content in order to change it, e.g. do_image_wic does this to update etc/fstab.
Ignored tar exit code 1 to be able to hardlink rootfs content while do_rootfs_tar is tarring it.
--- meta/classes/image_types.bbclass | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
meta/classes/image_types.bbclass +2 -1
@@ -119,7 +119,8 @@ IMAGE_CMD_squashfs-lzo = "mksquashfs ${IMAGE_ROOTFS} ${IMGDEPLOYDIR}/${IMAGE_NAM # In practice, it turned out to be not needed when creating archives and # required when extracting, but it seems prudent to use it in both cases. IMAGE_CMD_TAR ?= "tar" -IMAGE_CMD_tar = "${IMAGE_CMD_TAR} -cvf ${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.tar -C ${IMAGE_ROOTFS} ." +# ignore return code 1 "file changed as we read it" as other tasks(e.g. do_image_wic) may be hardlinking rootfs +IMAGE_CMD_tar = "${IMAGE_CMD_TAR} -cf ${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.tar -C ${IMAGE_ROOTFS} . || [[ $? -eq 1 ]]" do_image_cpio[cleandirs] += "${WORKDIR}/cpio_append" IMAGE_CMD_cpio () {
Signature · 1 line
2.1.4
PATCH v5 2/7 wic: copy rootfs directory before changing fstab
In reply to Ed Bartoshwic updates /etc/fstab on root partition if there are valid mount points in .wks
When wic runs from bitbake this can cause incorrect results or even breakage of other tasks working with the same rootfs directory in parallel with do_image_wic.
Implemented hardlinking rootfs directory to a temporary location before updating fstab to avoid conflicts with other tasks working with the same rootfs directory.
--- scripts/lib/wic/plugins/imager/direct.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)
scripts/lib/wic/plugins/imager/direct.py +15 -9
@@ -115,12 +115,20 @@ class DirectPlugin(ImagerPlugin): fstab_lines = fstab.readlines() if self._update_fstab(fstab_lines, self.parts): - shutil.copyfile(fstab_path, fstab_path + ".orig") + # hardlink rootfs content to the temporary directory to update fstab + # as rootfs can be used by other tasks and can't be modified + new_rootfs = os.path.realpath(os.path.join(self.workdir, "rootfs_copy")) + exec_cmd("cp -al %s %s" % (image_rootfs, new_rootfs), + self.native_sysroot) + + fstab_path = os.path.join(new_rootfs, 'etc/fstab') + + os.unlink(fstab_path) with open(fstab_path, "w") as fstab: fstab.writelines(fstab_lines) - return fstab_path + return new_rootfs def _update_fstab(self, fstab_lines, parts): """Assume partition order same as in wks"""
@@ -156,7 +164,10 @@ class DirectPlugin(ImagerPlugin): filesystems from the artifacts directly and combine them into a partitioned image. """ - fstab_path = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR")) + new_rootfs = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR")) + if new_rootfs: + # rootfs was copied to update fstab + self.rootfs_dir['ROOTFS_DIR'] = new_rootfs for part in self.parts: # get rootfs size from bitbake variable if it's not set in .ks file
@@ -172,12 +183,7 @@ class DirectPlugin(ImagerPlugin): if rsize_bb: part.size = int(round(float(rsize_bb))) - try: - self._image.prepare(self) - finally: - if fstab_path: - shutil.move(fstab_path + ".orig", fstab_path) - + self._image.prepare(self) self._image.layout_partitions() self._image.create()
Signature · 1 line
2.1.4
PATCH v5 3/7 wic: use absolute paths in rootfs plugin
In reply to Ed BartoshUsing relative paths can cause copyhardlinktree API to fail as it changes current directory when working. Converted all paths to absolute paths using os.path.realpath.
--- scripts/lib/wic/plugins/source/rootfs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
scripts/lib/wic/plugins/source/rootfs.py +2 -2
@@ -48,7 +48,7 @@ class RootfsPlugin(SourcePlugin): @staticmethod def __get_rootfs_dir(rootfs_dir): if os.path.isdir(rootfs_dir): - return rootfs_dir + return os.path.realpath(rootfs_dir) image_rootfs_dir = get_bitbake_var("IMAGE_ROOTFS", rootfs_dir) if not os.path.isdir(image_rootfs_dir):
@@ -56,7 +56,7 @@ class RootfsPlugin(SourcePlugin): "named %s has been found at %s, exiting." % (rootfs_dir, image_rootfs_dir)) - return image_rootfs_dir + return os.path.realpath(image_rootfs_dir) @classmethod def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
Signature · 1 line
2.1.4
PATCH v5 4/7 wic: rootfs: fix rootfs path reporting
In reply to Ed Bartoshwic gets rootfs paths from partition object property 'rootfs_dir' and shows them in final report.
rootfs plugin sets this property to the temporary path, which causes temporary paths appearing in the report.
Changed the code to prevent storing temporary rootfs path in part.rootfs_dir. This should fix the report.
--- scripts/lib/wic/plugins/source/rootfs.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
scripts/lib/wic/plugins/source/rootfs.py +4 -6
@@ -81,8 +81,9 @@ class RootfsPlugin(SourcePlugin): raise WicError("Couldn't find --rootfs-dir=%s connection or " "it is not a valid path, exiting" % part.rootfs_dir) - real_rootfs_dir = cls.__get_rootfs_dir(rootfs_dir) + part.rootfs_dir = cls.__get_rootfs_dir(rootfs_dir) + new_rootfs = None # Handle excluded paths. if part.exclude_path is not None: # We need a new rootfs directory we can delete files from. Copy to
@@ -92,9 +93,7 @@ class RootfsPlugin(SourcePlugin): if os.path.lexists(new_rootfs): shutil.rmtree(os.path.join(new_rootfs)) - copyhardlinktree(real_rootfs_dir, new_rootfs) - - real_rootfs_dir = new_rootfs + copyhardlinktree(part.rootfs_dir, new_rootfs) for orig_path in part.exclude_path: path = orig_path
@@ -123,6 +122,5 @@ class RootfsPlugin(SourcePlugin): # Delete whole directory. shutil.rmtree(full_path) - part.rootfs_dir = real_rootfs_dir part.prepare_rootfs(cr_workdir, oe_builddir, - real_rootfs_dir, native_sysroot) + new_rootfs or part.rootfs_dir, native_sysroot)
Signature · 1 line
2.1.4
PATCH v5 6/7 wic: add /boot mount point to fstab by default
In reply to Ed Bartoshwic avoided adding /boot to fstab for no reason.
This exception was hardcoded in the wic code.
There is no need for this as mountpoint in .wks file is an optional field. It can be used only if user wants to have partitions automatically mounted on system boot.
[YOCTO #11662]
--- scripts/lib/wic/plugins/imager/direct.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/lib/wic/plugins/imager/direct.py +1 -1
@@ -135,7 +135,7 @@ class DirectPlugin(ImagerPlugin): updated = False for part in parts: if not part.realnum or not part.mountpoint \ - or part.mountpoint in ("/", "/boot"): + or part.mountpoint == "/": continue # mmc device partitions are named mmcblk0p1, mmcblk0p2..
Signature · 1 line
2.1.4
PATCH v5 7/7 oe-selftest: wic: fix test_quemu
In reply to Ed BartoshThis test case boots the image in qemu and checks for mounted partitions. As /boot is mounted automatically the test case fails. Fixed this by adding /boot to the list of mounted partitions.
--- meta/lib/oeqa/selftest/cases/wic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
meta/lib/oeqa/selftest/cases/wic.py +1 -1
@@ -637,7 +637,7 @@ part /etc --source rootfs --ondisk mmcblk0 --fstype=ext4 --exclude-path bin/ --r cmd = "mount |grep '^/dev/' | cut -f1,3 -d ' '" status, output = qemu.run_serial(cmd) self.assertEqual(1, status, 'Failed to run command "%s": %s' % (cmd, output)) - self.assertEqual(output, '/dev/root /\r\n/dev/sda3 /mnt') + self.assertEqual(output, '/dev/root /\r\n/dev/sda1 /boot\r\n/dev/sda3 /mnt') @only_for_arch(['i586', 'i686', 'x86_64']) @OETestID(1852)
Signature · 1 line
2.1.4
PATCH v5 5/7 wic: rootfs: make copied rootfs unique
In reply to Ed BartoshUsed unique suffix (line number from .wks file) for the copied rootfs directory to avoid possible conflicts.
--- scripts/lib/wic/plugins/source/rootfs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/lib/wic/plugins/source/rootfs.py +1 -1
@@ -88,7 +88,7 @@ class RootfsPlugin(SourcePlugin): if part.exclude_path is not None: # We need a new rootfs directory we can delete files from. Copy to # workdir. - new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs")) + new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno)) if os.path.lexists(new_rootfs): shutil.rmtree(os.path.join(new_rootfs))
Signature · 1 line
2.1.4
Quoted 15 lines · Ed Bartosh
This patchset adds /boot to the
/etc/fstabof root partition, making it mounted on boot. It also fixes reporting and testing issues caused by this change.The patchset also fixes long standing bug: wic updated fstab inplace in rootfs directory. This causes other tasks working with rootfs directory to produce incorrect results or crash. This is fixed by hadlinking rootfs content to the temporary directory before updating fstab.
This approach caused
do_image_tarto fail with the error "file changed as we read it"
as hardlinking changes files ctime. In order to solve this we had to modifydo_image_tarto ignore file changes.
Quoted 5 lines
Changes in v2: squashed patches by reviewer's request Changes in v3: unlink
/etc/fstabin rootfs copy before updating it Changes in v4: used 'cp -a' instead of copyhardlinktree to avoiddo_image_tarfailure due to changed ctime Changes in v5: back to hardlinking. ignored tar exit code 1.
This patchset had a couple of issues, the tar command change was a bashism '[[' so I changed it to '['. The cp -a change also doesn't work cross device so I resurrected one of the previous versions of that patch that uses copyhardlinktree.
As I mentioned previously, if we can speed up that function, great. We do need to handle cross device linkage though (and there may be an optimisation to have wic use the same device for the temp rootfs?).
The updated series is going through tests again now.
Cheers,
Richard