From 83b9674176330e64102ffa1672615350ee7a1e27 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Wed, 16 Sep 2026 00:42:48 -0400 Subject: [PATCH] =?UTF-8?q?[REF]=20qemu=20firmware=20:=20une=20table=20pou?= =?UTF-8?q?r=20les=20deux=20voies,=20fuseau=20d=C3=A9cod=C3=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deux tables opposées vivaient dans le même fichier : l'une forçait le BIOS, l'autre l'UEFI, chacune lue d'un seul côté. Elles pouvaient se contredire sans que rien ne le dise. FIRMWARE_IMPOSE les remplace, et les deux suites passent inchangées. « --bios » l'emportait sur un fait physique : forcé sur une image sans secteur d'amorçage, il donnait une VM « running » à console muette. Il est ignoré, et l'appelant l'apprend. Le fuseau était lu en UTF-8 strict sous un « except OSError » : un octet qui n'en est pas faisait échouer tout le déploiement pour une traduction de confort. --- EN --- Two opposing tables lived in one file: one forced BIOS, the other UEFI, each read by one side only. They could contradict each other silently. FIRMWARE_IMPOSE replaces both, and both suites pass unchanged. "--bios" overrode a physical fact: forced on an image with no boot sector it gave a "running" VM with a mute console. It is ignored now, and the caller is told. The timezone was read as strict UTF-8 under an "except OSError": a byte that is not one failed the whole deployment for a convenience translation. Assisted-by: Claude Opus 5 --- long_test/deep_proxmox.py | 4 ++ script/qemu/deploy_qemu.py | 97 +++++++++++++++++++++++------------ test/test_proxmox_uefi.py | 43 ++++++++++++---- test/test_qemu_deploy_hote.py | 35 +++++++++++++ test/test_qemu_timezone.py | 77 +++++++++++++++++++++------ 5 files changed, 196 insertions(+), 60 deletions(-) diff --git a/long_test/deep_proxmox.py b/long_test/deep_proxmox.py index c440314..ea91dc6 100755 --- a/long_test/deep_proxmox.py +++ b/long_test/deep_proxmox.py @@ -258,6 +258,10 @@ class Descente(descente.Descente): "sshkey_path": "/root/.ssh/longtest.pub", "nameservers": dns, "start": True, + # Du CATALOGUE, jamais en dur : create_cmds lit cette clé, et un + # spec qui l'omet vaut SeaBIOS en silence — sur une image sans + # secteur d'amorçage BIOS, une VM « running » à la console muette. + "uefi": mod.requiert_uefi(DISTRO), } # La clé publique doit être un FICHIER sur le parent : « --sshkeys » # n'accepte pas la clé en ligne. diff --git a/script/qemu/deploy_qemu.py b/script/qemu/deploy_qemu.py index 16421d2..d40edc8 100755 --- a/script/qemu/deploy_qemu.py +++ b/script/qemu/deploy_qemu.py @@ -182,25 +182,6 @@ NIXOS_VERSIONS: dict[str, tuple[str, str, int, str]] = { "25.11": ("25.11", "nixos-25.11", 2048, "40G"), } -# Les images qui n'ont AUCUN secteur d'amorçage BIOS : elles ne démarrent que -# par UEFI. -# -# Ce chemin-ci amorce en UEFI pour TOUT LE MONDE et n'a donc jamais eu besoin -# de le savoir ; celui de Proxmox part en SeaBIOS, et c'est lui qui lit cette -# liste. Mesuré sur un Proxmox 9 : en SeaBIOS, une VM NixOS se déclare -# « running » et sa console reste muette ; la même en OVMF démarre — systemd, -# cloud-init, réseau. -# -# La liste reste COURTE plutôt que de basculer le défaut de tous : Debian 13, -# mesurée sur le même hôte, démarre en SeaBIOS sans rien lui devoir. -DISTROS_UEFI_SEUL: tuple[str, ...] = ("nixos",) - - -def requiert_uefi(distro: str) -> bool: - """L'image de `distro` refuse-t-elle un amorçage BIOS hérité ?""" - return distro in DISTROS_UEFI_SEUL - - DISTROS: dict[str, tuple[dict[str, tuple[str, str, int, str]], str]] = { "ubuntu": (UBUNTU_VERSIONS, "24.04"), "debian": (DEBIAN_VERSIONS, "12"), @@ -1890,23 +1871,58 @@ def hostname_valide(nom: str) -> str: TZ_ALIASES = "/usr/share/zoneinfo/tzdata.zi" -# Distributions dont la chaîne UEFI ne démarre pas sur les OVMF courants. +# Le firmware IMPOSÉ par l'image, sur x86, distribution par distribution. # -# L'image de Fedora charge et DÉMARRE son chargeur — le micrologiciel l'annonce +# UNE table et deux valeurs, parce que les deux voies de déploiement partent +# de défauts OPPOSÉS : celle de libvirt amorce en UEFI pour tout le monde, +# celle de Proxmox part en SeaBIOS. Chaque valeur écarte donc un défaut +# différent, et chacune nomme un fait de l'IMAGE — pas une préférence. +# +# « bios » — la chaîne UEFI de l'image ne démarre pas sur les OVMF courants. +# Celle de Fedora charge et DÉMARRE son chargeur — le micrologiciel l'annonce # — puis se fige sans écrire un octet sur le disque. La même image en BIOS # démarre son noyau normalement : ce n'est donc ni l'image, ni la partition # EFI, dont le chemin de repli est bien là. Ni l'entropie ni la machine q35 n'y # changent rien. # -# Le symptôme visible depuis le déploiement est muet : aucune console, aucun -# bail DHCP, une VM « en cours d'exécution » qui ne fait rien. D'où cette table -# plutôt qu'un diagnostic à refaire. -BIOS_OBLIGATOIRE = {"fedora"} +# « uefi » — l'image n'a AUCUN secteur d'amorçage BIOS. Mesuré sur un +# Proxmox 9 : en SeaBIOS, une VM NixOS se déclare « running » et sa console +# reste muette ; la même en OVMF démarre — systemd, cloud-init, réseau. +# +# Le symptôme est muet des deux côtés : aucune console, aucun bail DHCP, une +# VM « en cours d'exécution » qui ne fait rien. D'où cette table plutôt qu'un +# diagnostic à refaire. +# +# Elle reste COURTE plutôt que de renverser un défaut pour tous : Debian 13, +# mesurée sur le même hôte, démarre en SeaBIOS sans rien devoir à l'UEFI. +# +# AVEUGLE À L'ARCHITECTURE : elle ne vaut que sur x86. Sur arm64 il n'y a pas +# de SeaBIOS, et virt_install tranche par l'architecture avant d'arriver ici ; +# la lecture Proxmox, elle, ne crée que des VM x86. +FIRMWARE_IMPOSE: dict[str, str] = {"fedora": "bios", "nixos": "uefi"} def amorcage_bios(distro: str, demande: bool) -> bool: - """Faut-il amorcer en BIOS ? La demande explicite l'emporte toujours.""" - return bool(demande) or distro in BIOS_OBLIGATOIRE + """Faut-il amorcer en BIOS hérité ? + + La demande explicite l'emporte, SAUF sur une image sans secteur + d'amorçage BIOS : « --bios » ne peut pas en inventer un, et la VM se + déclarerait « running » avec une console muette. Un fait physique ne se + force pas ; l'appelant en est averti. + """ + impose = FIRMWARE_IMPOSE.get(distro) + if impose == "uefi": + return False + return bool(demande) or impose == "bios" + + +def requiert_uefi(distro: str) -> bool: + """L'image de `distro` refuse-t-elle un amorçage BIOS hérité ? + + Lue par la voie Proxmox, qui part en SeaBIOS et doit donc savoir à qui + poser « --bios ovmf » et un disque EFI. + """ + return FIRMWARE_IMPOSE.get(distro) == "uefi" def canonical_timezone(tz: str, table: str = TZ_ALIASES) -> str: @@ -1925,11 +1941,15 @@ def canonical_timezone(tz: str, table: str = TZ_ALIASES) -> str: jour de tzdata sans qu'on s'en occupe ; « table » n'existe que pour qu'un test en fournisse une autre sans dépendre du tzdata de sa machine. - DEUX tours, et non un seul : un lien peut désigner un autre lien — - « Universal » mène à « UTC », qui mène à « Etc/UTC ». S'arrêter au premier - rendrait un nom qui reste un alias, donc le défaut qu'on répare. Au-delà - de deux, la table est incohérente et le nom d'origine vaut mieux qu'une - boucle. + DEUX tours, et non un seul. Le format AUTORISE qu'un lien désigne un + autre lien, et un seul tour rendrait alors un nom qui reste un alias — + le défaut même qu'on répare. Le tzdata publié n'en contient aucune : tous + ses liens visent une zone. Le second tour est donc une assurance, au prix + d'une recherche dans un dictionnaire. Au-delà de deux, la table est + incohérente et le nom d'origine vaut mieux qu'une boucle. + + L'INVARIANT que la fonction tient, et que le nombre de tours sert : le nom + rendu n'est pas lui-même un alias de la table. """ if not tz: return tz @@ -1940,7 +1960,9 @@ def canonical_timezone(tz: str, table: str = TZ_ALIASES) -> str: champs = ligne.split() if len(champs) >= 3 and champs[0] == "L": alias[champs[2]] = champs[1] - except OSError: + except (OSError, UnicodeDecodeError): + # Le décodage aussi : un octet hors UTF-8 dans le fichier ferait + # échouer le déploiement ENTIER pour une traduction de confort. return tz for _ in range(2): tz = alias.get(tz, tz) @@ -4476,7 +4498,7 @@ def virt_install( # Boot UEFI par défaut (x86) : Debian 13 (trixie) et les images cloud # récentes n'embarquent plus le chargeur BIOS/GRUB-pc et partent en # boucle « Booting... » en SeaBIOS. --bios force l'ancien BIOS, que - # certaines distributions exigent — voir BIOS_OBLIGATOIRE. + # certaines distributions exigent — voir FIRMWARE_IMPOSE. # Secure Boot DÉSACTIVÉ : le chargeur d'Arch (GRUB) n'est pas signé et # OVMF Secure Boot le refuse (« Access Denied » -> pas de boot). cmd += [ @@ -5151,7 +5173,14 @@ def main() -> None: # rien d'autre qu'un avertissement de cloud-init ne le dise. Le nom de # DOMAINE, lui, peut le porter — les deux ne se ressemblent qu'en général. args.hostname = args.hostname or hostname_valide(args.name) + demande_bios = args.bios args.bios = amorcage_bios(args.distro, args.bios) + if demande_bios and not args.bios: + print( + f"\n --bios ignoré : l'image {args.distro} n'a pas de secteur" + " d'amorçage BIOS. Forcé, elle se déclarerait « running » avec" + " une console muette." + ) pw_hash = resolve_password(args) ssh_keys = load_ssh_keys(args.ssh_key) diff --git a/test/test_proxmox_uefi.py b/test/test_proxmox_uefi.py index ba2ae46..4aeae96 100644 --- a/test/test_proxmox_uefi.py +++ b/test/test_proxmox_uefi.py @@ -118,19 +118,42 @@ class LaSequenceDeCreation(unittest.TestCase): self.assertLess(i_efi, i_start) -class LeMenuTransmetLeMarqueur(unittest.TestCase): - """Un catalogue qui sait et un menu qui ne dit pas laisse la VM en - SeaBIOS : les deux points d'appel sont tenus.""" +class TousLesConstructeursDeSpecLeDisent(unittest.TestCase): + """create_cmds lit « uefi » dans le spec, et un spec qui l'omet vaut + SeaBIOS sans rien dire. - SRC = (RACINE / "script/todo/proxmox_menu.py").read_text(encoding="utf-8") + Compter les appels dans UN fichier laissait échapper tout constructeur + écrit ailleurs, et il y en avait un. Le garde-fou cherche donc les + appelants plutôt que de les supposer. + """ - def test_both_call_sites_pass_it(self): - self.assertEqual(2, self.SRC.count('"uefi": mod.requiert_uefi(')) + def _appelants(self): + trouves = [] + for dossier in ("script", "long_test"): + for chemin in sorted((RACINE / dossier).rglob("*.py")): + if chemin.name == "proxmox_deploy.py": + continue + texte = chemin.read_text(encoding="utf-8") + if "create_cmds(" in texte: + trouves.append((chemin, texte)) + return trouves - def test_it_is_read_from_the_catalogue_not_hardcoded(self): - """Écrire « nixos » ici ferait deux autorités sur une même question.""" - self.assertNotIn('"uefi": True', self.SRC) - self.assertNotIn('"uefi": "nixos"', self.SRC) + def test_the_callers_are_found_at_all(self): + """Un test qui ne trouve plus personne passerait en restant vert.""" + self.assertTrue(self._appelants()) + + def test_every_one_of_them_declares_the_firmware(self): + for chemin, texte in self._appelants(): + with self.subTest(fichier=chemin.name): + self.assertIn('"uefi"', texte) + + def test_none_of_them_decides_it_itself(self): + """Écrire « nixos » à côté du spec ferait deux autorités sur une même + question, et la seconde vieillirait sans qu'on le sache.""" + for chemin, texte in self._appelants(): + with self.subTest(fichier=chemin.name): + self.assertNotIn('"uefi": True', texte) + self.assertNotIn('"uefi": "nixos"', texte) if __name__ == "__main__": diff --git a/test/test_qemu_deploy_hote.py b/test/test_qemu_deploy_hote.py index 44ceb55..16e78a8 100644 --- a/test/test_qemu_deploy_hote.py +++ b/test/test_qemu_deploy_hote.py @@ -28,9 +28,11 @@ RACINE = Path(__file__).resolve().parent.parent sys.path.insert(0, str(RACINE)) from script.qemu.deploy_qemu import ( # noqa: E402 + FIRMWARE_IMPOSE, amorcage_bios, canonical_timezone, hostname_valide, + requiert_uefi, ) @@ -145,6 +147,39 @@ class TestAmorcage(unittest.TestCase): def test_une_distribution_inconnue_garde_le_defaut(self): self.assertFalse(amorcage_bios("inconnue", False)) + def test_la_demande_ne_peut_pas_inventer_un_secteur_damorcage(self): + """La seule limite de « la demande l'emporte » : une image sans + secteur d'amorçage BIOS n'en gagne pas un parce qu'on l'a tapé. Forcée, + elle se déclare « running » avec une console muette — soit la panne + que ce drapeau est censé éviter ailleurs.""" + self.assertFalse(amorcage_bios("nixos", True)) + self.assertFalse(amorcage_bios("nixos", False)) + + +class TestUneSeuleTableDeFirmware(unittest.TestCase): + """Les deux voies de déploiement partent de défauts OPPOSÉS — libvirt en + UEFI, Proxmox en SeaBIOS — et lisent la MÊME table. Deux tables se + contrediraient sans que rien ne le dise, chacune n'étant lue que d'un + côté.""" + + def test_les_deux_lectures_viennent_de_la_table(self): + for distro, impose in FIRMWARE_IMPOSE.items(): + with self.subTest(distro=distro): + self.assertEqual(impose == "uefi", requiert_uefi(distro)) + self.assertEqual( + impose == "bios", amorcage_bios(distro, False) + ) + + def test_la_table_ne_connait_que_deux_valeurs(self): + """Une troisième valeur serait lue comme « rien d'imposé » par les + deux lectures, en silence.""" + self.assertLessEqual(set(FIRMWARE_IMPOSE.values()), {"bios", "uefi"}) + + def test_une_distribution_hors_table_nimpose_rien(self): + self.assertFalse(requiert_uefi("")) + self.assertFalse(requiert_uefi("inconnue")) + self.assertFalse(amorcage_bios("inconnue", False)) + if __name__ == "__main__": unittest.main() diff --git a/test/test_qemu_timezone.py b/test/test_qemu_timezone.py index cd51674..9a42d0f 100644 --- a/test/test_qemu_timezone.py +++ b/test/test_qemu_timezone.py @@ -11,9 +11,15 @@ fuseau, la VM RESTE en UTC, et le module se solde par un échec qui ne se voit qu'aux horodatages, longtemps après le déploiement. Ce que ces tests gardent, et que la traduction ligne à ligne ne donne pas : -un lien peut désigner un AUTRE lien — « Universal » mène à « UTC », qui mène -à « Etc/UTC ». S'arrêter au premier rendrait un nom qui reste un alias, donc -le défaut même qu'on répare. +l'INVARIANT que le nom rendu n'est jamais lui-même un alias. Le format +autorise qu'un lien désigne un autre lien, et s'arrêter au premier rendrait +alors un nom qui reste un alias — le défaut même qu'on répare. + +La chaîne qui l'éprouve est FABRIQUÉE, et la table plus bas contredit sur ce +point le tzdata publié, où « Universal » vise « Etc/UTC » directement : aucun +lien n'y désigne un autre lien. L'inventer est la seule façon d'exercer le +second tour, et la dernière classe vérifie l'invariant sur la VRAIE table, +qu'une chaîne y apparaisse un jour ou non. La table est passée par le paramètre « table », qui existe exactement pour cela : un test qui lirait le tzdata de la machine qui l'exécute passerait ou @@ -45,8 +51,9 @@ def _deploy_qemu(): DQ = _deploy_qemu() # Une tzdata.zi réduite : les lignes « L » sont les seules -# qui portent un lien, le reste du fichier décrit les règles horaires. La -# dernière paire enchaîne deux liens, ce qu'aucune autre n'éprouve. +# qui portent un lien, le reste du fichier décrit les règles horaires. Les +# deux dernières lignes enchaînent deux liens — un cas que le format autorise +# et que le tzdata publié ne contient pas, d'où la fabrication. TABLE = """# tzdata.zi, extrait R d 1974 ma 1 - Ap Su>=1 2 1 D Z America/Toronto -5:17:32 - LMT 1895 @@ -95,11 +102,13 @@ class LaTraduction(unittest.TestCase): ) def test_an_alias_of_an_alias_resolves(self): - """« Universal » pointe « UTC », qui pointe « Etc/UTC ». + """Dans CETTE table, « Universal » pointe « UTC », qui pointe + « Etc/UTC ». - C'est ce que la lecture ligne à ligne, qui rend au premier lien - trouvé, ne sait pas faire : elle rendrait « UTC », un alias que - l'image cloud peut très bien ne pas porter non plus. + Un seul tour rendrait « UTC », un alias que l'image cloud peut très + bien ne pas porter non plus. Le tzdata publié n'enchaîne aucun lien : + la chaîne est fabriquée pour exercer le second tour, le format + l'autorisant. """ self.assertEqual( DQ.canonical_timezone("Universal", self.table), "Etc/UTC" @@ -109,6 +118,27 @@ class LaTraduction(unittest.TestCase): """Rien à traduire, et surtout rien à inventer.""" self.assertEqual(DQ.canonical_timezone("", self.table), "") + def test_a_third_link_is_left_where_two_passes_reach(self): + """La BORNE, celle que la docstring annonce : deux tours, pas une + boucle. Une table qui enchaîne trois liens est incohérente, et s'y + arrêter vaut mieux que tourner sur une table qui se mord la queue.""" + chemin = os.path.join(os.path.dirname(self.table), "trois.zi") + with open(chemin, "w", encoding="utf-8") as fh: + fh.write("L Etc/UTC A\nL A B\nL B C\n") + self.assertEqual(DQ.canonical_timezone("C", chemin), "A") + + def test_a_byte_outside_utf8_does_not_stop_the_deployment(self): + """Le fichier est lu en UTF-8 strict. Un octet qui n'en est pas + lèverait une erreur de DÉCODAGE, qui n'est pas une erreur de + fichier : non rattrapée, elle ferait échouer tout le déploiement + pour une traduction de confort.""" + chemin = os.path.join(os.path.dirname(self.table), "binaire.zi") + with open(chemin, "wb") as fh: + fh.write(b"L America/Toronto Canada/Eastern\n\xff\xfe\n") + self.assertEqual( + DQ.canonical_timezone("Canada/Eastern", chemin), "Canada/Eastern" + ) + class SansTable(unittest.TestCase): def test_a_host_without_the_table_keeps_the_zone_asked_for(self): @@ -149,13 +179,6 @@ class LesDeuxPointsDEcriture(unittest.TestCase): self.assertIn('TZ_ALIASES = "/usr/share/zoneinfo/tzdata.zi"', self.SRC) self.assertIn("table: str = TZ_ALIASES", self.SRC) - def test_two_passes_and_not_one(self): - """Le tour unique rendrait un alias d'alias inchangé au deuxième - niveau : c'est précisément ce que ce fichier garde.""" - i = self.SRC.index("def canonical_timezone(") - corps = self.SRC[i : i + 1800] - self.assertIn("for _ in range(2):", corps) - class LeVraiFichier(unittest.TestCase): """Ce que la vraie table garantit, sur un hôte qui la porte.""" @@ -164,6 +187,28 @@ class LeVraiFichier(unittest.TestCase): if not Path(DQ.TZ_ALIASES).exists(): self.skipTest("hôte sans tzdata.zi") + def _liens(self): + alias = {} + with open(DQ.TZ_ALIASES, encoding="utf-8") as fh: + for ligne in fh: + champs = ligne.split() + if len(champs) >= 3 and champs[0] == "L": + alias[champs[2]] = champs[1] + return alias + + def test_no_alias_survives_the_translation(self): + """L'invariant, sur la vraie table et sur TOUS ses liens : le nom + rendu n'est pas lui-même un alias. + + Ce test est celui qui compte le jour où le tzdata enchaînera deux + liens — il restera vert sans qu'on y touche, alors qu'une implémen- + tation à un seul tour deviendrait fausse ce jour-là.""" + alias = self._liens() + self.assertTrue(alias, "table sans lien") + for nom in alias: + with self.subTest(alias=nom): + self.assertNotIn(DQ.canonical_timezone(nom), alias) + def test_the_zone_written_exists_in_the_guest_tzdata(self): """L'épreuve qui compte : le nom rendu est un fichier de zoneinfo, ce que cloud-init vérifie chez l'invité avant de poser /etc/localtime."""