filesystem: store the actual size in bytes alongside the human readable size

Currently, the partition form stores the size as a human readable value.
(e.g., 123456K, 1.1G, 1.876G, 100G). When we exit the size field (i.e., upon
losing focus), we convert the value to a number of bytes and then align
it up to the nearest MiB (or whatever the alignment requirement is).

Unfortunately, after computing the aligned value, we turn it back into a
human-readable string and store it as is. It is not okay because the
conversion does not ensure that the alignment requirement is still
honored.

For instance, if the user types in 1.1G, we do the following:

 * convert it to a number of bytes -> 1181116006.4 (surprise, it is not
   even an integer).

 * round it up to the nearest MiB -> 1181745152 (this is the correct
   value)

 * transform it into a human readable string and store it as is -> 1.1G
   - which actually corresponds to the original value.

This leads to an exception later when creating the partition:

    File "subiquity/models/filesystem.py", line 1841, in add_partition
      raise Exception(
   Exception: ('size %s or offset %s not aligned to %s', 1181116006, 1048576, 1048576)

Fixed by storing the actual size as a number of bytes - alongside the
human readable size.

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
This commit is contained in:
Olivier Gayot 2023-08-09 13:28:07 +02:00
parent b0f10300eb
commit cda6c54b87
3 changed files with 39 additions and 2 deletions

View File

@ -21,6 +21,7 @@ configuration.
""" """
import logging import logging
import re import re
from typing import Optional
from urwid import Text, connect_signal from urwid import Text, connect_signal
@ -89,6 +90,7 @@ class FSTypeField(FormField):
class SizeWidget(StringEditor): class SizeWidget(StringEditor):
def __init__(self, form): def __init__(self, form):
self.form = form self.form = form
self.accurate_value: Optional[int] = None
super().__init__() super().__init__()
def lost_focus(self): def lost_focus(self):
@ -114,6 +116,9 @@ class SizeWidget(StringEditor):
), ),
) )
) )
# This will invoke self.form.clean_size() and it is expected that
# size_str (and therefore self.value) are propertly aligned.
self.accurate_value = self.form.size
else: else:
aligned_sz = align_up(sz, self.form.alignment) aligned_sz = align_up(sz, self.form.alignment)
aligned_sz_str = humanize_size(aligned_sz) aligned_sz_str = humanize_size(aligned_sz)
@ -125,6 +130,7 @@ class SizeWidget(StringEditor):
_("Rounded size up to {size}").format(size=aligned_sz_str), _("Rounded size up to {size}").format(size=aligned_sz_str),
) )
) )
self.accurate_value = aligned_sz
class SizeField(FormField): class SizeField(FormField):

View File

@ -5,7 +5,7 @@ import urwid
from subiquity.client.controllers.filesystem import FilesystemController from subiquity.client.controllers.filesystem import FilesystemController
from subiquity.common.filesystem import gaps from subiquity.common.filesystem import gaps
from subiquity.models.filesystem import dehumanize_size from subiquity.models.filesystem import MiB, dehumanize_size
from subiquity.models.tests.test_filesystem import make_model_and_disk from subiquity.models.tests.test_filesystem import make_model_and_disk
from subiquity.ui.views.filesystem.partition import ( from subiquity.ui.views.filesystem.partition import (
FormatEntireStretchy, FormatEntireStretchy,
@ -58,6 +58,7 @@ class PartitionViewTests(unittest.TestCase):
gap = gaps.Gap(device=disk, offset=1 << 20, size=99 << 30) gap = gaps.Gap(device=disk, offset=1 << 20, size=99 << 30)
view, stretchy = make_partition_view(model, disk, gap=gap) view, stretchy = make_partition_view(model, disk, gap=gap)
view_helpers.enter_data(stretchy.form, valid_data) view_helpers.enter_data(stretchy.form, valid_data)
stretchy.form.size.widget.lost_focus()
view_helpers.click(stretchy.form.done_btn.base_widget) view_helpers.click(stretchy.form.done_btn.base_widget)
valid_data["mount"] = "/" valid_data["mount"] = "/"
valid_data["size"] = dehumanize_size(valid_data["size"]) valid_data["size"] = dehumanize_size(valid_data["size"])
@ -77,6 +78,7 @@ class PartitionViewTests(unittest.TestCase):
view, stretchy = make_partition_view(model, disk, partition=partition) view, stretchy = make_partition_view(model, disk, partition=partition)
self.assertTrue(stretchy.form.done_btn.enabled) self.assertTrue(stretchy.form.done_btn.enabled)
view_helpers.enter_data(stretchy.form, form_data) view_helpers.enter_data(stretchy.form, form_data)
stretchy.form.size.widget.lost_focus()
view_helpers.click(stretchy.form.done_btn.base_widget) view_helpers.click(stretchy.form.done_btn.base_widget)
expected_data = { expected_data = {
"size": dehumanize_size(form_data["size"]), "size": dehumanize_size(form_data["size"]),
@ -111,6 +113,7 @@ class PartitionViewTests(unittest.TestCase):
self.assertFalse(stretchy.form.size.enabled) self.assertFalse(stretchy.form.size.enabled)
self.assertTrue(stretchy.form.done_btn.enabled) self.assertTrue(stretchy.form.done_btn.enabled)
view_helpers.enter_data(stretchy.form, form_data) view_helpers.enter_data(stretchy.form, form_data)
stretchy.form.size.widget.lost_focus()
view_helpers.click(stretchy.form.done_btn.base_widget) view_helpers.click(stretchy.form.done_btn.base_widget)
expected_data = { expected_data = {
"fstype": "xfs", "fstype": "xfs",
@ -177,6 +180,7 @@ class PartitionViewTests(unittest.TestCase):
self.assertEqual(stretchy.form.mount.value, "/boot/efi") self.assertEqual(stretchy.form.mount.value, "/boot/efi")
view_helpers.enter_data(stretchy.form, form_data) view_helpers.enter_data(stretchy.form, form_data)
stretchy.form.size.widget.lost_focus()
view_helpers.click(stretchy.form.done_btn.base_widget) view_helpers.click(stretchy.form.done_btn.base_widget)
expected_data = { expected_data = {
"size": dehumanize_size(form_data["size"]), "size": dehumanize_size(form_data["size"]),
@ -246,3 +250,26 @@ class PartitionViewTests(unittest.TestCase):
model._orig_config = model._render_actions() model._orig_config = model._render_actions()
view, stretchy = make_format_entire_view(model, disk) view, stretchy = make_format_entire_view(model, disk)
self.assertEqual(stretchy.form.fstype.value, None) self.assertEqual(stretchy.form.fstype.value, None)
def test_create_partition_unaligned_size(self):
# In LP: #2013201, the user would type in 1.1G and the partition
# created would not be aligned to a MiB boundary.
unaligned_data = {
"size": "1.1G", # Corresponds to 1181116006.4 bytes (not an int)
"fstype": "ext4",
}
valid_data = {
"mount": "/",
"size": 1127 * MiB, # ~1.10058 GiB
"use_swap": False,
"fstype": "ext4",
}
model, disk = make_model_and_disk()
gap = gaps.Gap(device=disk, offset=1 << 20, size=99 << 30)
view, stretchy = make_partition_view(model, disk, gap=gap)
view_helpers.enter_data(stretchy.form, unaligned_data)
stretchy.form.size.widget.lost_focus()
view_helpers.click(stretchy.form.done_btn.base_widget)
view.controller.partition_disk_handler.assert_called_once_with(
stretchy.disk, valid_data, partition=None, gap=gap
)

View File

@ -531,7 +531,11 @@ class Form(object, metaclass=MetaForm):
data = {} data = {}
for field in self._fields: for field in self._fields:
if field.enabled: if field.enabled:
data[field.field.name] = field.value accurate_value = getattr(field.widget, "accurate_value", None)
if accurate_value is not None:
data[field.field.name] = accurate_value
else:
data[field.field.name] = field.value
return data return data