From ee8e25818a0dbe284cad10ff79a05b4e0ee51a66 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Fri, 2 Oct 2015 13:49:10 -0500 Subject: [PATCH] Don't allow multiple devices to be bootable - Update filesystem model to check if we have an existing device that's marked bootable. - Update look at all disk mount points, not just the current one Signed-off-by: Ryan Harper --- subiquity/controllers/filesystem.py | 9 ++++-- subiquity/models/blockdev.py | 15 ++------- subiquity/models/filesystem.py | 47 +++++++++++++++++++++++------ subiquity/ui/views/filesystem.py | 11 ++++--- subiquity/ui/views/raid.py | 3 +- 5 files changed, 54 insertions(+), 31 deletions(-) diff --git a/subiquity/controllers/filesystem.py b/subiquity/controllers/filesystem.py index 3717679c..de9c2700 100644 --- a/subiquity/controllers/filesystem.py +++ b/subiquity/controllers/filesystem.py @@ -102,8 +102,13 @@ class FilesystemController(ControllerPolicy): log.debug('disk.freespace: {}'.format(current_disk.freespace)) try: - ''' create a gpt boot partition if one doesn't exist ''' - if current_disk.parttype == 'gpt' and \ + ''' create a gpt boot partition if one doesn't exist, only + one one disk''' + + system_bootable = self.model.bootable() + log.debug('model has bootable device? {}'.format(system_bootable)) + if system_bootable is False and \ + current_disk.parttype == 'gpt' and \ len(current_disk.disk.partitions) == 0: if self.is_uefi(): log.debug('Adding EFI partition first') diff --git a/subiquity/models/blockdev.py b/subiquity/models/blockdev.py index 7dca07cf..a7ea6386 100644 --- a/subiquity/models/blockdev.py +++ b/subiquity/models/blockdev.py @@ -171,7 +171,7 @@ class Blockdev(): @property def percent_free(self): ''' return the device free percentage of the whole device''' - percent = ( int((1.0 - (self.usedspace / self.size)) * 100)) + percent = (int((1.0 - (self.usedspace / self.size)) * 100)) return percent @property @@ -194,18 +194,12 @@ class Blockdev(): space += int(action.offset) space += int(action.size) - log.debug('{} usedspace: {}'.format(self.disk.devpath, space)) return space @property def freespace(self, unit='B'): ''' return amount of free space ''' - used = self.usedspace - size = self.size - log.debug('{} freespace: {} - {} = {}'.format(self.disk.devpath, - size, used, - size - used)) - return size - used + return self.size - self.usedspace @property def lastpartnumber(self): @@ -289,19 +283,14 @@ class Blockdev(): # dict to uniq the list of devices mounted mounted_devs = {} for mnt in re.findall('/dev/.*', mounts): - log.debug('mnt={}'.format(mnt)) (devpath, mount, *_) = mnt.split() # resolve any symlinks mounted_devs.update( {os.path.realpath(devpath): mount}) - log.debug('mounted_devs: {}'.format(mounted_devs)) matches = [dev for dev in mounted_devs.keys() if dev.startswith(self.disk.devpath)] - log.debug('Checking if {} is in {}'.format( - self.disk.devpath, matches)) if len(matches) > 0: - log.debug('Device is mounted: {}'.format(matches)) return True return False diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 8eca33c6..7d2310d4 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -25,10 +25,11 @@ HUMAN_UNITS = ['B', 'K', 'M', 'G', 'T', 'P'] log = logging.getLogger('subiquity.models.filesystem') -class AttrDict(dict): +class AttrDict(dict): __getattr__ = dict.__getitem__ __setattr__ = dict.__setitem__ + class FilesystemModel(ModelPolicy): """ Model representing storage options """ @@ -151,18 +152,27 @@ class FilesystemModel(ModelPolicy): self.info[disk] = self.prober.get_storage_info(disk) def get_disk(self, disk): - log.debug('probe_storage: get_disk()') + log.debug('probe_storage: get_disk({})'.format(disk)) if disk not in self.devices: self.devices[disk] = Blockdev(disk, self.info[disk].serial, self.info[disk].model, size=self.info[disk].size) return self.devices[disk] - def get_disks(self): + def get_available_disks(self): + ''' currently only returns available disks ''' + disks = [d for d in self.get_all_disks() if d.available] + log.debug('get_available_disks -> {}'.format( + ",".join([d.devpath for d in disks]))) + return disks + + def get_all_disks(self): possible_devices = list(set(list(self.devices.keys()) + list(self.info.keys()))) possible_disks = [self.get_disk(d) for d in sorted(possible_devices)] - return [d for d in possible_disks if d.available] + log.debug('get_all_disks -> {}'.format(",".join([d.devpath for d in + possible_disks]))) + return possible_disks def calculate_raid_size(self, raid_level, raid_devices, spare_devices): ''' @@ -290,20 +300,39 @@ class FilesystemModel(ModelPolicy): ''' one or more disks has used space and has "/" as a mount ''' - for disk in self.get_disks(): + for disk in self.get_all_disks(): if disk.usedspace > 0 and "/" in disk.mounts: return True - def get_available_disks(self): - return [dev.disk.devpath for dev in self.get_disks()] + def bootable(self): + ''' true if one disk has a boot partition ''' + log.debug('bootable check') + for disk in self.get_all_disks(): + for (num, action) in disk.partitions.items(): + if action.flags in ['bios_grub']: + log.debug('bootable check: we\'ve got boot!') + return True - def get_used_disks(self): - return [dev.disk.devpath for dev in self.devices.values() + log.debug('bootable check: no disks have been marked bootable') + return False + + def get_available_disk_names(self): + return [dev.disk.devpath for dev in self.get_available_disks()] + + def get_used_disk_names(self): + return [dev.disk.devpath for dev in self.get_all_disks() if dev.available is False] def get_disk_info(self, disk): return self.info.get(disk, {}) + def get_mounts(self): + mounts = [] + for dev in self.get_all_disks(): + mounts += dev.mounts + + return mounts + def get_disk_action(self, disk): return self.devices[disk].get_actions() diff --git a/subiquity/ui/views/filesystem.py b/subiquity/ui/views/filesystem.py index 7b84c5a2..f681155b 100644 --- a/subiquity/ui/views/filesystem.py +++ b/subiquity/ui/views/filesystem.py @@ -241,9 +241,10 @@ class AddPartitionView(WidgetWrap): self.selected_disk) return # Validate mountpoint input - if self.mountpoint.value in self.selected_disk.mounts: + all_mounts = self.model.get_mounts() + if self.mountpoint.value in all_mounts: log.error('provided mountpoint already allocated' - ' ({}'.format(self.mountpoint.value)) + ' ({})'.format(self.mountpoint.value)) # FIXME: update the error message widget instead self.mountpoint.set_error('ERROR: already mounted') self.signal.emit_signal( @@ -398,7 +399,7 @@ class FilesystemView(ViewPolicy): def _build_used_disks(self): log.debug('FileSystemView: building used disks') pl = [] - for disk in self.model.get_used_disks(): + for disk in self.model.get_used_disk_names(): log.debug('used disk: {}'.format(disk)) pl.append(Text(disk)) if len(pl): @@ -440,7 +441,7 @@ class FilesystemView(ViewPolicy): buttons = [] # don't enable done botton if we can't install - if self.model.installable: + if self.model.installable(): buttons.append( Color.button(done_btn(on_press=self.done), focus_map='button focus')) @@ -464,7 +465,7 @@ class FilesystemView(ViewPolicy): col_1 = [] col_2 = [] - avail_disks = self.model.get_available_disks() + avail_disks = self.model.get_available_disk_names() if len(avail_disks) == 0: return Pile([Color.info_minor(Text("No available disks."))]) diff --git a/subiquity/ui/views/raid.py b/subiquity/ui/views/raid.py index 711dc5bb..9d42e38e 100644 --- a/subiquity/ui/views/raid.py +++ b/subiquity/ui/views/raid.py @@ -47,9 +47,8 @@ class RaidView(ViewPolicy): items = [ Text("DISK SELECTION") ] - avail_disks = self.model.get_available_disks() + avail_disks = self.model.get_available_disk_names() if len(avail_disks) == 0: - self.installable = False return items.append( [Color.info_minor(Text("No available disks."))])