[FIX] qemu shrink : place mesurée avant sauvegarde, étape annoncée

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
This commit is contained in:
Mathieu Benoit 2026-08-31 07:51:19 -04:00
parent 87ad8c4439
commit 1cc74c83b4
3 changed files with 149 additions and 4 deletions

View file

@ -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 (

View file

@ -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…",

View file

@ -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()