diff --git a/AGENTS.md b/AGENTS.md index ba5fe55b..e22ab110 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,6 +24,8 @@ Users configure source/destination **endpoints** (credentials) and - Use `.tox/py3/bin/` (`stestr`, `ruff`); it has project deps. Ignore `.mypy_cache`, `.ruff_cache`, `.tox`. - Follow ruff (`tox.ini`, `ruff.toml`). +- Prefer f-strings over `%` formatting and `str.format` in new or + touched code. - Public methods need docstrings (subclasses may inherit). Use type hints when the type is known. - Do not add helpers for trivial checks such as diff --git a/coriolis/osmorphing/windows.py b/coriolis/osmorphing/windows.py index bd34fc27..f2a7f369 100644 --- a/coriolis/osmorphing/windows.py +++ b/coriolis/osmorphing/windows.py @@ -950,8 +950,11 @@ def _install_dism_drivers(self, driver_paths): """Installs the drivers located at the given paths into the offline Windows image via DISM. - Temporarily grants permissions on the DriverStore FileRepository + Attempts to grant permissions on the DriverStore FileRepository (required on Nano Server) for the duration of the installation. + A failure to grant permissions is logged and ignored, since driver + installation usually succeeds without the extra permissions. + Permissions are revoked only when the grant succeeded. Paths which do not exist are skipped with a warning. """ sid = self._get_sid() @@ -959,7 +962,16 @@ def _install_dism_drivers(self, driver_paths): file_repo_path = ( "%sWindows\\System32\\DriverStore\\FileRepository" % self._os_root_dir ) - self._grant_permissions(file_repo_path, "*%s" % sid) + user = "*%s" % sid + permissions_granted = False + try: + self._grant_permissions(file_repo_path, user) + permissions_granted = True + except Exception as err: + LOG.warning( + f"Failed to grant permissions on '{file_repo_path}' for " + f"'{user}'. Continuing with driver installation: {err}" + ) try: for driver_path in driver_paths: if self._conn.test_path(driver_path): @@ -969,7 +981,8 @@ def _install_dism_drivers(self, driver_paths): "Could not locate driver dir '%s', skipping.", driver_path ) finally: - self._revoke_permissions(file_repo_path, "*%s" % sid) + if permissions_granted: + self._revoke_permissions(file_repo_path, user) def _setup_qemu_agent_installation_local_script( self, msi_source_path=None, msi_url=None, qemu_ga_source_dir=None diff --git a/coriolis/tests/osmorphing/test_windows.py b/coriolis/tests/osmorphing/test_windows.py index 220eff56..88344b4e 100644 --- a/coriolis/tests/osmorphing/test_windows.py +++ b/coriolis/tests/osmorphing/test_windows.py @@ -1402,6 +1402,29 @@ def test_install_dism_drivers_revokes_on_failure( # Permissions must be revoked even if driver installation fails: mock_revoke_permissions.assert_called_once_with(exp_repo_path, f"*{null_sid}") + @mock.patch.object(windows.BaseWindowsMorphingTools, "_get_sid") + @mock.patch.object(windows.BaseWindowsMorphingTools, "_grant_permissions") + @mock.patch.object(windows.BaseWindowsMorphingTools, "_add_dism_driver") + @mock.patch.object(windows.BaseWindowsMorphingTools, "_revoke_permissions") + def test_install_dism_drivers_ignores_grant_failure( + self, + mock_revoke_permissions, + mock_add_dism_driver, + mock_grant_permissions, + mock_get_sid, + ): + null_sid = "S-1-0-0" + mock_get_sid.return_value = null_sid + mock_grant_permissions.side_effect = IOError("access denied") + self.conn.test_path.return_value = True + + self.morphing_tools._install_dism_drivers(["e:\\NetKVM"]) + + exp_repo_path = "C:\\Windows\\System32\\DriverStore\\FileRepository" + mock_grant_permissions.assert_called_once_with(exp_repo_path, f"*{null_sid}") + mock_add_dism_driver.assert_called_once_with("e:\\NetKVM") + mock_revoke_permissions.assert_not_called() + @mock.patch.object(windows.BaseWindowsMorphingTools, "register_firstboot_script") def test_setup_qemu_agent_installation_local_script_from_url( self, mock_register_firstboot_script