diff --git a/script/todo/qemu_install_monitor.py b/script/todo/qemu_install_monitor.py index d5eb71b..332960c 100644 --- a/script/todo/qemu_install_monitor.py +++ b/script/todo/qemu_install_monitor.py @@ -23,7 +23,11 @@ import socket import subprocess import time -from script.todo.qemu_privilege import sudo_prefix +from script.todo.qemu_privilege import ( + LIBVIRT_URI as URI, + sudo_prefix, + virsh_argv, +) from pathlib import Path try: @@ -1172,7 +1176,7 @@ def virsh_domstates() -> dict: pour tout le parc (à interroger à intervalle LENT).""" try: res = subprocess.run( - ["sudo", "virsh", "list", "--all"], + virsh_argv("list", "--all"), capture_output=True, text=True, timeout=15, @@ -1391,7 +1395,7 @@ def read_domstats() -> str: processus à chaque tour.""" try: res = subprocess.run( - ["sudo", "virsh", "domstats", "--balloon", "--block"], + virsh_argv("domstats", "--balloon", "--block"), capture_output=True, text=True, timeout=15, @@ -1760,15 +1764,7 @@ def arm_balloon(names) -> None: for name in names or (): try: subprocess.run( - [ - "sudo", - "virsh", - "dommemstat", - name, - "--period", - "5", - "--live", - ], + virsh_argv("dommemstat", name, "--period", "5", "--live"), capture_output=True, text=True, timeout=10, @@ -1976,16 +1972,18 @@ def delete_vm_cmd(name: str, with_disks: bool, uuid: str = "") -> str: cmd = "" if uuid: cmd = ( - f"vu=$({sudo_prefix()}virsh domuuid {q} 2>/dev/null" + f"vu=$({sudo_prefix()}virsh --connect {URI} domuuid {q}" + " 2>/dev/null" " | tr -d '[:space:]'); " f'if [ "$vu" != {shlex.quote(uuid)} ]; then ' f'echo "REFUS : {name} n\'est plus le même domaine"' f' "($vu). Rien n\'a été effacé."; exit 1; fi; ' ) cmd += ( - f"{sudo_prefix()}virsh destroy {q} 2>/dev/null; " - f"{sudo_prefix()}virsh undefine {q} --nvram 2>/dev/null " - f"|| {sudo_prefix()}virsh undefine {q}" + f"{sudo_prefix()}virsh --connect {URI} destroy {q} 2>/dev/null; " + f"{sudo_prefix()}virsh --connect {URI} " + f"undefine {q} --nvram 2>/dev/null " + f"|| {sudo_prefix()}virsh --connect {URI} undefine {q}" ) if with_disks: disk = shlex.quote(f"/var/lib/libvirt/images/{name}.qcow2") @@ -2920,7 +2918,10 @@ def run_monitor(manifest_path: str, run_app: bool = True): ) sortie = "Ctrl+O" else: - cmd = f"{sudo_prefix()}virsh console {shlex.quote(vm['name'])}" + cmd = ( + f"{sudo_prefix()}virsh --connect {URI} " + f"console {shlex.quote(vm['name'])}" + ) titre = f"virsh console {vm['name']}" sortie = "Ctrl+]" with self.suspend(): @@ -3107,7 +3108,7 @@ def run_monitor(manifest_path: str, run_app: bool = True): ) else: subprocess.run( - ["sudo", "virsh", action, nom], + virsh_argv(action, nom), capture_output=True, text=True, timeout=30, diff --git a/script/todo/qemu_manage.py b/script/todo/qemu_manage.py index de55f10..e348af9 100644 --- a/script/todo/qemu_manage.py +++ b/script/todo/qemu_manage.py @@ -13,7 +13,11 @@ import subprocess import time from script.todo import todo_install -from script.todo.qemu_privilege import sudo_prefix +from script.todo.qemu_privilege import ( + LIBVIRT_URI as URI, + sudo_prefix, + virsh_argv, +) from script.todo.todo_i18n import t @@ -128,7 +132,7 @@ class QemuManageMixin: self.execute.exec_command_live(cmd, source_erplibre=False) def _qemu_list_vms(self, ask_advanced=False): - cmd = f"{sudo_prefix()}virsh list --all" + cmd = f"{sudo_prefix()}virsh --connect {URI} list --all" print(f"{t('Will execute:')} {cmd}") self.execute.exec_command_live(cmd, source_erplibre=False) if not ask_advanced: @@ -212,7 +216,10 @@ class QemuManageMixin: print(t("Cancelled.")) return for real in resolved: - cmd = f"{sudo_prefix()}virsh {action} {shlex.quote(real)}" + cmd = ( + f"{sudo_prefix()}virsh --connect {URI} " + f"{action} {shlex.quote(real)}" + ) print(f"\n{t('Will execute:')} {cmd}") self.execute.exec_command_live(cmd, source_erplibre=False) @@ -229,7 +236,7 @@ class QemuManageMixin: EST OUVERT, c'est elle qui compte, un disque attaché à chaud n'existant que là. """ - argv = ["sudo", "virsh", "dumpxml"] + argv = virsh_argv("dumpxml") if inactive: argv.append("--inactive") argv.append(name) @@ -250,7 +257,7 @@ class QemuManageMixin: """Démarrage automatique activé ? (absent du XML : virsh seul le sait)""" try: res = subprocess.run( - ["sudo", "virsh", "dominfo", name], + virsh_argv("dominfo", name), capture_output=True, text=True, timeout=15, @@ -291,15 +298,11 @@ class QemuManageMixin: un qui contourne la gestion du réseau par libvirt. """ tokens = [] - nets = self._qemu_cmd_lines( - ["sudo", "virsh", "net-list", "--all", "--name"] - ) + nets = self._qemu_cmd_lines(virsh_argv("net-list", "--all", "--name")) owned = set() for net in nets: tokens.append(f"network:{net}") - for line in self._qemu_cmd_lines( - ["sudo", "virsh", "net-info", net] - ): + for line in self._qemu_cmd_lines(virsh_argv("net-info", net)): if line.startswith("Bridge:"): owned.add(line.split(":", 1)[1].strip()) for line in self._qemu_cmd_lines( @@ -477,7 +480,7 @@ class QemuManageMixin: """(vcpus, max_mem_kib) via « virsh dominfo », ou (0, 0).""" try: res = subprocess.run( - ["sudo", "virsh", "dominfo", name], + virsh_argv("dominfo", name), capture_output=True, text=True, timeout=15, @@ -562,22 +565,14 @@ class QemuManageMixin: disparaît au prochain démarrage du domaine.""" try: subprocess.run( - [ - "sudo", - "virsh", - "dommemstat", - name, - "--period", - "5", - "--live", - ], + virsh_argv("dommemstat", name, "--period", "5", "--live"), capture_output=True, text=True, timeout=15, env=QemuManageMixin._qemu_c_env(), ) res = subprocess.run( - ["sudo", "virsh", "dommemstat", name], + virsh_argv("dommemstat", name), capture_output=True, text=True, timeout=15, @@ -678,7 +673,8 @@ class QemuManageMixin: targets = [name] for tgt in targets: cmd = ( - f"{sudo_prefix()}virsh domifaddr {shlex.quote(tgt)}" + f"{sudo_prefix()}virsh --connect {URI} " + f"domifaddr {shlex.quote(tgt)}" " --source lease" ) print(f"\n{t('Will execute:')} {cmd}") @@ -697,7 +693,9 @@ class QemuManageMixin: print( f"👤 {t('Default login (if set at deploy): erplibre / erplibre')}" ) - cmd = f"{sudo_prefix()}virsh console {shlex.quote(name)}" + cmd = ( + f"{sudo_prefix()}virsh --connect {URI} console {shlex.quote(name)}" + ) print(f"{t('Will execute:')} {cmd}") self.execute.exec_command_live(cmd, source_erplibre=False) @@ -1001,7 +999,7 @@ class QemuManageMixin: """État libvirt de la VM (« running », « shut off », …) ou ''.""" try: res = subprocess.run( - ["sudo", "virsh", "domstate", name], + virsh_argv("domstate", name), capture_output=True, text=True, timeout=15, @@ -1020,7 +1018,7 @@ class QemuManageMixin: return name try: res = subprocess.run( - ["sudo", "virsh", "domname", str(name)], + virsh_argv("domname", str(name)), capture_output=True, text=True, timeout=15, @@ -1042,7 +1040,8 @@ class QemuManageMixin: # --mode acpi,agent : envoie le SIGNAL d'extinction (bouton ACPI) puis # tente l'agent invité si présent — plus fiable qu'un arrêt brutal. cmd = ( - f"{sudo_prefix()}virsh shutdown {shlex.quote(name)}" + f"{sudo_prefix()}virsh --connect {URI} " + f"shutdown {shlex.quote(name)}" " --mode acpi,agent" ) print(f"{t('Will execute:')} {cmd}") @@ -1075,7 +1074,10 @@ class QemuManageMixin: ) ) ): - cmd = f"{sudo_prefix()}virsh destroy {shlex.quote(name)}" + cmd = ( + f"{sudo_prefix()}virsh --connect {URI} " + f"destroy {shlex.quote(name)}" + ) print(f"{t('Will execute:')} {cmd}") self.execute.exec_command_live(cmd, source_erplibre=False) time.sleep(2) @@ -1088,7 +1090,7 @@ class QemuManageMixin: ignore le seed cloud-init (…-seed.iso, en lecture seule).""" try: res = subprocess.run( - ["sudo", "virsh", "domblklist", name, "--details"], + virsh_argv("domblklist", name, "--details"), capture_output=True, text=True, timeout=15, @@ -1269,7 +1271,8 @@ class QemuManageMixin: # Agrandissement À CHAUD : le disque virtuel grossit, le FS invité # devra être étendu ensuite. cmd = ( - f"{sudo_prefix()}virsh blockresize {shlex.quote(name)} " + f"{sudo_prefix()}virsh --connect {URI} " + f"blockresize {shlex.quote(name)} " f"{shlex.quote(disk)} {new_gb:g}G" ) else: @@ -1315,7 +1318,10 @@ class QemuManageMixin: if self._is_yes(input(t("Start the VM now? (y/N): "))): # `name` est déjà le nom canonique : « virsh start » # échouerait car l'ID disparaît quand la VM est éteinte. - cmd = f"{sudo_prefix()}virsh start {shlex.quote(name)}" + cmd = ( + f"{sudo_prefix()}virsh --connect {URI} " + f"start {shlex.quote(name)}" + ) print(f"{t('Will execute:')} {cmd}") self.execute.exec_command_live(cmd, source_erplibre=False) @@ -1808,13 +1814,9 @@ class QemuManageMixin: def agent(payload): try: res = subprocess.run( - [ - "sudo", - "virsh", - "qemu-agent-command", - name, - json.dumps(payload), - ], + virsh_argv( + "qemu-agent-command", name, json.dumps(payload) + ), capture_output=True, text=True, timeout=30, @@ -1875,7 +1877,9 @@ class QemuManageMixin: ) if not self._is_yes(input(t("Open the serial console now? (y/N): "))): return - cmd = f"{sudo_prefix()}virsh console {shlex.quote(name)}" + cmd = ( + f"{sudo_prefix()}virsh --connect {URI} console {shlex.quote(name)}" + ) print(f"{t('Will execute:')} {cmd}") self.execute.exec_command_live(cmd, source_erplibre=False) @@ -1883,7 +1887,7 @@ class QemuManageMixin: """Noms des VM libvirt définies (via virsh).""" try: res = subprocess.run( - ["sudo", "virsh", "list", "--all", "--name"], + virsh_argv("list", "--all", "--name"), capture_output=True, text=True, timeout=15, @@ -1936,9 +1940,11 @@ class QemuManageMixin: # Éteindre si en cours, puis retirer la définition (+ nvram si # UEFI ; repli sans l'option pour les vieilles versions de virsh). cmd = ( - f"{sudo_prefix()}virsh destroy {q} 2>/dev/null; " - f"{sudo_prefix()}virsh undefine {q} --nvram 2>/dev/null " - f"|| {sudo_prefix()}virsh undefine {q}" + f"{sudo_prefix()}virsh --connect {URI} " + f"destroy {q} 2>/dev/null; " + f"{sudo_prefix()}virsh --connect {URI} " + f"undefine {q} --nvram 2>/dev/null " + f"|| {sudo_prefix()}virsh --connect {URI} undefine {q}" ) if del_disks and fichiers: cmd += "; sudo rm -f " + " ".join( @@ -2009,7 +2015,7 @@ class QemuManageMixin: for name in self._qemu_list_domains(): try: res = subprocess.run( - ["sudo", "virsh", "domiflist", name], + virsh_argv("domiflist", name), capture_output=True, text=True, timeout=15, @@ -2128,7 +2134,7 @@ class QemuManageMixin: for name in self._qemu_list_domains(): try: res = subprocess.run( - ["sudo", "virsh", "domblklist", name, "--details"], + virsh_argv("domblklist", name, "--details"), capture_output=True, text=True, timeout=15, @@ -2161,9 +2167,11 @@ class QemuManageMixin: for name in ghosts: q = shlex.quote(name) cmd = ( - f"{sudo_prefix()}virsh destroy {q} 2>/dev/null; " - f"{sudo_prefix()}virsh undefine {q} --nvram 2>/dev/null " - f"|| {sudo_prefix()}virsh undefine {q}" + f"{sudo_prefix()}virsh --connect {URI} " + f"destroy {q} 2>/dev/null; " + f"{sudo_prefix()}virsh --connect {URI} " + f"undefine {q} --nvram 2>/dev/null " + f"|| {sudo_prefix()}virsh --connect {URI} undefine {q}" ) print(f"{t('Will execute:')} {cmd}") self.execute.exec_command_live(cmd, source_erplibre=False) @@ -2354,7 +2362,7 @@ class QemuManageMixin: """Vrai si une VM libvirt de ce nom est déjà définie.""" try: res = subprocess.run( - ["sudo", "virsh", "dominfo", name], + virsh_argv("dominfo", name), capture_output=True, text=True, timeout=15, @@ -2398,7 +2406,7 @@ class QemuManageMixin: for source in ("lease", "agent", "arp"): try: res = subprocess.run( - ["sudo", "virsh", "domifaddr", name, "--source", source], + virsh_argv("domifaddr", name, "--source", source), capture_output=True, text=True, timeout=15, @@ -2430,7 +2438,7 @@ class QemuManageMixin: for source in ("lease", "agent", "arp"): try: res = subprocess.run( - ["sudo", "virsh", "domifaddr", name, "--source", source], + virsh_argv("domifaddr", name, "--source", source), capture_output=True, text=True, timeout=15, @@ -2607,7 +2615,7 @@ class QemuManageMixin: """Architecture d'une VM (jeton amd64/arm64/s390x) via virsh dumpxml.""" try: res = subprocess.run( - ["sudo", "virsh", "dumpxml", name], + virsh_argv("dumpxml", name), capture_output=True, text=True, timeout=15, diff --git a/script/todo/qemu_privilege.py b/script/todo/qemu_privilege.py index e5a33fe..21b37e9 100644 --- a/script/todo/qemu_privilege.py +++ b/script/todo/qemu_privilege.py @@ -72,6 +72,27 @@ def sudo_prefix() -> str: return "sudo " if needs_sudo() else "" +# L'URI ne se laisse JAMAIS implicite. Pour un utilisateur non root, libvirt +# choisit « qemu:///session », un hyperviseur séparé où AUCUNE des VM du +# système n'existe : « virsh list --all » y rend une liste vide, sans erreur +# et sans avertissement. Appartenir au groupe libvirt donne le DROIT d'accéder +# à qemu:///system, mais ne change pas l'URI par défaut. Tant que les +# commandes passaient par sudo, l'URI de root masquait l'omission. +LIBVIRT_URI = "qemu:///system" + + +def virsh_argv(*args: str) -> list: + """Argv d'un virsh local : sudo si besoin, URI toujours.""" + prefixe = ["sudo"] if needs_sudo() else [] + return prefixe + ["virsh", "--connect", LIBVIRT_URI, *args] + + +def virsh_cmd(args: str = "") -> str: + """Même chose, en chaîne pour un shell. « args » est déjà échappé.""" + base = f"{sudo_prefix()}virsh --connect {LIBVIRT_URI}" + return f"{base} {args}" if args else base + + def group_state(user: str = "") -> tuple[bool, bool]: """(déclaré, actif) pour le groupe libvirt. diff --git a/test/test_qemu_monitor_pve.py b/test/test_qemu_monitor_pve.py index 19f66eb..0d1d3b2 100644 --- a/test/test_qemu_monitor_pve.py +++ b/test/test_qemu_monitor_pve.py @@ -571,9 +571,9 @@ class TestEffacerDepuisUnSuiviRouvert(unittest.TestCase): def test_a_local_delete_checks_the_uuid(self): cmd = mon.delete_vm_cmd("vm-a", True, "5d55d05a-1e77") - self.assertIn("virsh domuuid vm-a", cmd) + self.assertIn("domuuid vm-a", cmd) self.assertIn("5d55d05a-1e77", cmd) - self.assertLess(cmd.index("domuuid"), cmd.index("virsh destroy vm-a")) + self.assertLess(cmd.index("domuuid"), cmd.index("destroy vm-a")) def test_an_old_manifest_without_identity_still_deletes(self): # Un manifeste écrit avant ce correctif n'a pas d'UUID. Refuser toute @@ -581,7 +581,7 @@ class TestEffacerDepuisUnSuiviRouvert(unittest.TestCase): # d'avant, la confirmation à deux mains. cmd = mon.delete_vm_cmd("vm-a", True) self.assertNotIn("domuuid", cmd) - self.assertIn("virsh undefine vm-a", cmd) + self.assertIn("undefine vm-a", cmd) sans_nom = mon.delete_vm_cmd_pve({"target": "pve9", "vmid": 101}) self.assertNotIn("qm config", sans_nom) self.assertIn("qm destroy 101", sans_nom) @@ -930,7 +930,7 @@ class TestLeWebEtLaSuppression(unittest.TestCase): def test_deleting_a_local_vm_is_unchanged(self): cmd = mon.delete_vm_cmd("vm-a", True) - self.assertIn("virsh undefine", cmd) + self.assertIn("undefine vm-a", cmd) self.assertIn("/var/lib/libvirt/images/vm-a.qcow2", cmd) diff --git a/test/test_qemu_privilege.py b/test/test_qemu_privilege.py index 3385d58..341fbbb 100644 --- a/test/test_qemu_privilege.py +++ b/test/test_qemu_privilege.py @@ -20,6 +20,7 @@ import io import sys import unittest from contextlib import redirect_stdout +from pathlib import Path from unittest import mock sys.argv = ["todo.py"] @@ -135,5 +136,51 @@ class AvertissementAvantInstallation(unittest.TestCase): self.assertEqual("", self._rendu(False, True, True)) +class LUriEstToujoursExplicite(unittest.TestCase): + """Sans « --connect », un virsh non root vise qemu:///session. + + Cet hyperviseur-là est SÉPARÉ : aucune VM du système n'y existe, et + « list --all » y rend une liste vide, sans erreur ni avertissement. Tant + que les commandes passaient par sudo, l'URI de root masquait l'omission ; + la retirer l'a mise au jour. + """ + + def test_the_builder_always_names_the_uri(self): + for besoin in (True, False): + with mock.patch.object(qp, "needs_sudo", return_value=besoin): + argv = qp.virsh_argv("list", "--all") + self.assertIn("--connect", argv) + self.assertEqual( + qp.LIBVIRT_URI, argv[argv.index("--connect") + 1] + ) + self.assertIn(f"--connect {qp.LIBVIRT_URI}", qp.virsh_cmd("x")) + + def test_sudo_only_when_the_probe_asks_for_it(self): + with mock.patch.object(qp, "needs_sudo", return_value=False): + self.assertNotIn("sudo", qp.virsh_argv("list")) + with mock.patch.object(qp, "needs_sudo", return_value=True): + self.assertEqual("sudo", qp.virsh_argv("list")[0]) + + def test_no_menu_call_builds_virsh_by_hand(self): + """Un virsh écrit à la main échapperait au constructeur, donc à + l'URI : c'est exactement ce qui vidait la liste des VM.""" + for chemin in ( + "script/todo/qemu_manage.py", + "script/todo/qemu_install_monitor.py", + ): + source = Path(chemin).read_text(encoding="utf-8") + for num, ligne in enumerate(source.splitlines(), 1): + if '"virsh"' not in ligne and "}virsh " not in ligne: + continue + voisin = "\n".join( + source.splitlines()[max(0, num - 4) : num + 4] + ) + self.assertIn( + "--connect", + voisin, + f"{chemin}:{num} appelle virsh sans URI", + ) + + if __name__ == "__main__": unittest.main()