From b855c9b44661d6b8a9432ab41f69c322c72230b4 Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 06:36:44 +0000 Subject: [PATCH 01/12] boot_control.common: split SlotMountHelper as a new module, refine is_target_mounted method --- src/otaclient/boot_control/_common.py | 106 +++----------------------- 1 file changed, 11 insertions(+), 95 deletions(-) diff --git a/src/otaclient/boot_control/_common.py b/src/otaclient/boot_control/_common.py index 38fd93088..d303831bb 100644 --- a/src/otaclient/boot_control/_common.py +++ b/src/otaclient/boot_control/_common.py @@ -11,14 +11,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -r"""Shared utils for boot_controller.""" +"""Shared utils for boot_controller.""" from __future__ import annotations import contextlib import logging -import shutil import sys from pathlib import Path from subprocess import CalledProcessError @@ -153,7 +152,7 @@ def get_dev_by_mount_point( @classmethod def is_target_mounted( - cls, target: Path | str, *, raise_exception: bool = True + cls, target: StrOrPath, *, raise_exception: bool = False ) -> bool: """Check if is mounted or not. can be a dev or a mount point. @@ -166,10 +165,17 @@ def is_target_mounted( Defaults to True. Returns: - bool: return True if the target has at least one mount_point. + Return True if the target has at least one mount_point. Return False if is False and + is not a mount point or not mounted. """ cmd = ["findmnt", target] - return bool(subprocess_check_output(cmd, raise_exception=raise_exception)) + try: + subprocess_call(cmd, raise_exception=True) + return True + except CalledProcessError: + if raise_exception: + raise + return False @classmethod def get_parent_dev(cls, child_device: str, *, raise_exception: bool = True) -> str: @@ -699,95 +705,5 @@ def booted_ota_status(self) -> api_types.StatusOta: return self._ota_status -class SlotMountHelper: - """Helper class that provides methods for mounting slots.""" - - def __init__( - self, - *, - standby_slot_dev: Union[str, Path], - standby_slot_mount_point: Union[str, Path], - active_slot_dev: Union[str, Path], - active_slot_mount_point: Union[str, Path], - ) -> None: - # dev - self.standby_slot_dev = str(standby_slot_dev) - self.active_slot_dev = str(active_slot_dev) - # mount points - self.standby_slot_mount_point = Path(standby_slot_mount_point) - self.active_slot_mount_point = Path(active_slot_mount_point) - self.standby_slot_mount_point.mkdir(exist_ok=True, parents=True) - self.active_slot_mount_point.mkdir(exist_ok=True, parents=True) - # standby slot /boot dir - # NOTE(20230907): this will always be /boot, - # in the future this attribute will not be used by - # standby slot creater. - self.standby_boot_dir = self.standby_slot_mount_point / Path( - cfg.BOOT_DPATH - ).relative_to("/") - - def mount_standby(self) -> None: - """Mount standby slot dev to .""" - logger.debug("mount standby slot rootfs dev...") - if CMDHelperFuncs.is_target_mounted( - self.standby_slot_dev, raise_exception=False - ): - logger.debug(f"{self.standby_slot_dev=} is mounted, try to umount it ...") - CMDHelperFuncs.umount(self.standby_slot_dev, raise_exception=False) - - CMDHelperFuncs.mount_rw( - target=self.standby_slot_dev, - mount_point=self.standby_slot_mount_point, - ) - - def mount_active(self) -> None: - """Mount active rootfs ready-only.""" - logger.debug("mount active slot rootfs dev...") - CMDHelperFuncs.mount_ro( - target=self.active_slot_dev, - mount_point=self.active_slot_mount_point, - ) - - def preserve_ota_folder_to_standby(self): - """Copy the /boot/ota folder to standby slot to preserve it. - - /boot/ota folder contains the ota setting for this device, - so we should preserve it for each slot, accross each update. - """ - logger.debug("copy /boot/ota from active to standby.") - try: - _src = self.active_slot_mount_point / Path(cfg.OTA_DPATH).relative_to("/") - _dst = self.standby_slot_mount_point / Path(cfg.OTA_DPATH).relative_to("/") - shutil.copytree(_src, _dst, dirs_exist_ok=True) - except Exception as e: - raise ValueError( - f"failed to copy /boot/ota from active to standby: {e!r}" - ) from e - - def prepare_standby_dev( - self, - *, - erase_standby: bool = False, - fslabel: Optional[str] = None, - ) -> None: - CMDHelperFuncs.umount(self.standby_slot_dev, raise_exception=False) - if erase_standby: - return CMDHelperFuncs.mkfs_ext4(self.standby_slot_dev, fslabel=fslabel) - - # TODO: in the future if in-place update mode is implemented, do a - # fschck over the standby slot file system. - if fslabel: - CMDHelperFuncs.set_ext4_fslabel(self.standby_slot_dev, fslabel=fslabel) - - def umount_all(self, *, ignore_error: bool = True): - logger.debug("unmount standby slot and active slot mount point...") - CMDHelperFuncs.umount( - self.standby_slot_mount_point, raise_exception=ignore_error - ) - CMDHelperFuncs.umount( - self.active_slot_mount_point, raise_exception=ignore_error - ) - - def cat_proc_cmdline(target: str = "/proc/cmdline") -> str: return read_str_from_file(target, _default="") From c46d0fb2c57883ba8f5eb6ce21ff33262aafa02a Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 06:55:26 +0000 Subject: [PATCH 02/12] re-implement slot mount helper --- .../boot_control/_slot_mnt_helper.py | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 src/otaclient/boot_control/_slot_mnt_helper.py diff --git a/src/otaclient/boot_control/_slot_mnt_helper.py b/src/otaclient/boot_control/_slot_mnt_helper.py new file mode 100644 index 000000000..e76fc8cc5 --- /dev/null +++ b/src/otaclient/boot_control/_slot_mnt_helper.py @@ -0,0 +1,188 @@ +# Copyright 2022 TIER IV, INC. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Helper for mounting/umount slots during OTA.""" + + +from __future__ import annotations + +import logging +import shutil +from pathlib import Path +from subprocess import CalledProcessError +from time import sleep +from typing import Optional + +from otaclient.boot_control._common import CMDHelperFuncs +from otaclient.configs.cfg import cfg +from otaclient_common import replace_root +from otaclient_common.typing import StrOrPath + +logger = logging.getLogger(__name__) + +MAX_RETRY_COUNT = 6 +RETRY_INTERVAL = 2 + + +class SlotMountHelper: + """Helper class that provides methods for mounting slots.""" + + def __init__( + self, + *, + standby_slot_dev: StrOrPath, + standby_slot_mount_point: StrOrPath, + active_rootfs: StrOrPath, + active_slot_mount_point: StrOrPath, + ) -> None: + self.standby_slot_dev = str(standby_slot_dev) + self.active_rootfs = str(active_rootfs) + + self.standby_slot_mount_point = Path(standby_slot_mount_point) + self.active_slot_mount_point = Path(active_slot_mount_point) + + # standby slot /boot dir + # NOTE(20230907): this will always be /boot, + # in the future this attribute will not be used by + # standby slot creater. + self.standby_boot_dir = Path( + replace_root( + cfg.BOOT_DPATH, cfg.CANONICAL_ROOT, self.standby_slot_mount_point + ) + ) + + @staticmethod + def _umount(mnt_point: StrOrPath, *, ignore_error: bool) -> None: + """Try to umount the at our best. + + Raises: + If is False, raises the last failed attemp's CalledProcessError. + """ + if not CMDHelperFuncs.is_target_mounted(mnt_point): + return + + for _retry in range(MAX_RETRY_COUNT + 1): + try: + CMDHelperFuncs.umount(mnt_point, raise_exception=True) + break + except CalledProcessError as e: + logger.warning(f"retry#{_retry} failed to umount standby slot: {e!r}") + logger.warning(f"{e.stderr}\n{e.stdout}") + + if _retry >= MAX_RETRY_COUNT: + logger.error(f"reached max retry on umounting {mnt_point}, abort") + if not ignore_error: + raise + + sleep(RETRY_INTERVAL) + continue + + def _ensure_mointpoint(self, mnt_point: Path) -> None: + """Ensure the exists, has no mount on it and ready for mount. + + If the is valid, but we failed to umount any previous mounts on it, + we still keep use the mountpoint as later mount will override the previous one. + """ + if not mnt_point.is_dir(): + mnt_point.unlink(missing_ok=True) + + if not mnt_point.exists(): + mnt_point.mkdir(exist_ok=True, parents=True) + return + + try: + self._umount(mnt_point, ignore_error=False) + except Exception: + logger.warning(f"still use {mnt_point} and override the previous mount") + + @staticmethod + def _ensure_mount(target: StrOrPath, mnt_point: StrOrPath, *, mount_func) -> None: + for _retry in range(MAX_RETRY_COUNT + 1): + try: + mount_func(target=target, mount_point=mnt_point) + CMDHelperFuncs.is_target_mounted(target, raise_exception=True) + return + except CalledProcessError as e: + logger.error(f"retry#{_retry} failed to mout standby slot: {e!r}") + logger.error(f"{e.stderr=}\n{e.stdout=}") + + if _retry == MAX_RETRY_COUNT: + logger.error("exceed max retry count on mounting, abort") + raise + + sleep(RETRY_INTERVAL) + continue + + def mount_standby(self) -> None: + """Mount standby slot dev to . + + Raises: + CalledProcessedError on the last failed attemp. + """ + logger.debug("mount standby slot rootfs dev...") + self._ensure_mointpoint(self.standby_slot_mount_point) + self._ensure_mount( + target=self.standby_slot_dev, + mnt_point=self.standby_slot_mount_point, + mount_func=CMDHelperFuncs.mount_rw, + ) + + def mount_active(self) -> None: + """Mount current active rootfs ready-only. + + Raises: + CalledProcessedError on the last failed attemp. + """ + logger.debug("mount active slot rootfs dev...") + self._ensure_mointpoint(self.active_slot_mount_point) + self._ensure_mount( + target=self.active_rootfs, + mnt_point=self.active_slot_mount_point, + mount_func=CMDHelperFuncs.bind_mount_ro, + ) + + def preserve_ota_folder_to_standby(self): + """Copy the /boot/ota folder to standby slot to preserve it. + + /boot/ota folder contains the ota setting for this device, + so we should preserve it for each slot, accross each update. + """ + logger.debug("copy /boot/ota from active to standby.") + try: + _src = self.active_slot_mount_point / Path(cfg.OTA_DPATH).relative_to("/") + _dst = self.standby_slot_mount_point / Path(cfg.OTA_DPATH).relative_to("/") + shutil.copytree(_src, _dst, dirs_exist_ok=True) + except Exception as e: + raise ValueError( + f"failed to copy /boot/ota from active to standby: {e!r}" + ) from e + + def prepare_standby_dev( + self, + *, + erase_standby: bool = False, + fslabel: Optional[str] = None, + ) -> None: + CMDHelperFuncs.umount(self.standby_slot_dev, raise_exception=False) + if erase_standby: + return CMDHelperFuncs.mkfs_ext4(self.standby_slot_dev, fslabel=fslabel) + + # TODO: in the future if in-place update mode is implemented, do a + # fschck over the standby slot file system. + if fslabel: + CMDHelperFuncs.set_ext4_fslabel(self.standby_slot_dev, fslabel=fslabel) + + def umount_all(self, *, ignore_error: bool = True): + logger.debug("unmount standby slot and active slot mount point...") + self._umount(self.active_slot_mount_point, ignore_error=ignore_error) + self._umount(self.standby_slot_mount_point, ignore_error=ignore_error) From 0a064416cf5002d7c83caadb6f299e62d272ffc4 Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 06:59:00 +0000 Subject: [PATCH 03/12] integrate new slot mount helper --- src/otaclient/boot_control/_grub.py | 4 ++-- src/otaclient/boot_control/_jetson_cboot.py | 5 +++-- src/otaclient/boot_control/_jetson_uefi.py | 5 +++-- src/otaclient/boot_control/_rpi_boot.py | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/otaclient/boot_control/_grub.py b/src/otaclient/boot_control/_grub.py index a3b0df1e3..c4a2389f2 100644 --- a/src/otaclient/boot_control/_grub.py +++ b/src/otaclient/boot_control/_grub.py @@ -43,6 +43,7 @@ from typing import ClassVar, Dict, Generator, List, Optional, Tuple from otaclient import errors as ota_errors +from otaclient.boot_control._slot_mnt_helper import SlotMountHelper from otaclient.configs.cfg import cfg from otaclient_api.v2 import types as api_types from otaclient_common._io import ( @@ -55,7 +56,6 @@ from ._common import ( CMDHelperFuncs, OTAStatusFilesControl, - SlotMountHelper, cat_proc_cmdline, ) from .configs import grub_cfg as boot_cfg @@ -751,7 +751,7 @@ def __init__(self) -> None: self._mp_control = SlotMountHelper( standby_slot_dev=self._boot_control.standby_root_dev, standby_slot_mount_point=cfg.STANDBY_SLOT_MNT, - active_slot_dev=self._boot_control.active_root_dev, + active_rootfs=cfg.ACTIVE_ROOT, active_slot_mount_point=cfg.ACTIVE_SLOT_MNT, ) self._ota_status_control = OTAStatusFilesControl( diff --git a/src/otaclient/boot_control/_jetson_cboot.py b/src/otaclient/boot_control/_jetson_cboot.py index 8556c1f43..df015b90f 100644 --- a/src/otaclient/boot_control/_jetson_cboot.py +++ b/src/otaclient/boot_control/_jetson_cboot.py @@ -31,6 +31,7 @@ PayloadType, load_firmware_package, ) +from otaclient.boot_control._slot_mnt_helper import SlotMountHelper from otaclient.configs.cfg import cfg from otaclient_api.v2 import types as api_types from otaclient_common import replace_root @@ -38,7 +39,7 @@ from otaclient_common.common import subprocess_run_wrapper from otaclient_common.typing import StrOrPath -from ._common import CMDHelperFuncs, OTAStatusFilesControl, SlotMountHelper +from ._common import CMDHelperFuncs, OTAStatusFilesControl from ._jetson_common import ( SLOT_PAR_MAP, BSPVersion, @@ -453,7 +454,7 @@ def __init__(self) -> None: self._mp_control = SlotMountHelper( standby_slot_dev=self._cboot_control.standby_rootfs_devpath, standby_slot_mount_point=cfg.STANDBY_SLOT_MNT, - active_slot_dev=self._cboot_control.curent_rootfs_devpath, + active_rootfs=cfg.ACTIVE_ROOT, active_slot_mount_point=cfg.ACTIVE_SLOT_MNT, ) diff --git a/src/otaclient/boot_control/_jetson_uefi.py b/src/otaclient/boot_control/_jetson_uefi.py index 82cd67c3b..599527db3 100644 --- a/src/otaclient/boot_control/_jetson_uefi.py +++ b/src/otaclient/boot_control/_jetson_uefi.py @@ -39,6 +39,7 @@ PayloadType, load_firmware_package, ) +from otaclient.boot_control._slot_mnt_helper import SlotMountHelper from otaclient.configs.cfg import cfg from otaclient_api.v2 import types as api_types from otaclient_common import replace_root @@ -46,7 +47,7 @@ from otaclient_common.common import subprocess_call from otaclient_common.typing import StrOrPath -from ._common import CMDHelperFuncs, OTAStatusFilesControl, SlotMountHelper +from ._common import CMDHelperFuncs, OTAStatusFilesControl from ._jetson_common import ( SLOT_PAR_MAP, BSPVersion, @@ -861,7 +862,7 @@ def __init__(self) -> None: self._mp_control = SlotMountHelper( standby_slot_dev=uefi_control.standby_rootfs_devpath, standby_slot_mount_point=cfg.STANDBY_SLOT_MNT, - active_slot_dev=self._uefi_control.curent_rootfs_devpath, + active_rootfs=cfg.ACTIVE_ROOT, active_slot_mount_point=cfg.ACTIVE_SLOT_MNT, ) diff --git a/src/otaclient/boot_control/_rpi_boot.py b/src/otaclient/boot_control/_rpi_boot.py index aae47c192..02ab95f63 100644 --- a/src/otaclient/boot_control/_rpi_boot.py +++ b/src/otaclient/boot_control/_rpi_boot.py @@ -27,6 +27,7 @@ from typing_extensions import Self import otaclient.errors as ota_errors +from otaclient.boot_control._slot_mnt_helper import SlotMountHelper from otaclient.configs.cfg import cfg from otaclient_api.v2 import types as api_types from otaclient_common._io import copyfile_atomic, write_str_to_file_atomic @@ -36,7 +37,6 @@ from ._common import ( CMDHelperFuncs, OTAStatusFilesControl, - SlotMountHelper, ) from .configs import rpi_boot_cfg as boot_cfg from .protocol import BootControllerProtocol @@ -436,7 +436,7 @@ def __init__(self) -> None: self._mp_control = SlotMountHelper( standby_slot_dev=self._rpiboot_control.standby_slot_dev, standby_slot_mount_point=cfg.STANDBY_SLOT_MNT, - active_slot_dev=self._rpiboot_control.active_slot_dev, + active_rootfs=cfg.ACTIVE_ROOT, active_slot_mount_point=cfg.ACTIVE_SLOT_MNT, ) # init ota-status files From b461f989a7ccad6ab3af45a99590598bbd403d05 Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 07:04:52 +0000 Subject: [PATCH 04/12] fix up test files --- tests/test_otaclient/test_boot_control/test_grub.py | 3 ++- tests/test_otaclient/test_boot_control/test_rpi_boot.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_otaclient/test_boot_control/test_grub.py b/tests/test_otaclient/test_boot_control/test_grub.py index 5df87ea01..5cb655f2b 100644 --- a/tests/test_otaclient/test_boot_control/test_grub.py +++ b/tests/test_otaclient/test_boot_control/test_grub.py @@ -237,8 +237,9 @@ def mock_setup( grub_ab_slot: tuple[Path, Path, Path], _grub_mkconfig_fsm: GrubMkConfigFSM, ): - from otaclient.boot_control._common import CMDHelperFuncs, SlotMountHelper + from otaclient.boot_control._common import CMDHelperFuncs from otaclient.boot_control._grub import GrubABPartitionDetector + from otaclient.boot_control._slot_mnt_helper import SlotMountHelper slot_a, slot_b, _ = grub_ab_slot diff --git a/tests/test_otaclient/test_boot_control/test_rpi_boot.py b/tests/test_otaclient/test_boot_control/test_rpi_boot.py index 16b1f0155..3760f0326 100644 --- a/tests/test_otaclient/test_boot_control/test_rpi_boot.py +++ b/tests/test_otaclient/test_boot_control/test_rpi_boot.py @@ -24,8 +24,9 @@ import pytest_mock from otaclient.boot_control import _rpi_boot -from otaclient.boot_control._common import CMDHelperFuncs, SlotMountHelper +from otaclient.boot_control._common import CMDHelperFuncs from otaclient.boot_control._rpi_boot import RPIBootController +from otaclient.boot_control._slot_mnt_helper import SlotMountHelper from otaclient.boot_control.configs import RPIBootControlConfig, rpi_boot_cfg from otaclient.configs import DefaultOTAClientConfigs from otaclient.configs.cfg import cfg as otaclient_cfg From 0eb3cb51ec0fead2ee7295d03782ddcca92c5801 Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 07:36:54 +0000 Subject: [PATCH 05/12] slot mnt helper: now expose ensure_mount, ensure_umount and ensure_mountpoint funcs --- src/otaclient/boot_control/_jetson_uefi.py | 2 +- .../boot_control/_slot_mnt_helper.py | 147 ++++++++++-------- 2 files changed, 80 insertions(+), 69 deletions(-) diff --git a/src/otaclient/boot_control/_jetson_uefi.py b/src/otaclient/boot_control/_jetson_uefi.py index 599527db3..08a0159bf 100644 --- a/src/otaclient/boot_control/_jetson_uefi.py +++ b/src/otaclient/boot_control/_jetson_uefi.py @@ -177,7 +177,7 @@ def verify(cls) -> str | None: # pragma: no cover @contextlib.contextmanager def _ensure_efivarfs_mounted() -> Generator[None, Any, None]: # pragma: no cover """Ensure the efivarfs is mounted as rw, and then umount it.""" - if CMDHelperFuncs.is_target_mounted(EFIVARS_SYS_MOUNT_POINT): + if CMDHelperFuncs.is_target_mounted(EFIVARS_SYS_MOUNT_POINT, raise_exception=False): options = "remount,rw,nosuid,nodev,noexec,relatime" else: logger.warning( diff --git a/src/otaclient/boot_control/_slot_mnt_helper.py b/src/otaclient/boot_control/_slot_mnt_helper.py index e76fc8cc5..0ec488f08 100644 --- a/src/otaclient/boot_control/_slot_mnt_helper.py +++ b/src/otaclient/boot_control/_slot_mnt_helper.py @@ -34,6 +34,79 @@ RETRY_INTERVAL = 2 +def ensure_mount( + target: StrOrPath, mnt_point: StrOrPath, *, mount_func, raise_exception: bool = True +) -> None: + """Ensure the mounted on by our best. + + Raises: + If is True, raises the last failed attemp's CalledProcessError. + """ + for _retry in range(MAX_RETRY_COUNT + 1): + try: + mount_func(target=target, mount_point=mnt_point) + CMDHelperFuncs.is_target_mounted(target, raise_exception=True) + return + except CalledProcessError as e: + logger.error(f"retry#{_retry} failed to mout standby slot: {e!r}") + logger.error(f"{e.stderr=}\n{e.stdout=}") + + if _retry >= MAX_RETRY_COUNT: + logger.error("exceed max retry count on mounting, abort") + if raise_exception: + raise + return + + sleep(RETRY_INTERVAL) + continue + + +def ensure_umount(mnt_point: StrOrPath, *, ignore_error: bool) -> None: + """Try to umount the at our best. + + Raises: + If is False, raises the last failed attemp's CalledProcessError. + """ + if not CMDHelperFuncs.is_target_mounted(mnt_point): + return + + for _retry in range(MAX_RETRY_COUNT + 1): + try: + CMDHelperFuncs.umount(mnt_point, raise_exception=True) + break + except CalledProcessError as e: + logger.warning(f"retry#{_retry} failed to umount standby slot: {e!r}") + logger.warning(f"{e.stderr}\n{e.stdout}") + + if _retry >= MAX_RETRY_COUNT: + logger.error(f"reached max retry on umounting {mnt_point}, abort") + if not ignore_error: + raise + return + + sleep(RETRY_INTERVAL) + continue + + +def ensure_mointpoint(mnt_point: Path) -> None: + """Ensure the exists, has no mount on it and ready for mount. + + If the is valid, but we failed to umount any previous mounts on it, + we still keep use the mountpoint as later mount will override the previous one. + """ + if not mnt_point.is_dir(): + mnt_point.unlink(missing_ok=True) + + if not mnt_point.exists(): + mnt_point.mkdir(exist_ok=True, parents=True) + return + + try: + ensure_umount(mnt_point, ignore_error=False) + except Exception: + logger.warning(f"still use {mnt_point} and override the previous mount") + + class SlotMountHelper: """Helper class that provides methods for mounting slots.""" @@ -61,68 +134,6 @@ def __init__( ) ) - @staticmethod - def _umount(mnt_point: StrOrPath, *, ignore_error: bool) -> None: - """Try to umount the at our best. - - Raises: - If is False, raises the last failed attemp's CalledProcessError. - """ - if not CMDHelperFuncs.is_target_mounted(mnt_point): - return - - for _retry in range(MAX_RETRY_COUNT + 1): - try: - CMDHelperFuncs.umount(mnt_point, raise_exception=True) - break - except CalledProcessError as e: - logger.warning(f"retry#{_retry} failed to umount standby slot: {e!r}") - logger.warning(f"{e.stderr}\n{e.stdout}") - - if _retry >= MAX_RETRY_COUNT: - logger.error(f"reached max retry on umounting {mnt_point}, abort") - if not ignore_error: - raise - - sleep(RETRY_INTERVAL) - continue - - def _ensure_mointpoint(self, mnt_point: Path) -> None: - """Ensure the exists, has no mount on it and ready for mount. - - If the is valid, but we failed to umount any previous mounts on it, - we still keep use the mountpoint as later mount will override the previous one. - """ - if not mnt_point.is_dir(): - mnt_point.unlink(missing_ok=True) - - if not mnt_point.exists(): - mnt_point.mkdir(exist_ok=True, parents=True) - return - - try: - self._umount(mnt_point, ignore_error=False) - except Exception: - logger.warning(f"still use {mnt_point} and override the previous mount") - - @staticmethod - def _ensure_mount(target: StrOrPath, mnt_point: StrOrPath, *, mount_func) -> None: - for _retry in range(MAX_RETRY_COUNT + 1): - try: - mount_func(target=target, mount_point=mnt_point) - CMDHelperFuncs.is_target_mounted(target, raise_exception=True) - return - except CalledProcessError as e: - logger.error(f"retry#{_retry} failed to mout standby slot: {e!r}") - logger.error(f"{e.stderr=}\n{e.stdout=}") - - if _retry == MAX_RETRY_COUNT: - logger.error("exceed max retry count on mounting, abort") - raise - - sleep(RETRY_INTERVAL) - continue - def mount_standby(self) -> None: """Mount standby slot dev to . @@ -130,8 +141,8 @@ def mount_standby(self) -> None: CalledProcessedError on the last failed attemp. """ logger.debug("mount standby slot rootfs dev...") - self._ensure_mointpoint(self.standby_slot_mount_point) - self._ensure_mount( + ensure_mointpoint(self.standby_slot_mount_point) + ensure_mount( target=self.standby_slot_dev, mnt_point=self.standby_slot_mount_point, mount_func=CMDHelperFuncs.mount_rw, @@ -144,8 +155,8 @@ def mount_active(self) -> None: CalledProcessedError on the last failed attemp. """ logger.debug("mount active slot rootfs dev...") - self._ensure_mointpoint(self.active_slot_mount_point) - self._ensure_mount( + ensure_mointpoint(self.active_slot_mount_point) + ensure_mount( target=self.active_rootfs, mnt_point=self.active_slot_mount_point, mount_func=CMDHelperFuncs.bind_mount_ro, @@ -184,5 +195,5 @@ def prepare_standby_dev( def umount_all(self, *, ignore_error: bool = True): logger.debug("unmount standby slot and active slot mount point...") - self._umount(self.active_slot_mount_point, ignore_error=ignore_error) - self._umount(self.standby_slot_mount_point, ignore_error=ignore_error) + ensure_umount(self.active_slot_mount_point, ignore_error=ignore_error) + ensure_umount(self.standby_slot_mount_point, ignore_error=ignore_error) From f093fe914989a81bf7496bf8828d152743ee2d1b Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 07:53:26 +0000 Subject: [PATCH 06/12] minor update --- .../boot_control/_slot_mnt_helper.py | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/otaclient/boot_control/_slot_mnt_helper.py b/src/otaclient/boot_control/_slot_mnt_helper.py index 0ec488f08..6676c7218 100644 --- a/src/otaclient/boot_control/_slot_mnt_helper.py +++ b/src/otaclient/boot_control/_slot_mnt_helper.py @@ -21,7 +21,6 @@ from pathlib import Path from subprocess import CalledProcessError from time import sleep -from typing import Optional from otaclient.boot_control._common import CMDHelperFuncs from otaclient.configs.cfg import cfg @@ -36,7 +35,7 @@ def ensure_mount( target: StrOrPath, mnt_point: StrOrPath, *, mount_func, raise_exception: bool = True -) -> None: +) -> None: # pragma: no cover """Ensure the mounted on by our best. Raises: @@ -48,11 +47,15 @@ def ensure_mount( CMDHelperFuncs.is_target_mounted(target, raise_exception=True) return except CalledProcessError as e: - logger.error(f"retry#{_retry} failed to mout standby slot: {e!r}") + logger.error( + f"retry#{_retry} failed to mount {target} on {mnt_point}: {e!r}" + ) logger.error(f"{e.stderr=}\n{e.stdout=}") if _retry >= MAX_RETRY_COUNT: - logger.error("exceed max retry count on mounting, abort") + logger.error( + f"exceed max retry count mounting {target} on {mnt_point}, abort" + ) if raise_exception: raise return @@ -61,13 +64,15 @@ def ensure_mount( continue -def ensure_umount(mnt_point: StrOrPath, *, ignore_error: bool) -> None: +def ensure_umount( + mnt_point: StrOrPath, *, ignore_error: bool +) -> None: # pragma: no cover """Try to umount the at our best. Raises: If is False, raises the last failed attemp's CalledProcessError. """ - if not CMDHelperFuncs.is_target_mounted(mnt_point): + if not CMDHelperFuncs.is_target_mounted(mnt_point, raise_exception=False): return for _retry in range(MAX_RETRY_COUNT + 1): @@ -75,7 +80,7 @@ def ensure_umount(mnt_point: StrOrPath, *, ignore_error: bool) -> None: CMDHelperFuncs.umount(mnt_point, raise_exception=True) break except CalledProcessError as e: - logger.warning(f"retry#{_retry} failed to umount standby slot: {e!r}") + logger.warning(f"retry#{_retry} failed to umount {mnt_point}: {e!r}") logger.warning(f"{e.stderr}\n{e.stdout}") if _retry >= MAX_RETRY_COUNT: @@ -88,7 +93,7 @@ def ensure_umount(mnt_point: StrOrPath, *, ignore_error: bool) -> None: continue -def ensure_mointpoint(mnt_point: Path) -> None: +def ensure_mointpoint(mnt_point: Path) -> None: # pragma: no cover """Ensure the exists, has no mount on it and ready for mount. If the is valid, but we failed to umount any previous mounts on it, @@ -104,10 +109,13 @@ def ensure_mointpoint(mnt_point: Path) -> None: try: ensure_umount(mnt_point, ignore_error=False) except Exception: - logger.warning(f"still use {mnt_point} and override the previous mount") + logger.warning( + f"{mnt_point} still has other mounts on it, " + f"but still use {mnt_point} and override the previous mount" + ) -class SlotMountHelper: +class SlotMountHelper: # pragma: no cover """Helper class that provides methods for mounting slots.""" def __init__( @@ -135,7 +143,7 @@ def __init__( ) def mount_standby(self) -> None: - """Mount standby slot dev to . + """Mount standby slot dev rw to . Raises: CalledProcessedError on the last failed attemp. @@ -182,9 +190,9 @@ def prepare_standby_dev( self, *, erase_standby: bool = False, - fslabel: Optional[str] = None, + fslabel: str | None = None, ) -> None: - CMDHelperFuncs.umount(self.standby_slot_dev, raise_exception=False) + ensure_umount(self.standby_slot_dev, ignore_error=True) if erase_standby: return CMDHelperFuncs.mkfs_ext4(self.standby_slot_dev, fslabel=fslabel) From 8694a0dea29a26c8dfdd651d2dd50a6ab453dde6 Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 08:06:21 +0000 Subject: [PATCH 07/12] otaclient_common.linux: subprocess_run_wrapper: also mask stderr if check_output is False --- src/otaclient_common/linux.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/otaclient_common/linux.py b/src/otaclient_common/linux.py index 5d1e427f3..b6f6ebfff 100644 --- a/src/otaclient_common/linux.py +++ b/src/otaclient_common/linux.py @@ -200,7 +200,7 @@ def _chroot(): return subprocess.run( cmd, check=check, - stderr=subprocess.PIPE, + stderr=subprocess.PIPE if check_output else None, stdout=subprocess.PIPE if check_output else None, timeout=timeout, preexec_fn=preexec_fn, From a4615d7c69b08add855f74e211a7c5cd5ee31143 Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 08:07:01 +0000 Subject: [PATCH 08/12] use is_target_mounted to check if target is umounted --- src/otaclient/boot_control/_slot_mnt_helper.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/otaclient/boot_control/_slot_mnt_helper.py b/src/otaclient/boot_control/_slot_mnt_helper.py index 6676c7218..43e2f3f2a 100644 --- a/src/otaclient/boot_control/_slot_mnt_helper.py +++ b/src/otaclient/boot_control/_slot_mnt_helper.py @@ -78,7 +78,8 @@ def ensure_umount( for _retry in range(MAX_RETRY_COUNT + 1): try: CMDHelperFuncs.umount(mnt_point, raise_exception=True) - break + if not CMDHelperFuncs.is_target_mounted(mnt_point, raise_exception=False): + break except CalledProcessError as e: logger.warning(f"retry#{_retry} failed to umount {mnt_point}: {e!r}") logger.warning(f"{e.stderr}\n{e.stdout}") From 4fd8b69ba0856ac1849bb346b79043677980510c Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 08:15:13 +0000 Subject: [PATCH 09/12] Revert "otaclient_common.linux: subprocess_run_wrapper: also mask stderr if check_output is False" This reverts commit 8694a0dea29a26c8dfdd651d2dd50a6ab453dde6. --- src/otaclient_common/linux.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/otaclient_common/linux.py b/src/otaclient_common/linux.py index b6f6ebfff..5d1e427f3 100644 --- a/src/otaclient_common/linux.py +++ b/src/otaclient_common/linux.py @@ -200,7 +200,7 @@ def _chroot(): return subprocess.run( cmd, check=check, - stderr=subprocess.PIPE if check_output else None, + stderr=subprocess.PIPE, stdout=subprocess.PIPE if check_output else None, timeout=timeout, preexec_fn=preexec_fn, From 775d673c1869e2a4e8f343f8c6caa60569e1f663 Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 08:40:57 +0000 Subject: [PATCH 10/12] fix mount standby: also try to umount the standby slot dev before mounting --- src/otaclient/boot_control/_slot_mnt_helper.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/otaclient/boot_control/_slot_mnt_helper.py b/src/otaclient/boot_control/_slot_mnt_helper.py index 43e2f3f2a..b08123d25 100644 --- a/src/otaclient/boot_control/_slot_mnt_helper.py +++ b/src/otaclient/boot_control/_slot_mnt_helper.py @@ -44,7 +44,7 @@ def ensure_mount( for _retry in range(MAX_RETRY_COUNT + 1): try: mount_func(target=target, mount_point=mnt_point) - CMDHelperFuncs.is_target_mounted(target, raise_exception=True) + CMDHelperFuncs.is_target_mounted(mnt_point, raise_exception=True) return except CalledProcessError as e: logger.error( @@ -72,14 +72,11 @@ def ensure_umount( Raises: If is False, raises the last failed attemp's CalledProcessError. """ - if not CMDHelperFuncs.is_target_mounted(mnt_point, raise_exception=False): - return - for _retry in range(MAX_RETRY_COUNT + 1): try: - CMDHelperFuncs.umount(mnt_point, raise_exception=True) if not CMDHelperFuncs.is_target_mounted(mnt_point, raise_exception=False): break + CMDHelperFuncs.umount(mnt_point, raise_exception=True) except CalledProcessError as e: logger.warning(f"retry#{_retry} failed to umount {mnt_point}: {e!r}") logger.warning(f"{e.stderr}\n{e.stdout}") @@ -151,6 +148,8 @@ def mount_standby(self) -> None: """ logger.debug("mount standby slot rootfs dev...") ensure_mointpoint(self.standby_slot_mount_point) + ensure_umount(self.standby_slot_dev, ignore_error=False) + ensure_mount( target=self.standby_slot_dev, mnt_point=self.standby_slot_mount_point, From 269d06b497adcaa9fae046fc7b93d30b0ec5632c Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 12:14:58 +0000 Subject: [PATCH 11/12] ensure_mointpoint: is_dir also return true on symlink --- src/otaclient/boot_control/_slot_mnt_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/otaclient/boot_control/_slot_mnt_helper.py b/src/otaclient/boot_control/_slot_mnt_helper.py index b08123d25..2a8a1f14f 100644 --- a/src/otaclient/boot_control/_slot_mnt_helper.py +++ b/src/otaclient/boot_control/_slot_mnt_helper.py @@ -97,7 +97,7 @@ def ensure_mointpoint(mnt_point: Path) -> None: # pragma: no cover If the is valid, but we failed to umount any previous mounts on it, we still keep use the mountpoint as later mount will override the previous one. """ - if not mnt_point.is_dir(): + if mnt_point.is_symlink() or not mnt_point.is_dir(): mnt_point.unlink(missing_ok=True) if not mnt_point.exists(): From 5bccbbd9dfeefdb1c223e568f94746a303b31865 Mon Sep 17 00:00:00 2001 From: "bodong.yang" Date: Mon, 11 Nov 2024 12:15:27 +0000 Subject: [PATCH 12/12] explicitly define raise_exception or ignore_error --- src/otaclient/boot_control/_slot_mnt_helper.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/otaclient/boot_control/_slot_mnt_helper.py b/src/otaclient/boot_control/_slot_mnt_helper.py index 2a8a1f14f..2ae9da2c3 100644 --- a/src/otaclient/boot_control/_slot_mnt_helper.py +++ b/src/otaclient/boot_control/_slot_mnt_helper.py @@ -34,7 +34,7 @@ def ensure_mount( - target: StrOrPath, mnt_point: StrOrPath, *, mount_func, raise_exception: bool = True + target: StrOrPath, mnt_point: StrOrPath, *, mount_func, raise_exception: bool ) -> None: # pragma: no cover """Ensure the mounted on by our best. @@ -91,7 +91,9 @@ def ensure_umount( continue -def ensure_mointpoint(mnt_point: Path) -> None: # pragma: no cover +def ensure_mointpoint( + mnt_point: Path, *, ignore_error: bool +) -> None: # pragma: no cover """Ensure the exists, has no mount on it and ready for mount. If the is valid, but we failed to umount any previous mounts on it, @@ -105,7 +107,7 @@ def ensure_mointpoint(mnt_point: Path) -> None: # pragma: no cover return try: - ensure_umount(mnt_point, ignore_error=False) + ensure_umount(mnt_point, ignore_error=ignore_error) except Exception: logger.warning( f"{mnt_point} still has other mounts on it, " @@ -147,13 +149,14 @@ def mount_standby(self) -> None: CalledProcessedError on the last failed attemp. """ logger.debug("mount standby slot rootfs dev...") - ensure_mointpoint(self.standby_slot_mount_point) + ensure_mointpoint(self.standby_slot_mount_point, ignore_error=True) ensure_umount(self.standby_slot_dev, ignore_error=False) ensure_mount( target=self.standby_slot_dev, mnt_point=self.standby_slot_mount_point, mount_func=CMDHelperFuncs.mount_rw, + raise_exception=True, ) def mount_active(self) -> None: @@ -163,11 +166,12 @@ def mount_active(self) -> None: CalledProcessedError on the last failed attemp. """ logger.debug("mount active slot rootfs dev...") - ensure_mointpoint(self.active_slot_mount_point) + ensure_mointpoint(self.active_slot_mount_point, ignore_error=True) ensure_mount( target=self.active_rootfs, mnt_point=self.active_slot_mount_point, mount_func=CMDHelperFuncs.bind_mount_ro, + raise_exception=True, ) def preserve_ota_folder_to_standby(self):