diff --git a/tests/test_audit_fixes.py b/tests/test_audit_fixes.py index 86b7f3d..85dbbd8 100644 --- a/tests/test_audit_fixes.py +++ b/tests/test_audit_fixes.py @@ -55,16 +55,44 @@ def test_vm_action_success_redirects_without_error(logged_in, data_dirs, monkeyp def test_vm_delete_undefines_uefi_machines(logged_in, data_dirs, monkeypatch): + import core + calls = recording(monkeypatch, app) + core_calls = recording(monkeypatch, core) + disk = data_dirs["images"] / "web01.qcow2" + disk.write_bytes(b"QFI\xfb") response = logged_in.post("/vm/web01/delete", follow_redirects=False) assert response.headers["location"].startswith("/?message=") assert find(calls, "virsh", "destroy", "web01") - undefine = find(calls, "virsh", "undefine", "web01")[0] - assert "--nvram" in undefine and "--remove-all-storage" in undefine and "--snapshots-metadata" in undefine + undefine = find(core_calls, "virsh", "undefine", "web01")[0] + assert "--nvram" in undefine and "--snapshots-metadata" in undefine + assert not disk.exists() and find(core_calls, "virsh", "pool-refresh", "virtuality-images") + + +def test_vm_delete_keeps_installer_iso_and_foreign_disks(logged_in, data_dirs, monkeypatch): + """--remove-all-storage удалял вставленный ISO: загруженные образы пропадали вместе с машиной.""" + import core + + iso = data_dirs["iso"] / "smk-server-_1.iso" + iso.write_bytes(b"ISO") + disk = data_dirs["images"] / "web01.qcow2" + disk.write_bytes(b"QFI\xfb") + foreign = data_dirs["disk_images"] / "shared.raw" + foreign.write_bytes(b"raw") + listing = f" Type Device Target Source\n----\n file disk vda {disk}\n file disk vdb {foreign}\n file cdrom sda {iso}\n" + recording(monkeypatch, app) + core_calls = recording(monkeypatch, core, [(["virsh", "domblklist"], conftest.fake_result(listing))]) + response = logged_in.post("/vm/web01/delete", follow_redirects=False) + assert not disk.exists() and iso.exists() and foreign.exists() + assert "--remove-all-storage" not in find(core_calls, "virsh", "undefine", "web01")[0] + assert str(foreign) in unquote(response.headers["location"]) def test_vm_delete_failure_is_reported(logged_in, data_dirs, monkeypatch): - recording(monkeypatch, app, [(["virsh", "undefine"], conftest.fake_result(ok=False, stderr="error: Requested operation is not valid: cannot undefine domain with nvram"))]) + import core + + recording(monkeypatch, app) + recording(monkeypatch, core, [(["virsh", "undefine"], conftest.fake_result(ok=False, stderr="error: Requested operation is not valid: cannot undefine domain with nvram"))]) response = logged_in.post("/vm/web01/delete", follow_redirects=False) assert response.headers["location"].startswith("/vm/web01?error=") @@ -73,9 +101,9 @@ def test_vm_delete_stops_paused_machine_first(logged_in, data_dirs, monkeypatch) import core calls = recording(monkeypatch, app) - recording(monkeypatch, core, [(["virsh", "domstate"], conftest.fake_result("paused"))]) + core_calls = recording(monkeypatch, core, [(["virsh", "domstate"], conftest.fake_result("paused"))]) logged_in.post("/vm/web01/delete", follow_redirects=False) - assert find(calls, "virsh", "destroy", "web01") and find(calls, "virsh", "undefine", "web01") + assert find(calls, "virsh", "destroy", "web01") and find(core_calls, "virsh", "undefine", "web01") def test_vm_delete_waits_for_running_task(logged_in, data_dirs, monkeypatch): diff --git a/tests/test_backups.py b/tests/test_backups.py index 4121242..6d151e1 100644 --- a/tests/test_backups.py +++ b/tests/test_backups.py @@ -420,7 +420,7 @@ def test_restore_requires_replace_confirmation(fast_worker): assert exc.value.code == "exists" -UNDEFINE = ["virsh", "undefine", "web01", "--remove-all-storage", "--nvram", "--managed-save", "--snapshots-metadata"] +UNDEFINE = ["virsh", "undefine", "web01", "--nvram", "--managed-save", "--snapshots-metadata", "--checkpoints-metadata"] def test_restore_replaces_existing_vm_only_after_disks_are_ready(fast_worker, monkeypatch): @@ -464,23 +464,23 @@ def test_restore_refuses_when_space_is_tight_even_if_old_vm_would_free_it(fast_w assert exc.value.code == "space" and "рядом со старой машиной" not in str(exc.value) -def test_restore_ejects_iso_before_undefine(fast_worker, monkeypatch): +def test_restore_keeps_inserted_iso(fast_worker, monkeypatch): make_backup(fast_worker) + iso = fast_worker["iso"] / "debian.iso" + iso.write_bytes(b"ISO") virsh = FakeVirsh(running=False) base = virsh.__call__ def with_iso(cmd, timeout=12, **kw): if cmd[:2] == ["virsh", "domblklist"]: virsh.calls.append(list(cmd)) - return fake_result(DOMBLKLIST_DETAILS.replace("sda -", "sda /var/lib/virtuality/iso/debian.iso")) + return fake_result(DOMBLKLIST_DETAILS.replace("/var/lib/virtuality/images", str(core.IMAGES_DIR)).replace("sda -", f"sda {iso}")) return base(cmd, timeout, **kw) monkeypatch.setattr(core, "run_cmd", with_iso) monkeypatch.setattr(backups, "stream_cmd", fake_stream()) backups.start_restore("web01", "20260924-0300", "web01", True) - eject = ["virsh", "change-media", "web01", "sda", "--eject", "--config"] - assert eject in virsh.calls - assert virsh.calls.index(eject) < virsh.calls.index(UNDEFINE) + assert UNDEFINE in virsh.calls and iso.exists() def test_restore_rejects_bad_name_and_incomplete_backup(fast_worker): diff --git a/tests/test_quickvm.py b/tests/test_quickvm.py index 87133e6..1410bd5 100644 --- a/tests/test_quickvm.py +++ b/tests/test_quickvm.py @@ -146,7 +146,10 @@ def test_reservation_is_reused_and_released(data_dirs, monkeypatch): def test_vm_delete_releases_address(logged_in, data_dirs, monkeypatch): - calls = recording(monkeypatch, app, []) + import core + + recording(monkeypatch, app, []) + calls = recording(monkeypatch, core, []) net_calls = recording(monkeypatch, network_core, [(["virsh", "net-dumpxml"], conftest.fake_result((conftest.FIXTURES / "net-dumpxml.xml").read_text()))]) assert logged_in.post("/vm/web01/delete", follow_redirects=False).status_code == 303 assert find(calls, "virsh", "undefine") and find(net_calls, "virsh", "net-update", "virtuality-nat", "delete") diff --git a/web/app.py b/web/app.py index 77d7b84..21bcf1b 100644 --- a/web/app.py +++ b/web/app.py @@ -2101,13 +2101,15 @@ def vm_action(request: Request, name: str, action: str): return redirect_with_message(f"/vm/{name}", "error", f"С машиной сейчас идёт задача «{busy[0].get('title') or 'операция'}». Дождитесь её завершения и удалите машину снова.") if core.vm_is_running(name): # и на паузе тоже: undefine активной машины оставил бы её работать без настроек run_cmd(["virsh", "destroy", name], timeout=20) - # --nvram: UEFI machines keep firmware variables that block a plain undefine. - result = run_cmd(["virsh", "undefine", name, "--remove-all-storage", "--nvram", "--managed-save", "--snapshots-metadata", "--checkpoints-metadata"], timeout=120) - if not result["ok"]: - return redirect_with_message(f"/vm/{name}", "error", f"Не удалось удалить машину: {virsh_error(result)}") + # Установочные ISO и образы систем остаются: удаляются только диски самой машины. + ok, error, _removed, kept = core.undefine_vm(name) + if not ok: + return redirect_with_message(f"/vm/{name}", "error", f"Не удалось удалить машину: {error}") network_core.release_nat_address(name) - removed = network_core.remove_vm_forwards(name) - message = f"Машина {name} удалена" + (f", вместе с ней убраны пробросы портов: {removed}" if removed else "") + forwards = network_core.remove_vm_forwards(name) + message = f"Машина {name} удалена" + (f", вместе с ней убраны пробросы портов: {forwards}" if forwards else "") + if kept: + message += ". Не удалены диски вне папки машин: " + ", ".join(str(path) for path in kept) return redirect_with_message("/", "message", message) if action not in allowed: return JSONResponse({"ok": False, "error": "Unsupported action"}, status_code=400) diff --git a/web/core.py b/web/core.py index 3574a03..7a99de3 100644 --- a/web/core.py +++ b/web/core.py @@ -227,6 +227,43 @@ def vm_is_running(name: str) -> bool: return vm_state(name) in ("running", "idle", "blocked", "paused", "pmsuspended", "in shutdown") +def vm_disk_files(name: str) -> tuple[list[Path], list[Path]]: + """Файлы дисков машины: (свои — в IMAGES_DIR, чужие — где-то ещё). Приводы CD/DVD сюда не входят.""" + own: list[Path] = [] + foreign: list[Path] = [] + result = run_cmd(["virsh", "domblklist", name, "--details"], timeout=10) + for disk in presenters.parse_domblklist(result.get("stdout") or ""): + if disk["device"] == "disk" and disk["source"]: + path = Path(disk["source"]) + (own if IMAGES_DIR.resolve() in path.resolve().parents else foreign).append(path) + return own, foreign + + +def undefine_vm(name: str) -> tuple[bool, str, list[Path], list[Path]]: + """Удалить машину и файлы её дисков из IMAGES_DIR. Возвращает (ok, ошибка, удалённые, оставленные). + + Без `--remove-all-storage`: он удаляет все тома libvirt, подключённые к машине, в том числе + вставленный установочный ISO из пула virtuality-iso. Диски вне IMAGES_DIR не трогаем.""" + own, foreign = vm_disk_files(name) # после undefine libvirt о дисках машины уже не расскажет + # --nvram: UEFI machines keep firmware variables that block a plain undefine. + result = run_cmd(["virsh", "undefine", name, "--nvram", "--managed-save", "--snapshots-metadata", "--checkpoints-metadata"], timeout=120) + if not result["ok"]: + return False, cmd_error(result, "virsh undefine"), [], [] + removed = [] + for disk in own: + try: + disk.unlink() + except FileNotFoundError: + continue + except OSError: + foreign.append(disk) + continue + removed.append(disk) + if removed: + run_cmd(["virsh", "pool-refresh", "virtuality-images"], timeout=20) + return True, "", removed, foreign + + # ---------------------------------------------------------------- operations def operation_meta_path(operation_id: str) -> Path: return OPERATIONS_DIR / f"{operation_id}.json" diff --git a/web/features/backups.py b/web/features/backups.py index 117de7e..6a42f7a 100644 --- a/web/features/backups.py +++ b/web/features/backups.py @@ -140,13 +140,6 @@ def vm_disks(name: str) -> list[dict[str, str]]: return [disk for disk in presenters.parse_domblklist(result["stdout"]) if disk["device"] == "disk" and disk["source"]] -def vm_cdroms_with_media(name: str) -> list[dict[str, str]]: - result = run(["virsh", "domblklist", name, "--details"], timeout=10) - if not result["ok"]: - return [] - return [disk for disk in presenters.parse_domblklist(result["stdout"]) if disk["device"] == "cdrom" and disk["source"]] - - def existing_parent(path: Path) -> Path: """Сам путь или ближайший существующий родитель — чтобы спросить у ФС о разделе.""" target = path @@ -691,16 +684,12 @@ def remove_vm(operation: dict[str, Any], name: str) -> None: result = run(["virsh", "destroy", name], timeout=30) if not result["ok"]: raise BackupError("Не удалось выключить старую машину: " + core.cmd_error(result, "virsh destroy")) - for cdrom in vm_cdroms_with_media(name): - # Иначе `undefine --remove-all-storage` удалит и вставленный ISO-образ. - result = run(["virsh", "change-media", name, cdrom["target"], "--eject", "--config"], timeout=20) - if not result["ok"]: - raise BackupError(f"Не удалось извлечь образ из привода {cdrom['target']}: " + core.cmd_error(result, "virsh change-media")) - log(f"Образ извлечён из привода {cdrom['target']}, чтобы он не был удалён вместе с машиной.") - result = run(["virsh", "undefine", name, "--remove-all-storage", "--nvram", "--managed-save", "--snapshots-metadata"], timeout=180) - if not result["ok"]: - raise BackupError("Не удалось удалить старую машину: " + core.cmd_error(result, "virsh undefine")) + ok, error, _removed, kept = core.undefine_vm(name) # вставленный ISO остаётся на месте + if not ok: + raise BackupError("Не удалось удалить старую машину: " + error) log(f"Старая машина {name} удалена вместе с дисками.") + if kept: + log("Диски вне папки машин не удалены: " + ", ".join(str(path) for path in kept)) def restore_worker(operation: dict[str, Any]) -> None: