Merge branch 'master' into mwhudson/guided-partitioning

Fixing a few conflicts.

snapcraft.yaml

@@ -23,9 +23,11 @@ parts:
       - pyudev
       - attrs
     source: .
+    source-type: git
   wrappers:
     plugin: dump
     source: .
+    source-type: git
     organize:
       'bin/console-conf-tui': usr/bin/console-conf
       'bin/subiquity-tui': usr/bin/subiquity

subiquity/controllers/filesystem.py

@@ -27,15 +27,15 @@ from subiquity.curtin import (
 from subiquity.models import (FilesystemModel, RaidModel)
 from subiquity.models.filesystem import humanize_size
 from subiquity.ui.views import (
-    AddFormatView,
-    AddPartitionView,
     BcacheView,
     DiskInfoView,
     DiskPartitionView,
     FilesystemView,
+    FormatEntireView,
     GuidedDiskSelectionView,
     GuidedFilesystemView,
     LVMVolumeGroupView,
+    PartitionView,
     RaidView,
     )
 
@@ -150,13 +150,51 @@ class FilesystemController(BaseController):
         log.debug("Adding partition to {}".format(disk))
         footer = ("Select whole disk, or partition, to format and mount.")
         self.ui.set_footer(footer)
-        adp_view = AddPartitionView(self.model, self, disk)
+        adp_view = PartitionView(self.model, self, disk)
         self.ui.set_body(adp_view)
 
-    def do_add_disk_partition(self, disk, spec):
+    def edit_partition(self, disk, partition):
+        log.debug("Editing partition {}".format(partition))
+        footer = ("Edit partition details format and mount.")
+        self.ui.set_footer(footer)
+        adp_view = PartitionView(self.model, self, disk, partition)
+        self.ui.set_body(adp_view)
+
+    def delete_partition(self, part):
+        old_fs = part.fs()
+        if old_fs is not None:
+            self.model._filesystems.remove(old_fs)
+            part._fs = None
+            mount = old_fs.mount()
+            if mount is not None:
+                old_fs._mount = None
+                self.model._mounts.remove(mount)
+        part.device.partitions().remove(part)
+        self.model._partitions.remove(part)
+        self.partition_disk(part.device)
+
+    def partition_disk_handler(self, disk, partition, spec):
         log.debug('spec: {}'.format(spec))
         log.debug('disk.freespace: {}'.format(disk.free))
 
+        if partition is not None:
+            partition.number = spec['partnum']
+            partition.size = spec['size']
+            old_fs = partition.fs()
+            if old_fs is not None:
+                self.model._filesystems.remove(old_fs)
+                partition._fs = None
+                mount = old_fs.mount()
+                if mount is not None:
+                    old_fs._mount = None
+                    self.model._mounts.remove(mount)
+            if spec['fstype'].label is not None:
+                fs = self.model.add_filesystem(partition, spec['fstype'].label)
+                if spec['mount']:
+                  self.model.add_mount(fs, spec['mount'])
+            self.partition_disk(disk)
+            return
+
         system_bootable = self.model.bootable()
         log.debug('model has bootable device? {}'.format(system_bootable))
         if not system_bootable and len(disk.partitions()) == 0:
@@ -174,33 +212,34 @@ class FilesystemController(BaseController):
             # the offset and bios/grub partition
             # XXX should probably only do this if the partition is now too big to fit on the disk?
             log.debug("Adjusting request down:" +
-                      "{} - {} = {}".format(spec['bytes'], part.size,
+                      "{} - {} = {}".format(spec['size'], part.size,
-                                            spec['bytes'] - part.size))
+                                            spec['size'] - part.size))
-            spec['bytes'] -= part.size
+            spec['size'] -= part.size
             spec['partnum'] = 2
 
-        part = self.model.add_partition(disk=disk, partnum=spec["partnum"], size=spec["bytes"])
+        part = self.model.add_partition(disk=disk, partnum=spec["partnum"], size=spec["size"])
         if spec['fstype'] is not None:
-            fs = self.model.add_filesystem(part, spec['fstype'])
+            fs = self.model.add_filesystem(part, spec['fstype'].label)
-            if spec['mountpoint']:
+            if spec['mount']:
-                self.model.add_mount(fs, spec['mountpoint'])
+                self.model.add_mount(fs, spec['mount'])
 
         log.info("Successfully added partition")
-
-    def add_disk_partition_handler(self, disk, spec):
-        self.do_add_disk_partition(disk, spec)
         self.partition_disk(disk)
 
     def add_format_handler(self, volume, spec, back):
         log.debug('add_format_handler')
-        if spec['fstype'] is not None:
+        old_fs = volume.fs()
-            fs = self.model.add_filesystem(volume, spec['fstype'])
+        if old_fs is not None:
-        else:
+            self.model._filesystems.remove(old_fs)
-            fs = volume.fs()
+            volume._fs = None
-        if spec['mountpoint']:
+            mount = old_fs.mount()
-            if fs is None:
+            if mount is not None:
-                raise Exception("{} is not formatted".format(volume.path))
+                old_fs._mount = None
-            self.model.add_mount(fs, spec['mountpoint'])
+                self.model._mounts.remove(mount)
+        if spec['fstype'].label is not None:
+            fs = self.model.add_filesystem(volume, spec['fstype'].label)
+            if spec['mount']:
+                self.model.add_mount(fs, spec['mount'])
         back()
 
     def connect_iscsi_disk(self, *args, **kwargs):
@@ -268,7 +307,7 @@ class FilesystemController(BaseController):
         footer = ("Format or mount whole disk.")
         self.ui.set_header(header)
         self.ui.set_footer(footer)
-        afv_view = AddFormatView(self.model, self, disk, lambda : self.partition_disk(disk))
+        afv_view = FormatEntireView(self.model, self, disk, lambda : self.partition_disk(disk))
         self.ui.set_body(afv_view)
 
     def format_mount_partition(self, partition):
@@ -281,7 +320,7 @@ class FilesystemController(BaseController):
             footer = ("Format and mount partition.")
         self.ui.set_header(header)
         self.ui.set_footer(footer)
-        afv_view = AddFormatView(self.model, self, partition, self.default)
+        afv_view = FormatEntireView(self.model, self, partition, self.default)
         self.ui.set_body(afv_view)
 
     def show_disk_information_next(self, disk):

subiquity/models/filesystem.py

@@ -147,7 +147,13 @@ class Disk:
 
     @property
     def next_partnum(self):
-        return len(self._partitions) + 1
+        partnums = set()
+        for p in self._partitions:
+            partnums.add(p.number)
+        i = 1
+        while i in partnums:
+            i += 1
+        return i
 
     @property
     def size(self):
@@ -239,6 +245,7 @@ class FilesystemModel(object):
         ('ext4', True, FS('ext4', True)),
         ('xfs', True, FS('xfs', True)),
         ('btrfs', True, FS('btrfs', True)),
+        ('fat32', True, FS('fat32', True)),
         ('---', False),
         ('swap', True, FS('swap', False)),
         #('bcache cache', True, FS('bcache cache', False)),
@@ -370,17 +377,6 @@ class FilesystemModel(object):
         # Do we need to check that there is a disk with the boot flag?
         return '/' in self.get_mountpoint_to_devpath_mapping() and self.bootable()
 
-    def validate_mount(self, mountpoint):
-        if mountpoint is None:
-            return
-        # /usr/include/linux/limits.h:PATH_MAX
-        if len(mountpoint) > 4095:
-            return 'Path exceeds PATH_MAX'
-        mnts = self.get_mountpoint_to_devpath_mapping()
-        dev = mnts.get(mountpoint)
-        if dev is not None:
-            return "%s is already mounted at %s"%(dev, mountpoint)
-
     def bootable(self):
         ''' true if one disk has a boot partition '''
         for p in self._partitions:

subiquity/ui/mount.py

@@ -36,13 +36,12 @@ OTHER = object()
 LEAVE_UNMOUNTED = object()
 
 class MountSelector(WidgetWrap):
-    def __init__(self, model):
+    def __init__(self, mountpoint_to_devpath_mapping):
-        mounts = model.get_mountpoint_to_devpath_mapping()
         opts = []
         first_opt = None
         max_len = max(map(len, common_mountpoints))
         for i, mnt in enumerate(common_mountpoints):
-            devpath = mounts.get(mnt)
+            devpath = mountpoint_to_devpath_mapping.get(mnt)
             if devpath is None:
                 if first_opt is None:
                     first_opt = i
@@ -58,18 +57,20 @@ class MountSelector(WidgetWrap):
         connect_signal(self._selector, 'select', self._select_mount)
         self._other = _MountEditor(edit_text='/')
         super().__init__(Pile([self._selector]))
+        self._other_showing = False
         if self._selector.value is OTHER:
             # This can happen if all the common_mountpoints are in use.
             self._showhide_other(True)
 
     def _showhide_other(self, show):
-        if show:
+        if show and not self._other_showing:
             self._w.contents.append((Padding(self._other, left=4), self._w.options('pack')))
-        else:
+            self._other_showing = True
+        elif self._other_showing:
             del self._w.contents[-1]
+            self._other_showing = False
 
     def _select_mount(self, sender, value):
-        if (self._selector.value == OTHER) != (value == OTHER):
         self._showhide_other(value==OTHER)
         if value == OTHER:
             self._w.focus_position = 1
@@ -83,11 +84,21 @@ class MountSelector(WidgetWrap):
         else:
             return self._selector.value
 
+    @value.setter
+    def value(self, val):
+        if val is None:
+            self._selector.value = LEAVE_UNMOUNTED
+        elif val in common_mountpoints:
+            self._selector.value = val
+        else:
+            self._selector.value = OTHER
+            self._other.value = val
+
 
 class MountField(FormField):
 
     def _make_widget(self, form):
-        return MountSelector(form.model)
+        return MountSelector(form.mountpoint_to_devpath_mapping)
 
     def clean(self, value):
         if value is None:

subiquity/ui/views/__init__.py

@@ -14,8 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from .filesystem import (FilesystemView,  # NOQA
-                         AddPartitionView,
+                         PartitionView,
-                         AddFormatView,
+                         FormatEntireView,
                          DiskPartitionView,
                          DiskInfoView,
                          GuidedDiskSelectionView,

subiquity/ui/views/filesystem/__init__.py

@@ -20,9 +20,8 @@ configuration.
 
 """
 
-from .add_format import AddFormatView
-from .add_partition import AddPartitionView
 from .disk_info import DiskInfoView
 from .disk_partition import DiskPartitionView
 from .filesystem import FilesystemView
 from .guided import GuidedDiskSelectionView, GuidedFilesystemView
+from .partition import FormatEntireView, PartitionView

subiquity/ui/views/filesystem/add_format.py

@@ -1,101 +0,0 @@
-# Copyright 2015 Canonical, Ltd.
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU Affero General Public License as
-# published by the Free Software Foundation, either version 3 of the
-# License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU Affero General Public License for more details.
-#
-# You should have received a copy of the GNU Affero General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-import logging
-from urwid import connect_signal
-
-from subiquitycore.ui.utils import Padding
-from subiquitycore.ui.container import ListBox
-from subiquitycore.ui.form import Form
-from subiquitycore.view import BaseView
-
-from subiquity.ui.mount import MountField
-from subiquity.ui.views.filesystem.add_partition import FSTypeField
-
-
-log = logging.getLogger('subiquity.ui.filesystem.add_format')
-
-
-class AddFormatForm(Form):
-
-    def __init__(self, model):
-        self.model = model
-        super().__init__()
-        connect_signal(self.fstype.widget, 'select', self.select_fstype)
-
-    fstype = FSTypeField("Format")
-    mount = MountField("Mount")
-
-    def select_fstype(self, sender, fs):
-        self.mount.enabled = fs.is_mounted
-
-    def validate_mount(self):
-        return self.model.validate_mount(self.mount.value)
-
-
-class AddFormatView(BaseView):
-    def __init__(self, model, controller, volume, back):
-        self.model = model
-        self.controller = controller
-        self.volume = volume
-        self.back = back
-
-        self.form = AddFormatForm(model)
-        if self.volume.fs() is not None:
-            for i, fs_spec in enumerate(self.model.supported_filesystems):
-                if len(fs_spec) == 3:
-                    fs = fs_spec[2]
-                    if fs.label == self.volume.fs().fstype:
-                        self.form.fstype.widget.index = i
-            self.form.fstype.enabled = False
-        connect_signal(self.form, 'submit', self.done)
-        connect_signal(self.form, 'cancel', self.cancel)
-
-        body = [
-            Padding.line_break(""),
-            self.form.as_rows(self),
-            Padding.line_break(""),
-            Padding.fixed_10(self.form.buttons)
-        ]
-        format_box = Padding.center_50(ListBox(body))
-        super().__init__(format_box)
-
-    def cancel(self, button=None):
-        self.back()
-
-    def done(self, result):
-        """ format spec
-
-        {
-          'format' Str(ext4|btrfs..,
-          'mountpoint': Str
-        }
-        """
-        fstype = self.form.fstype.value
-
-        if fstype.is_mounted:
-            mount = self.form.mount.value
-        else:
-            mount = None
-
-        result = {
-            "fstype": fstype.label,
-            "mountpoint": mount,
-        }
-        if self.volume.fs() is not None:
-            result['fstype'] = None
-
-        log.debug("Add Format Result: {}".format(result))
-        self.controller.add_format_handler(self.volume, result, self.back)

subiquity/ui/views/filesystem/add_partition.py

@@ -1,140 +0,0 @@
-# Copyright 2015 Canonical, Ltd.
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU Affero General Public License as
-# published by the Free Software Foundation, either version 3 of the
-# License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU Affero General Public License for more details.
-#
-# You should have received a copy of the GNU Affero General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-""" Filesystem
-
-Provides storage device selection and additional storage
-configuration.
-
-"""
-import logging
-from urwid import connect_signal, Text
-
-from subiquitycore.ui.container import ListBox
-from subiquitycore.ui.form import (
-    Form,
-    FormField,
-    IntegerField,
-    StringField,
-    )
-from subiquitycore.ui.utils import Padding, Color
-from subiquitycore.ui.interactive import Selector
-from subiquitycore.view import BaseView
-
-from subiquity.models.filesystem import (
-    humanize_size,
-    dehumanize_size,
-    HUMAN_UNITS,
-    )
-from subiquity.ui.mount import MountField
-
-
-log = logging.getLogger('subiquity.ui.filesystem.add_partition')
-
-
-class FSTypeField(FormField):
-    def _make_widget(self, form):
-        return Selector(opts=form.model.supported_filesystems)
-
-
-class AddPartitionForm(Form):
-
-    def __init__(self, model, disk):
-        self.model = model
-        self.disk = disk
-        self.size_str = humanize_size(disk.free)
-        super().__init__()
-        self.size.caption = "Size (max {})".format(self.size_str)
-        self.partnum.value = self.disk.next_partnum
-        connect_signal(self.fstype.widget, 'select', self.select_fstype)
-
-    def select_fstype(self, sender, fs):
-        self.mount.enabled = fs.is_mounted
-
-    partnum = IntegerField("Partition number")
-    size = StringField()
-    fstype = FSTypeField("Format")
-    mount = MountField("Mount")
-
-    def validate_size(self):
-        v = self.size.value
-        if not v:
-            return
-        suffixes = ''.join(HUMAN_UNITS) + ''.join(HUMAN_UNITS).lower()
-        if v[-1] not in suffixes:
-            unit = self.size_str[-1]
-            v += unit
-            self.size.value = v
-        try:
-            sz = dehumanize_size(v)
-        except ValueError as v:
-            return str(v)
-        if sz > self.disk.free:
-            self.size.value = self.size_str
-            self.size.show_extra(Color.info_minor(Text("Capped partition size at %s"%(self.size_str,), align="center")))
-
-    def validate_mount(self):
-        return self.model.validate_mount(self.mount.value)
-
-
-class AddPartitionView(BaseView):
-
-    def __init__(self, model, controller, disk):
-        log.debug('AddPartitionView: selected_disk=[{}]'.format(disk.path))
-        self.model = model
-        self.controller = controller
-        self.disk = disk
-
-        self.form = AddPartitionForm(model, self.disk)
-
-        connect_signal(self.form, 'submit', self.done)
-        connect_signal(self.form, 'cancel', self.cancel)
-
-        body = [
-            self.form.as_rows(self),
-            Padding.line_break(""),
-            Padding.fixed_10(self.form.buttons),
-        ]
-        partition_box = Padding.center_50(ListBox(body))
-        super().__init__(partition_box)
-
-    def cancel(self, button=None):
-        self.controller.partition_disk(self.disk)
-
-    def done(self, result):
-
-        fstype = self.form.fstype.value
-
-        if fstype.is_mounted:
-            mount = self.form.mount.value
-        else:
-            mount = None
-
-        if self.form.size.value:
-            size = dehumanize_size(self.form.size.value)
-            if size > self.disk.free:
-                size = self.disk.free
-        else:
-            size = self.disk.free
-
-        result = {
-            "partnum": self.form.partnum.value,
-            "bytes": size,
-            "fstype": fstype.label,
-            "mountpoint": mount,
-        }
-
-        log.debug("Add Partition Result: {}".format(result))
-        self.controller.add_disk_partition_handler(self.disk, result)

subiquity/ui/views/filesystem/disk_partition.py

@@ -14,7 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import logging
-from urwid import BoxAdapter, Text
+from urwid import BoxAdapter, connect_signal, Text
 
 from subiquitycore.ui.lists import SimpleList
 from subiquitycore.ui.buttons import done_btn, cancel_btn, menu_btn
@@ -37,7 +37,7 @@ class DiskPartitionView(BaseView):
         self.body = [
             Padding.center_79(self._build_model_inputs()),
             Padding.line_break(""),
-            Padding.center_79(self._build_menu()),
+            Padding.center_79(self.show_disk_info_w()),
             Padding.line_break(""),
             Padding.fixed_10(self._build_buttons()),
         ]
@@ -67,41 +67,51 @@ class DiskPartitionView(BaseView):
             else:
                 fstype = part.fs().fstype
                 mountpoint = part.fs().mount().path
+            part_btn = menu_btn(label)
+            if part.type == 'disk':
+                connect_signal(part_btn, 'click', self._click_disk)
+            else:
+                connect_signal(part_btn, 'click', self._click_part, part)
             return Columns([
-                (15, Text(label)),
+                (25, Color.menu_button(part_btn)),
-                Text(size),
+                (9, Text(size, align="right")),
                 Text(fstype),
                 Text(mountpoint),
-            ], 4)
+            ], 2)
         if self.disk.fs() is not None:
             partitioned_disks.append(format_volume("entire disk", self.disk))
         else:
             for part in self.disk.partitions():
-                partitioned_disks.append(format_volume("partition {}".format(part.number), part))
+                partitioned_disks.append(format_volume("Partition {}".format(part.number), part))
         if self.disk.free > 0:
             free_space = humanize_size(self.disk.free)
+            if len(self.disk.partitions()) > 0:
+                label = "Add another partition"
+            else:
+                label = "Add first partition"
+            add_btn = menu_btn(label)
+            connect_signal(add_btn, 'click', self.add_partition)
             partitioned_disks.append(Columns([
-                (15, Text("FREE SPACE")),
+                (25, Color.menu_button(add_btn)),
-                Text(free_space),
+                (9, Text(free_space, align="right")),
-                Text(""),
+                Text("free space"),
-                Text("")
+            ], 2))
-            ], 4))
+        if len(self.disk.partitions()) == 0 and \
+           self.disk.available:
+            text = ("Format or create swap on entire "
+                    "device (unusual, advanced)")
+            partitioned_disks.append(Text(""))
+            partitioned_disks.append(Color.menu_button(
+                menu_btn(label=text, on_press=self.format_entire)))
 
-        return BoxAdapter(SimpleList(partitioned_disks, is_selectable=False),
+        return BoxAdapter(SimpleList(partitioned_disks),
                           height=len(partitioned_disks))
 
-    def _build_menu(self):
+    def _click_part(self, sender, part):
-        """
+        self.controller.edit_partition(self.disk, part)
-        Builds the add partition menu with user visible
+
-        changes to the button depending on if existing
+    def _click_disk(self, sender):
-        partitions exist or not.
+        self.controller.format_entire(self.disk)
-        """
-        menus = [
-            self.add_partition_w(),
-            self.create_swap_w(),
-            self.show_disk_info_w(),
-        ]
-        return Pile([m for m in menus if m])
 
     def show_disk_info_w(self):
         """ Runs hdparm against device and displays its output
@@ -112,32 +122,6 @@ class DiskPartitionView(BaseView):
                 label=text,
                 on_press=self.show_disk_info))
 
-    def create_swap_w(self):
-        """ Handles presenting an enabled create swap on
-        entire device button if no partition exists, otherwise
-        it is disabled.
-        """
-        text = ("Format or create swap on entire "
-                "device (unusual, advanced)")
-        if len(self.disk.partitions()) == 0 and \
-           self.disk.available:
-            return Color.menu_button(
-                menu_btn(label=text, on_press=self.format_entire))
-
-    def add_partition_w(self):
-        """ Handles presenting the add partition widget button
-        depending on if partitions exist already or not.
-        """
-        if not self.disk.available:
-            return None
-        text = "Add first partition"
-        if len(self.disk.partitions()) > 0:
-            text = "Add partition (max size {})".format(
-                humanize_size(self.disk.free))
-
-        return Color.menu_button(
-            menu_btn(label=text, on_press=self.add_partition))
-
     def show_disk_info(self, result):
         self.controller.show_disk_information(self.disk)
 

subiquity/ui/views/filesystem/filesystem.py

@@ -80,11 +80,11 @@ class FilesystemView(BaseView):
         self.body = [
             Text("FILE SYSTEM SUMMARY"),
             Text(""),
-            self._build_filesystem_list(),
+            Padding.push_4(self._build_filesystem_list()),
             Text(""),
             Text("AVAILABLE DEVICES"),
             Text(""),
-            self._build_available_inputs(),
+            Padding.push_4(self._build_available_inputs()),
             Text(""),
             #self._build_menu(),
             #Text(""),
@@ -97,7 +97,7 @@ class FilesystemView(BaseView):
         w = ListBox(self.body)
         if self.model.can_install():
             w.set_focus_path([len(self.body)-1, 0])
-        super().__init__(Padding.center_90(w))
+        super().__init__(Padding.center_95(w))
         log.debug('FileSystemView init complete()')
 
     def _build_used_disks(self):
@@ -118,7 +118,7 @@ class FilesystemView(BaseView):
             cols.append((m.path, path, humanize_size(m.device.volume.size), m.device.fstype, m.device.volume.desc()))
         for fs in self.model._filesystems:
             if fs.fstype == 'swap':
-                cols.append((None, 'SWAP', humanize_size(fs.volume.size), fs.fstype, fs.device.volume.desc()))
+                cols.append((None, 'SWAP', humanize_size(fs.volume.size), fs.fstype, fs.volume.desc()))
 
         if len(cols) == 0:
             return Pile([Color.info_minor(
@@ -164,17 +164,15 @@ class FilesystemView(BaseView):
             size = Text(humanize_size(disk.size).rjust(9))
             typ = Text(disk.desc())
             col3(disk_label, size, typ)
-            if disk.fs() is not None:
-                label = "entire device, "
             fs = disk.fs()
             if fs is not None:
+                label = "entire device, "
+                fs_obj = self.model.fs_by_name[fs.fstype]
                 if fs.mount():
                     label += "%-*s"%(self.model.longest_fs_name+2, fs.fstype+',') + fs.mount().path
                 else:
                     label += fs.fstype
-                else:
+                if fs_obj.label and fs_obj.is_mounted and not fs.mount():
-                    label += "unformatted"
-                if not fs.mount():
                     disk_btn = menu_btn(label=label)
                     connect_signal(disk_btn, 'click', self.click_disk, disk)
                     disk_btn = Color.menu_button(disk_btn)
@@ -201,23 +199,25 @@ class FilesystemView(BaseView):
                     part_btn = Color.info_minor(Text("  " + label))
                     size = Color.info_minor(size)
                     col2(part_btn, size)
-            if disk.available:
-                if disk.used > 0:
-                    disk_btn = menu_btn(label="FREE SPACE")
-                    connect_signal(disk_btn, 'click', self.click_disk, disk)
-                    disk_btn = Color.menu_button(disk_btn)
             size = disk.size
             free = disk.free
             percent = int(100*free/size)
-                    if percent == 0:
+            if disk.available and disk.used > 0 and percent > 0:
-                        continue
+                disk_btn = menu_btn(label="ADD/EDIT PARTITIONS")
-                    size = Text("{:>9} ({}%)".format(humanize_size(free), percent))
+                connect_signal(disk_btn, 'click', self.click_disk, disk)
+                disk_btn = Color.menu_button(disk_btn)
+                size = Text("{:>9} ({}%) free".format(humanize_size(free), percent))
                 col2(disk_btn, size)
-                else:
+            elif disk.available and percent > 0:
                 disk_btn = menu_btn(label="ADD FIRST PARTITION")
                 connect_signal(disk_btn, 'click', self.click_disk, disk)
                 disk_btn = Color.menu_button(disk_btn)
                 col2(disk_btn, Text(""))
+            else:
+                disk_btn = menu_btn(label="EDIT PARTITIONS")
+                connect_signal(disk_btn, 'click', self.click_disk, disk)
+                disk_btn = Color.menu_button(disk_btn)
+                col2(disk_btn, Text(""))
 
         if len(inputs) == 1:
             return Pile([Color.info_minor(
@@ -226,9 +226,6 @@ class FilesystemView(BaseView):
         return Pile(inputs)
 
     def click_disk(self, sender, disk):
-        if disk.fs() is not None:
-            self.controller.format_entire(disk)
-        else:
         self.controller.partition_disk(disk)
 
     def click_partition(self, sender, partition):

subiquity/ui/views/filesystem/guided.py

@@ -83,9 +83,9 @@ class GuidedDiskSelectionView(BaseView):
     def choose_disk(self, btn, disk):
         result = {
             "partnum": 1,
-            "bytes": disk.free,
+            "size": disk.free,
-            "fstype": "ext4",
+            "fstype": self.model.fs_by_name["ext4"],
-            "mountpoint": "/",
+            "mount": "/",
         }
-        self.controller.do_add_disk_partition(disk, result)
+        self.controller.partition_disk_handler(disk, None, result)
         self.controller.manual()

subiquity/ui/views/filesystem/partition.py

@@ -0,0 +1,191 @@
+# Copyright 2015 Canonical, Ltd.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+""" Filesystem
+
+Provides storage device selection and additional storage
+configuration.
+
+"""
+import logging
+from urwid import connect_signal, Text
+
+from subiquitycore.ui.buttons import PlainButton
+from subiquitycore.ui.container import ListBox
+from subiquitycore.ui.form import (
+    Form,
+    FormField,
+    IntegerField,
+    StringField,
+    )
+from subiquitycore.ui.utils import Padding, Color
+from subiquitycore.ui.interactive import Selector
+from subiquitycore.view import BaseView
+
+from subiquity.models.filesystem import (
+    FilesystemModel,
+    HUMAN_UNITS,
+    dehumanize_size,
+    humanize_size,
+    )
+from subiquity.ui.mount import MountField
+
+
+log = logging.getLogger('subiquity.ui.filesystem.add_partition')
+
+
+class FSTypeField(FormField):
+    def _make_widget(self, form):
+        return Selector(opts=FilesystemModel.supported_filesystems)
+
+
+class PartitionForm(Form):
+
+    def __init__(self, mountpoint_to_devpath_mapping, max_size, initial={}):
+        self.mountpoint_to_devpath_mapping = mountpoint_to_devpath_mapping
+        super().__init__(initial)
+        if max_size is not None:
+            self.max_size = max_size
+            self.size_str = humanize_size(max_size)
+            self.size.caption = "Size (max {})".format(self.size_str)
+        else:
+            self.remove_field('partnum')
+            self.remove_field('size')
+        connect_signal(self.fstype.widget, 'select', self.select_fstype)
+
+    def select_fstype(self, sender, fs):
+        self.mount.enabled = fs.is_mounted
+
+    partnum = IntegerField("Partition number")
+    size = StringField()
+    fstype = FSTypeField("Format")
+    mount = MountField("Mount")
+
+    def clean_size(self, val):
+        if not val:
+            return self.max_size
+        suffixes = ''.join(HUMAN_UNITS) + ''.join(HUMAN_UNITS).lower()
+        if val[-1] not in suffixes:
+            unit = self.size_str[-1]
+            val += unit
+            self.size.widget.value = val
+        sz = dehumanize_size(val)
+        if sz > self.max_size:
+            self.size.show_extra(Color.info_minor(Text("Capped partition size at %s"%(self.size_str,), align="center")))
+            self.size.widget.value = self.size_str
+            return self.max_size
+        return sz
+
+    def clean_mount(self, val):
+        if self.fstype.value.is_mounted:
+            return val
+        else:
+            return None
+
+    def validate_mount(self):
+        mount = self.mount.value
+        if mount is None:
+            return
+        # /usr/include/linux/limits.h:PATH_MAX
+        if len(mount) > 4095:
+            return 'Path exceeds PATH_MAX'
+        dev = self.mountpoint_to_devpath_mapping.get(mount)
+        if dev is not None:
+            return "%s is already mounted at %s"%(dev, mount)
+
+
+class PartitionFormatView(BaseView):
+    def __init__(self, size, existing, initial, back):
+
+        mountpoint_to_devpath_mapping = self.model.get_mountpoint_to_devpath_mapping()
+        if existing is not None:
+            fs = existing.fs()
+            if fs is not None:
+                initial['fstype'] = self.model.fs_by_name[fs.fstype]
+                mount = fs.mount()
+                if mount is not None:
+                    initial['mount'] = mount.path
+                    if mount.path in mountpoint_to_devpath_mapping:
+                        del mountpoint_to_devpath_mapping[mount.path]
+        self.form = PartitionForm(mountpoint_to_devpath_mapping, size, initial)
+        self.back = back
+
+        connect_signal(self.form, 'submit', self.done)
+        connect_signal(self.form, 'cancel', self.cancel)
+
+        partition_box = Padding.center_50(ListBox(self.make_body()))
+        super().__init__(partition_box)
+
+    def make_body(self):
+        return [
+            self.form.as_rows(self),
+            Padding.line_break(""),
+            Padding.fixed_10(self.form.buttons),
+        ]
+
+    def cancel(self, button=None):
+        self.back()
+
+
+class PartitionView(PartitionFormatView):
+
+    def __init__(self, model, controller, disk, partition=None):
+        log.debug('PartitionView: selected_disk=[{}]'.format(disk.path))
+        self.model = model
+        self.controller = controller
+        self.disk = disk
+        self.partition = partition
+
+        max_size = disk.free
+        if partition is None:
+            initial = {'partnum': disk.next_partnum}
+        else:
+            max_size += partition.size
+            initial = {
+                'partnum': partition.number,
+                'size': humanize_size(partition.size),
+                }
+        super().__init__(max_size, partition, initial, lambda : self.controller.partition_disk(disk))
+
+    def make_body(self):
+        body = super().make_body()
+        if self.partition is not None:
+            delete_btn = PlainButton("Delete")
+            connect_signal(delete_btn, 'click', self.delete)
+            body[-2:-2] = [
+                Text(""),
+                Padding.fixed_10(Color.info_error(delete_btn)),
+                ]
+            pass
+        return body
+
+    def delete(self, sender):
+        self.controller.delete_partition(self.partition)
+
+    def done(self, form):
+        log.debug("Add Partition Result: {}".format(form.as_data()))
+        self.controller.partition_disk_handler(self.disk, self.partition, form.as_data())
+
+
+class FormatEntireView(PartitionFormatView):
+    def __init__(self, model, controller, volume, back):
+        self.model = model
+        self.controller = controller
+        self.volume = volume
+        super().__init__(None, volume, {}, back)
+
+    def done(self, form):
+        log.debug("Add Partition Result: {}".format(form.as_data()))
+        self.controller.add_format_handler(self.volume, form.as_data(), self.back)

subiquitycore/ui/form.py

@@ -13,6 +13,8 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import logging
+
 from urwid import (
     AttrMap,
     connect_signal,
@@ -35,6 +37,8 @@ from subiquitycore.ui.interactive import (
     )
 from subiquitycore.ui.utils import Color
 
+log = logging.getLogger("subiquitycore.ui.form")
+
 class Toggleable(delegate_to_widget_mixin('_original_widget'), WidgetDecoration):
 
     def __init__(self, original, active_color):
@@ -254,7 +258,7 @@ class Form(object, metaclass=MetaForm):
 
     opts = {}
 
-    def __init__(self):
+    def __init__(self, initial={}):
         self.done_btn = Toggleable(done_btn(), 'button')
         self.cancel_btn = Toggleable(cancel_btn(), 'button')
         connect_signal(self.done_btn.base_widget, 'click', self._click_done)
@@ -265,6 +269,8 @@ class Form(object, metaclass=MetaForm):
             bf = field.bind(self)
             setattr(self, bf.field.name, bf)
             self._fields.append(bf)
+            if field.name in initial:
+                bf.value = initial[field.name]
 
     def _click_done(self, sender):
         emit_signal(self, 'submit', self)
@@ -304,3 +310,9 @@ class Form(object, metaclass=MetaForm):
             self.buttons.focus_position = 1
         else:
             self.buttons.contents[0][0].enable()
+
+    def as_data(self):
+        data = {}
+        for field in self._fields:
+            data[field.field.name] = field.value
+        return data

subiquitycore/ui/interactive.py

@@ -22,21 +22,17 @@ import re
 
 from urwid import (
     ACTIVATE,
-    AttrWrap,
-    connect_signal,
     Edit,
-    Filler,
     IntEdit,
     LineBox,
-    PopUpLauncher,
     SelectableIcon,
     Text,
-    TOP,
     WidgetWrap,
     )
 
 from subiquitycore.ui.buttons import PlainButton
 from subiquitycore.ui.container import Pile
+from subiquitycore.ui.selector import Selector
 from subiquitycore.ui.utils import Color, Padding
 
 log = logging.getLogger("subiquitycore.ui.input")
@@ -111,125 +107,6 @@ class IntegerEditor(WidgetWrap):
         return self._edit.set_edit_text(str(val))
 
 
-class _PopUpButton(SelectableIcon):
-    """It looks a bit like a radio button, but it just emits 'click' on activation."""
-
-    signals = ['click']
-
-    states = {
-        True: "(+) ",
-        False: "( ) ",
-        }
-
-    def __init__(self, option, state):
-        p = self.states[state]
-        super().__init__(p + option, len(p))
-
-    def keypress(self, size, key):
-        if self._command_map[key] != ACTIVATE:
-            return key
-        self._emit('click')
-
-
-class _PopUpSelectDialog(WidgetWrap):
-    """A list of PopUpButtons with a box around them."""
-
-    def __init__(self, parent, cur_index):
-        self.parent = parent
-        group = []
-        for i, option in enumerate(self.parent._options):
-            if option[1]:
-                btn = _PopUpButton(option[0], state=i==cur_index)
-                connect_signal(btn, 'click', self.click, i)
-                group.append(AttrWrap(btn, 'menu_button', 'menu_button focus'))
-            else:
-                btn = Text("    " + option[0])
-                group.append(AttrWrap(btn, 'info_minor'))
-        pile = Pile(group)
-        pile.set_focus(group[cur_index])
-        fill = Filler(pile, valign=TOP)
-        super().__init__(LineBox(fill))
-
-    def click(self, btn, index):
-        self.parent.index = index
-        self.parent.close_pop_up()
-
-    def keypress(self, size, key):
-        if key == 'esc':
-            self.parent.close_pop_up()
-        else:
-            return super().keypress(size, key)
-
-class SelectorError(Exception):
-    pass
-
-class Selector(PopUpLauncher):
-    """A widget that allows the user to chose between options by popping up this list of options.
-
-    (A bit like <select> in an HTML form).
-    """
-
-    _prefix = "(+) "
-
-    signals = ['select']
-
-    def __init__(self, opts, index=0):
-        self._options = []
-        for opt in opts:
-            if not isinstance(opt, tuple):
-                if not isinstance(opt, str):
-                    raise SelectorError("invalid option %r", opt)
-                opt = (opt, True, opt)
-            elif len(opt) == 1:
-                opt = (opt[0], True, opt[0])
-            elif len(opt) == 2:
-                opt = (opt[0], opt[1], opt[0])
-            elif len(opt) != 3:
-                raise SelectorError("invalid option %r", opt)
-            self._options.append(opt)
-        self._button = SelectableIcon(self._prefix, len(self._prefix))
-        self._set_index(index)
-        super().__init__(self._button)
-
-    def keypress(self, size, key):
-        if self._command_map[key] != ACTIVATE:
-            return key
-        self.open_pop_up()
-
-    def _set_index(self, val):
-        self._button.set_text(self._prefix + self._options[val][0])
-        self._index = val
-
-    @property
-    def index(self):
-        return self._index
-
-    @index.setter
-    def index(self, val):
-        self._emit('select', self._options[val][2])
-        self._set_index(val)
-
-    @property
-    def value(self):
-        return self._options[self._index][2]
-
-    @value.setter
-    def value(self, val):
-        for i, (label, enabled, value) in enumerate(self._options):
-            if value == val:
-                self.index = i
-                return
-        raise AttributeError("cannot set value to %r", val)
-
-    def create_pop_up(self):
-        return _PopUpSelectDialog(self, self.index)
-
-    def get_pop_up_parameters(self):
-        width = max([len(o[0]) for o in self._options]) \
-          + len(self._prefix) +  3 # line on left, space, line on right
-        return {'left':-1, 'top':-self.index-1, 'overlay_width':width, 'overlay_height':len(self._options) + 2}
-
-
 class YesNo(Selector):
     """ Yes/No selector
     """

subiquitycore/ui/selector.py

@@ -0,0 +1,180 @@
+# Copyright 2017 Canonical, Ltd.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from urwid import (
+    ACTIVATE,
+    AttrWrap,
+    connect_signal,
+    Filler,
+    LineBox,
+    PopUpLauncher,
+    SelectableIcon,
+    Text,
+    TOP,
+    WidgetWrap,
+    )
+
+from subiquitycore.ui.container import Pile
+
+
+class _PopUpButton(SelectableIcon):
+    """It looks a bit like a radio button, but it just emits 'click' on activation."""
+
+    signals = ['click']
+
+    states = {
+        True: "(+) ",
+        False: "( ) ",
+        }
+
+    def __init__(self, option, state):
+        p = self.states[state]
+        super().__init__(p + option, len(p))
+
+    def keypress(self, size, key):
+        if self._command_map[key] != ACTIVATE:
+            return key
+        self._emit('click')
+
+
+class _PopUpSelectDialog(WidgetWrap):
+    """A list of PopUpButtons with a box around them."""
+
+    def __init__(self, parent, cur_index):
+        self.parent = parent
+        group = []
+        for i, option in enumerate(self.parent._options):
+            if option.enabled:
+                btn = _PopUpButton(option.label, state=i==cur_index)
+                connect_signal(btn, 'click', self.click, i)
+                group.append(AttrWrap(btn, 'menu_button', 'menu_button focus'))
+            else:
+                btn = Text("    " + option.label)
+                group.append(AttrWrap(btn, 'info_minor'))
+        pile = Pile(group)
+        pile.set_focus(group[cur_index])
+        fill = Filler(pile, valign=TOP)
+        super().__init__(LineBox(fill))
+
+    def click(self, btn, index):
+        self.parent.index = index
+        self.parent.close_pop_up()
+
+    def keypress(self, size, key):
+        if key == 'esc':
+            self.parent.close_pop_up()
+        else:
+            return super().keypress(size, key)
+
+
+class SelectorError(Exception):
+    pass
+
+
+class Option:
+
+    def __init__(self, val):
+        if not isinstance(val, tuple):
+            if not isinstance(val, str):
+                raise SelectorError("invalid option %r", val)
+            self.label = val
+            self.enabled = True
+            self.value = val
+        elif len(val) == 1:
+            self.label = val[0]
+            self.enabled = True
+            self.value = val[0]
+        elif len(val) == 2:
+            self.label = val[0]
+            self.enabled = val[1]
+            self.value = val[0]
+        elif len(val) == 3:
+            self.label = val[0]
+            self.enabled = val[1]
+            self.value = val[2]
+        else:
+            raise SelectorError("invalid option %r", val)
+
+
+class Selector(PopUpLauncher):
+    """A widget that allows the user to chose between options by popping up a list of options.
+
+    (A bit like <select> in an HTML form).
+    """
+
+    _prefix = "(+) "
+
+    signals = ['select']
+
+    def __init__(self, opts, index=0):
+        self._options = []
+        for opt in opts:
+            if not isinstance(opt, Option):
+                opt = Option(opt)
+            self._options.append(opt)
+        self._button = SelectableIcon(self._prefix, len(self._prefix))
+        self._set_index(index)
+        super().__init__(self._button)
+
+    def keypress(self, size, key):
+        if self._command_map[key] != ACTIVATE:
+            return key
+        self.open_pop_up()
+
+    def _set_index(self, val):
+        self._button.set_text(self._prefix + self._options[val].label)
+        self._index = val
+
+    @property
+    def index(self):
+        return self._index
+
+    @index.setter
+    def index(self, val):
+        self._emit('select', self._options[val].value)
+        self._set_index(val)
+
+    def option_by_label(self, label):
+        for opt in self._options:
+            if opt.label == label:
+                return opt
+
+    def option_by_value(self, value):
+        for opt in self._options:
+            if opt.value == value:
+                return opt
+
+    def option_by_index(self, index):
+        return self._options[index]
+
+    @property
+    def value(self):
+        return self._options[self._index].value
+
+    @value.setter
+    def value(self, val):
+        for i, opt in enumerate(self._options):
+            if opt.value == val:
+                self.index = i
+                return
+        raise AttributeError("cannot set value to %r", val)
+
+    def create_pop_up(self):
+        return _PopUpSelectDialog(self, self.index)
+
+    def get_pop_up_parameters(self):
+        width = max([len(o.label) for o in self._options]) \
+          + len(self._prefix) +  3 # line on left, space, line on right
+        return {'left':-1, 'top':-self.index-1, 'overlay_width':width, 'overlay_height':len(self._options) + 2}