From 383a9d20f49d64008dfd7cb094bfddb82d9cee26 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Thu, 11 Jan 2024 16:10:54 +0100 Subject: [PATCH 1/9] examples: update machine-configs to include NVMe controllers Signed-off-by: Olivier Gayot --- examples/machines/imsm.json | 84 +++++++++++++++++++ .../machines/lp-1986676-missing-osprober.json | 43 ++++++++++ 2 files changed, 127 insertions(+) diff --git a/examples/machines/imsm.json b/examples/machines/imsm.json index 3c0d5bc2..91805a9d 100644 --- a/examples/machines/imsm.json +++ b/examples/machines/imsm.json @@ -1580,6 +1580,90 @@ } ] }, + "nvme": { + "nvme0": { + "DEVNAME": "/dev/nvme0", + "DEVPATH": "/devices/pci0000:b2/0000:b2:05.5/pci10000:00/10000:00:02.0/10000:01:00.0/nvme/nvme0", + "MAJOR": "238", + "MINOR": "0", + "NVME_TRTYPE": "pcie", + "SUBSYSTEM": "nvme", + "attrs": { + "address": "0000:b2:05.5", + "cntlid": "5", + "cntrltype": "io", + "dctype": "none", + "dev": "238:0", + "device": null, + "firmware_rev": "A3550012", + "hmb": "1", + "kato": "0", + "model": "A400 NVMe SanDisk 256GB", + "numa_node": "0", + "power/async": "disabled", + "power/autosuspend_delay_ms": null, + "power/control": "auto", + "power/pm_qos_latency_tolerance_us": "100000", + "power/runtime_active_kids": "0", + "power/runtime_active_time": "0", + "power/runtime_enabled": "disabled", + "power/runtime_status": "unsupported", + "power/runtime_suspended_time": "0", + "power/runtime_usage": "0", + "queue_count": "9", + "rescan_controller": null, + "reset_controller": null, + "serial": "171877421152", + "sqsize": "1023", + "state": "live", + "subsysnqn": "nqn.1994-11.com.sandisk:nvme:A400M.2:171877421152", + "subsystem": "nvme", + "transport": "pcie", + "uevent": "MAJOR=238\nMINOR=0\nDEVNAME=nvme0\nNVME_TRTYPE=pcie" + } + }, + "nvme1": { + "DEVNAME": "/dev/nvme1", + "DEVPATH": "/devices/pci0000:b2/0000:b2:05.5/pci10000:00/10000:00:03.0/10000:02:00.0/nvme/nvme1/nvme1n1", + "MAJOR": "238", + "MINOR": "0", + "NVME_TRTYPE": "pcie", + "SUBSYSTEM": "nvme", + "attrs": { + "address": "0000:b2:05.5", + "cntlid": "5", + "cntrltype": "io", + "dctype": "none", + "dev": "238:0", + "device": null, + "firmware_rev": "A3550012", + "hmb": "1", + "kato": "0", + "model": "A400 NVMe SanDisk 256GB", + "numa_node": "0", + "power/async": "disabled", + "power/autosuspend_delay_ms": null, + "power/control": "auto", + "power/pm_qos_latency_tolerance_us": "100000", + "power/runtime_active_kids": "0", + "power/runtime_active_time": "0", + "power/runtime_enabled": "disabled", + "power/runtime_status": "unsupported", + "power/runtime_suspended_time": "0", + "power/runtime_usage": "0", + "queue_count": "9", + "rescan_controller": null, + "reset_controller": null, + "serial": "171877424191", + "sqsize": "1023", + "state": "live", + "subsysnqn": "nqn.1994-11.com.sandisk:nvme:A400M.2:171877424191", + "subsystem": "nvme", + "transport": "pcie", + "uevent": "MAJOR=238\nMINOR=0\nDEVNAME=nvme0\nNVME_TRTYPE=pcie" + } + } + }, "raid": { "/dev/md126": { "DEVLINKS": "/dev/disk/by-id/md-uuid-ac4bee3d:2607dd80:76f9390f:f2d72638 /dev/md/subvol", diff --git a/examples/machines/lp-1986676-missing-osprober.json b/examples/machines/lp-1986676-missing-osprober.json index 3ab42fd5..c602d4d6 100644 --- a/examples/machines/lp-1986676-missing-osprober.json +++ b/examples/machines/lp-1986676-missing-osprober.json @@ -2872,6 +2872,49 @@ } } }, + "nvme": { + "nvme0": { + "DEVNAME": "/dev/nvme0", + "DEVPATH": "/devices/pci0000:00/0000:00:1c.0/0000:02:00.0/nvme/nvme0/", + "MAJOR": "238", + "MINOR": "0", + "NVME_TRTYPE": "pcie", + "SUBSYSTEM": "nvme", + "attrs": { + "address": "0000:00:1c.0", + "cntlid": "5", + "cntrltype": "io", + "dctype": "none", + "dev": "238:0", + "device": null, + "firmware_rev": "S2681102", + "hmb": "1", + "kato": "0", + "model": "KINGSTON SKC2000M8250G", + "numa_node": "0", + "power/async": "disabled", + "power/autosuspend_delay_ms": null, + "power/control": "auto", + "power/pm_qos_latency_tolerance_us": "100000", + "power/runtime_active_kids": "0", + "power/runtime_active_time": "0", + "power/runtime_enabled": "disabled", + "power/runtime_status": "unsupported", + "power/runtime_suspended_time": "0", + "power/runtime_usage": "0", + "queue_count": "9", + "rescan_controller": null, + "reset_controller": null, + "serial": "50026B72823D1475", + "sqsize": "1023", + "state": "live", + "subsysnqn": "nqn.1994-11.com.kingston:nvme:SK2000M82560GM.2:50026B72823D1475", + "subsystem": "nvme", + "transport": "pcie", + "uevent": "MAJOR=238\nMINOR=0\nDEVNAME=nvme0\nNVME_TRTYPE=pcie" + } + } + }, "dmcrypt": {}, "dasd": {}, "zfs": { From c95716669cc2eb6be4e1b7e66b521c8ab9d7166b Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Fri, 12 Jan 2024 15:24:35 +0100 Subject: [PATCH 2/9] storage: expect optional NVMe controllers in the storage config Signed-off-by: Olivier Gayot --- snapcraft.yaml | 2 +- subiquity/models/filesystem.py | 10 ++++++++++ subiquity/ui/views/filesystem/disk_info.py | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/snapcraft.yaml b/snapcraft.yaml index c5ca2f22..ba6dc260 100644 --- a/snapcraft.yaml +++ b/snapcraft.yaml @@ -70,7 +70,7 @@ parts: source: https://git.launchpad.net/curtin source-type: git - source-commit: a349105809af6188cb394e300a99846df3d117f0 + source-commit: 237053d9d18916dd72cf861280474d4df0e9fd24 override-pull: | craftctl default diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 6e532edd..77ff8f17 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -704,12 +704,21 @@ class Dasd: preserve: bool = False +@fsobj("nvme_controller") +class NVMeController: + transport: str + tcp_port: Optional[int] = None + tcp_addr: Optional[str] = None + preserve: bool = False + + @fsobj("disk") class Disk(_Device): ptable: Optional[str] = attributes.ptable() serial: Optional[str] = None wwn: Optional[str] = None multipath: Optional[str] = None + nvme_controller: Optional[NVMeController] = attributes.ref(default=None) path: Optional[str] = None wipe: Optional[str] = None preserve: bool = False @@ -755,6 +764,7 @@ class Disk(_Device): "serial": self.serial or "unknown", "wwn": self.wwn or "unknown", "multipath": self.multipath or "unknown", + "nvme-controller": self.nvme_controller, "size": self.size, "humansize": humanize_size(self.size), "vendor": self._info.vendor or "unknown", diff --git a/subiquity/ui/views/filesystem/disk_info.py b/subiquity/ui/views/filesystem/disk_info.py index bf7af250..74bcbd35 100644 --- a/subiquity/ui/views/filesystem/disk_info.py +++ b/subiquity/ui/views/filesystem/disk_info.py @@ -37,6 +37,7 @@ labels_keys = [ ("Bus:", "bus"), ("Rotational:", "rotational"), ("Path:", "devpath"), + ("NVMe controller:", "nvme-controller"), ] From e83343c02cd53608b2934f918acdf0cfb3253d8b Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 16 Jan 2024 15:00:13 +0100 Subject: [PATCH 3/9] filesystem: LVM_VolGroup has a list of devices, not a set Signed-off-by: Olivier Gayot --- subiquity/models/tests/test_filesystem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index ea11ab38..cac31d0c 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -224,7 +224,7 @@ def make_vg(model, pvs=None): name = "vg%s" % len(model._actions) if pvs is None: - pvs = {make_disk(model)} + pvs = [make_disk(model)] return model.add_volgroup(name, pvs) From 802e9ef8db8d3f80da5d851e5cda2f7a0a32c2d3 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Fri, 12 Jan 2024 15:14:08 +0100 Subject: [PATCH 4/9] storage: add property to tell if FS objects are "on" remote storage Signed-off-by: Olivier Gayot --- subiquity/models/filesystem.py | 30 ++++++ subiquity/models/tests/test_filesystem.py | 114 ++++++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 77ff8f17..223a651f 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -693,6 +693,9 @@ class _Device(_Formattable, ABC): part.number = next_num next_num += 1 + def on_remote_storage(self) -> bool: + raise NotImplementedError + @fsobj("dasd") class Dasd: @@ -820,6 +823,11 @@ class Disk(_Device): return None return id.encode("utf-8").decode("unicode_escape").strip() + def on_remote_storage(self) -> bool: + if self.nvme_controller and self.nvme_controller.transport == "tcp": + return True + return False + @fsobj("partition") class Partition(_Formattable): @@ -933,6 +941,9 @@ class Partition(_Formattable): # check the partition number. return self.device.ptable == "msdos" and self.number > 4 + def on_remote_storage(self) -> bool: + return self.device.on_remote_storage() + @fsobj("raid") class Raid(_Device): @@ -1017,6 +1028,13 @@ class Raid(_Device): # What is a device that makes up this device referred to as? component_name = "component" + def on_remote_storage(self) -> bool: + for dev in self.devices: + if dev.on_remote_storage(): + return True + + return False + @fsobj("lvm_volgroup") class LVM_VolGroup(_Device): @@ -1042,6 +1060,12 @@ class LVM_VolGroup(_Device): # What is a device that makes up this device referred to as? component_name = "PV" + def on_remote_storage(self) -> bool: + for dev in self.devices: + if dev.on_remote_storage(): + return True + return False + @fsobj("lvm_partition") class LVM_LogicalVolume(_Formattable): @@ -1073,6 +1097,9 @@ class LVM_LogicalVolume(_Formattable): ok_for_raid = False ok_for_lvm_vg = False + def on_remote_storage(self) -> bool: + return self.volgroup.on_remote_storage() + LUKS_OVERHEAD = 16 * (2**20) @@ -1152,6 +1179,9 @@ class DM_Crypt: def size(self): return self.volume.size - LUKS_OVERHEAD + def on_remote_storage(self) -> bool: + return self.volume.on_remote_storage() + @fsobj("device") class ArbitraryDevice(_Device): diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index cac31d0c..97cf00fe 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -15,6 +15,7 @@ import pathlib import unittest +from typing import Optional from unittest import mock import attr @@ -31,6 +32,7 @@ from subiquity.models.filesystem import ( Filesystem, FilesystemModel, NotFinalPartitionError, + NVMeController, Partition, RecoveryKeyHandler, ZPool, @@ -263,6 +265,20 @@ def make_zfs(model, *, pool, **kw): return zfs +def make_nvme_controller( + model, + *, + transport: str, + tcp_addr: Optional[str] = None, + tcp_port: Optional[str] = None, +) -> NVMeController: + ctrler = NVMeController( + m=model, transport=transport, tcp_addr=tcp_addr, tcp_port=tcp_port + ) + model._actions.append(ctrler) + return ctrler + + class TestFilesystemModel(unittest.TestCase): def _test_ok_for_xxx(self, model, make_new_device, attr, test_partitions=True): # Newly formatted devs are ok_for_raid @@ -1617,3 +1633,101 @@ class TestRecoveryKeyHandler(SubiTestCase): ), expected, ) + + +class TestOnRemoteStorage(SubiTestCase): + def test_disk__on_local_storage(self): + m, d = make_model_and_disk(name="sda", serial="sata0") + self.assertFalse(d.on_remote_storage()) + + d = make_disk(name="nvme0n1", serial="pcie0") + self.assertFalse(d.on_remote_storage()) + + ctrler = make_nvme_controller(model=m, transport="pcie") + + d = make_disk( + fs_model=m, name="nvme1n1", nvme_controller=ctrler, serial="pcie1" + ) + self.assertFalse(d.on_remote_storage()) + + def test_disk__on_remote_storage(self): + m = make_model() + + ctrler = make_nvme_controller( + model=m, transport="tcp", tcp_addr="172.16.82.78", tcp_port=4420 + ) + + d = make_disk(fs_model=m, name="nvme0n1", nvme_controller=ctrler, serial="tcp0") + self.assertTrue(d.on_remote_storage()) + + def test_partition(self): + m, d = make_model_and_disk(name="sda", serial="sata0") + p = make_partition(model=m, device=d) + + # For partitions, this is directly dependent on the underlying device. + with mock.patch.object(d, "on_remote_storage", return_value=False): + self.assertFalse(p.on_remote_storage()) + with mock.patch.object(d, "on_remote_storage", return_value=True): + self.assertTrue(p.on_remote_storage()) + + def test_raid(self): + m, raid = make_model_and_raid() + + d0, d1 = list(raid.devices) + + d0_local = mock.patch.object(d0, "on_remote_storage", return_value=False) + d1_local = mock.patch.object(d1, "on_remote_storage", return_value=False) + d0_remote = mock.patch.object(d0, "on_remote_storage", return_value=True) + d1_remote = mock.patch.object(d1, "on_remote_storage", return_value=True) + + # If at least one of the underlying disk is on remote storage, the raid + # should be considered on remote storage too. + with d0_local, d1_local: + self.assertFalse(raid.on_remote_storage()) + with d0_local, d1_remote: + self.assertTrue(raid.on_remote_storage()) + with d0_remote, d1_local: + self.assertTrue(raid.on_remote_storage()) + with d0_remote, d1_remote: + self.assertTrue(raid.on_remote_storage()) + + def test_lvm_volgroup(self): + m, vg = make_model_and_vg() + + # make_vg creates a VG with a single PV (i.e., a disk). + d0 = vg.devices[0] + + with mock.patch.object(d0, "on_remote_storage", return_value=False): + self.assertFalse(vg.on_remote_storage()) + with mock.patch.object(d0, "on_remote_storage", return_value=True): + self.assertTrue(vg.on_remote_storage()) + + d1 = make_disk(fs_model=m) + + vg.devices.append(d1) + + d0_local = mock.patch.object(d0, "on_remote_storage", return_value=False) + d1_local = mock.patch.object(d1, "on_remote_storage", return_value=False) + d0_remote = mock.patch.object(d0, "on_remote_storage", return_value=True) + d1_remote = mock.patch.object(d1, "on_remote_storage", return_value=True) + + # Just like RAIDs, if at least one of the underlying PV is on remote + # storage, the VG should be considered on remote storage too. + with d0_local, d1_local: + self.assertFalse(vg.on_remote_storage()) + with d0_local, d1_remote: + self.assertTrue(vg.on_remote_storage()) + with d0_remote, d1_local: + self.assertTrue(vg.on_remote_storage()) + with d0_remote, d1_remote: + self.assertTrue(vg.on_remote_storage()) + + def test_lvm_logical_volume(self): + m, lv = make_model_and_lv() + + vg = lv.volgroup + # For LVs, this is directly dependent on the underlying VG. + with mock.patch.object(vg, "on_remote_storage", return_value=False): + self.assertFalse(lv.on_remote_storage()) + with mock.patch.object(vg, "on_remote_storage", return_value=True): + self.assertTrue(lv.on_remote_storage()) From c3de13d10c0b15e8972dfee124f701ad2f5c724e Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Wed, 10 Jan 2024 00:05:31 +0100 Subject: [PATCH 5/9] filesystem: only allow /home on remote storage Signed-off-by: Olivier Gayot --- subiquity/common/filesystem/boot.py | 4 ++ subiquity/common/filesystem/labels.py | 9 ++++- subiquity/models/filesystem.py | 4 ++ .../controllers/tests/test_filesystem.py | 18 +++++++++ subiquity/ui/mount.py | 11 ++++++ subiquity/ui/views/filesystem/partition.py | 39 ++++++++++++++++++- 6 files changed, 82 insertions(+), 3 deletions(-) diff --git a/subiquity/common/filesystem/boot.py b/subiquity/common/filesystem/boot.py index 77098114..d86e03db 100644 --- a/subiquity/common/filesystem/boot.py +++ b/subiquity/common/filesystem/boot.py @@ -336,6 +336,8 @@ def can_be_boot_device(device, *, resize_partition=None, with_reformatting=False @can_be_boot_device.register(Disk) def _can_be_boot_device_disk(disk, *, resize_partition=None, with_reformatting=False): + if disk.on_remote_storage(): + return False if with_reformatting: disk = disk._reformatted() plan = get_boot_device_plan(disk, resize_partition=resize_partition) @@ -344,6 +346,8 @@ def _can_be_boot_device_disk(disk, *, resize_partition=None, with_reformatting=F @can_be_boot_device.register(Raid) def _can_be_boot_device_raid(raid, *, resize_partition=None, with_reformatting=False): + if raid.on_remote_storage(): + return False bl = raid._m.bootloader if bl != Bootloader.UEFI: return False diff --git a/subiquity/common/filesystem/labels.py b/subiquity/common/filesystem/labels.py index 0ff29a2c..301f49e4 100644 --- a/subiquity/common/filesystem/labels.py +++ b/subiquity/common/filesystem/labels.py @@ -117,6 +117,13 @@ def desc(device): def _desc_disk(disk): if disk.multipath: return _("multipath device") + if disk.on_remote_storage(): + if disk.nvme_controller is not None and disk.nvme_controller.transport == "tcp": + return _("NVMe/TCP drive") + # At time of writing, only NVMe/TCP drives will report as "remote". + # Let's set a default label for potential transports that we may + # support in the future. + return _("remote drive") return _("local disk") @@ -318,7 +325,7 @@ def _for_client_disk(disk, *, min_size=0): partitions=[for_client(p) for p in gaps.parts_and_gaps(disk)], boot_device=boot.is_boot_device(disk), can_be_boot_device=boot.can_be_boot_device(disk), - ok_for_guided=disk.size >= min_size, + ok_for_guided=disk.size >= min_size and not disk.on_remote_storage(), model=getattr(disk, "model", None), vendor=getattr(disk, "vendor", None), has_in_use_partition=disk._has_in_use_partition, diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 223a651f..3aad81e8 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -803,6 +803,8 @@ class Disk(_Device): return False if len(self._partitions) > 0: return False + if self.on_remote_storage(): + return False return True @property @@ -913,6 +915,8 @@ class Partition(_Formattable): return False if self._constructed_device is not None: return False + if self.on_remote_storage(): + return False return True @property diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index 4321601d..8b0e3195 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -47,6 +47,7 @@ from subiquity.models.tests.test_filesystem import ( FakeStorageInfo, make_disk, make_model, + make_nvme_controller, make_partition, ) from subiquity.server import snapdapi @@ -1187,11 +1188,17 @@ class TestManualBoot(IsolatedAsyncioTestCase): @parameterized.expand(bootloaders_and_ptables) async def test_get_boot_disks_some(self, bootloader, ptable): self._setup(bootloader, ptable) + ctrler = make_nvme_controller( + model=self.model, transport="tcp", tcp_addr="172.16.82.78", tcp_port=4420 + ) + d1 = make_disk(self.model) d2 = make_disk(self.model) + make_disk(self.model, nvme_controller=ctrler) make_partition(self.model, d1, size=gaps.largest_gap_size(d1), preserve=True) if bootloader == Bootloader.NONE: # NONE will always pass the boot check, even on a full disk + # .. well unless if it is a "remote" disk. bootable = set([d1.id, d2.id]) else: bootable = set([d2.id]) @@ -1199,6 +1206,17 @@ class TestManualBoot(IsolatedAsyncioTestCase): for d in resp.disks: self.assertEqual(d.id in bootable, d.can_be_boot_device) + @parameterized.expand(bootloaders_and_ptables) + async def test_get_boot_disks_no_remote(self, bootloader, ptable): + self._setup(bootloader, ptable) + d = make_disk(self.model) + with mock.patch.object(d, "on_remote_storage", return_value=False): + resp = await self.fsc.v2_GET() + self.assertTrue(resp.disks[0].can_be_boot_device) + with mock.patch.object(d, "on_remote_storage", return_value=True): + resp = await self.fsc.v2_GET() + self.assertFalse(resp.disks[0].can_be_boot_device) + class TestCoreBootInstallMethods(IsolatedAsyncioTestCase): def setUp(self): diff --git a/subiquity/ui/mount.py b/subiquity/ui/mount.py index 92af83c2..649dd68e 100644 --- a/subiquity/ui/mount.py +++ b/subiquity/ui/mount.py @@ -79,6 +79,17 @@ class MountSelector(WidgetWrap): if isinstance(opt.value, str): opt.enabled = opt.value in common_mountpoints + def disable_all_mountpoints_but_home(self): + """Currently, we only want filesystems of non-local disks mounted at + /home. This is not completely enforced by the UI though. Users can + still select "other" and type "/", "/boot", "/var" or anything else.""" + for opt in self._selector._options: + if not isinstance(opt.value, str): + continue + if opt.value == "/home": + continue + opt.enabled = False + def _showhide_other(self, show): if show and not self._other_showing: self._w.contents.append( diff --git a/subiquity/ui/views/filesystem/partition.py b/subiquity/ui/views/filesystem/partition.py index eb0743b6..1f14c506 100644 --- a/subiquity/ui/views/filesystem/partition.py +++ b/subiquity/ui/views/filesystem/partition.py @@ -161,15 +161,27 @@ LVNameField = simple_field(LVNameEditor) class PartitionForm(Form): - def __init__(self, model, max_size, initial, lvm_names, device, alignment): + def __init__( + self, + model, + max_size, + initial, + lvm_names, + device, + alignment, + remote_storage: bool, + ): self.model = model self.device = device self.existing_fs_type = None + self.remote_storage: bool = remote_storage if device: ofstype = device.original_fstype() if ofstype: self.existing_fs_type = ofstype initial_path = initial.get("mount") + if not initial_path and remote_storage: + initial["mount"] = "/home" self.mountpoints = { m.path: m.device.volume for m in self.model.all_mounts() @@ -196,6 +208,9 @@ class PartitionForm(Form): else: self.mount.widget.enable_common_mountpoints() self.mount.value = self.mount.value + if self.remote_storage: + self.mount.widget.disable_all_mountpoints_but_home() + self.mount.value = self.mount.value if fstype is None: if self.existing_fs_type == "swap": show_use = True @@ -297,6 +312,17 @@ class PartitionForm(Form): ).format(mountpoint=mount), ) ) + if self.remote_storage and mount != "/home": + self.mount.show_extra( + ( + "info_error", + _( + "This filesystem is on remote storage. It is usually a " + "bad idea to mount it anywhere but at /home, proceed " + "only with caution." + ), + ) + ) def as_rows(self): r = super().as_rows() @@ -450,6 +476,7 @@ class PartitionStretchy(Stretchy): if isinstance(disk, LVM_VolGroup): initial["name"] = partition.name lvm_names.remove(partition.name) + remote_storage = partition.on_remote_storage() else: initial["fstype"] = "ext4" max_size = self.gap.size @@ -461,9 +488,16 @@ class PartitionStretchy(Stretchy): break x += 1 initial["name"] = name + remote_storage = disk.on_remote_storage() self.form = PartitionForm( - self.model, max_size, initial, lvm_names, partition, alignment + self.model, + max_size, + initial, + lvm_names, + partition, + alignment, + remote_storage, ) if not isinstance(disk, LVM_VolGroup): @@ -626,6 +660,7 @@ class FormatEntireStretchy(Stretchy): None, device, alignment=device.alignment_data().part_align, + remote_storage=device.on_remote_storage(), ) self.form.remove_field("size") self.form.remove_field("name") From 376131b04d2f8854bf25ea601c3b070cf1e7c023 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Wed, 10 Jan 2024 15:36:32 +0100 Subject: [PATCH 6/9] filesystem: pass _netdev option for mounts on remote storage Signed-off-by: Olivier Gayot --- subiquity/common/filesystem/manipulator.py | 10 +++++++++- subiquity/models/filesystem.py | 7 +++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/subiquity/common/filesystem/manipulator.py b/subiquity/common/filesystem/manipulator.py index b5b4ec26..a55b6915 100644 --- a/subiquity/common/filesystem/manipulator.py +++ b/subiquity/common/filesystem/manipulator.py @@ -42,7 +42,9 @@ class FilesystemManipulator: def create_mount(self, fs, spec): if spec.get("mount") is None: return - mount = self.model.add_mount(fs, spec["mount"]) + mount = self.model.add_mount( + fs, spec["mount"], on_remote_storage=spec.get("on-remote-storage", False) + ) if self.model.needs_bootloader_partition(): vol = fs.volume if vol.type == "partition" and boot.can_be_boot_device(vol.device): @@ -240,6 +242,9 @@ class FilesystemManipulator: def partition_disk_handler(self, disk, spec, *, partition=None, gap=None): log.debug("partition_disk_handler: %s %s %s %s", disk, spec, partition, gap) + if disk.on_remote_storage(): + spec["on-remote-storage"] = True + if partition is not None: if "size" in spec and spec["size"] != partition.size: trailing, gap_size = gaps.movable_trailing_partitions_and_gap_size( @@ -294,6 +299,9 @@ class FilesystemManipulator: log.debug("logical_volume_handler: %s %s %s", vg, lv, spec) + if vg.on_remote_storage(): + spec["on-remote-storage"] = True + if lv is not None: if "name" in spec: lv.name = spec["name"] diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 3aad81e8..ffc1f7f4 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -2166,10 +2166,13 @@ class FilesystemModel: raise Exception("can only remove unmounted filesystem") self._remove(fs) - def add_mount(self, fs, path): + def add_mount(self, fs, path, *, on_remote_storage=False): if fs._mount is not None: raise Exception(f"{fs} is already mounted") - m = Mount(m=self, device=fs, path=path) + options = None + if on_remote_storage: + options = "defaults,_netdev" + m = Mount(m=self, device=fs, path=path, options=options) self._actions.append(m) return m From 76f04697058e1b0b969c835f987d3ba31d4e98ca Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Fri, 19 Jan 2024 17:00:09 +0100 Subject: [PATCH 7/9] filesystem: fix run-answers for deleting a partition When trying to delete a partition using the answers-based mechanism, subiquity tries to call .done() on the ConfirmDeletesStretchy overlay. However, this method does not exist. The .confirm() method is what we should use instead. Signed-off-by: Olivier Gayot --- subiquity/client/controllers/filesystem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subiquity/client/controllers/filesystem.py b/subiquity/client/controllers/filesystem.py index 0f408793..3a5b463d 100644 --- a/subiquity/client/controllers/filesystem.py +++ b/subiquity/client/controllers/filesystem.py @@ -216,7 +216,7 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator): return if isinstance(body.stretchy, ConfirmDeleteStretchy): if action.get("submit", True): - body.stretchy.done() + body.stretchy.confirm() else: async for _ in self._enter_form_data( body.stretchy.form, action["data"], action.get("submit", True) From 88b6dcb6c69778dd502492f6ffdc26bb71746dd8 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Fri, 19 Jan 2024 17:02:13 +0100 Subject: [PATCH 8/9] filesystem: add support for reformatting in run-answers Signed-off-by: Olivier Gayot --- subiquity/client/controllers/filesystem.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/subiquity/client/controllers/filesystem.py b/subiquity/client/controllers/filesystem.py index 3a5b463d..b17d5667 100644 --- a/subiquity/client/controllers/filesystem.py +++ b/subiquity/client/controllers/filesystem.py @@ -193,7 +193,10 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator): return raidlevels_by_value[level] async def _answers_action(self, action): - from subiquity.ui.views.filesystem.delete import ConfirmDeleteStretchy + from subiquity.ui.views.filesystem.delete import ( + ConfirmDeleteStretchy, + ConfirmReformatStretchy, + ) from subiquitycore.ui.stretchy import StretchyOverlay log.debug("_answers_action %r", action) @@ -214,7 +217,9 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator): body = self.ui.body._w if not isinstance(body, StretchyOverlay): return - if isinstance(body.stretchy, ConfirmDeleteStretchy): + if isinstance( + body.stretchy, (ConfirmDeleteStretchy, ConfirmReformatStretchy) + ): if action.get("submit", True): body.stretchy.confirm() else: From c1105dffee6e319ca66e14cd1fb05fb317ed8e72 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Fri, 19 Jan 2024 17:04:56 +0100 Subject: [PATCH 9/9] filesystem: add integration test using nvme-o-tcp Signed-off-by: Olivier Gayot --- examples/answers/nvme-over-tcp.yaml | 58 +++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 examples/answers/nvme-over-tcp.yaml diff --git a/examples/answers/nvme-over-tcp.yaml b/examples/answers/nvme-over-tcp.yaml new file mode 100644 index 00000000..cb2bba74 --- /dev/null +++ b/examples/answers/nvme-over-tcp.yaml @@ -0,0 +1,58 @@ +#machine-config: examples/machines/nvme-over-tcp.json +Source: + source: ubuntu-server +Welcome: + lang: en_US +Refresh: + update: no +Keyboard: + layout: us +Network: + accept-default: yes +Proxy: + proxy: "" +Mirror: + mirror: "http://fr.archive.ubuntu.com" +Filesystem: + # 1. Reformat both disks + # 2. Create the root filesystem on disk 0 (which is a local disk) + # 3. Create the /home filesystem on disk 1 (which is a remote disk) + manual: + - obj: [disk index 0] + action: REFORMAT + - obj: [disk index 1] + action: REFORMAT + - obj: [disk index 0] + action: PARTITION + data: + fstype: ext4 + mount: / + - obj: [disk index 1] + action: PARTITION + data: + fstype: ext4 + mount: /home + - action: done +Identity: + realname: Ubuntu + username: ubuntu + hostname: ubuntu-server + # ubuntu + password: '$6$wdAcoXrU039hKYPd$508Qvbe7ObUnxoj15DRCkzC3qO7edjH0VV7BPNRDYK4QR8ofJaEEF2heacn0QgD.f8pO8SNp83XNdWG6tocBM1' +UbuntuPro: + token: "" +SSH: + install_server: true + pwauth: false + authorized_keys: + - | + ssh-rsa AAAAAAAAAAAAAAAAAAAAAAAAA # ssh-import-id lp:subiquity +SnapList: + snaps: + hello: + channel: stable + classic: false +InstallProgress: + reboot: yes +Drivers: + install: no