Merge lp:~mvo/snappy/snappy-improved-developer-mode2 into lp:~snappy-dev/snappy/snappy-moved-to-github

[Michael Vogt]

Refactor partition/ and split code into more files.

A bit of refactoring in the partition/ system, mostly moving stuff around. The goal is to eventually decouple bootloader and partition a bit more. The branch is rather large, sorry for that. Please let me know if I should split it up further.

[John Lenton (chipaca)]

I'm pretending you removed the stray comment :-) feel free to top-approve once that's done. Also, commit message.

review: Approve

[Michael Vogt (mvo)]

Thanks! I fixed the mentioned issues and it should be good to go now.

[Snappy Tarmac (snappydevtarmac)]

Attempt to merge into lp:snappy failed due to conflicts:

text conflict in partition/bootloader_uboot_test.go

[lp:~mvo/snappy/snappy-improved-developer-mode2]

514. By Michael Vogt on 2015-06-24

merged lp:snappy and resolved conflicts

[Michael Vogt (mvo)]

review: Approve

=== added file 'partition/assets.go'
--- partition/assets.go	1970-01-01 00:00:00 +0000
+++ partition/assets.go	2015-06-24 13:28:39 +0000
@@ -0,0 +1,87 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2014-2015 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package partition
+
+import (
+	"errors"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	"gopkg.in/yaml.v2"
+)
+
+// Representation of the yaml in the hardwareSpecFile
+type hardwareSpecType struct {
+	Kernel          string         `yaml:"kernel"`
+	Initrd          string         `yaml:"initrd"`
+	DtbDir          string         `yaml:"dtbs"`
+	PartitionLayout string         `yaml:"partition-layout"`
+	Bootloader      bootloaderName `yaml:"bootloader"`
+}
+
+var (
+	// ErrNoHardwareYaml is returned when no hardware yaml is found in
+	// the update, this means that there is nothing to process with regards
+	// to device parts.
+	ErrNoHardwareYaml = errors.New("no hardware.yaml")
+
+	// Declarative specification of the type of system which specifies such
+	// details as:
+	//
+	// - the location of initrd+kernel within the system-image archive.
+	// - the location of hardware-specific .dtb files within the
+	//   system-image archive.
+	// - the type of bootloader that should be used for this system.
+	// - expected system partition layout (single or dual rootfs's).
+	hardwareSpecFileReal = filepath.Join(cacheDir, "hardware.yaml")
+
+	// useful to override in the tests
+	hardwareSpecFile = hardwareSpecFileReal
+
+	// Directory that _may_ get automatically created on unpack that
+	// contains updated hardware-specific boot assets (such as initrd,
+	// kernel)
+	assetsDir = filepath.Join(cacheDir, "assets")
+
+	// Directory that _may_ get automatically created on unpack that
+	// contains updated hardware-specific assets that require flashing
+	// to the disk (such as uBoot, MLO)
+	flashAssetsDir = filepath.Join(cacheDir, "flashtool-assets")
+)
+
+func readHardwareSpec() (*hardwareSpecType, error) {
+	var h hardwareSpecType
+
+	data, err := ioutil.ReadFile(hardwareSpecFile)
+	// if hardware.yaml does not exist it just means that there was no
+	// device part in the update.
+	if os.IsNotExist(err) {
+		return nil, ErrNoHardwareYaml
+	} else if err != nil {
+		return nil, err
+	}
+
+	if err := yaml.Unmarshal([]byte(data), &h); err != nil {
+		return nil, err
+	}
+
+	return &h, nil
+}

=== added file 'partition/assets_test.go'
--- partition/assets_test.go	1970-01-01 00:00:00 +0000
+++ partition/assets_test.go	2015-06-24 13:28:39 +0000
@@ -0,0 +1,36 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2014-2015 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package partition
+
+import (
+	. "gopkg.in/check.v1"
+)
+
+func (s *PartitionTestSuite) TestHardwareSpec(c *C) {
+
+	hardwareSpecFile = makeHardwareYaml(c, "")
+	hw, err := readHardwareSpec()
+	c.Assert(err, IsNil)
+	c.Assert(hw.Kernel, Equals, "assets/vmlinuz")
+	c.Assert(hw.Initrd, Equals, "assets/initrd.img")
+	c.Assert(hw.DtbDir, Equals, "assets/dtbs")
+	c.Assert(hw.PartitionLayout, Equals, bootloaderSystemAB)
+	c.Assert(hw.Bootloader, Equals, bootloaderNameUboot)
+}

=== modified file 'partition/bootloader_grub.go'
--- partition/bootloader_grub.go	2015-06-09 17:22:59 +0000
+++ partition/bootloader_grub.go	2015-06-24 13:28:39 +0000
@@ -80,7 +80,7 @@
 func (g *grub) ToggleRootFS() (err error) {
 
 	// create the grub config
-	if err := runInChroot(g.partition.MountTarget(), bootloaderGrubUpdateCmd); err != nil {
+	if err := runInChroot(mountTarget, bootloaderGrubUpdateCmd); err != nil {
 		return err
 	}
 

=== modified file 'partition/bootloader_grub_test.go'
--- partition/bootloader_grub_test.go	2015-06-09 17:43:20 +0000
+++ partition/bootloader_grub_test.go	2015-06-24 13:28:39 +0000
@@ -92,10 +92,10 @@
 	c.Assert(err, IsNil)
 
 	// this is always called
-	mp := singleCommand{"/bin/mountpoint", "/writable/cache/system"}
+	mp := singleCommand{"/bin/mountpoint", mountTarget}
 	c.Assert(allCommands[0], DeepEquals, mp)
 
-	expectedGrubUpdate := singleCommand{"/usr/sbin/chroot", "/writable/cache/system", bootloaderGrubUpdateCmd}
+	expectedGrubUpdate := singleCommand{"/usr/sbin/chroot", mountTarget, bootloaderGrubUpdateCmd}
 	c.Assert(allCommands[1], DeepEquals, expectedGrubUpdate)
 
 	expectedGrubSet := singleCommand{bootloaderGrubEnvCmd, bootloaderGrubEnvFile, "set", "snappy_mode=try"}
@@ -145,7 +145,7 @@
 	c.Assert(err, IsNil)
 
 	// this is always called
-	mp := singleCommand{"/bin/mountpoint", "/writable/cache/system"}
+	mp := singleCommand{"/bin/mountpoint", mountTarget}
 	c.Assert(allCommands[0], DeepEquals, mp)
 
 	expectedGrubSet := singleCommand{bootloaderGrubEnvCmd, bootloaderGrubEnvFile, "unset", "snappy_trial_boot"}

=== modified file 'partition/bootloader_uboot.go'
--- partition/bootloader_uboot.go	2015-06-09 17:22:59 +0000
+++ partition/bootloader_uboot.go	2015-06-24 13:28:39 +0000
@@ -227,14 +227,18 @@
 func (u *uboot) HandleAssets() (err error) {
 	// check if we have anything, if there is no hardware yaml, there is nothing
 	// to process.
-	hardware, err := u.partition.hardwareSpec()
+	hardware, err := readHardwareSpec()
 	if err == ErrNoHardwareYaml {
 		return nil
 	} else if err != nil {
 		return err
 	}
-	// ensure to remove the file once we are done
-	defer os.Remove(u.partition.hardwareSpecFile)
+	// ensure to remove the file once we are done (and all was good)
+	defer func() {
+		if err == nil {
+			os.Remove(hardwareSpecFile)
+		}
+	}()
 
 	// validate bootloader
 	if hardware.Bootloader != u.Name() {
@@ -263,7 +267,7 @@
 		}
 
 		// expand path
-		path := filepath.Join(u.partition.cacheDir(), file)
+		path := filepath.Join(cacheDir, file)
 
 		if !helpers.FileExists(path) {
 			return fmt.Errorf("can not find file %s", path)
@@ -281,7 +285,7 @@
 	//       fully speced
 
 	// install .dtb files
-	dtbSrcDir := filepath.Join(u.partition.cacheDir(), hardware.DtbDir)
+	dtbSrcDir := filepath.Join(cacheDir, hardware.DtbDir)
 	if helpers.FileExists(dtbSrcDir) {
 		// ensure we cleanup the source dir
 		defer os.RemoveAll(dtbSrcDir)
@@ -303,8 +307,6 @@
 		}
 	}
 
-	flashAssetsDir := u.partition.flashAssetsDir()
-
 	if helpers.FileExists(flashAssetsDir) {
 		// FIXME: we don't currently do anything with the
 		// MLO + uImage files since they are not specified in

=== modified file 'partition/bootloader_uboot_test.go'
--- partition/bootloader_uboot_test.go	2015-06-12 05:23:40 +0000
+++ partition/bootloader_uboot_test.go	2015-06-24 13:28:39 +0000
@@ -158,7 +158,7 @@
 
 func makeMockAssetsDir(c *C) {
 	for _, f := range []string{"assets/vmlinuz", "assets/initrd.img", "assets/dtbs/foo.dtb", "assets/dtbs/bar.dtb"} {
-		p := filepath.Join(defaultCacheDir, f)
+		p := filepath.Join(cacheDir, f)
 		os.MkdirAll(filepath.Dir(p), 0755)
 		err := ioutil.WriteFile(p, []byte(f), 0644)
 		c.Assert(err, IsNil)
@@ -172,8 +172,8 @@
 	c.Assert(err, IsNil)
 
 	// mock the hardwareYaml and the cacheDir
-	p.hardwareSpecFile = makeHardwareYaml(c, "")
-	defaultCacheDir = c.MkDir()
+	hardwareSpecFile = makeHardwareYaml(c, "")
+	cacheDir = c.MkDir()
 
 	// create mock assets/
 	makeMockAssetsDir(c)
@@ -192,8 +192,8 @@
 	}
 
 	// ensure nothing left behind
-	c.Assert(helpers.FileExists(filepath.Join(defaultCacheDir, "assets")), Equals, false)
-	c.Assert(helpers.FileExists(p.hardwareSpecFile), Equals, false)
+	c.Assert(helpers.FileExists(filepath.Join(cacheDir, "assets")), Equals, false)
+	c.Assert(helpers.FileExists(hardwareSpecFile), Equals, false)
 }
 
 func (s *PartitionTestSuite) TestHandleAssetsVerifyBootloader(c *C) {
@@ -203,7 +203,8 @@
 	c.Assert(err, IsNil)
 
 	// mock the hardwareYaml and the cacheDir
-	p.hardwareSpecFile = makeHardwareYaml(c, "bootloader: grub")
+	hardwareSpecFile = makeHardwareYaml(c, "bootloader: grub")
+	cacheDir = c.MkDir()
 
 	err = bootloader.HandleAssets()
 	c.Assert(err, NotNil)
@@ -216,18 +217,16 @@
 	c.Assert(err, IsNil)
 
 	// mock the hardwareYaml and the cacheDir
-	p.hardwareSpecFile = makeHardwareYaml(c, `
+	hardwareSpecFile = makeHardwareYaml(c, `
 bootloader: u-boot
 partition-layout: inplace
 `)
-
 	err = bootloader.HandleAssets()
 	c.Assert(err, NotNil)
 }
 
 func (s *PartitionTestSuite) TestHandleAssetsNoHardwareYaml(c *C) {
 	s.makeFakeUbootEnv(c)
-	defaultCacheDir = c.MkDir()
 
 	p := New()
 	bootloader, err := bootloader(p)
@@ -242,7 +241,7 @@
 	bootloader, err := bootloader(p)
 	c.Assert(err, IsNil)
 
-	p.hardwareSpecFile = makeHardwareYaml(c, `
+	hardwareSpecFile = makeHardwareYaml(c, `
 bootloader u-boot
 `)
 

=== added file 'partition/dirs.go'
--- partition/dirs.go	1970-01-01 00:00:00 +0000
+++ partition/dirs.go	2015-06-24 13:28:39 +0000
@@ -0,0 +1,41 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2014-2015 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package partition
+
+import (
+	"path/filepath"
+)
+
+// The full path to the cache directory, which is used as a
+// scratch pad, for downloading new images to and bind mounting the
+// rootfs.
+const cacheDirReal = "/writable/cache"
+
+var (
+	// useful for overwriting in the tests
+	cacheDir = cacheDirReal
+
+	// Directory to mount writable root filesystem below the cache
+	// diretory.
+	mountTargetReal = filepath.Join(cacheDir, "system")
+
+	// useful to override in tests
+	mountTarget = mountTargetReal
+)

=== added file 'partition/mount.go'
--- partition/mount.go	1970-01-01 00:00:00 +0000
+++ partition/mount.go	2015-06-24 13:28:39 +0000
@@ -0,0 +1,153 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2014-2015 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package partition
+
+import (
+	"fmt"
+	"sort"
+)
+
+// MountOption represents how the partition should be mounted, currently
+// RO (read-only) and RW (read-write) are supported
+type MountOption int
+
+const (
+	// RO mounts the partition read-only
+	RO MountOption = iota
+	// RW mounts the partition read-only
+	RW
+)
+
+// mountEntry represents a mount this package has created.
+type mountEntry struct {
+	source string
+	target string
+
+	options string
+
+	// true if target refers to a bind mount. We could derive this
+	// from options, but this field saves the effort.
+	bindMount bool
+}
+
+// mountEntryArray represents an array of mountEntry objects.
+type mountEntryArray []mountEntry
+
+// current mounts that this package has created.
+var mounts mountEntryArray
+
+// Len is part of the sort interface, required to allow sort to work
+// with an array of Mount objects.
+func (mounts mountEntryArray) Len() int {
+	return len(mounts)
+}
+
+// Less is part of the sort interface, required to allow sort to work
+// with an array of Mount objects.
+func (mounts mountEntryArray) Less(i, j int) bool {
+	return mounts[i].target < mounts[j].target
+}
+
+// Swap is part of the sort interface, required to allow sort to work
+// with an array of Mount objects.
+func (mounts mountEntryArray) Swap(i, j int) {
+	mounts[i], mounts[j] = mounts[j], mounts[i]
+}
+
+// removeMountByTarget removes the Mount specified by the target from
+// the global mounts array.
+func removeMountByTarget(mnts mountEntryArray, target string) (results mountEntryArray) {
+
+	for _, m := range mnts {
+		if m.target != target {
+			results = append(results, m)
+		}
+	}
+
+	return results
+}
+
+// undoMounts unmounts all mounts this package has mounted optionally
+// only unmounting bind mounts and leaving all remaining mounts.
+func undoMounts(bindMountsOnly bool) error {
+
+	mountsCopy := make(mountEntryArray, len(mounts), cap(mounts))
+	copy(mountsCopy, mounts)
+
+	// reverse sort to ensure unmounts are handled in the correct
+	// order.
+	sort.Sort(sort.Reverse(mountsCopy))
+
+	// Iterate backwards since we want a reverse-sorted list of
+	// mounts to ensure we can unmount in order.
+	for _, mount := range mountsCopy {
+		if bindMountsOnly && !mount.bindMount {
+			continue
+		}
+
+		if err := unmountAndRemoveFromGlobalMountList(mount.target); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// FIXME: use syscall.Mount() here
+func mount(source, target, options string) (err error) {
+	var args []string
+
+	args = append(args, "/bin/mount")
+	if options != "" {
+		args = append(args, fmt.Sprintf("-o%s", options))
+	}
+
+	args = append(args, source)
+	args = append(args, target)
+
+	return runCommand(args...)
+}
+
+// Mount the given directory and add it to the global mounts slice
+func mountAndAddToGlobalMountList(m mountEntry) (err error) {
+
+	err = mount(m.source, m.target, m.options)
+	if err == nil {
+		mounts = append(mounts, m)
+	}
+
+	return err
+}
+
+// Unmount the given directory and remove it from the global "mounts" slice
+func unmountAndRemoveFromGlobalMountList(target string) (err error) {
+	err = runCommand("/bin/umount", target)
+	if err != nil {
+		return err
+
+	}
+
+	results := removeMountByTarget(mounts, target)
+
+	// Update global
+	mounts = results
+
+	return nil
+}

=== added file 'partition/mount_test.go'
--- partition/mount_test.go	1970-01-01 00:00:00 +0000
+++ partition/mount_test.go	2015-06-24 13:28:39 +0000
@@ -0,0 +1,64 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2014-2015 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package partition
+
+import (
+	. "gopkg.in/check.v1"
+)
+
+func (s *PartitionTestSuite) TestMountEntryArray(c *C) {
+	mea := mountEntryArray{}
+
+	c.Assert(mea.Len(), Equals, 0)
+
+	me := mountEntry{source: "/dev",
+		target:    "/dev",
+		options:   "bind",
+		bindMount: true}
+
+	mea = append(mea, me)
+	c.Assert(mea.Len(), Equals, 1)
+
+	me = mountEntry{source: "/foo",
+		target:    "/foo",
+		options:   "",
+		bindMount: false}
+
+	mea = append(mea, me)
+	c.Assert(mea.Len(), Equals, 2)
+
+	c.Assert(mea.Less(0, 1), Equals, true)
+	c.Assert(mea.Less(1, 0), Equals, false)
+
+	mea.Swap(0, 1)
+	c.Assert(mea.Less(0, 1), Equals, false)
+	c.Assert(mea.Less(1, 0), Equals, true)
+
+	results := removeMountByTarget(mea, "invalid")
+
+	// No change expected
+	c.Assert(results, DeepEquals, mea)
+
+	results = removeMountByTarget(mea, "/dev")
+
+	c.Assert(len(results), Equals, 1)
+	c.Assert(results[0], Equals, mountEntry{source: "/foo",
+		target: "/foo", options: "", bindMount: false})
+}

=== modified file 'partition/partition.go'
--- partition/partition.go	2015-06-09 17:43:20 +0000
+++ partition/partition.go	2015-06-24 13:28:39 +0000
@@ -23,50 +23,37 @@
 import (
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"os/signal"
 	"path/filepath"
 	"regexp"
-	"sort"
 	"strings"
+	"sync"
 	"syscall"
 
-	"gopkg.in/yaml.v2"
-
 	"launchpad.net/snappy/logger"
 )
 
-var signalHandlerRegistered = false
-
-// Name of writable user data partition label as created by
-// ubuntu-device-flash(1).
-const writablePartitionLabel = "writable"
-
-// Name of primary root filesystem partition label as created by
-// ubuntu-device-flash(1).
-const rootfsAlabel = "system-a"
-
-// Name of primary root filesystem partition label as created by
-// ubuntu-device-flash(1). Note that this partition will
-// only be present if this is an A/B upgrade system.
-const rootfsBlabel = "system-b"
-
-// name of boot partition label as created by ubuntu-device-flash(1).
-const bootPartitionLabel = "system-boot"
-
-// its useful to override this in tests
-const realDefaultCacheDir = "/writable/cache"
-
-// FIXME: Should query system-image-cli (see bug LP:#1380574).
-var defaultCacheDir = realDefaultCacheDir
-
-// Directory to mount writable root filesystem below the cache
-// diretory.
-const mountTarget = "system"
-
-// File creation mode used when any directories are created
-const dirMode = 0750
+const (
+	// Name of writable user data partition label as created by
+	// ubuntu-device-flash(1).
+	writablePartitionLabel = "writable"
+
+	// Name of primary root filesystem partition label as created by
+	// ubuntu-device-flash(1).
+	rootfsAlabel = "system-a"
+
+	// Name of primary root filesystem partition label as created by
+	// ubuntu-device-flash(1). Note that this partition will
+	// only be present if this is an A/B upgrade system.
+	rootfsBlabel = "system-b"
+
+	// name of boot partition label as created by ubuntu-device-flash(1).
+	bootPartitionLabel = "system-boot"
+
+	// File creation mode used when any directories are created
+	dirMode = 0750
+)
 
 var (
 	// ErrBootloader is returned if the bootloader can not be determined
@@ -79,41 +66,6 @@
 	// ErrNoDualPartition is returned if you try to use a dual
 	// partition feature on a single partition
 	ErrNoDualPartition = errors.New("No dual partition")
-
-	// ErrNoHardwareYaml is returned when no hardware yaml is found in
-	// the update, this means that there is nothing to process with regards
-	// to device parts.
-	ErrNoHardwareYaml = errors.New("no hardware.yaml")
-)
-
-// Declarative specification of the type of system which specifies such
-// details as:
-//
-// - the location of initrd+kernel within the system-image archive.
-// - the location of hardware-specific .dtb files within the
-//   system-image archive.
-// - the type of bootloader that should be used for this system.
-// - expected system partition layout (single or dual rootfs's).
-const hardwareSpecFile = "hardware.yaml"
-
-// Directory that _may_ get automatically created on unpack that
-// contains updated hardware-specific boot assets (such as initrd, kernel)
-const assetsDir = "assets"
-
-// Directory that _may_ get automatically created on unpack that
-// contains updated hardware-specific assets that require flashing
-// to the disk (such as uBoot, MLO)
-const flashAssetsDir = "flashtool-assets"
-
-// MountOption represents how the partition should be mounted, currently
-// RO (read-only) and RW (read-write) are supported
-type MountOption int
-
-const (
-	// RO mounts the partition read-only
-	RO MountOption = iota
-	// RW mounts the partition read-only
-	RW
 )
 
 // Interface provides the interface to interact with a partition
@@ -134,24 +86,6 @@
 	BootloaderDir() string
 }
 
-// mountEntry represents a mount this package has created.
-type mountEntry struct {
-	source string
-	target string
-
-	options string
-
-	// true if target refers to a bind mount. We could derive this
-	// from options, but this field saves the effort.
-	bindMount bool
-}
-
-// mountEntryArray represents an array of mountEntry objects.
-type mountEntryArray []mountEntry
-
-// current mounts that this package has created.
-var mounts mountEntryArray
-
 // Partition is the type to interact with the partition
 type Partition struct {
 	// all partitions
@@ -159,8 +93,6 @@
 
 	// just root partitions
 	roots []string
-
-	hardwareSpecFile string
 }
 
 type blockDevice struct {
@@ -178,77 +110,10 @@
 	mountpoint string
 }
 
-// Representation of HARDWARE_SPEC_FILE
-type hardwareSpecType struct {
-	Kernel          string         `yaml:"kernel"`
-	Initrd          string         `yaml:"initrd"`
-	DtbDir          string         `yaml:"dtbs"`
-	PartitionLayout string         `yaml:"partition-layout"`
-	Bootloader      bootloaderName `yaml:"bootloader"`
-}
-
-// Len is part of the sort interface, required to allow sort to work
-// with an array of Mount objects.
-func (mounts mountEntryArray) Len() int {
-	return len(mounts)
-}
-
-// Less is part of the sort interface, required to allow sort to work
-// with an array of Mount objects.
-func (mounts mountEntryArray) Less(i, j int) bool {
-	return mounts[i].target < mounts[j].target
-}
-
-// Swap is part of the sort interface, required to allow sort to work
-// with an array of Mount objects.
-func (mounts mountEntryArray) Swap(i, j int) {
-	mounts[i], mounts[j] = mounts[j], mounts[i]
-}
-
-// removeMountByTarget removes the Mount specified by the target from
-// the global mounts array.
-func removeMountByTarget(mnts mountEntryArray, target string) (results mountEntryArray) {
-
-	for _, m := range mnts {
-		if m.target != target {
-			results = append(results, m)
-		}
-	}
-
-	return results
-}
+var once sync.Once
 
 func init() {
-	if !signalHandlerRegistered {
-		setupSignalHandler()
-		signalHandlerRegistered = true
-	}
-}
-
-// undoMounts unmounts all mounts this package has mounted optionally
-// only unmounting bind mounts and leaving all remaining mounts.
-func undoMounts(bindMountsOnly bool) error {
-
-	mountsCopy := make(mountEntryArray, len(mounts), cap(mounts))
-	copy(mountsCopy, mounts)
-
-	// reverse sort to ensure unmounts are handled in the correct
-	// order.
-	sort.Sort(sort.Reverse(mountsCopy))
-
-	// Iterate backwards since we want a reverse-sorted list of
-	// mounts to ensure we can unmount in order.
-	for _, mount := range mountsCopy {
-		if bindMountsOnly && !mount.bindMount {
-			continue
-		}
-
-		if err := unmountAndRemoveFromGlobalMountList(mount.target); err != nil {
-			return err
-		}
-	}
-
-	return nil
+	once.Do(setupSignalHandler)
 }
 
 func signalHandler(sig os.Signal) {
@@ -291,66 +156,6 @@
 	return labels
 }
 
-func mount(source, target, options string) (err error) {
-	var args []string
-
-	args = append(args, "/bin/mount")
-	if options != "" {
-		args = append(args, fmt.Sprintf("-o%s", options))
-	}
-
-	args = append(args, source)
-	args = append(args, target)
-
-	return runCommand(args...)
-}
-
-// Mount the given directory and add it to the global mounts slice
-func mountAndAddToGlobalMountList(m mountEntry) (err error) {
-
-	err = mount(m.source, m.target, m.options)
-	if err == nil {
-		mounts = append(mounts, m)
-	}
-
-	return err
-}
-
-// Unmount the given directory and remove it from the global "mounts" slice
-func unmountAndRemoveFromGlobalMountList(target string) (err error) {
-	err = runCommand("/bin/umount", target)
-	if err != nil {
-		return err
-
-	}
-
-	results := removeMountByTarget(mounts, target)
-
-	// Update global
-	mounts = results
-
-	return nil
-}
-
-// Run fsck(8) on specified device.
-func fsck(device string) (err error) {
-	return runCommand(
-		"/sbin/fsck",
-		"-M", // Paranoia - don't fsck if already mounted
-		"-av", device)
-}
-
-// Returns the position of the string in the given slice or -1 if its not found
-func stringInSlice(slice []string, value string) int {
-	for i, s := range slice {
-		if s == value {
-			return i
-		}
-	}
-
-	return -1
-}
-
 var runLsblk = func() (out []string, err error) {
 	output, err := runCommandWithStdout(
 		"/bin/lsblk",
@@ -443,20 +248,11 @@
 	return partitions, nil
 }
 
-var makeDirectory = func(path string, mode os.FileMode) error {
-	return os.MkdirAll(path, mode)
-}
-
-func (p *Partition) makeMountPoint() (err error) {
-	return makeDirectory(p.MountTarget(), dirMode)
-}
-
 // New creates a new partition type
 func New() *Partition {
 	p := new(Partition)
 
 	p.getPartitionDetails()
-	p.hardwareSpecFile = filepath.Join(p.cacheDir(), hardwareSpecFile)
 
 	return p
 }
@@ -500,7 +296,7 @@
 		}()
 	}
 
-	err = f(p.MountTarget())
+	err = f(mountTarget)
 	return err
 }
 
@@ -542,47 +338,6 @@
 	return isNextBootOther(bootloader)
 }
 
-// Returns the full path to the cache directory, which is used as a
-// scratch pad, for downloading new images to and bind mounting the
-// rootfs.
-func (p *Partition) cacheDir() string {
-	return defaultCacheDir
-}
-
-func (p *Partition) hardwareSpec() (hardwareSpecType, error) {
-	h := hardwareSpecType{}
-
-	data, err := ioutil.ReadFile(p.hardwareSpecFile)
-	// if hardware.yaml does not exist it just means that there was no
-	// device part in the update.
-	if os.IsNotExist(err) {
-		return h, ErrNoHardwareYaml
-	} else if err != nil {
-		return h, err
-	}
-
-	if err := yaml.Unmarshal([]byte(data), &h); err != nil {
-		return h, err
-	}
-
-	return h, nil
-}
-
-// Return full path to the main assets directory
-func (p *Partition) assetsDir() string {
-	return filepath.Join(p.cacheDir(), assetsDir)
-}
-
-// Return the full path to the hardware-specific flash assets directory.
-func (p *Partition) flashAssetsDir() string {
-	return filepath.Join(p.cacheDir(), flashAssetsDir)
-}
-
-// MountTarget gets the full path to the mount target directory
-func (p *Partition) MountTarget() string {
-	return filepath.Join(p.cacheDir(), mountTarget)
-}
-
 func (p *Partition) getPartitionDetails() (err error) {
 	p.partitions, err = loadPartitionDetails()
 	if err != nil {
@@ -676,11 +431,13 @@
 func (p *Partition) mountOtherRootfs(readOnly bool) (err error) {
 	var other *blockDevice
 
-	p.makeMountPoint()
+	if err := os.MkdirAll(mountTarget, dirMode); err != nil {
+		return err
+	}
 
 	other = p.otherRootPartition()
 
-	m := mountEntry{source: other.device, target: p.MountTarget()}
+	m := mountEntry{source: other.device, target: mountTarget}
 
 	if readOnly {
 		m.options = "ro"
@@ -707,9 +464,7 @@
 
 // Ensure the other partition is mounted read-only.
 func (p *Partition) ensureOtherMountedRO() (err error) {
-	mountpoint := p.MountTarget()
-
-	if err = runCommand("/bin/mountpoint", mountpoint); err == nil {
+	if err = runCommand("/bin/mountpoint", mountTarget); err == nil {
 		// already mounted
 		return err
 	}
@@ -741,14 +496,14 @@
 
 		return mountAndAddToGlobalMountList(mountEntry{
 			source: other.device,
-			target: p.MountTarget()})
+			target: mountTarget})
 	}
 	// r/w -> r/o: no fsck required.
-	return mount(other.device, p.MountTarget(), "remount,ro")
+	return mount(other.device, mountTarget, "remount,ro")
 }
 
 func (p *Partition) unmountOtherRootfs() (err error) {
-	return unmountAndRemoveFromGlobalMountList(p.MountTarget())
+	return unmountAndRemoveFromGlobalMountList(mountTarget)
 }
 
 // The bootloader requires a few filesystems to be mounted when
@@ -773,7 +528,7 @@
 	}
 
 	for _, fs := range requiredChrootMounts {
-		target := filepath.Join(p.MountTarget(), fs)
+		target := filepath.Join(mountTarget, fs)
 
 		err := mountAndAddToGlobalMountList(mountEntry{source: fs,
 			target:    target,
@@ -787,11 +542,11 @@
 	// Grub also requires access to both rootfs's when run from
 	// within a chroot (to allow it to create menu entries for
 	// both), so bindmount the real rootfs.
-	targetInChroot := filepath.Join(p.MountTarget(), p.MountTarget())
+	targetInChroot := filepath.Join(mountTarget, mountTarget)
 
 	// FIXME: we should really remove this after the unmount
 
-	if err = makeDirectory(targetInChroot, dirMode); err != nil {
+	if err = os.MkdirAll(targetInChroot, dirMode); err != nil {
 		return err
 	}
 

=== modified file 'partition/partition_test.go'
--- partition/partition_test.go	2015-06-09 17:43:20 +0000
+++ partition/partition_test.go	2015-06-24 13:28:39 +0000
@@ -44,14 +44,13 @@
 	return err
 }
 
-func mockMakeDirectory(path string, mode os.FileMode) error {
-	return nil
-}
-
 func (s *PartitionTestSuite) SetUpTest(c *C) {
 	s.tempdir = c.MkDir()
 	runLsblk = mockRunLsblkDualSnappy
 
+	// custom mount target
+	mountTarget = c.MkDir()
+
 	// setup fake paths for grub
 	bootloaderGrubDir = filepath.Join(s.tempdir, "boot", "grub")
 	bootloaderGrubConfigFile = filepath.Join(bootloaderGrubDir, "grub.cfg")
@@ -72,8 +71,10 @@
 
 	// always restore what we might have mocked away
 	runCommand = runCommandImpl
-	defaultCacheDir = realDefaultCacheDir
 	bootloader = bootloaderImpl
+	cacheDir = cacheDirReal
+	hardwareSpecFile = hardwareSpecFileReal
+	mountTarget = mountTargetReal
 
 	// grub vars
 	bootloaderGrubConfigFile = bootloaderGrubConfigFileReal
@@ -109,20 +110,6 @@
 	return tmp.Name()
 }
 
-func (s *PartitionTestSuite) TestHardwareSpec(c *C) {
-	p := New()
-	c.Assert(p, NotNil)
-
-	p.hardwareSpecFile = makeHardwareYaml(c, "")
-	hw, err := p.hardwareSpec()
-	c.Assert(err, IsNil)
-	c.Assert(hw.Kernel, Equals, "assets/vmlinuz")
-	c.Assert(hw.Initrd, Equals, "assets/initrd.img")
-	c.Assert(hw.DtbDir, Equals, "assets/dtbs")
-	c.Assert(hw.PartitionLayout, Equals, bootloaderSystemAB)
-	c.Assert(hw.Bootloader, Equals, bootloaderNameUboot)
-}
-
 func mockRunLsblkDualSnappy() (output []string, err error) {
 	dualData := `
 NAME="sda" LABEL="" PKNAME="" MOUNTPOINT=""
@@ -136,46 +123,6 @@
 	return strings.Split(dualData, "\n"), err
 }
 
-func (s *PartitionTestSuite) TestMountEntryArray(c *C) {
-	mea := mountEntryArray{}
-
-	c.Assert(mea.Len(), Equals, 0)
-
-	me := mountEntry{source: "/dev",
-		target:    "/dev",
-		options:   "bind",
-		bindMount: true}
-
-	mea = append(mea, me)
-	c.Assert(mea.Len(), Equals, 1)
-
-	me = mountEntry{source: "/foo",
-		target:    "/foo",
-		options:   "",
-		bindMount: false}
-
-	mea = append(mea, me)
-	c.Assert(mea.Len(), Equals, 2)
-
-	c.Assert(mea.Less(0, 1), Equals, true)
-	c.Assert(mea.Less(1, 0), Equals, false)
-
-	mea.Swap(0, 1)
-	c.Assert(mea.Less(0, 1), Equals, false)
-	c.Assert(mea.Less(1, 0), Equals, true)
-
-	results := removeMountByTarget(mea, "invalid")
-
-	// No change expected
-	c.Assert(results, DeepEquals, mea)
-
-	results = removeMountByTarget(mea, "/dev")
-
-	c.Assert(len(results), Equals, 1)
-	c.Assert(results[0], Equals, mountEntry{source: "/foo",
-		target: "/foo", options: "", bindMount: false})
-}
-
 func (s *PartitionTestSuite) TestSnappyDualRoot(c *C) {
 	p := New()
 	c.Assert(p.dualRootPartitions(), Equals, true)
@@ -218,14 +165,13 @@
 		return nil
 	})
 	c.Assert(err, IsNil)
-	c.Assert(reportedRoot, Equals, (&Partition{}).MountTarget())
+	c.Assert(reportedRoot, Equals, mountTarget)
 }
 
 func (s *PartitionTestSuite) TestRunWithOtherDualParitionRWFuncErr(c *C) {
 	c.Assert(mounts, DeepEquals, mountEntryArray(nil))
 
 	runCommand = mockRunCommand
-	makeDirectory = mockMakeDirectory
 
 	p := New()
 	err := p.RunWithOther(RW, func(otherRoot string) (err error) {
@@ -239,8 +185,12 @@
 	// ensure cleanup happend
 
 	// FIXME: mounts are global
-	expected := mountEntry{source: "/dev/sda4",
-		target: "/writable/cache/system", options: "", bindMount: false}
+	expected := mountEntry{
+		source:    "/dev/sda4",
+		target:    mountTarget,
+		options:   "",
+		bindMount: false,
+	}
 
 	// At program exit, "other" should still be mounted
 	c.Assert(mounts, DeepEquals, mountEntryArray{expected})
@@ -295,8 +245,12 @@
 	c.Assert(p, NotNil)
 
 	p.mountOtherRootfs(false)
-	expected := mountEntry{source: "/dev/sda4",
-		target: "/writable/cache/system", options: "", bindMount: false}
+	expected := mountEntry{
+		source:    "/dev/sda4",
+		target:    mountTarget,
+		options:   "",
+		bindMount: false,
+	}
 
 	c.Assert(mounts, DeepEquals, mountEntryArray{expected})
 
@@ -313,26 +267,26 @@
 
 	p.bindmountRequiredFilesystems()
 	c.Assert(mounts, DeepEquals, mountEntryArray{
-		mountEntry{source: "/dev", target: p.MountTarget() + "/dev",
-			options: "bind", bindMount: true},
-
-		mountEntry{source: "/proc", target: p.MountTarget() + "/proc",
-			options: "bind", bindMount: true},
-
-		mountEntry{source: "/sys", target: p.MountTarget() + "/sys",
-			options: "bind", bindMount: true},
-		mountEntry{source: "/boot/efi", target: p.MountTarget() + "/boot/efi",
+		mountEntry{source: "/dev", target: mountTarget + "/dev",
+			options: "bind", bindMount: true},
+
+		mountEntry{source: "/proc", target: mountTarget + "/proc",
+			options: "bind", bindMount: true},
+
+		mountEntry{source: "/sys", target: mountTarget + "/sys",
+			options: "bind", bindMount: true},
+		mountEntry{source: "/boot/efi", target: mountTarget + "/boot/efi",
 			options: "bind", bindMount: true},
 
 		// this comes from the grub bootloader via AdditionalBindMounts
-		mountEntry{source: "/boot/grub", target: p.MountTarget() + "/boot/grub",
+		mountEntry{source: "/boot/grub", target: mountTarget + "/boot/grub",
 			options: "bind", bindMount: true},
 
 		// Required to allow grub inside the chroot to access
 		// the "current" rootfs outside the chroot (used
 		// to generate the grub menuitems).
 		mountEntry{source: "/",
-			target:  p.MountTarget() + p.MountTarget(),
+			target:  mountTarget + mountTarget,
 			options: "bind,ro", bindMount: true},
 	})
 	p.unmountRequiredFilesystems()
@@ -351,23 +305,23 @@
 	p.bindmountRequiredFilesystems()
 	c.Assert(mounts, DeepEquals, mountEntryArray{
 
-		mountEntry{source: "/dev/sda4", target: "/writable/cache/system",
+		mountEntry{source: "/dev/sda4", target: mountTarget,
 			options: "", bindMount: false},
 
-		mountEntry{source: "/dev", target: p.MountTarget() + "/dev",
-			options: "bind", bindMount: true},
-
-		mountEntry{source: "/proc", target: p.MountTarget() + "/proc",
-			options: "bind", bindMount: true},
-
-		mountEntry{source: "/sys", target: p.MountTarget() + "/sys",
-			options: "bind", bindMount: true},
-
-		mountEntry{source: "/boot/efi", target: p.MountTarget() + "/boot/efi",
+		mountEntry{source: "/dev", target: mountTarget + "/dev",
+			options: "bind", bindMount: true},
+
+		mountEntry{source: "/proc", target: mountTarget + "/proc",
+			options: "bind", bindMount: true},
+
+		mountEntry{source: "/sys", target: mountTarget + "/sys",
+			options: "bind", bindMount: true},
+
+		mountEntry{source: "/boot/efi", target: mountTarget + "/boot/efi",
 			options: "bind", bindMount: true},
 
 		mountEntry{source: "/",
-			target:  p.MountTarget() + p.MountTarget(),
+			target:  mountTarget + mountTarget,
 			options: "bind,ro", bindMount: true},
 	})
 
@@ -375,8 +329,12 @@
 	undoMounts(true)
 
 	c.Assert(mounts, DeepEquals, mountEntryArray{
-		mountEntry{source: "/dev/sda4", target: "/writable/cache/system",
-			options: "", bindMount: false},
+		mountEntry{
+			source:    "/dev/sda4",
+			target:    mountTarget,
+			options:   "",
+			bindMount: false,
+		},
 	})
 
 	// should unmount everything

=== modified file 'partition/utils.go'
--- partition/utils.go	2015-05-19 14:09:19 +0000
+++ partition/utils.go	2015-06-24 13:28:39 +0000
@@ -69,3 +69,22 @@
 
 // This is a var instead of a function to making mocking in the tests easier
 var runCommandWithStdout = runCommandWithStdoutImpl
+
+// Run fsck(8) on specified device.
+func fsck(device string) (err error) {
+	return runCommand(
+		"/sbin/fsck",
+		"-M", // Paranoia - don't fsck if already mounted
+		"-av", device)
+}
+
+// Returns the position of the string in the given slice or -1 if its not found
+func stringInSlice(slice []string, value string) int {
+	for i, s := range slice {
+		if s == value {
+			return i
+		}
+	}
+
+	return -1
+}