From 1cc74c83b4817559345e8eea6f21e571fb692f67 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Mon, 31 Aug 2026 07:51:19 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20qemu=20shrink=20:=20place=20mesur=C3=A9?= =?UTF-8?q?e=20avant=20sauvegarde,=20=C3=A9tape=20annonc=C3=A9e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit La sauvegarde double la place occupée et le défaut était OUI : sur un disque presque plein, une entrée vide lançait une copie qui s'arrête à mi-course et laisse un .bak tronqué. Les deux tailles passent donc avant la question, et le défaut bascule à NON quand la place manque. Le besoin annoncé est la taille ALLOUÉE : « cp --sparse=always » ne recopie pas les trous d'un qcow2. La sortie du gestionnaire de paquets enchaînait par ailleurs sur une question portant sur autre chose ; l'étape se referme d'une ligne. Vérifié : le défaut remis à OUI sans place, comme la taille apparente au lieu de l'allouée, font tomber les tests. 4144 verts. --- EN --- A backup doubles the space used and the default was YES: on a nearly full disk, an empty answer started a copy that stops midway and leaves a truncated .bak. Both sizes now come before the question, and the default flips to NO when the room is short. The need shown is the ALLOCATED size: "cp --sparse=always" does not copy the holes of a qcow2. The package manager output also ran straight into a question about something else; the step now closes with a line of its own. Checked: putting the default back to YES without room, like the apparent size instead of the allocated one, makes the tests fail. 4144 green. Assisted-by: Claude Opus 5 --- script/todo/qemu_manage.py | 57 +++++++++++++++++++++++-- script/todo/todo_i18n.py | 20 +++++++++ test/test_qemu_shrink_tools.py | 76 ++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 4 deletions(-) diff --git a/script/todo/qemu_manage.py b/script/todo/qemu_manage.py index 51f77aa..56d47be 100644 --- a/script/todo/qemu_manage.py +++ b/script/todo/qemu_manage.py @@ -1350,7 +1350,58 @@ class QemuManageMixin: ) if status: print(f" {t('Error installing the tools: ')}{status}") - return [b for b in self._SHRINK_TOOLS if not shutil.which(b)] + reste = [b for b in self._SHRINK_TOOLS if not shutil.which(b)] + if not reste: + # Sans cette ligne, la sortie du gestionnaire de paquets est + # suivie directement de la question suivante, qui porte sur tout + # autre chose : rien ne dit que l'installation a abouti ni qu'on + # a changé d'étape. + print(f" ✅ {t('Tools installed; on with the shrink.')}") + return reste + + @staticmethod + def _qemu_backup_need_and_free(disk): + """(besoin, libre) en octets pour la copie de sauvegarde du disque. + + Le besoin est la taille ALLOUÉE et non la taille apparente : + « cp --sparse=always » ne recopie pas les trous d'un qcow2. C'est une + borne haute — « --reflink=auto » rend la copie presque gratuite sur + btrfs et XFS — mais rien ne garantit le reflink, et se tromper par + excès est le bon sens ici : une copie qui manque de place s'arrête à + mi-chemin et laisse un .bak tronqué. + """ + besoin = os.stat(disk).st_blocks * 512 + libre = shutil.disk_usage(os.path.dirname(disk) or ".").free + return besoin, libre + + def _qemu_ask_backup(self, disk): + """Proposer la sauvegarde du disque, chiffres en main. True si oui. + + Les deux tailles passent AVANT la question : une copie qui ne tient + pas s'arrête à mi-course et laisse un .bak tronqué sur un système de + fichiers désormais plein. Quand la place manque, le défaut bascule à + NON — une entrée distraite ne doit pas remplir le disque — sans pour + autant décider à la place de l'opérateur, qui peut insister. + """ + besoin, libre = self._qemu_backup_need_and_free(disk) + print( + f"\n{t('A backup doubles the space used:')}" + f" {self._human_size(besoin)} — {t('free here:')}" + f" {self._human_size(libre)}" + ) + if libre > besoin * 1.05: + return self._is_yes_default_yes( + input(t("Back up the disk before shrinking? (Y/n): ")) + ) + print(f"⚠ {t('Not enough free space for a full backup.')}") + return self._is_yes( + input( + t( + "Back up anyway, at the risk of filling the disk?" + " (y/N): " + ) + ) + ) def _qemu_safe_shrink(self, name, disk, new_gb): """Réduit le disque SANS casser l'OS, via qemu-nbd + resize2fs + @@ -1377,9 +1428,7 @@ class QemuManageMixin: # d'échec, et de tester la VM avant de la supprimer (proposé à la fin). self._shrink_backup = None bak = None - if self._is_yes_default_yes( - input(t("Back up the disk before shrinking? (Y/n): ")) - ): + if self._qemu_ask_backup(disk): bak = f"{disk}.bak" print(f"\n{t('Backing up the disk before shrinking…')}") if ( diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 284cd9a..b905c61 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -2829,6 +2829,26 @@ TRANSLATIONS = { "fr": "Aucun paquet connu ici pour :", "en": "No package known here for:", }, + "Tools installed; on with the shrink.": { + "fr": "Outils installés ; on passe à la réduction.", + "en": "Tools installed; on with the shrink.", + }, + "A backup doubles the space used:": { + "fr": "Une sauvegarde double la place occupée :", + "en": "A backup doubles the space used:", + }, + "free here:": { + "fr": "libre ici :", + "en": "free here:", + }, + "Not enough free space for a full backup.": { + "fr": "Pas assez de place libre pour une sauvegarde complète.", + "en": "Not enough free space for a full backup.", + }, + "Back up anyway, at the risk of filling the disk? (y/N): ": { + "fr": "Sauvegarder quand même, au risque de remplir le disque? (y/N) : ", + "en": "Back up anyway, at the risk of filling the disk? (y/N): ", + }, "Backing up the disk before shrinking…": { "fr": "Sauvegarde du disque avant réduction…", "en": "Backing up the disk before shrinking…", diff --git a/test/test_qemu_shrink_tools.py b/test/test_qemu_shrink_tools.py index 364381e..38c2cbc 100644 --- a/test/test_qemu_shrink_tools.py +++ b/test/test_qemu_shrink_tools.py @@ -18,6 +18,7 @@ ses outils. """ import io +import os import sys import unittest from contextlib import redirect_stdout @@ -192,5 +193,80 @@ class TestGivingUp(ShrinkToolsBase): self.assertIn("100", out) +class TestBackupSpace(unittest.TestCase): + """La sauvegarde avant réduction, et la place qu'elle demande. + + Elle doublait l'occupation sans rien annoncer : sur un disque presque + plein la copie s'arrête à mi-course et laisse un .bak tronqué, sur un + système de fichiers désormais saturé. Les deux chiffres passent donc + avant la question, et le défaut bascule quand la place manque — une + entrée distraite ne doit pas remplir le disque. + """ + + GIB = 1 << 30 + + def setUp(self): + self.todo = TODO.__new__(TODO) + + def test_the_need_is_the_allocated_size_not_the_apparent_one(self): + """« cp --sparse=always » ne recopie pas les trous d'un qcow2 : un + disque de 60 Go apparents mais 8 Go alloués ne demande que 8 Go.""" + faux = os.stat_result((0o644, 0, 0, 1, 0, 0, 60 * self.GIB, 0, 0, 0)) + # st_blocks n'est pas dans le tuple : on le pose à part. + with patch("script.todo.qemu_manage.os.stat") as stat, patch( + "script.todo.qemu_manage.shutil.disk_usage" + ) as du: + stat.return_value = type( + "S", (), {"st_blocks": 8 * self.GIB // 512} + )() + du.return_value = type("U", (), {"free": 99 * self.GIB})() + besoin, libre = TODO._qemu_backup_need_and_free("/x/d.qcow2") + self.assertEqual(besoin, 8 * self.GIB) + self.assertEqual(libre, 99 * self.GIB) + self.assertNotEqual(besoin, faux.st_size) + + def _decision(self, besoin, libre, answer): + """(question posée, sauvegarde retenue) — par le VRAI code. + + Ce helper appelle _qemu_ask_backup et ne réimplémente rien : une + copie de la logique dans le test aurait laissé passer un défaut + remis à OUI sans place, ce qui est précisément le défaut à garder. + """ + vu = [] + + def demande(invite=""): + vu.append(invite) + return answer + + with patch.object( + TODO, + "_qemu_backup_need_and_free", + staticmethod(lambda d: (besoin, libre)), + ), patch("builtins.input", demande), redirect_stdout(io.StringIO()): + retenu = self.todo._qemu_ask_backup("/x/d.qcow2") + return vu[-1], retenu + + def test_with_room_the_default_stays_yes(self): + question, retenu = self._decision(12 * self.GIB, 40 * self.GIB, "") + self.assertIn("(O/n", question) + self.assertTrue(retenu) + + def test_without_room_the_default_flips_to_no(self): + """Le cœur du correctif : entrée vide ne doit PAS remplir le disque.""" + question, retenu = self._decision(12 * self.GIB, 3 * self.GIB, "") + self.assertIn("(y/N", question) + self.assertFalse(retenu) + + def test_without_room_insisting_still_works(self): + """On informe, on ne décide pas à la place de l'opérateur.""" + _, retenu = self._decision(12 * self.GIB, 3 * self.GIB, "y") + self.assertTrue(retenu) + + def test_a_margin_guards_the_exactly_equal_case(self): + """Une place égale au besoin n'en laisse aucune : refusé.""" + _, retenu = self._decision(12 * self.GIB, 12 * self.GIB, "") + self.assertFalse(retenu) + + if __name__ == "__main__": unittest.main()