From 612d9436712592b78f82a56469e44efef98d4dd5 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Thu, 3 Sep 2026 03:49:48 -0400 Subject: [PATCH] =?UTF-8?q?[ADD]=20qemu=20diagnostic=20:=20l'=C3=A9tat=203?= =?UTF-8?q?D=20de=20chaque=20VM,=20ABI=20fig=C3=A9e=20comprise?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le rapport disait ce que l'HÔTE sait faire, jamais ce que chaque VM recevra au prochain démarrage. Trois valeurs y répondent ensemble et aucune seule : le type de vidéo, « accel3d », et le device figé par libvirt, qui l'emporte sur les deux autres. La section les lit dans la définition persistante et écrit la commande qui défige, sans la lancer. Les sections Python du rapport sont désormais isolées. Le fichier s'écrit d'un bloc à la fin : une section qui lève emportait tout ce qui avait été relevé avant elle, laissant sans rapport au moment d'en avoir besoin. Le double de test rendait un objet sans « returncode », ce qui masquait le défaut. --- EN --- The report said what the HOST can do, never what each VM will get on its next boot. Three values answer that together and none alone: video type, « accel3d », and the device libvirt pinned, which outranks both. The section reads them from the persistent definition and writes out the unpinning command without running it. Python sections of the report are now isolated. The file is written in one block at the end: a section that raised took down everything gathered before it, leaving no report exactly when one is needed. The test double returned an object with no « returncode », hiding the fault. Assisted-by: Claude Opus 5 --- script/todo/qemu_manage.py | 94 +++++++++++++++++++++++-- script/todo/todo_i18n.py | 20 ++++++ test/test_qemu_start_egl.py | 135 +++++++++++++++++++++++++++++++++++- 3 files changed, 243 insertions(+), 6 deletions(-) diff --git a/script/todo/qemu_manage.py b/script/todo/qemu_manage.py index 8ee735c..e565838 100644 --- a/script/todo/qemu_manage.py +++ b/script/todo/qemu_manage.py @@ -438,6 +438,70 @@ class QemuManageMixin: print(" lspci -nnk | grep -A3 -iE 'vga|3d|display'") self._qemu_nvidia_acl_advice() + def _qemu_vm_3d_report(self): + """État 3D de chaque VM définie, lu dans sa définition PERSISTANTE. + + Le rapport sur l'hôte dit ce que la machine peut faire ; celui-ci + dit ce que chaque VM recevra au prochain démarrage, ce qui n'est + pas la même question. Trois valeurs y répondent ensemble et aucune + seule : le type de vidéo, « accel3d », et le device figé. + + Ce dernier (attribut « device », depuis libvirt 12.5.0) grave le + device QEMU retenu pour tenir l'ABI de l'invité stable d'un + démarrage à l'autre, et il l'emporte sur « accel3d ». Une VM + démarrée une première fois sans 3D garde donc un device sans GL, + que cocher la 3D ensuite ne change pas : la définition et la ligne + de commande se contredisent alors sans que rien ne le signale. + """ + from script.todo import qemu_hardware as hw + + print(f"\n── {t('3D per VM')} ──") + noms = self._qemu_list_domains() + if not noms: + print(f" {t('none')}") + return + figees = [] + for nom in noms: + etat = hw.hw_state(self._qemu_dumpxml(nom)) + if not etat.get("video"): + print(f" · {nom} — {t('no video device')}") + continue + champs = [ + f"video={etat['video']}", + f"accel3d={'on' if etat.get('accel3d') else 'off'}", + f"device={etat.get('video_device') or t('not pinned')}", + ] + if etat.get("render"): + champs.append(f"rendernode={etat['render']}") + if hw.pin_defeats_3d(etat): + marque = "❌" + figees.append(nom) + elif etat.get("accel3d"): + marque = "✅" + else: + marque = "· " + print(f" {marque} {nom} — {', '.join(champs)}") + if nom in figees: + print( + f" ⚠ {t('non-GL device pinned: 3D will not start')}" + ) + if figees: + # Le rapport ne modifie rien : la commande est ÉCRITE, jamais + # lancée. Elle passe par la définition persistante et non par + # virt-xml, dont le vocabulaire ne connaît pas partout cet + # attribut ; « define » l'accepte quelle que soit sa version. + print(f"\n {t('To unpin, VM stopped:')}") + print(" v=$(mktemp) ; vm=" + figees[0]) + print( + " virsh --connect qemu:///system dumpxml --inactive" + " $vm > $v" + ) + print( + " sed -i \"s/device='virtio-vga'/device='virtio-vga-gl'/\"" + " $v" + ) + print(" virsh --connect qemu:///system define $v") + # Les relevés du diagnostic : (titre, commande). Tous en LECTURE — un # rapport qui modifie l'hôte n'est plus un rapport, et celui-ci est fait # pour être envoyé à quelqu'un qui n'a pas accès à la machine. @@ -670,6 +734,28 @@ class QemuManageMixin: print(f" {t('enough: the list applies when QEMU is launched).')}") return True + @staticmethod + def _diag_section(titre, rendu): + """Une section du rapport, isolée : elle échoue SEULE. + + Les sondes shell sont déjà protégées une à une ; les sections + écrites en Python ne l'étaient pas. Or le rapport s'écrit d'un + bloc à la FIN : une section qui lève emporte avec elle tout ce qui + a été relevé avant, et l'utilisateur se retrouve sans fichier — + au moment précis où il en a besoin. L'échec devient donc une ligne + du rapport, ce qui est en soi une information. + """ + import io + from contextlib import redirect_stdout + + tampon = io.StringIO() + try: + with redirect_stdout(tampon): + rendu() + except Exception as exc: + tampon.write(f"\n({type(exc).__name__}: {exc})\n") + return f"\n===== {titre} =====\n" + tampon.getvalue() + def _qemu_diagnostics(self): """Relevé complet de l'hôte, écrit dans un fichier à transmettre. @@ -702,10 +788,10 @@ class QemuManageMixin: morceaux.append("(timeout)\n") except OSError as exc: morceaux.append(f"({exc})\n") - tampon = io.StringIO() - with redirect_stdout(tampon): - self._qemu_gpu_3d_report() - morceaux.append("\n===== 3D =====\n" + tampon.getvalue()) + morceaux.append(self._diag_section("3D", self._qemu_gpu_3d_report)) + morceaux.append( + self._diag_section("3D par VM", self._qemu_vm_3d_report) + ) try: with open(chemin, "w", encoding="utf-8") as fh: fh.write("".join(morceaux)) diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 193b987..9066c29 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -5008,6 +5008,26 @@ TRANSLATIONS = { ), }, # Récupération de fichiers dans le disque d'une VM (libguestfs) + "3D per VM": { + "fr": "3D par VM", + "en": "3D per VM", + }, + "no video device": { + "fr": "aucun périphérique vidéo", + "en": "no video device", + }, + "not pinned": { + "fr": "non figé", + "en": "not pinned", + }, + "non-GL device pinned: 3D will not start": { + "fr": "device sans GL figé : la 3D ne démarrera pas", + "en": "non-GL device pinned: 3D will not start", + }, + "To unpin, VM stopped:": { + "fr": "Pour défiger, la VM arrêtée :", + "en": "To unpin, VM stopped:", + }, # Une VM dont libvirt a figé le device vidéo sur une variante sans GL # tourne sans 3D, même avec « accel3d=yes » dans sa définition. "frozen on": { diff --git a/test/test_qemu_start_egl.py b/test/test_qemu_start_egl.py index d8c6e03..cff2e3c 100644 --- a/test/test_qemu_start_egl.py +++ b/test/test_qemu_start_egl.py @@ -201,6 +201,10 @@ class LeDiagnostic(unittest.TestCase): class R: stdout = f"sortie de {cmd}\n" stderr = "" + # Le vrai objet de subprocess en porte un, et les helpers + # qui appellent virsh s'en servent pour distinguer une + # sortie d'un échec. L'omettre faisait mentir le double. + returncode = 0 return R() @@ -224,7 +228,16 @@ class LeDiagnostic(unittest.TestCase): contenu = Path(tmp, fichiers[0]).read_text(encoding="utf-8") # Les quatre familles qui décident d'un problème QEMU : la machine, # l'hyperviseur, le GPU, et l'interpréteur qui porte virt-xml. - for attendu in ("uname", "virsh", "dri", "virt-xml", "3D"): + # « 3D » couvre l'hôte, « 3D par VM » ce que chaque définition + # livrera : deux questions distinctes, deux sections. + for attendu in ( + "uname", + "virsh", + "dri", + "virt-xml", + "===== 3D =====", + "===== 3D par VM =====", + ): self.assertIn(attendu, contenu) # Programmes qui ne peuvent que LIRE. « command -v » cherche un outil @@ -247,7 +260,7 @@ class LeDiagnostic(unittest.TestCase): "sort", "echo", } - VIRSH_LECTURE = {"version", "list", "net-list"} + VIRSH_LECTURE = {"version", "list", "net-list", "dumpxml", "dominfo"} def test_every_probe_is_read_only(self): """Un rapport qui modifie l'hôte n'est plus un rapport.""" @@ -255,6 +268,12 @@ class LeDiagnostic(unittest.TestCase): self._lancer(tmp) operateurs = {";", "|", "||", "&&"} for cmd in self.lances: + # Le relevé lance deux formes : les sondes, chaînes passées au + # shell, et les helpers qui appellent virsh en argv-liste. Une + # liste est DÉJÀ découpée — la passer à shlex lèverait. + if isinstance(cmd, (list, tuple)): + self._juger(list(cmd), cmd) + continue # shlex plutôt qu'un découpage sur « | » : le motif de grep en # contient un, et le couper au milieu ferait juger « 3d » comme # s'il était un programme. @@ -617,3 +636,115 @@ class LesPieces3D(unittest.TestCase): "qemu/hw-display-virtio-gpu-gl.so", ): self.assertIn(module, motifs) + + +class La3DParVM(unittest.TestCase): + """Le rapport dit, VM par VM, ce que le prochain démarrage livrera. + + Trois valeurs y répondent ENSEMBLE et aucune seule : le type de vidéo, + « accel3d », et le device figé par libvirt. Une VM dont l'ABI est + figée sur un device sans GL tourne sans 3D quoi que demande sa + définition, et c'est le cas qu'un rapport doit nommer. + """ + + XML = ( + "{n}" + "" + "" + "" + ) + + def _rendu(self, vms): + """Sortie du relevé pour {nom: (device figé, accel3d)}.""" + faux = { + nom: self.XML.format( + n=nom, + attr=f"device='{dev}'" if dev else "", + a=accel, + ) + for nom, (dev, accel) in vms.items() + } + todo = TODO.__new__(TODO) + vus = [] + with mock.patch.object( + TODO, "_qemu_list_domains", lambda s: list(faux) + ), mock.patch.object( + TODO, + "_qemu_dumpxml", + staticmethod(lambda n, inactive=True: faux[n]), + ), mock.patch( + "builtins.print", + side_effect=lambda *a, **k: vus.append( + " ".join(str(x) for x in a) + ), + ): + todo._qemu_vm_3d_report() + return "\n".join(vus) + + def test_a_non_gl_pin_is_named_and_explained(self): + rendu = self._rendu({"vm": ("virtio-vga", "yes")}) + self.assertIn("❌", rendu) + self.assertIn("device=virtio-vga", rendu) + self.assertIn("accel3d=on", rendu) + + def test_the_repair_is_written_not_run(self): + """Un rapport ne modifie rien : la commande est ÉCRITE. + + Elle passe par « define » et non par virt-xml, dont le vocabulaire + ne connaît pas partout cet attribut. + """ + rendu = self._rendu({"vm": ("virtio-vga", "yes")}) + self.assertIn("virtio-vga-gl", rendu) + self.assertIn("define", rendu) + + def test_a_gl_pin_is_left_alone(self): + """Le suffixe « -gl » distingue les deux devices : sans cette + lecture, une VM correctement accélérée serait accusée à tort.""" + rendu = self._rendu({"vm": ("virtio-vga-gl", "yes")}) + self.assertIn("✅", rendu) + self.assertNotIn("❌", rendu) + self.assertNotIn("define", rendu) + + def test_a_vm_that_never_asked_for_3d_is_not_accused(self): + rendu = self._rendu({"vm": ("virtio-vga", "no")}) + self.assertNotIn("❌", rendu) + self.assertNotIn("define", rendu) + + def test_only_the_frozen_one_is_counted(self): + """Un lot mêlé : le bloc de réparation ne doit paraître qu'une + fois, et nommer une VM réellement figée.""" + rendu = self._rendu( + { + "saine": ("virtio-vga-gl", "yes"), + "figee": ("virtio-vga", "yes"), + "sans": ("virtio-vga", "no"), + } + ) + self.assertEqual(1, rendu.count("❌")) + self.assertEqual(1, rendu.count("define")) + self.assertIn("vm=figee", rendu) + + +class LesSectionsDuRapport(unittest.TestCase): + """Le rapport s'écrit d'un bloc à la FIN. + + Une section qui lève emporterait donc tout ce qui a été relevé avant + elle, et l'utilisateur se retrouverait sans fichier au moment précis + où il en a besoin. + """ + + def test_a_failing_section_becomes_a_line_not_a_loss(self): + def casse(): + raise RuntimeError("sonde cassée") + + texte = TODO._diag_section("essai", casse) + self.assertIn("===== essai =====", texte) + self.assertIn("RuntimeError", texte) + self.assertIn("sonde cassée", texte) + + def test_a_healthy_section_keeps_its_output(self): + texte = TODO._diag_section("essai", lambda: print("relevé")) + self.assertIn("relevé", texte) + self.assertNotIn("Error", texte)