From 620d7b1973c0dba9330f5dff8fd84dce1d0d782a Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Tue, 28 May 2019 22:35:15 +1200 Subject: [PATCH] call extract_storage_config with probert config to create actions --- subiquity/controllers/filesystem.py | 8 +- subiquity/models/filesystem.py | 119 ++++++++++-------- .../views/filesystem/tests/test_filesystem.py | 15 +-- 3 files changed, 74 insertions(+), 68 deletions(-) diff --git a/subiquity/controllers/filesystem.py b/subiquity/controllers/filesystem.py index 03808370..6aa780b6 100644 --- a/subiquity/controllers/filesystem.py +++ b/subiquity/controllers/filesystem.py @@ -16,8 +16,6 @@ import enum import logging -from probert.storage import StorageInfo - from subiquitycore.controller import BaseController from subiquity.models.filesystem import ( @@ -75,11 +73,7 @@ class FilesystemController(BaseController): 5.0, lambda loop, ud: self._check_probe_timeout()) def _bg_probe(self, probe_types=None): - probed_data = self.prober.get_storage(probe_types=probe_types) - storage = {} - for path, data in probed_data["blockdev"].items(): - storage[path] = StorageInfo({path: data}) - return storage + return self.prober.get_storage(probe_types=probe_types) def _probed(self, fut, restricted=False): if not restricted and self._probe_state != ProbeState.PROBING: diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index e7af440d..b93335e0 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -17,15 +17,17 @@ from abc import ABC, abstractmethod import attr import collections import enum -import glob +import itertools import logging import math import os import pathlib import platform -import sys from curtin.util import human2bytes +from curtin import storage_config + +from probert.storage import StorageInfo log = logging.getLogger('subiquity.models.filesystem') @@ -70,6 +72,9 @@ def _remove_backlinks(obj): setattr(vv, backlink, None) +_type_to_cls = {} + + def fsobj(typ): def wrapper(c): c.__attrs_post_init__ = _set_backlinks @@ -77,6 +82,7 @@ def fsobj(typ): c.id = attributes.idfield(typ) c._m = attr.ib(repr=None, default=None) c = attr.s(cmp=False)(c) + _type_to_cls[typ] = c return c return wrapper @@ -554,14 +560,6 @@ class Disk(_Device): _info = attr.ib(default=None) - @classmethod - def from_info(self, model, info): - d = Disk(m=model, info=info) - d.serial = info.serial - d.path = info.name - d.model = info.model - return d - def info_for_display(self): bus = self._info.raw.get('ID_BUS', None) major = self._info.raw.get('MAJOR', None) @@ -1003,14 +1001,67 @@ class FilesystemModel(object): def __init__(self): self.bootloader = self._probe_bootloader() - self._disk_info = [] + self._probe_data = None self.reset() def reset(self): - self._actions = [ - Disk.from_info(self, info) for info in self._disk_info] + if self._probe_data is not None: + config = storage_config.extract_storage_config(self._probe_data) + self._actions = self._actions_from_config( + config["storage"]["config"], + self._probe_data['blockdev']) + else: + self._actions = [] self.grub_install_device = None + def _actions_from_config(self, config, blockdevs): + byid = {} + objs = [] + exclusions = set() + for action in config: + if action['type'] == 'mount': + exclusions.add(byid[action['device']]) + continue + c = _type_to_cls.get(action['type'], None) + if c is None: + # Ignore any action we do not know how to process yet + # (e.g. bcache) + continue + kw = {} + for f in attr.fields(c): + n = f.name + if n not in action: + continue + v = action[n] + if f.metadata.get('ref', False): + kw[n] = byid[v] + elif f.metadata.get('reflist', False): + kw[n] = [byid[id] for id in v] + else: + kw[n] = v + if kw['type'] == 'disk': + path = kw['path'] + kw['info'] = StorageInfo({path: blockdevs[path]}) + kw['preserve'] = True + obj = byid[action['id']] = c(m=self, **kw) + objs.append(obj) + + # We filter out anything that can be reached from a currently + # mounted device. The motivation here is only to exclude the + # media subiquity is mounted from, so this might be a bit + # excessive but hey it works. + while True: + log.debug("exclusions %s", {e.id for e in exclusions}) + next_exclusions = exclusions.copy() + for e in exclusions: + next_exclusions.update(itertools.chain( + dependencies(e), reverse_dependencies(e))) + if len(exclusions) == len(next_exclusions): + break + exclusions = next_exclusions + + return [o for o in objs if o not in exclusions] + def _render_actions(self): # The curtin storage config has the constraint that an action must be # preceded by all the things that it depends on. We handle this by @@ -1086,45 +1137,9 @@ class FilesystemModel(object): } return config - def _get_system_mounted_disks(self): - # This assumes a fairly vanilla setup. It won't list as - # mounted a disk that is only mounted via lvm, for example. - mounted_devs = [] - with open('/proc/mounts', encoding=sys.getfilesystemencoding()) as pm: - for line in pm: - if line.startswith('/dev/'): - mounted_devs.append(line.split()[0][5:]) - mounted_disks = set() - for dev in mounted_devs: - if os.path.exists('/sys/block/{}'.format(dev)): - mounted_disks.add('/dev/' + dev) - else: - paths = glob.glob('/sys/block/*/{}/partition'.format(dev)) - if len(paths) == 1: - mounted_disks.add('/dev/' + paths[0].split('/')[3]) - return mounted_disks - - def load_probe_data(self, storage): - currently_mounted = self._get_system_mounted_disks() - for path, info in storage.items(): - log.debug("fs probe %s", path) - if path in currently_mounted: - continue - if info.type == 'disk': - if info.is_virtual: - continue - if info.raw["MAJOR"] in ("2", "11"): # serial and cd devices - continue - if info.raw['attrs'].get('ro') == "1": - continue - if "ID_CDROM" in info.raw: - continue - # log.debug('disk={}\n{}'.format( - # path, json.dumps(data, indent=4, sort_keys=True))) - if info.size < self.lower_size_limit: - continue - self._disk_info.append(info) - self._actions.append(Disk.from_info(self, info)) + def load_probe_data(self, probe_data): + self._probe_data = probe_data + self.reset() def disk_by_path(self, path): for a in self._actions: diff --git a/subiquity/ui/views/filesystem/tests/test_filesystem.py b/subiquity/ui/views/filesystem/tests/test_filesystem.py index b97b09dc..c931529c 100644 --- a/subiquity/ui/views/filesystem/tests/test_filesystem.py +++ b/subiquity/ui/views/filesystem/tests/test_filesystem.py @@ -1,6 +1,5 @@ import unittest from unittest import mock -from collections import namedtuple import urwid @@ -12,14 +11,12 @@ from subiquity.models.filesystem import ( Disk, FilesystemModel, ) +from subiquity.models.tests.test_filesystem import ( + FakeStorageInfo, + ) from subiquity.ui.views.filesystem.filesystem import FilesystemView -FakeStorageInfo = namedtuple( - 'FakeStorageInfo', ['name', 'size', 'free', 'serial', 'model']) -FakeStorageInfo.__new__.__defaults__ = (None,) * len(FakeStorageInfo._fields) - - class FilesystemViewTests(unittest.TestCase): def make_view(self, model, devices=[]): @@ -35,9 +32,9 @@ class FilesystemViewTests(unittest.TestCase): def test_one_disk(self): model = mock.create_autospec(spec=FilesystemModel) - disk = Disk.from_info(model, FakeStorageInfo( - name='disk-name', size=100*(2**20), free=50*(2**20), - serial="DISK-SERIAL")) + disk = Disk( + m=model, serial="DISK-SERIAL", + info=FakeStorageInfo(size=100*(2**20), free=50*(2**20))) view = self.make_view(model, [disk]) w = view_helpers.find_with_pred( view,