[lxc-devel] [lxd/master] Rework ZFS utility functions

[albertodonato on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 505 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170808/a7763d4b/attachment.bin>
-------------- next part --------------
From 8d4208ef81731d0dac5659534b77ec742914c21b Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 10:07:01 +0200
Subject: [PATCH 01/21] storage/zfs: move some helper functions to
 storage_zfs_utils.go

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 18 ------------------
 lxd/storage_zfs_utils.go | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index e74958356..643d675bf 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -36,24 +36,6 @@ func (s *storageZfs) getOnDiskPoolName() string {
 	return s.pool.Name
 }
 
-func zfsIsEnabled() bool {
-	out, err := exec.LookPath("zfs")
-	if err != nil || len(out) == 0 {
-		return false
-	}
-
-	return true
-}
-
-func zfsModuleVersionGet() (string, error) {
-	zfsVersion, err := ioutil.ReadFile("/sys/module/zfs/version")
-	if err != nil {
-		return "", fmt.Errorf("could not determine ZFS module version")
-	}
-
-	return strings.TrimSpace(string(zfsVersion)), nil
-}
-
 // Only initialize the minimal information we need about a given storage type.
 func (s *storageZfs) StorageCoreInit() error {
 	s.sType = storageTypeZfs
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 03ff896fb..fb827ff49 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -2,7 +2,9 @@ package main
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -13,6 +15,26 @@ import (
 	"github.com/lxc/lxd/shared/logger"
 )
 
+// zfsIsEnabled returns whether zfs backend is supported.
+func zfsIsEnabled() bool {
+	out, err := exec.LookPath("zfs")
+	if err != nil || len(out) == 0 {
+		return false
+	}
+
+	return true
+}
+
+// zfsModuleVersionGet returhs the ZFS module version
+func zfsModuleVersionGet() (string, error) {
+	zfsVersion, err := ioutil.ReadFile("/sys/module/zfs/version")
+	if err != nil {
+		return "", fmt.Errorf("could not determine ZFS module version")
+	}
+
+	return strings.TrimSpace(string(zfsVersion)), nil
+}
+
 // zfsPoolVolumeCreate creates a ZFS dataset with a set of given properties.
 func zfsPoolVolumeCreate(dataset string, properties ...string) (string, error) {
 	cmd := []string{"zfs", "create"}

From 413b08bcdeb6b743290cf487b5319e1fbd8a4c2d Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 11:07:09 +0200
Subject: [PATCH 02/21] storage/zfs: make zfsPoolCheck a standalone function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs_utils.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index fb827ff49..0b77e2086 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -55,7 +55,7 @@ func zfsPoolVolumeSet(dataset string, key string, value string) (string, error)
 		dataset)
 }
 
-func (s *storageZfs) zfsPoolCheck(pool string) error {
+func zfsPoolCheck(pool string) error {
 	output, err := shared.RunCommand(
 		"zfs", "get", "type", "-H", "-o", "value", pool)
 	if err != nil {
@@ -165,7 +165,7 @@ func (s *storageZfs) zfsPoolCreate() error {
 					}
 				}
 			} else {
-				err := s.zfsPoolCheck(vdev)
+				err := zfsPoolCheck(vdev)
 				if err != nil {
 					return err
 				}

From d785aba42beeaf375f412ba10dabd1f7e204e5cb Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 11:21:31 +0200
Subject: [PATCH 03/21] storage/zfs: make zfsFilesystemEntityDelete a
 standalone function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       |  2 +-
 lxd/storage_zfs_utils.go | 10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 643d675bf..019cd042c 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -140,7 +140,7 @@ func (s *storageZfs) StoragePoolCreate() error {
 func (s *storageZfs) StoragePoolDelete() error {
 	logger.Infof("Deleting ZFS storage pool \"%s\".", s.pool.Name)
 
-	err := s.zfsFilesystemEntityDelete()
+	err := zfsFilesystemEntityDelete(s.pool.Config["source"], s.getOnDiskPoolName())
 	if err != nil {
 		return err
 	}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 0b77e2086..2f58f40ed 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -320,23 +320,21 @@ func (s *storageZfs) zfsPoolVolumeClone(source string, name string, dest string,
 	return nil
 }
 
-func (s *storageZfs) zfsFilesystemEntityDelete() error {
+func zfsFilesystemEntityDelete(vdev string, pool string) error {
 	var output string
 	var err error
-	poolName := s.getOnDiskPoolName()
-	if strings.Contains(poolName, "/") {
+	if strings.Contains(pool, "/") {
 		// Command to destroy a zfs dataset.
-		output, err = shared.RunCommand("zfs", "destroy", "-r", poolName)
+		output, err = shared.RunCommand("zfs", "destroy", "-r", pool)
 	} else {
 		// Command to destroy a zfs pool.
-		output, err = shared.RunCommand("zpool", "destroy", "-f", poolName)
+		output, err = shared.RunCommand("zpool", "destroy", "-f", pool)
 	}
 	if err != nil {
 		return fmt.Errorf("Failed to delete the ZFS pool: %s", output)
 	}
 
 	// Cleanup storage
-	vdev := s.pool.Config["source"]
 	if filepath.IsAbs(vdev) && !shared.IsBlockdevPath(vdev) {
 		os.RemoveAll(vdev)
 	}

From e87df547d40a3d3792039917700f50fc714b129b Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 11:31:17 +0200
Subject: [PATCH 04/21] storage/zfs: drop zfsPoolVolumeMount

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 10 +++++-----
 lxd/storage_zfs_utils.go |  4 ----
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 019cd042c..3b8078d02 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -192,7 +192,7 @@ func (s *storageZfs) StoragePoolVolumeCreate() error {
 	}
 
 	if !shared.IsMountPoint(customPoolVolumeMntPoint) {
-		s.zfsPoolVolumeMount(fs)
+		zfsMount(poolName, fs)
 	}
 
 	// apply quota
@@ -271,7 +271,7 @@ func (s *storageZfs) StoragePoolVolumeMount() (bool, error) {
 	var customerr error
 	ourMount := false
 	if !shared.IsMountPoint(customPoolVolumeMntPoint) {
-		customerr = s.zfsPoolVolumeMount(fs)
+		customerr = zfsMount(s.getOnDiskPoolName(), fs)
 		ourMount = true
 	}
 
@@ -1521,7 +1521,7 @@ func (s *storageZfs) ContainerSnapshotStart(container container) (bool, error) {
 		return false, err
 	}
 
-	err = s.zfsPoolVolumeMount(destFs)
+	err = zfsMount(s.getOnDiskPoolName(), destFs)
 	if err != nil {
 		return false, err
 	}
@@ -1649,7 +1649,7 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 
 	// Make sure that the image actually got mounted.
 	if !shared.IsMountPoint(tmpImageDir) {
-		s.zfsPoolVolumeMount(fs)
+		zfsMount(poolName, fs)
 	}
 
 	// Unpack the image into the temporary mountpoint.
@@ -2078,7 +2078,7 @@ func (s *storageZfs) MigrationSink(live bool, container container, snapshots []*
 	 * but sometimes it doesn't. Let's try to mount, but not complain about
 	 * failure.
 	 */
-	s.zfsPoolVolumeMount(zfsName)
+	zfsMount(poolName, zfsName)
 	return nil
 }
 
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 2f58f40ed..885556209 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -620,10 +620,6 @@ func zfsMount(poolName string, path string) error {
 	return nil
 }
 
-func (s *storageZfs) zfsPoolVolumeMount(path string) error {
-	return zfsMount(s.getOnDiskPoolName(), path)
-}
-
 func zfsUmount(poolName string, path string, mountpoint string) error {
 	output, err := shared.TryRunCommand(
 		"zfs",

From f379576a30c3fa641a182a4b7768015050be0a20 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 11:34:46 +0200
Subject: [PATCH 05/21] storage/zfs: drop zfsPoolVolumeUmount

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 8 ++++----
 lxd/storage_zfs_utils.go | 4 ----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 3b8078d02..bfd27f07e 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -314,7 +314,7 @@ func (s *storageZfs) StoragePoolVolumeUmount() (bool, error) {
 	var customerr error
 	ourUmount := false
 	if shared.IsMountPoint(customPoolVolumeMntPoint) {
-		customerr = s.zfsPoolVolumeUmount(fs, customPoolVolumeMntPoint)
+		customerr = zfsUmount(s.getOnDiskPoolName(), fs, customPoolVolumeMntPoint)
 		ourUmount = true
 	}
 
@@ -514,7 +514,7 @@ func (s *storageZfs) ContainerUmount(name string, path string) (bool, error) {
 	var imgerr error
 	ourUmount := false
 	if shared.IsMountPoint(containerPoolVolumeMntPoint) {
-		imgerr = s.zfsPoolVolumeUmount(fs, containerPoolVolumeMntPoint)
+		imgerr = zfsUmount(s.getOnDiskPoolName(), fs, containerPoolVolumeMntPoint)
 		ourUmount = true
 	}
 
@@ -1672,7 +1672,7 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 
 	// Make sure that the image actually got unmounted.
 	if shared.IsMountPoint(tmpImageDir) {
-		s.zfsPoolVolumeUmount(fs, tmpImageDir)
+		zfsUmount(poolName, fs, tmpImageDir)
 	}
 
 	// Create a snapshot of that image on the storage pool which we clone for
@@ -1975,7 +1975,7 @@ func (s *storageZfs) MigrationSink(live bool, container container, snapshots []*
 	zfsName := fmt.Sprintf("containers/%s", container.Name())
 	containerMntPoint := getContainerMountPoint(s.pool.Name, container.Name())
 	if shared.IsMountPoint(containerMntPoint) {
-		err := s.zfsPoolVolumeUmount(zfsName, containerMntPoint)
+		err := zfsUmount(poolName, zfsName, containerMntPoint)
 		if err != nil {
 			return err
 		}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 885556209..ce1c9f13c 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -637,10 +637,6 @@ func zfsUmount(poolName string, path string, mountpoint string) error {
 	return nil
 }
 
-func (s *storageZfs) zfsPoolVolumeUmount(path string, mountpoint string) error {
-	return zfsUmount(s.getOnDiskPoolName(), path, mountpoint)
-}
-
 func (s *storageZfs) zfsPoolListSubvolumes(path string) ([]string, error) {
 	output, err := shared.RunCommand(
 		"zfs",

From 105d447947b07fb4de86c3614bd699c71b980981 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 11:41:19 +0200
Subject: [PATCH 06/21] storage/zfs: make zfsPoolListSnapshots a standalone
 function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       |  6 +++---
 lxd/storage_zfs_utils.go | 13 ++++++-------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index bfd27f07e..5055b3efd 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -710,7 +710,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 
 	if s.zfsFilesystemEntityExists(fs, true) {
 		removable := true
-		snaps, err := s.zfsPoolListSnapshots(fs)
+		snaps, err := zfsPoolListSnapshots(s.getOnDiskPoolName(), fs)
 		if err != nil {
 			return err
 		}
@@ -1898,7 +1898,7 @@ func (s *storageZfs) MigrationSource(ct container, containerOnly bool) (Migratio
 	* is that we send the oldest to newest snapshot, hopefully saving on
 	* xfer costs. Then, after all that, we send the container itself.
 	 */
-	snapshots, err := s.zfsPoolListSnapshots(fmt.Sprintf("containers/%s", ct.Name()))
+	snapshots, err := zfsPoolListSnapshots(s.getOnDiskPoolName(), fmt.Sprintf("containers/%s", ct.Name()))
 	if err != nil {
 		return nil, err
 	}
@@ -2043,7 +2043,7 @@ func (s *storageZfs) MigrationSink(live bool, container container, snapshots []*
 
 	defer func() {
 		/* clean up our migration-send snapshots that we got from recv. */
-		zfsSnapshots, err := s.zfsPoolListSnapshots(fmt.Sprintf("containers/%s", container.Name()))
+		zfsSnapshots, err := zfsPoolListSnapshots(poolName, fmt.Sprintf("containers/%s", container.Name()))
 		if err != nil {
 			logger.Errorf("failed listing snapshots post migration: %s.", err)
 			return
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index ce1c9f13c..21adaefd9 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -291,7 +291,7 @@ func (s *storageZfs) zfsPoolVolumeClone(source string, name string, dest string,
 	}
 
 	for _, sub := range subvols {
-		snaps, err := s.zfsPoolListSnapshots(sub)
+		snaps, err := zfsPoolListSnapshots(poolName, sub)
 		if err != nil {
 			return err
 		}
@@ -391,7 +391,7 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 
 				// Check if the parent can now be deleted
 				subPath := strings.SplitN(path, "@", 2)[0]
-				snaps, err := s.zfsPoolListSnapshots(subPath)
+				snaps, err := zfsPoolListSnapshots(s.getOnDiskPoolName(), subPath)
 				if err != nil {
 					return err
 				}
@@ -570,7 +570,7 @@ func (s *storageZfs) zfsPoolVolumeSnapshotRestore(path string, name string) erro
 	}
 
 	for _, sub := range subvols {
-		snaps, err := s.zfsPoolListSnapshots(sub)
+		snaps, err := zfsPoolListSnapshots(poolName, sub)
 		if err != nil {
 			return err
 		}
@@ -667,12 +667,11 @@ func (s *storageZfs) zfsPoolListSubvolumes(path string) ([]string, error) {
 	return children, nil
 }
 
-func (s *storageZfs) zfsPoolListSnapshots(path string) ([]string, error) {
-	poolName := s.getOnDiskPoolName()
+func zfsPoolListSnapshots(pool string, path string) ([]string, error) {
 	path = strings.TrimRight(path, "/")
-	fullPath := poolName
+	fullPath := pool
 	if path != "" {
-		fullPath = fmt.Sprintf("%s/%s", poolName, path)
+		fullPath = fmt.Sprintf("%s/%s", pool, path)
 	}
 
 	output, err := shared.RunCommand(

From 9584d741868b19f6ef75fe9d504cbda0646a1459 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 12:00:52 +0200
Subject: [PATCH 07/21] storage/zfs: make zfsPoolListSubvolumes a standalone
 function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs_utils.go | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 21adaefd9..d816c0e32 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -170,7 +170,7 @@ func (s *storageZfs) zfsPoolCreate() error {
 					return err
 				}
 
-				subvols, err := s.zfsPoolListSubvolumes(vdev)
+				subvols, err := zfsPoolListSubvolumes(zpoolName, vdev)
 				if err != nil {
 					return err
 				}
@@ -285,7 +285,7 @@ func (s *storageZfs) zfsPoolVolumeClone(source string, name string, dest string,
 		return fmt.Errorf("Failed to clone the filesystem: %s", output)
 	}
 
-	subvols, err := s.zfsPoolListSubvolumes(fmt.Sprintf("%s/%s", poolName, source))
+	subvols, err := zfsPoolListSubvolumes(poolName, fmt.Sprintf("%s/%s", poolName, source))
 	if err != nil {
 		return err
 	}
@@ -564,7 +564,7 @@ func (s *storageZfs) zfsPoolVolumeSnapshotRestore(path string, name string) erro
 		return fmt.Errorf("Failed to restore ZFS snapshot: %s", output)
 	}
 
-	subvols, err := s.zfsPoolListSubvolumes(fmt.Sprintf("%s/%s", poolName, path))
+	subvols, err := zfsPoolListSubvolumes(poolName, fmt.Sprintf("%s/%s", poolName, path))
 	if err != nil {
 		return err
 	}
@@ -637,7 +637,7 @@ func zfsUmount(poolName string, path string, mountpoint string) error {
 	return nil
 }
 
-func (s *storageZfs) zfsPoolListSubvolumes(path string) ([]string, error) {
+func zfsPoolListSubvolumes(pool string, path string) ([]string, error) {
 	output, err := shared.RunCommand(
 		"zfs",
 		"list",
@@ -660,8 +660,7 @@ func (s *storageZfs) zfsPoolListSubvolumes(path string) ([]string, error) {
 			continue
 		}
 
-		poolName := s.getOnDiskPoolName()
-		children = append(children, strings.TrimPrefix(entry, fmt.Sprintf("%s/", poolName)))
+		children = append(children, strings.TrimPrefix(entry, fmt.Sprintf("%s/", pool)))
 	}
 
 	return children, nil
@@ -726,7 +725,7 @@ func (s *storageZfs) zfsPoolVolumeSnapshotRemovable(path string, name string) (b
 
 func (s *storageZfs) zfsPoolGetUsers() ([]string, error) {
 	poolName := s.getOnDiskPoolName()
-	subvols, err := s.zfsPoolListSubvolumes(poolName)
+	subvols, err := zfsPoolListSubvolumes(poolName, poolName)
 	if err != nil {
 		return []string{}, err
 	}

From a9b41e81d94351236606439e10f714933d384cbd Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 12:19:20 +0200
Subject: [PATCH 08/21] storage/zfs: make zfsFilesystemEntityPropertyGet a
 standalone function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 14 +++++++++-----
 lxd/storage_zfs_utils.go | 28 +++++++++++++++-------------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 5055b3efd..3927474d4 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -704,13 +704,14 @@ func (s *storageZfs) ContainerCanRestore(container container, sourceContainer co
 func (s *storageZfs) ContainerDelete(container container) error {
 	logger.Debugf("Deleting ZFS storage volume for container \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
 
+	poolName := s.getOnDiskPoolName()
 	containerName := container.Name()
 	fs := fmt.Sprintf("containers/%s", containerName)
 	containerPoolVolumeMntPoint := getContainerMountPoint(s.pool.Name, containerName)
 
 	if s.zfsFilesystemEntityExists(fs, true) {
 		removable := true
-		snaps, err := zfsPoolListSnapshots(s.getOnDiskPoolName(), fs)
+		snaps, err := zfsPoolListSnapshots(poolName, fs)
 		if err != nil {
 			return err
 		}
@@ -728,7 +729,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 		}
 
 		if removable {
-			origin, err := s.zfsFilesystemEntityPropertyGet(fs, "origin", true)
+			origin, err := zfsFilesystemEntityPropertyGet(poolName, fs, "origin", true)
 			if err != nil {
 				return err
 			}
@@ -1309,7 +1310,7 @@ func (s *storageZfs) ContainerGetUsage(container container) (int64, error) {
 		property = "usedbydataset"
 	}
 
-	value, err := s.zfsFilesystemEntityPropertyGet(fs, property, true)
+	value, err := zfsFilesystemEntityPropertyGet(s.getOnDiskPoolName(), fs, property, true)
 	if err != nil {
 		return -1, err
 	}
@@ -1381,7 +1382,9 @@ func (s *storageZfs) ContainerSnapshotDelete(snapshotContainer container) error
 				return err
 			}
 		} else {
-			err = s.zfsPoolVolumeSnapshotRename(fmt.Sprintf("containers/%s", sourceContainerName), snapName, fmt.Sprintf("copy-%s", uuid.NewRandom().String()))
+			err = zfsPoolVolumeSnapshotRename(
+				s.getOnDiskPoolName(), fmt.Sprintf("containers/%s", sourceContainerName), snapName,
+				fmt.Sprintf("copy-%s", uuid.NewRandom().String()))
 			if err != nil {
 				return err
 			}
@@ -1455,7 +1458,8 @@ func (s *storageZfs) ContainerSnapshotRename(snapshotContainer container, newNam
 	newZfsDatasetName := fmt.Sprintf("snapshot-%s", newSnapOnlyName)
 
 	if oldZfsDatasetName != newZfsDatasetName {
-		err := s.zfsPoolVolumeSnapshotRename(fmt.Sprintf("containers/%s", oldcName), oldZfsDatasetName, newZfsDatasetName)
+		err := zfsPoolVolumeSnapshotRename(
+			s.getOnDiskPoolName(), fmt.Sprintf("containers/%s", oldcName), oldZfsDatasetName, newZfsDatasetName)
 		if err != nil {
 			return err
 		}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index d816c0e32..8cbc6e584 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -343,7 +343,8 @@ func zfsFilesystemEntityDelete(vdev string, pool string) error {
 }
 
 func (s *storageZfs) zfsPoolVolumeDestroy(path string) error {
-	mountpoint, err := s.zfsFilesystemEntityPropertyGet(path, "mountpoint", true)
+	poolName := s.getOnDiskPoolName()
+	mountpoint, err := zfsFilesystemEntityPropertyGet(poolName, path, "mountpoint", true)
 	if err != nil {
 		return err
 	}
@@ -356,7 +357,6 @@ func (s *storageZfs) zfsPoolVolumeDestroy(path string) error {
 		}
 	}
 
-	poolName := s.getOnDiskPoolName()
 	// Due to open fds or kernel refs, this may fail for a bit, give it 10s
 	output, err := shared.TryRunCommand(
 		"zfs",
@@ -373,6 +373,8 @@ func (s *storageZfs) zfsPoolVolumeDestroy(path string) error {
 }
 
 func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
+	poolName := s.getOnDiskPoolName()
+
 	if strings.HasPrefix(path, "deleted/") {
 		// Cleanup of filesystems kept for refcount reason
 		removablePath, err := s.zfsPoolVolumeSnapshotRemovable(path, "")
@@ -391,7 +393,7 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 
 				// Check if the parent can now be deleted
 				subPath := strings.SplitN(path, "@", 2)[0]
-				snaps, err := zfsPoolListSnapshots(s.getOnDiskPoolName(), subPath)
+				snaps, err := zfsPoolListSnapshots(poolName, subPath)
 				if err != nil {
 					return err
 				}
@@ -404,7 +406,7 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 				}
 			} else {
 				// Cleanup filesystems
-				origin, err := s.zfsFilesystemEntityPropertyGet(path, "origin", true)
+				origin, err := zfsFilesystemEntityPropertyGet(poolName, path, "origin", true)
 				if err != nil {
 					return err
 				}
@@ -439,13 +441,14 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 }
 
 func (s *storageZfs) zfsFilesystemEntityExists(path string, prefixPathWithPool bool) bool {
-	output, _ := s.zfsFilesystemEntityPropertyGet(path, "name", prefixPathWithPool)
+	poolName := s.getOnDiskPoolName()
+	output, _ := zfsFilesystemEntityPropertyGet(poolName, path, "name", prefixPathWithPool)
 
 	// If prefixPathWithPool is false we assume that the path passed in
 	// already is a valid zfs entity we want to check for.
 	fsToCheck := path
 	if prefixPathWithPool {
-		fsToCheck = fmt.Sprintf("%s/%s", s.getOnDiskPoolName(), path)
+		fsToCheck = fmt.Sprintf("%s/%s", poolName, path)
 	}
 	if output == fsToCheck {
 		return true
@@ -454,12 +457,12 @@ func (s *storageZfs) zfsFilesystemEntityExists(path string, prefixPathWithPool b
 	return false
 }
 
-func (s *storageZfs) zfsFilesystemEntityPropertyGet(path string, key string, prefixPathWithPool bool) (string, error) {
+func zfsFilesystemEntityPropertyGet(pool string, path string, key string, prefixPathWithPool bool) (string, error) {
 	// If prefixPathWithPool is false we assume that the path passed in
 	// already is a valid zfs entity we want to check for.
 	fsToCheck := path
 	if prefixPathWithPool {
-		fsToCheck = fmt.Sprintf("%s/%s", s.getOnDiskPoolName(), path)
+		fsToCheck = fmt.Sprintf("%s/%s", pool, path)
 	}
 
 	output, err := shared.RunCommand(
@@ -592,14 +595,13 @@ func (s *storageZfs) zfsPoolVolumeSnapshotRestore(path string, name string) erro
 	return nil
 }
 
-func (s *storageZfs) zfsPoolVolumeSnapshotRename(path string, oldName string, newName string) error {
-	poolName := s.getOnDiskPoolName()
+func zfsPoolVolumeSnapshotRename(pool string, path string, oldName string, newName string) error {
 	output, err := shared.RunCommand(
 		"zfs",
 		"rename",
 		"-r",
-		fmt.Sprintf("%s/%s@%s", poolName, path, oldName),
-		fmt.Sprintf("%s/%s@%s", poolName, path, newName))
+		fmt.Sprintf("%s/%s@%s", pool, path, oldName),
+		fmt.Sprintf("%s/%s@%s", pool, path, newName))
 	if err != nil {
 		logger.Errorf("zfs snapshot rename failed: %s.", output)
 		return fmt.Errorf("Failed to rename ZFS snapshot: %s", output)
@@ -711,7 +713,7 @@ func (s *storageZfs) zfsPoolVolumeSnapshotRemovable(path string, name string) (b
 		snap = fmt.Sprintf("%s@%s", path, name)
 	}
 
-	clones, err := s.zfsFilesystemEntityPropertyGet(snap, "clones", true)
+	clones, err := zfsFilesystemEntityPropertyGet(s.getOnDiskPoolName(), snap, "clones", true)
 	if err != nil {
 		return false, err
 	}

From 03cc6ba61304dcc6c87a6ad185d690af523408ba Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 12:28:27 +0200
Subject: [PATCH 09/21] storage/zfs: make zfsPoolVolumeDestroy a standalone
 function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 14 ++++++++------
 lxd/storage_zfs_utils.go | 14 ++++++--------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 3927474d4..e134d1d2b 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -220,7 +220,7 @@ func (s *storageZfs) StoragePoolVolumeDelete() error {
 	fs := fmt.Sprintf("custom/%s", s.volume.Name)
 	customPoolVolumeMntPoint := getStoragePoolVolumeMountPoint(s.pool.Name, s.volume.Name)
 
-	err := s.zfsPoolVolumeDestroy(fs)
+	err := zfsPoolVolumeDestroy(s.getOnDiskPoolName(), fs)
 	if err != nil {
 		return err
 	}
@@ -736,7 +736,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 			poolName := s.getOnDiskPoolName()
 			origin = strings.TrimPrefix(origin, fmt.Sprintf("%s/", poolName))
 
-			err = s.zfsPoolVolumeDestroy(fs)
+			err = zfsPoolVolumeDestroy(poolName, fs)
 			if err != nil {
 				return err
 			}
@@ -764,7 +764,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 	}
 
 	snapshotZfsDataset := fmt.Sprintf("snapshots/%s", containerName)
-	s.zfsPoolVolumeDestroy(snapshotZfsDataset)
+	zfsPoolVolumeDestroy(poolName, snapshotZfsDataset)
 
 	// Delete potential leftover snapshot mountpoints.
 	snapshotMntPoint := getSnapshotMountPoint(s.pool.Name, containerName)
@@ -790,6 +790,8 @@ func (s *storageZfs) ContainerDelete(container container) error {
 }
 
 func (s *storageZfs) copyWithoutSnapshotsSparse(target container, source container) error {
+	poolName := s.getOnDiskPoolName()
+
 	sourceContainerName := source.Name()
 	sourceContainerPath := source.Path()
 
@@ -839,7 +841,7 @@ func (s *storageZfs) copyWithoutSnapshotsSparse(target container, source contain
 			if !revert {
 				return
 			}
-			s.zfsPoolVolumeDestroy(targetZfsDataset)
+			zfsPoolVolumeDestroy(poolName, targetZfsDataset)
 		}()
 
 		ourMount, err := s.ContainerMount(target)
@@ -1540,7 +1542,7 @@ func (s *storageZfs) ContainerSnapshotStop(container container) (bool, error) {
 	cName, sName, _ := containerGetParentAndSnapshotName(container.Name())
 	destFs := fmt.Sprintf("snapshots/%s/%s", cName, sName)
 
-	err := s.zfsPoolVolumeDestroy(destFs)
+	err := zfsPoolVolumeDestroy(s.getOnDiskPoolName(), destFs)
 	if err != nil {
 		return false, err
 	}
@@ -1704,7 +1706,7 @@ func (s *storageZfs) ImageDelete(fingerprint string) error {
 		}
 
 		if removable {
-			err := s.zfsPoolVolumeDestroy(fs)
+			err := zfsPoolVolumeDestroy(s.getOnDiskPoolName(), fs)
 			if err != nil {
 				return err
 			}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 8cbc6e584..a963fc45d 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -342,9 +342,8 @@ func zfsFilesystemEntityDelete(vdev string, pool string) error {
 	return nil
 }
 
-func (s *storageZfs) zfsPoolVolumeDestroy(path string) error {
-	poolName := s.getOnDiskPoolName()
-	mountpoint, err := zfsFilesystemEntityPropertyGet(poolName, path, "mountpoint", true)
+func zfsPoolVolumeDestroy(pool string, path string) error {
+	mountpoint, err := zfsFilesystemEntityPropertyGet(pool, path, "mountpoint", true)
 	if err != nil {
 		return err
 	}
@@ -362,7 +361,7 @@ func (s *storageZfs) zfsPoolVolumeDestroy(path string) error {
 		"zfs",
 		"destroy",
 		"-r",
-		fmt.Sprintf("%s/%s", poolName, path))
+		fmt.Sprintf("%s/%s", pool, path))
 
 	if err != nil {
 		logger.Errorf("zfs destroy failed: %s.", output)
@@ -386,7 +385,7 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 		if removablePath {
 			if strings.Contains(path, "@") {
 				// Cleanup snapshots
-				err = s.zfsPoolVolumeDestroy(path)
+				err = zfsPoolVolumeDestroy(poolName, path)
 				if err != nil {
 					return err
 				}
@@ -410,10 +409,9 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 				if err != nil {
 					return err
 				}
-				poolName := s.getOnDiskPoolName()
 				origin = strings.TrimPrefix(origin, fmt.Sprintf("%s/", poolName))
 
-				err = s.zfsPoolVolumeDestroy(path)
+				err = zfsPoolVolumeDestroy(poolName, path)
 				if err != nil {
 					return err
 				}
@@ -431,7 +429,7 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 		}
 	} else if strings.HasPrefix(path, "containers") && strings.Contains(path, "@copy-") {
 		// Just remove the copy- snapshot for copies of active containers
-		err := s.zfsPoolVolumeDestroy(path)
+		err := zfsPoolVolumeDestroy(poolName, path)
 		if err != nil {
 			return err
 		}

From abca18bf9295c05da834ed2d7bb390443c066b01 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 12:35:09 +0200
Subject: [PATCH 10/21] storage/zfs: make zfsPoolVolumeSnapshotRemovable a
 standalone function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 9 +++++----
 lxd/storage_zfs_utils.go | 6 +++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index e134d1d2b..868818926 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -718,7 +718,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 
 		for _, snap := range snaps {
 			var err error
-			removable, err = s.zfsPoolVolumeSnapshotRemovable(fs, snap)
+			removable, err = zfsPoolVolumeSnapshotRemovable(poolName, fs, snap)
 			if err != nil {
 				return err
 			}
@@ -1377,7 +1377,7 @@ func (s *storageZfs) ContainerSnapshotDelete(snapshotContainer container) error
 	snapName := fmt.Sprintf("snapshot-%s", sourceContainerSnapOnlyName)
 
 	if s.zfsFilesystemEntityExists(fmt.Sprintf("containers/%s@%s", sourceContainerName, snapName), true) {
-		removable, err := s.zfsPoolVolumeSnapshotRemovable(fmt.Sprintf("containers/%s", sourceContainerName), snapName)
+		removable, err := zfsPoolVolumeSnapshotRemovable(s.getOnDiskPoolName(), fmt.Sprintf("containers/%s", sourceContainerName), snapName)
 		if removable {
 			err = s.zfsPoolVolumeSnapshotDestroy(fmt.Sprintf("containers/%s", sourceContainerName), snapName)
 			if err != nil {
@@ -1697,16 +1697,17 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 func (s *storageZfs) ImageDelete(fingerprint string) error {
 	logger.Debugf("Deleting ZFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
+	poolName := s.getOnDiskPoolName()
 	fs := fmt.Sprintf("images/%s", fingerprint)
 
 	if s.zfsFilesystemEntityExists(fs, true) {
-		removable, err := s.zfsPoolVolumeSnapshotRemovable(fs, "readonly")
+		removable, err := zfsPoolVolumeSnapshotRemovable(poolName, fs, "readonly")
 		if err != nil {
 			return err
 		}
 
 		if removable {
-			err := zfsPoolVolumeDestroy(s.getOnDiskPoolName(), fs)
+			err := zfsPoolVolumeDestroy(poolName, fs)
 			if err != nil {
 				return err
 			}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index a963fc45d..370ed7191 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -376,7 +376,7 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 
 	if strings.HasPrefix(path, "deleted/") {
 		// Cleanup of filesystems kept for refcount reason
-		removablePath, err := s.zfsPoolVolumeSnapshotRemovable(path, "")
+		removablePath, err := zfsPoolVolumeSnapshotRemovable(poolName, path, "")
 		if err != nil {
 			return err
 		}
@@ -703,7 +703,7 @@ func zfsPoolListSnapshots(pool string, path string) ([]string, error) {
 	return children, nil
 }
 
-func (s *storageZfs) zfsPoolVolumeSnapshotRemovable(path string, name string) (bool, error) {
+func zfsPoolVolumeSnapshotRemovable(pool string, path string, name string) (bool, error) {
 	var snap string
 	if name == "" {
 		snap = path
@@ -711,7 +711,7 @@ func (s *storageZfs) zfsPoolVolumeSnapshotRemovable(path string, name string) (b
 		snap = fmt.Sprintf("%s@%s", path, name)
 	}
 
-	clones, err := zfsFilesystemEntityPropertyGet(s.getOnDiskPoolName(), snap, "clones", true)
+	clones, err := zfsFilesystemEntityPropertyGet(pool, snap, "clones", true)
 	if err != nil {
 		return false, err
 	}

From 55e250124fd4e5e701c9ae22be949bfd3951e0af Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 12:37:14 +0200
Subject: [PATCH 11/21] storage/zfs: make zfsPoolVolumeCleanup a standalone
 function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       |  2 +-
 lxd/storage_zfs_utils.go | 22 ++++++++++------------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 868818926..c480c90ea 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -741,7 +741,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 				return err
 			}
 
-			err = s.zfsPoolVolumeCleanup(origin)
+			err = zfsPoolVolumeCleanup(poolName, origin)
 			if err != nil {
 				return err
 			}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 370ed7191..2781ec148 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -371,12 +371,10 @@ func zfsPoolVolumeDestroy(pool string, path string) error {
 	return nil
 }
 
-func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
-	poolName := s.getOnDiskPoolName()
-
+func zfsPoolVolumeCleanup(pool string, path string) error {
 	if strings.HasPrefix(path, "deleted/") {
 		// Cleanup of filesystems kept for refcount reason
-		removablePath, err := zfsPoolVolumeSnapshotRemovable(poolName, path, "")
+		removablePath, err := zfsPoolVolumeSnapshotRemovable(pool, path, "")
 		if err != nil {
 			return err
 		}
@@ -385,40 +383,40 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 		if removablePath {
 			if strings.Contains(path, "@") {
 				// Cleanup snapshots
-				err = zfsPoolVolumeDestroy(poolName, path)
+				err = zfsPoolVolumeDestroy(pool, path)
 				if err != nil {
 					return err
 				}
 
 				// Check if the parent can now be deleted
 				subPath := strings.SplitN(path, "@", 2)[0]
-				snaps, err := zfsPoolListSnapshots(poolName, subPath)
+				snaps, err := zfsPoolListSnapshots(pool, subPath)
 				if err != nil {
 					return err
 				}
 
 				if len(snaps) == 0 {
-					err := s.zfsPoolVolumeCleanup(subPath)
+					err := zfsPoolVolumeCleanup(pool, subPath)
 					if err != nil {
 						return err
 					}
 				}
 			} else {
 				// Cleanup filesystems
-				origin, err := zfsFilesystemEntityPropertyGet(poolName, path, "origin", true)
+				origin, err := zfsFilesystemEntityPropertyGet(pool, path, "origin", true)
 				if err != nil {
 					return err
 				}
-				origin = strings.TrimPrefix(origin, fmt.Sprintf("%s/", poolName))
+				origin = strings.TrimPrefix(origin, fmt.Sprintf("%s/", pool))
 
-				err = zfsPoolVolumeDestroy(poolName, path)
+				err = zfsPoolVolumeDestroy(pool, path)
 				if err != nil {
 					return err
 				}
 
 				// Attempt to remove its parent
 				if origin != "-" {
-					err := s.zfsPoolVolumeCleanup(origin)
+					err := zfsPoolVolumeCleanup(pool, origin)
 					if err != nil {
 						return err
 					}
@@ -429,7 +427,7 @@ func (s *storageZfs) zfsPoolVolumeCleanup(path string) error {
 		}
 	} else if strings.HasPrefix(path, "containers") && strings.Contains(path, "@copy-") {
 		// Just remove the copy- snapshot for copies of active containers
-		err := zfsPoolVolumeDestroy(poolName, path)
+		err := zfsPoolVolumeDestroy(pool, path)
 		if err != nil {
 			return err
 		}

From fdae529d644c4f028be4b393716c1c1d24a4f364 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 12:50:40 +0200
Subject: [PATCH 12/21] storage/zfs: make zfsPoolVolumeSnapshotRestore a
 standalone function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       |  2 +-
 lxd/storage_zfs_utils.go | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index c480c90ea..78f7ab805 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -1285,7 +1285,7 @@ func (s *storageZfs) ContainerRestore(target container, source container) error
 	cName, snapOnlyName, _ := containerGetParentAndSnapshotName(source.Name())
 	snapName := fmt.Sprintf("snapshot-%s", snapOnlyName)
 
-	err = s.zfsPoolVolumeSnapshotRestore(fmt.Sprintf("containers/%s", cName), snapName)
+	err = zfsPoolVolumeSnapshotRestore(s.getOnDiskPoolName(), fmt.Sprintf("containers/%s", cName), snapName)
 	if err != nil {
 		return err
 	}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 2781ec148..dcfcd8dd7 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -552,24 +552,23 @@ func (s *storageZfs) zfsPoolVolumeSnapshotDestroy(path string, name string) erro
 	return nil
 }
 
-func (s *storageZfs) zfsPoolVolumeSnapshotRestore(path string, name string) error {
-	poolName := s.getOnDiskPoolName()
+func zfsPoolVolumeSnapshotRestore(pool string, path string, name string) error {
 	output, err := shared.TryRunCommand(
 		"zfs",
 		"rollback",
-		fmt.Sprintf("%s/%s@%s", poolName, path, name))
+		fmt.Sprintf("%s/%s@%s", pool, path, name))
 	if err != nil {
 		logger.Errorf("zfs rollback failed: %s.", output)
 		return fmt.Errorf("Failed to restore ZFS snapshot: %s", output)
 	}
 
-	subvols, err := zfsPoolListSubvolumes(poolName, fmt.Sprintf("%s/%s", poolName, path))
+	subvols, err := zfsPoolListSubvolumes(pool, fmt.Sprintf("%s/%s", pool, path))
 	if err != nil {
 		return err
 	}
 
 	for _, sub := range subvols {
-		snaps, err := zfsPoolListSnapshots(poolName, sub)
+		snaps, err := zfsPoolListSnapshots(pool, sub)
 		if err != nil {
 			return err
 		}
@@ -581,7 +580,7 @@ func (s *storageZfs) zfsPoolVolumeSnapshotRestore(path string, name string) erro
 		output, err := shared.TryRunCommand(
 			"zfs",
 			"rollback",
-			fmt.Sprintf("%s/%s@%s", poolName, sub, name))
+			fmt.Sprintf("%s/%s@%s", pool, sub, name))
 		if err != nil {
 			logger.Errorf("zfs rollback failed: %s.", output)
 			return fmt.Errorf("Failed to restore ZFS sub-volume snapshot: %s", output)

From 29b6ef3fc8af49c448d0eb3b823217d673a0df14 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 12:54:32 +0200
Subject: [PATCH 13/21] storage/zfs: make zfsPoolVolumeSnapshotDestroy a
 standalone function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 24 +++++++++++++-----------
 lxd/storage_zfs_utils.go |  5 ++---
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 78f7ab805..2faac6b5c 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -822,7 +822,7 @@ func (s *storageZfs) copyWithoutSnapshotsSparse(target container, source contain
 				if !revert {
 					return
 				}
-				s.zfsPoolVolumeSnapshotDestroy(sourceZfsDataset, sourceZfsDatasetSnapshot)
+				zfsPoolVolumeSnapshotDestroy(poolName, sourceZfsDataset, sourceZfsDatasetSnapshot)
 			}()
 		}
 	} else {
@@ -921,7 +921,7 @@ func (s *storageZfs) copyWithoutSnapshotFull(target container, source container)
 			return err
 		}
 		defer func() {
-			err := s.zfsPoolVolumeSnapshotDestroy(fs, snapshotSuffix)
+			err := zfsPoolVolumeSnapshotDestroy(poolName, fs, snapshotSuffix)
 			if err != nil {
 				logger.Warnf("Failed to delete temporary ZFS snapshot \"%s\". Manual cleanup needed.", sourceDataset)
 			}
@@ -970,7 +970,7 @@ func (s *storageZfs) copyWithoutSnapshotFull(target container, source container)
 		return err
 	}
 
-	err = s.zfsPoolVolumeSnapshotDestroy(targetfs, snapshotSuffix)
+	err = zfsPoolVolumeSnapshotDestroy(poolName, targetfs, snapshotSuffix)
 	if err != nil {
 		return err
 	}
@@ -1140,8 +1140,8 @@ func (s *storageZfs) ContainerCopy(target container, source container, container
 			return err
 		}
 
-		s.zfsPoolVolumeSnapshotDestroy(fmt.Sprintf("containers/%s", source.Name()), tmpSnapshotName)
-		s.zfsPoolVolumeSnapshotDestroy(fmt.Sprintf("containers/%s", target.Name()), tmpSnapshotName)
+		zfsPoolVolumeSnapshotDestroy(poolName, fmt.Sprintf("containers/%s", source.Name()), tmpSnapshotName)
+		zfsPoolVolumeSnapshotDestroy(poolName, fmt.Sprintf("containers/%s", target.Name()), tmpSnapshotName)
 
 		fs := fmt.Sprintf("containers/%s", target.Name())
 
@@ -1373,19 +1373,21 @@ func (s *storageZfs) ContainerSnapshotCreate(snapshotContainer container, source
 func (s *storageZfs) ContainerSnapshotDelete(snapshotContainer container) error {
 	logger.Debugf("Deleting ZFS storage volume for snapshot \"%s\" on storage pool \"%s\".", s.volume.Name, s.pool.Name)
 
+	poolName := s.getOnDiskPoolName()
+
 	sourceContainerName, sourceContainerSnapOnlyName, _ := containerGetParentAndSnapshotName(snapshotContainer.Name())
 	snapName := fmt.Sprintf("snapshot-%s", sourceContainerSnapOnlyName)
 
 	if s.zfsFilesystemEntityExists(fmt.Sprintf("containers/%s@%s", sourceContainerName, snapName), true) {
 		removable, err := zfsPoolVolumeSnapshotRemovable(s.getOnDiskPoolName(), fmt.Sprintf("containers/%s", sourceContainerName), snapName)
 		if removable {
-			err = s.zfsPoolVolumeSnapshotDestroy(fmt.Sprintf("containers/%s", sourceContainerName), snapName)
+			err = zfsPoolVolumeSnapshotDestroy(poolName, fmt.Sprintf("containers/%s", sourceContainerName), snapName)
 			if err != nil {
 				return err
 			}
 		} else {
 			err = zfsPoolVolumeSnapshotRename(
-				s.getOnDiskPoolName(), fmt.Sprintf("containers/%s", sourceContainerName), snapName,
+				poolName, fmt.Sprintf("containers/%s", sourceContainerName), snapName,
 				fmt.Sprintf("copy-%s", uuid.NewRandom().String()))
 			if err != nil {
 				return err
@@ -1865,12 +1867,12 @@ func (s *zfsMigrationSourceDriver) SendAfterCheckpoint(conn *websocket.Conn, bwl
 }
 
 func (s *zfsMigrationSourceDriver) Cleanup() {
+	poolName := s.zfs.getOnDiskPoolName()
 	if s.stoppedSnapName != "" {
-		s.zfs.zfsPoolVolumeSnapshotDestroy(fmt.Sprintf("containers/%s", s.container.Name()), s.stoppedSnapName)
+		zfsPoolVolumeSnapshotDestroy(poolName, fmt.Sprintf("containers/%s", s.container.Name()), s.stoppedSnapName)
 	}
-
 	if s.runningSnapName != "" {
-		s.zfs.zfsPoolVolumeSnapshotDestroy(fmt.Sprintf("containers/%s", s.container.Name()), s.runningSnapName)
+		zfsPoolVolumeSnapshotDestroy(poolName, fmt.Sprintf("containers/%s", s.container.Name()), s.runningSnapName)
 	}
 }
 
@@ -2062,7 +2064,7 @@ func (s *storageZfs) MigrationSink(live bool, container container, snapshots []*
 				continue
 			}
 
-			s.zfsPoolVolumeSnapshotDestroy(fmt.Sprintf("containers/%s", container.Name()), snap)
+			zfsPoolVolumeSnapshotDestroy(poolName, fmt.Sprintf("containers/%s", container.Name()), snap)
 		}
 	}()
 
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index dcfcd8dd7..5e1ffc9cb 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -537,13 +537,12 @@ func (s *storageZfs) zfsPoolVolumeSnapshotCreate(path string, name string) error
 	return nil
 }
 
-func (s *storageZfs) zfsPoolVolumeSnapshotDestroy(path string, name string) error {
-	poolName := s.getOnDiskPoolName()
+func zfsPoolVolumeSnapshotDestroy(pool, path string, name string) error {
 	output, err := shared.RunCommand(
 		"zfs",
 		"destroy",
 		"-r",
-		fmt.Sprintf("%s/%s@%s", poolName, path, name))
+		fmt.Sprintf("%s/%s@%s", pool, path, name))
 	if err != nil {
 		logger.Errorf("zfs destroy failed: %s.", output)
 		return fmt.Errorf("Failed to destroy ZFS snapshot: %s", output)

From f3a1073b648e03c35942407d0b4edf846d11d571 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 12:59:33 +0200
Subject: [PATCH 14/21] storage/zfs: make zfsPoolVolumeSnapshotCreate a
 standalone function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 17 +++++++++--------
 lxd/storage_zfs_utils.go |  5 ++---
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 2faac6b5c..35f48c3d2 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -814,7 +814,7 @@ func (s *storageZfs) copyWithoutSnapshotsSparse(target container, source contain
 		if s.zfsFilesystemEntityExists(fmt.Sprintf("containers/%s", sourceName), true) {
 			sourceZfsDatasetSnapshot = fmt.Sprintf("copy-%s", uuid.NewRandom().String())
 			sourceZfsDataset = fmt.Sprintf("containers/%s", sourceName)
-			err := s.zfsPoolVolumeSnapshotCreate(sourceZfsDataset, sourceZfsDatasetSnapshot)
+			err := zfsPoolVolumeSnapshotCreate(poolName, sourceZfsDataset, sourceZfsDatasetSnapshot)
 			if err != nil {
 				return err
 			}
@@ -916,7 +916,7 @@ func (s *storageZfs) copyWithoutSnapshotFull(target container, source container)
 		targetSnapshotDataset = fmt.Sprintf("%s/containers/%s@%s", poolName, targetName, snapshotSuffix)
 
 		fs := fmt.Sprintf("containers/%s", sourceName)
-		err := s.zfsPoolVolumeSnapshotCreate(fs, snapshotSuffix)
+		err := zfsPoolVolumeSnapshotCreate(poolName, fs, snapshotSuffix)
 		if err != nil {
 			return err
 		}
@@ -1102,14 +1102,15 @@ func (s *storageZfs) ContainerCopy(target container, source container, container
 			}
 		}
 
+		poolName := s.getOnDiskPoolName()
+
 		// send actual container
 		tmpSnapshotName := fmt.Sprintf("copy-send-%s", uuid.NewRandom().String())
-		err = s.zfsPoolVolumeSnapshotCreate(fmt.Sprintf("containers/%s", source.Name()), tmpSnapshotName)
+		err = zfsPoolVolumeSnapshotCreate(poolName, fmt.Sprintf("containers/%s", source.Name()), tmpSnapshotName)
 		if err != nil {
 			return err
 		}
 
-		poolName := s.getOnDiskPoolName()
 		currentSnapshotDataset := fmt.Sprintf("%s/containers/%s@%s", poolName, source.Name(), tmpSnapshotName)
 		args := []string{"send", currentSnapshotDataset}
 		if prevSnapOnlyName != "" {
@@ -1335,7 +1336,7 @@ func (s *storageZfs) ContainerSnapshotCreate(snapshotContainer container, source
 	snapName := fmt.Sprintf("snapshot-%s", snapshotSnapOnlyName)
 
 	sourceZfsDataset := fmt.Sprintf("containers/%s", cName)
-	err := s.zfsPoolVolumeSnapshotCreate(sourceZfsDataset, snapName)
+	err := zfsPoolVolumeSnapshotCreate(s.getOnDiskPoolName(), sourceZfsDataset, snapName)
 	if err != nil {
 		return err
 	}
@@ -1685,7 +1686,7 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 
 	// Create a snapshot of that image on the storage pool which we clone for
 	// container creation.
-	err = s.zfsPoolVolumeSnapshotCreate(fs, "readonly")
+	err = zfsPoolVolumeSnapshotCreate(poolName, fs, "readonly")
 	if err != nil {
 		return err
 	}
@@ -1841,7 +1842,7 @@ func (s *zfsMigrationSourceDriver) SendWhileRunning(conn *websocket.Conn, op *op
 	}
 
 	s.runningSnapName = fmt.Sprintf("migration-send-%s", uuid.NewRandom().String())
-	if err := s.zfs.zfsPoolVolumeSnapshotCreate(fmt.Sprintf("containers/%s", s.container.Name()), s.runningSnapName); err != nil {
+	if err := zfsPoolVolumeSnapshotCreate(s.zfs.getOnDiskPoolName(), fmt.Sprintf("containers/%s", s.container.Name()), s.runningSnapName); err != nil {
 		return err
 	}
 
@@ -1855,7 +1856,7 @@ func (s *zfsMigrationSourceDriver) SendWhileRunning(conn *websocket.Conn, op *op
 
 func (s *zfsMigrationSourceDriver) SendAfterCheckpoint(conn *websocket.Conn, bwlimit string) error {
 	s.stoppedSnapName = fmt.Sprintf("migration-send-%s", uuid.NewRandom().String())
-	if err := s.zfs.zfsPoolVolumeSnapshotCreate(fmt.Sprintf("containers/%s", s.container.Name()), s.stoppedSnapName); err != nil {
+	if err := zfsPoolVolumeSnapshotCreate(s.zfs.getOnDiskPoolName(), fmt.Sprintf("containers/%s", s.container.Name()), s.stoppedSnapName); err != nil {
 		return err
 	}
 
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 5e1ffc9cb..a9b6c66f4 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -522,13 +522,12 @@ func (s *storageZfs) zfsPoolVolumeSet(path string, key string, value string) err
 	return nil
 }
 
-func (s *storageZfs) zfsPoolVolumeSnapshotCreate(path string, name string) error {
-	poolName := s.getOnDiskPoolName()
+func zfsPoolVolumeSnapshotCreate(pool string, path string, name string) error {
 	output, err := shared.RunCommand(
 		"zfs",
 		"snapshot",
 		"-r",
-		fmt.Sprintf("%s/%s@%s", poolName, path, name))
+		fmt.Sprintf("%s/%s@%s", pool, path, name))
 	if err != nil {
 		logger.Errorf("zfs snapshot failed: %s.", output)
 		return fmt.Errorf("Failed to create ZFS snapshot: %s", output)

From 4f9e393a6d2c22726f7928d6e0c3d4d2a042bab2 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 13:21:57 +0200
Subject: [PATCH 15/21] storage/zfs: make zfsPoolVolumeClone a standalone
 function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 10 ++++++----
 lxd/storage_zfs_utils.go | 17 ++++++++---------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 35f48c3d2..abf6c3a4b 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -603,6 +603,7 @@ func (s *storageZfs) ContainerCreateFromImage(container container, fingerprint s
 	fs := fmt.Sprintf("containers/%s", containerName)
 	containerPoolVolumeMntPoint := getContainerMountPoint(s.pool.Name, containerName)
 
+	poolName := s.getOnDiskPoolName()
 	fsImage := fmt.Sprintf("images/%s", fingerprint)
 
 	imageStoragePoolLockID := getImageCreateLockID(s.pool.Name, fingerprint)
@@ -633,7 +634,7 @@ func (s *storageZfs) ContainerCreateFromImage(container container, fingerprint s
 		}
 	}
 
-	err := s.zfsPoolVolumeClone(fsImage, "readonly", fs, containerPoolVolumeMntPoint)
+	err := zfsPoolVolumeClone(poolName, fsImage, "readonly", fs, containerPoolVolumeMntPoint)
 	if err != nil {
 		return err
 	}
@@ -833,7 +834,7 @@ func (s *storageZfs) copyWithoutSnapshotsSparse(target container, source contain
 	}
 
 	if sourceZfsDataset != "" {
-		err := s.zfsPoolVolumeClone(sourceZfsDataset, sourceZfsDatasetSnapshot, targetZfsDataset, targetContainerMountPoint)
+		err := zfsPoolVolumeClone(poolName, sourceZfsDataset, sourceZfsDatasetSnapshot, targetZfsDataset, targetContainerMountPoint)
 		if err != nil {
 			return err
 		}
@@ -1524,13 +1525,14 @@ func (s *storageZfs) ContainerSnapshotStart(container container) (bool, error) {
 	sourceSnap := fmt.Sprintf("snapshot-%s", sName)
 	destFs := fmt.Sprintf("snapshots/%s/%s", cName, sName)
 
+	poolName := s.getOnDiskPoolName()
 	snapshotMntPoint := getSnapshotMountPoint(s.pool.Name, container.Name())
-	err := s.zfsPoolVolumeClone(sourceFs, sourceSnap, destFs, snapshotMntPoint)
+	err := zfsPoolVolumeClone(poolName, sourceFs, sourceSnap, destFs, snapshotMntPoint)
 	if err != nil {
 		return false, err
 	}
 
-	err = zfsMount(s.getOnDiskPoolName(), destFs)
+	err = zfsMount(poolName, destFs)
 	if err != nil {
 		return false, err
 	}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index a9b6c66f4..c0490da9d 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -270,28 +270,27 @@ func (s *storageZfs) zfsPoolCreate() error {
 	return nil
 }
 
-func (s *storageZfs) zfsPoolVolumeClone(source string, name string, dest string, mountpoint string) error {
-	poolName := s.getOnDiskPoolName()
+func zfsPoolVolumeClone(pool string, source string, name string, dest string, mountpoint string) error {
 	output, err := shared.RunCommand(
 		"zfs",
 		"clone",
 		"-p",
 		"-o", fmt.Sprintf("mountpoint=%s", mountpoint),
 		"-o", "canmount=noauto",
-		fmt.Sprintf("%s/%s@%s", poolName, source, name),
-		fmt.Sprintf("%s/%s", poolName, dest))
+		fmt.Sprintf("%s/%s@%s", pool, source, name),
+		fmt.Sprintf("%s/%s", pool, dest))
 	if err != nil {
 		logger.Errorf("zfs clone failed: %s.", output)
 		return fmt.Errorf("Failed to clone the filesystem: %s", output)
 	}
 
-	subvols, err := zfsPoolListSubvolumes(poolName, fmt.Sprintf("%s/%s", poolName, source))
+	subvols, err := zfsPoolListSubvolumes(pool, fmt.Sprintf("%s/%s", pool, source))
 	if err != nil {
 		return err
 	}
 
 	for _, sub := range subvols {
-		snaps, err := zfsPoolListSnapshots(poolName, sub)
+		snaps, err := zfsPoolListSnapshots(pool, sub)
 		if err != nil {
 			return err
 		}
@@ -301,7 +300,7 @@ func (s *storageZfs) zfsPoolVolumeClone(source string, name string, dest string,
 		}
 
 		destSubvol := dest + strings.TrimPrefix(sub, source)
-		snapshotMntPoint := getSnapshotMountPoint(s.pool.Name, destSubvol)
+		snapshotMntPoint := getSnapshotMountPoint(pool, destSubvol)
 
 		output, err := shared.RunCommand(
 			"zfs",
@@ -309,8 +308,8 @@ func (s *storageZfs) zfsPoolVolumeClone(source string, name string, dest string,
 			"-p",
 			"-o", fmt.Sprintf("mountpoint=%s", snapshotMntPoint),
 			"-o", "canmount=noauto",
-			fmt.Sprintf("%s/%s@%s", poolName, sub, name),
-			fmt.Sprintf("%s/%s", poolName, destSubvol))
+			fmt.Sprintf("%s/%s@%s", pool, sub, name),
+			fmt.Sprintf("%s/%s", pool, destSubvol))
 		if err != nil {
 			logger.Errorf("zfs clone failed: %s.", output)
 			return fmt.Errorf("Failed to clone the sub-volume: %s", output)

From 90baa25bddcc8ab67f2b1106ca25e5984ca40e60 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 15:52:04 +0200
Subject: [PATCH 16/21] storage/zfs: make zfsPoolVolumeSet a standalone
 function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 38 ++++++++++++++++++--------------------
 lxd/storage_zfs_utils.go | 24 ++++++++----------------
 2 files changed, 26 insertions(+), 36 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index abf6c3a4b..24b1e2956 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -186,7 +186,7 @@ func (s *storageZfs) StoragePoolVolumeCreate() error {
 		s.StoragePoolVolumeDelete()
 	}()
 
-	err = s.zfsPoolVolumeSet(fs, "mountpoint", customPoolVolumeMntPoint)
+	err = zfsPoolVolumeSet(poolName, fs, "mountpoint", customPoolVolumeMntPoint)
 	if err != nil {
 		return err
 	}
@@ -566,7 +566,7 @@ func (s *storageZfs) ContainerCreate(container container) error {
 	}()
 
 	// Set mountpoint.
-	err = s.zfsPoolVolumeSet(fs, "mountpoint", containerPoolVolumeMntPoint)
+	err = zfsPoolVolumeSet(poolName, fs, "mountpoint", containerPoolVolumeMntPoint)
 	if err != nil {
 		return err
 	}
@@ -747,7 +747,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 				return err
 			}
 		} else {
-			err := s.zfsPoolVolumeSet(fs, "mountpoint", "none")
+			err := zfsPoolVolumeSet(poolName, fs, "mountpoint", "none")
 			if err != nil {
 				return err
 			}
@@ -961,12 +961,12 @@ func (s *storageZfs) copyWithoutSnapshotFull(target container, source container)
 	targetContainerMountPoint := getContainerMountPoint(s.pool.Name, targetName)
 	targetfs := fmt.Sprintf("containers/%s", targetName)
 
-	err = s.zfsPoolVolumeSet(targetfs, "canmount", "noauto")
+	err = zfsPoolVolumeSet(poolName, targetfs, "canmount", "noauto")
 	if err != nil {
 		return err
 	}
 
-	err = s.zfsPoolVolumeSet(targetfs, "mountpoint", targetContainerMountPoint)
+	err = zfsPoolVolumeSet(poolName, targetfs, "mountpoint", targetContainerMountPoint)
 	if err != nil {
 		return err
 	}
@@ -1146,13 +1146,12 @@ func (s *storageZfs) ContainerCopy(target container, source container, container
 		zfsPoolVolumeSnapshotDestroy(poolName, fmt.Sprintf("containers/%s", target.Name()), tmpSnapshotName)
 
 		fs := fmt.Sprintf("containers/%s", target.Name())
-
-		err = s.zfsPoolVolumeSet(fs, "canmount", "noauto")
+		err = zfsPoolVolumeSet(poolName, fs, "canmount", "noauto")
 		if err != nil {
 			return err
 		}
 
-		err = s.zfsPoolVolumeSet(fs, "mountpoint", targetContainerMountPoint)
+		err = zfsPoolVolumeSet(poolName, fs, "mountpoint", targetContainerMountPoint)
 		if err != nil {
 			return err
 		}
@@ -1166,6 +1165,7 @@ func (s *storageZfs) ContainerCopy(target container, source container, container
 func (s *storageZfs) ContainerRename(container container, newName string) error {
 	logger.Debugf("Renaming ZFS storage volume for container \"%s\" from %s -> %s.", s.volume.Name, s.volume.Name, newName)
 
+	poolName := s.getOnDiskPoolName()
 	oldName := container.Name()
 
 	// Unmount the dataset.
@@ -1191,7 +1191,7 @@ func (s *storageZfs) ContainerRename(container container, newName string) error
 
 	// Set the new mountpoint for the dataset.
 	newContainerMntPoint := getContainerMountPoint(s.pool.Name, newName)
-	err = s.zfsPoolVolumeSet(newZfsDataset, "mountpoint", newContainerMntPoint)
+	err = zfsPoolVolumeSet(poolName, newZfsDataset, "mountpoint", newContainerMntPoint)
 	if err != nil {
 		return err
 	}
@@ -1571,6 +1571,7 @@ func (s *storageZfs) ContainerSnapshotCreateEmpty(snapshotContainer container) e
 func (s *storageZfs) ImageCreate(fingerprint string) error {
 	logger.Debugf("Creating ZFS storage volume for image \"%s\" on storage pool \"%s\".", fingerprint, s.pool.Name)
 
+	poolName := s.getOnDiskPoolName()
 	imageMntPoint := getImageMountPoint(s.pool.Name, fingerprint)
 	fs := fmt.Sprintf("images/%s", fingerprint)
 	revert := true
@@ -1602,7 +1603,7 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 
 		// In case this is an image from an older lxd instance, wipe the
 		// mountpoint.
-		err = s.zfsPoolVolumeSet(fs, "mountpoint", "none")
+		err = zfsPoolVolumeSet(poolName, fs, "mountpoint", "none")
 		if err != nil {
 			return err
 		}
@@ -1637,7 +1638,6 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 	imagePath := shared.VarPath("images", fingerprint)
 
 	// Create a new storage volume on the storage pool for the image.
-	poolName := s.getOnDiskPoolName()
 	dataset := fmt.Sprintf("%s/%s", poolName, fs)
 	msg, err := zfsPoolVolumeCreate(dataset, "mountpoint=none")
 	if err != nil {
@@ -1653,7 +1653,7 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 	}()
 
 	// Set a temporary mountpoint for the image.
-	err = s.zfsPoolVolumeSet(fs, "mountpoint", tmpImageDir)
+	err = zfsPoolVolumeSet(poolName, fs, "mountpoint", tmpImageDir)
 	if err != nil {
 		return err
 	}
@@ -1670,14 +1670,12 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 	}
 
 	// Mark the new storage volume for the image as readonly.
-	err = s.zfsPoolVolumeSet(fs, "readonly", "on")
-	if err != nil {
+	if err = zfsPoolVolumeSet(poolName, fs, "readonly", "on"); err != nil {
 		return err
 	}
 
 	// Remove the temporary mountpoint from the image storage volume.
-	err = s.zfsPoolVolumeSet(fs, "mountpoint", "none")
-	if err != nil {
+	if err = zfsPoolVolumeSet(poolName, fs, "mountpoint", "none"); err != nil {
 		return err
 	}
 
@@ -1717,8 +1715,7 @@ func (s *storageZfs) ImageDelete(fingerprint string) error {
 				return err
 			}
 		} else {
-			err := s.zfsPoolVolumeSet(fs, "mountpoint", "none")
-			if err != nil {
+			if err := zfsPoolVolumeSet(poolName, fs, "mountpoint", "none"); err != nil {
 				return err
 			}
 
@@ -2124,11 +2121,12 @@ func (s *storageZfs) StorageEntitySetQuota(volumeType int, size int64, data inte
 		property = "refquota"
 	}
 
+	poolName := s.getOnDiskPoolName()
 	var err error
 	if size > 0 {
-		err = s.zfsPoolVolumeSet(fs, property, fmt.Sprintf("%d", size))
+		err = zfsPoolVolumeSet(poolName, fs, property, fmt.Sprintf("%d", size))
 	} else {
-		err = s.zfsPoolVolumeSet(fs, property, "none")
+		err = zfsPoolVolumeSet(poolName, fs, property, "none")
 	}
 
 	if err != nil {
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index c0490da9d..d3b16beca 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -48,13 +48,6 @@ func zfsPoolVolumeCreate(dataset string, properties ...string) (string, error) {
 	return shared.RunCommand(cmd[0], cmd[1:]...)
 }
 
-func zfsPoolVolumeSet(dataset string, key string, value string) (string, error) {
-	return shared.RunCommand("zfs",
-		"set",
-		fmt.Sprintf("%s=%s", key, value),
-		dataset)
-}
-
 func zfsPoolCheck(pool string) error {
 	output, err := shared.RunCommand(
 		"zfs", "get", "type", "-H", "-o", "value", pool)
@@ -158,9 +151,7 @@ func (s *storageZfs) zfsPoolCreate() error {
 						return fmt.Errorf("Failed to create ZFS filesystem: %s", output)
 					}
 				} else {
-					msg, err := zfsPoolVolumeSet(vdev, "mountpoint", "none")
-					if err != nil {
-						logger.Errorf("zfs set failed to unset dataset mountpoint %s", msg)
+					if err := zfsPoolVolumeSet(vdev, "", "mountpoint", "none"); err != nil {
 						return err
 					}
 				}
@@ -179,9 +170,7 @@ func (s *storageZfs) zfsPoolCreate() error {
 					return fmt.Errorf("Provided ZFS pool (or dataset) isn't empty")
 				}
 
-				msg, err := zfsPoolVolumeSet(vdev, "mountpoint", "none")
-				if err != nil {
-					logger.Errorf("zfs set failed to unset dataset mountpoint %s", msg)
+				if err := zfsPoolVolumeSet(vdev, "", "mountpoint", "none"); err != nil {
 					return err
 				}
 			}
@@ -506,13 +495,16 @@ func (s *storageZfs) zfsPoolVolumeRename(source string, dest string) error {
 	return fmt.Errorf("Failed to rename ZFS filesystem: %s", output)
 }
 
-func (s *storageZfs) zfsPoolVolumeSet(path string, key string, value string) error {
-	poolName := s.getOnDiskPoolName()
+func zfsPoolVolumeSet(pool string, path string, key string, value string) error {
+	vdev := pool
+	if path != "" {
+		vdev = fmt.Sprintf("%s/%s", pool, path)
+	}
 	output, err := shared.RunCommand(
 		"zfs",
 		"set",
 		fmt.Sprintf("%s=%s", key, value),
-		fmt.Sprintf("%s/%s", poolName, path))
+		vdev)
 	if err != nil {
 		logger.Errorf("zfs set failed: %s.", output)
 		return fmt.Errorf("Failed to set ZFS config: %s", output)

From 265e69ada96534b31726dfcfbe3ce6ee32372db0 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 18:40:45 +0200
Subject: [PATCH 17/21] storage/zfs: drop unused zfsPoolGetUsers

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs_utils.go | 35 -----------------------------------
 1 file changed, 35 deletions(-)

diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index d3b16beca..3373fae9f 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -709,41 +709,6 @@ func zfsPoolVolumeSnapshotRemovable(pool string, path string, name string) (bool
 	return false, nil
 }
 
-func (s *storageZfs) zfsPoolGetUsers() ([]string, error) {
-	poolName := s.getOnDiskPoolName()
-	subvols, err := zfsPoolListSubvolumes(poolName, poolName)
-	if err != nil {
-		return []string{}, err
-	}
-
-	exceptions := []string{
-		"containers",
-		"images",
-		"snapshots",
-		"deleted",
-		"deleted/containers",
-		"deleted/images"}
-
-	users := []string{}
-	for _, subvol := range subvols {
-		path := strings.Split(subvol, "/")
-
-		// Only care about plausible LXD paths
-		if !shared.StringInSlice(path[0], exceptions) {
-			continue
-		}
-
-		// Ignore empty paths
-		if shared.StringInSlice(subvol, exceptions) {
-			continue
-		}
-
-		users = append(users, subvol)
-	}
-
-	return users, nil
-}
-
 func zfsFilesystemEntityExists(zfsEntity string) bool {
 	output, err := shared.RunCommand(
 		"zfs",

From 245fb1ac6c7e96c5ef0961ed49f395ef090a5a42 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Mon, 7 Aug 2017 19:02:02 +0200
Subject: [PATCH 18/21] storage/zfs: make zfsFilesystemEntityExists a
 standalone function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 21 ++++++++++-----------
 lxd/storage_zfs_utils.go | 36 +++++++++---------------------------
 2 files changed, 19 insertions(+), 38 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 24b1e2956..af78fdb6e 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -85,7 +85,7 @@ func (s *storageZfs) StoragePoolCheck() error {
 
 	poolName := s.getOnDiskPoolName()
 	if filepath.IsAbs(source) {
-		if zfsFilesystemEntityExists(poolName) {
+		if zfsFilesystemEntityExists(poolName, "") {
 			return nil
 		}
 		logger.Debugf("ZFS storage pool \"%s\" does not exist. Trying to import it.", poolName)
@@ -535,9 +535,8 @@ func (s *storageZfs) ContainerUmount(name string, path string) (bool, error) {
 
 // Things we do have to care about
 func (s *storageZfs) ContainerStorageReady(name string) bool {
-	poolName := s.getOnDiskPoolName()
-	fs := fmt.Sprintf("%s/containers/%s", poolName, name)
-	return s.zfsFilesystemEntityExists(fs, false)
+	fs := fmt.Sprintf("containers/%s", name)
+	return zfsFilesystemEntityExists(s.getOnDiskPoolName(), fs)
 }
 
 func (s *storageZfs) ContainerCreate(container container) error {
@@ -618,7 +617,7 @@ func (s *storageZfs) ContainerCreateFromImage(container container, fingerprint s
 		lxdStorageMapLock.Unlock()
 
 		var imgerr error
-		if !s.zfsFilesystemEntityExists(fsImage, true) {
+		if !zfsFilesystemEntityExists(poolName, fsImage) {
 			imgerr = s.ImageCreate(fingerprint)
 		}
 
@@ -710,7 +709,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 	fs := fmt.Sprintf("containers/%s", containerName)
 	containerPoolVolumeMntPoint := getContainerMountPoint(s.pool.Name, containerName)
 
-	if s.zfsFilesystemEntityExists(fs, true) {
+	if zfsFilesystemEntityExists(poolName, fs) {
 		removable := true
 		snaps, err := zfsPoolListSnapshots(poolName, fs)
 		if err != nil {
@@ -812,7 +811,7 @@ func (s *storageZfs) copyWithoutSnapshotsSparse(target container, source contain
 
 	revert := true
 	if sourceZfsDatasetSnapshot == "" {
-		if s.zfsFilesystemEntityExists(fmt.Sprintf("containers/%s", sourceName), true) {
+		if zfsFilesystemEntityExists(poolName, fmt.Sprintf("containers/%s", sourceName)) {
 			sourceZfsDatasetSnapshot = fmt.Sprintf("copy-%s", uuid.NewRandom().String())
 			sourceZfsDataset = fmt.Sprintf("containers/%s", sourceName)
 			err := zfsPoolVolumeSnapshotCreate(poolName, sourceZfsDataset, sourceZfsDatasetSnapshot)
@@ -827,7 +826,7 @@ func (s *storageZfs) copyWithoutSnapshotsSparse(target container, source contain
 			}()
 		}
 	} else {
-		if s.zfsFilesystemEntityExists(fmt.Sprintf("containers/%s at snapshot-%s", sourceName, sourceZfsDatasetSnapshot), true) {
+		if zfsFilesystemEntityExists(poolName, fmt.Sprintf("containers/%s at snapshot-%s", sourceName, sourceZfsDatasetSnapshot)) {
 			sourceZfsDataset = fmt.Sprintf("containers/%s", sourceName)
 			sourceZfsDatasetSnapshot = fmt.Sprintf("snapshot-%s", sourceZfsDatasetSnapshot)
 		}
@@ -1380,7 +1379,7 @@ func (s *storageZfs) ContainerSnapshotDelete(snapshotContainer container) error
 	sourceContainerName, sourceContainerSnapOnlyName, _ := containerGetParentAndSnapshotName(snapshotContainer.Name())
 	snapName := fmt.Sprintf("snapshot-%s", sourceContainerSnapOnlyName)
 
-	if s.zfsFilesystemEntityExists(fmt.Sprintf("containers/%s@%s", sourceContainerName, snapName), true) {
+	if zfsFilesystemEntityExists(poolName, fmt.Sprintf("containers/%s@%s", sourceContainerName, snapName)) {
 		removable, err := zfsPoolVolumeSnapshotRemovable(s.getOnDiskPoolName(), fmt.Sprintf("containers/%s", sourceContainerName), snapName)
 		if removable {
 			err = zfsPoolVolumeSnapshotDestroy(poolName, fmt.Sprintf("containers/%s", sourceContainerName), snapName)
@@ -1588,7 +1587,7 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 		s.deleteImageDbPoolVolume(fingerprint)
 	}()
 
-	if s.zfsFilesystemEntityExists(fmt.Sprintf("deleted/%s", fs), true) {
+	if zfsFilesystemEntityExists(poolName, fmt.Sprintf("deleted/%s", fs)) {
 		err := s.zfsPoolVolumeRename(fmt.Sprintf("deleted/%s", fs), fs)
 		if err != nil {
 			return err
@@ -1703,7 +1702,7 @@ func (s *storageZfs) ImageDelete(fingerprint string) error {
 	poolName := s.getOnDiskPoolName()
 	fs := fmt.Sprintf("images/%s", fingerprint)
 
-	if s.zfsFilesystemEntityExists(fs, true) {
+	if zfsFilesystemEntityExists(poolName, fs) {
 		removable, err := zfsPoolVolumeSnapshotRemovable(poolName, fs, "readonly")
 		if err != nil {
 			return err
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 3373fae9f..7c003d04b 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -137,8 +137,7 @@ func (s *storageZfs) zfsPoolCreate() error {
 			s.dataset = vdev
 
 			if strings.Contains(vdev, "/") {
-				ok := s.zfsFilesystemEntityExists(vdev, false)
-				if !ok {
+				if !zfsFilesystemEntityExists(vdev, "") {
 					output, err := shared.RunCommand(
 						"zfs",
 						"create",
@@ -424,23 +423,6 @@ func zfsPoolVolumeCleanup(pool string, path string) error {
 	return nil
 }
 
-func (s *storageZfs) zfsFilesystemEntityExists(path string, prefixPathWithPool bool) bool {
-	poolName := s.getOnDiskPoolName()
-	output, _ := zfsFilesystemEntityPropertyGet(poolName, path, "name", prefixPathWithPool)
-
-	// If prefixPathWithPool is false we assume that the path passed in
-	// already is a valid zfs entity we want to check for.
-	fsToCheck := path
-	if prefixPathWithPool {
-		fsToCheck = fmt.Sprintf("%s/%s", poolName, path)
-	}
-	if output == fsToCheck {
-		return true
-	}
-
-	return false
-}
-
 func zfsFilesystemEntityPropertyGet(pool string, path string, key string, prefixPathWithPool bool) (string, error) {
 	// If prefixPathWithPool is false we assume that the path passed in
 	// already is a valid zfs entity we want to check for.
@@ -483,7 +465,7 @@ func (s *storageZfs) zfsPoolVolumeRename(source string, dest string) error {
 		}
 
 		// zfs rename can fail because of descendants, yet still manage the rename
-		if !s.zfsFilesystemEntityExists(source, true) && s.zfsFilesystemEntityExists(dest, true) {
+		if !zfsFilesystemEntityExists(poolName, source) && zfsFilesystemEntityExists(poolName, dest) {
 			return nil
 		}
 
@@ -709,7 +691,11 @@ func zfsPoolVolumeSnapshotRemovable(pool string, path string, name string) (bool
 	return false, nil
 }
 
-func zfsFilesystemEntityExists(zfsEntity string) bool {
+func zfsFilesystemEntityExists(pool string, path string) bool {
+	vdev := pool
+	if path != "" {
+		vdev = fmt.Sprintf("%s/%s", pool, path)
+	}
 	output, err := shared.RunCommand(
 		"zfs",
 		"get",
@@ -717,15 +703,11 @@ func zfsFilesystemEntityExists(zfsEntity string) bool {
 		"-H",
 		"-o",
 		"name",
-		zfsEntity)
+		vdev)
 	if err != nil {
 		return false
 	}
 
 	detectedName := strings.TrimSpace(output)
-	if detectedName != zfsEntity {
-		return false
-	}
-
-	return true
+	return detectedName == vdev
 }

From 3ce40ca7acc7a4e58faf6adbae48c8be8e44bcd9 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Tue, 8 Aug 2017 10:38:04 +0200
Subject: [PATCH 19/21] storage/zfs: drop unneeded param from
 zfsFilesystemEntityPropertyGet

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       |  4 ++--
 lxd/storage_zfs_utils.go | 17 +++++------------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index af78fdb6e..e42e38183 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -729,7 +729,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 		}
 
 		if removable {
-			origin, err := zfsFilesystemEntityPropertyGet(poolName, fs, "origin", true)
+			origin, err := zfsFilesystemEntityPropertyGet(poolName, fs, "origin")
 			if err != nil {
 				return err
 			}
@@ -1313,7 +1313,7 @@ func (s *storageZfs) ContainerGetUsage(container container) (int64, error) {
 		property = "usedbydataset"
 	}
 
-	value, err := zfsFilesystemEntityPropertyGet(s.getOnDiskPoolName(), fs, property, true)
+	value, err := zfsFilesystemEntityPropertyGet(s.getOnDiskPoolName(), fs, property)
 	if err != nil {
 		return -1, err
 	}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 7c003d04b..5cc06c0fa 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -330,7 +330,7 @@ func zfsFilesystemEntityDelete(vdev string, pool string) error {
 }
 
 func zfsPoolVolumeDestroy(pool string, path string) error {
-	mountpoint, err := zfsFilesystemEntityPropertyGet(pool, path, "mountpoint", true)
+	mountpoint, err := zfsFilesystemEntityPropertyGet(pool, path, "mountpoint")
 	if err != nil {
 		return err
 	}
@@ -390,7 +390,7 @@ func zfsPoolVolumeCleanup(pool string, path string) error {
 				}
 			} else {
 				// Cleanup filesystems
-				origin, err := zfsFilesystemEntityPropertyGet(pool, path, "origin", true)
+				origin, err := zfsFilesystemEntityPropertyGet(pool, path, "origin")
 				if err != nil {
 					return err
 				}
@@ -423,14 +423,7 @@ func zfsPoolVolumeCleanup(pool string, path string) error {
 	return nil
 }
 
-func zfsFilesystemEntityPropertyGet(pool string, path string, key string, prefixPathWithPool bool) (string, error) {
-	// If prefixPathWithPool is false we assume that the path passed in
-	// already is a valid zfs entity we want to check for.
-	fsToCheck := path
-	if prefixPathWithPool {
-		fsToCheck = fmt.Sprintf("%s/%s", pool, path)
-	}
-
+func zfsFilesystemEntityPropertyGet(pool string, path string, key string) (string, error) {
 	output, err := shared.RunCommand(
 		"zfs",
 		"get",
@@ -438,7 +431,7 @@ func zfsFilesystemEntityPropertyGet(pool string, path string, key string, prefix
 		"-p",
 		"-o", "value",
 		key,
-		fsToCheck)
+		fmt.Sprintf("%s/%s", pool, path))
 	if err != nil {
 		return "", fmt.Errorf("Failed to get ZFS config: %s", output)
 	}
@@ -679,7 +672,7 @@ func zfsPoolVolumeSnapshotRemovable(pool string, path string, name string) (bool
 		snap = fmt.Sprintf("%s@%s", path, name)
 	}
 
-	clones, err := zfsFilesystemEntityPropertyGet(pool, snap, "clones", true)
+	clones, err := zfsFilesystemEntityPropertyGet(pool, snap, "clones")
 	if err != nil {
 		return false, err
 	}

From 81034aa68848765a9bf49228c3f6f688750217bd Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Tue, 8 Aug 2017 10:43:09 +0200
Subject: [PATCH 20/21] storage/zfs: make zfsPoolVolumeRename a standalone
 function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 10 ++++------
 lxd/storage_zfs_utils.go |  9 ++++-----
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index e42e38183..3218c863c 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -751,7 +751,7 @@ func (s *storageZfs) ContainerDelete(container container) error {
 				return err
 			}
 
-			err = s.zfsPoolVolumeRename(fs, fmt.Sprintf("deleted/containers/%s", uuid.NewRandom().String()))
+			err = zfsPoolVolumeRename(poolName, fs, fmt.Sprintf("deleted/containers/%s", uuid.NewRandom().String()))
 			if err != nil {
 				return err
 			}
@@ -1176,7 +1176,7 @@ func (s *storageZfs) ContainerRename(container container, newName string) error
 	// Rename the dataset.
 	oldZfsDataset := fmt.Sprintf("containers/%s", oldName)
 	newZfsDataset := fmt.Sprintf("containers/%s", newName)
-	err = s.zfsPoolVolumeRename(oldZfsDataset, newZfsDataset)
+	err = zfsPoolVolumeRename(poolName, oldZfsDataset, newZfsDataset)
 	if err != nil {
 		return err
 	}
@@ -1588,8 +1588,7 @@ func (s *storageZfs) ImageCreate(fingerprint string) error {
 	}()
 
 	if zfsFilesystemEntityExists(poolName, fmt.Sprintf("deleted/%s", fs)) {
-		err := s.zfsPoolVolumeRename(fmt.Sprintf("deleted/%s", fs), fs)
-		if err != nil {
+		if err := zfsPoolVolumeRename(poolName, fmt.Sprintf("deleted/%s", fs), fs); err != nil {
 			return err
 		}
 
@@ -1718,8 +1717,7 @@ func (s *storageZfs) ImageDelete(fingerprint string) error {
 				return err
 			}
 
-			err = s.zfsPoolVolumeRename(fs, fmt.Sprintf("deleted/%s", fs))
-			if err != nil {
+			if err := zfsPoolVolumeRename(poolName, fs, fmt.Sprintf("deleted/%s", fs)); err != nil {
 				return err
 			}
 		}
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 5cc06c0fa..423b3b6a6 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -439,18 +439,17 @@ func zfsFilesystemEntityPropertyGet(pool string, path string, key string) (strin
 	return strings.TrimRight(output, "\n"), nil
 }
 
-func (s *storageZfs) zfsPoolVolumeRename(source string, dest string) error {
+func zfsPoolVolumeRename(pool string, source string, dest string) error {
 	var err error
 	var output string
 
-	poolName := s.getOnDiskPoolName()
 	for i := 0; i < 20; i++ {
 		output, err = shared.RunCommand(
 			"zfs",
 			"rename",
 			"-p",
-			fmt.Sprintf("%s/%s", poolName, source),
-			fmt.Sprintf("%s/%s", poolName, dest))
+			fmt.Sprintf("%s/%s", pool, source),
+			fmt.Sprintf("%s/%s", pool, dest))
 
 		// Success
 		if err == nil {
@@ -458,7 +457,7 @@ func (s *storageZfs) zfsPoolVolumeRename(source string, dest string) error {
 		}
 
 		// zfs rename can fail because of descendants, yet still manage the rename
-		if !zfsFilesystemEntityExists(poolName, source) && zfsFilesystemEntityExists(poolName, dest) {
+		if !zfsFilesystemEntityExists(pool, source) && zfsFilesystemEntityExists(pool, dest) {
 			return nil
 		}
 

From cb788dbb1b96bc9cbba9c0b86885c9247a025c58 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Tue, 8 Aug 2017 11:22:19 +0200
Subject: [PATCH 21/21] storage/zfs: extract some logic to a standalone
 zfsPoolCreate function

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 lxd/storage_zfs.go       | 179 ++++++++++++++++++++++++++++++++++++++++++
 lxd/storage_zfs_utils.go | 198 +++--------------------------------------------
 2 files changed, 191 insertions(+), 186 deletions(-)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 3218c863c..04761ec7c 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -137,6 +137,185 @@ func (s *storageZfs) StoragePoolCreate() error {
 	return nil
 }
 
+func (s *storageZfs) zfsPoolCreate() error {
+	zpoolName := s.getOnDiskPoolName()
+	vdev := s.pool.Config["source"]
+	if vdev == "" {
+		vdev = filepath.Join(shared.VarPath("disks"), fmt.Sprintf("%s.img", s.pool.Name))
+		s.pool.Config["source"] = vdev
+
+		if s.pool.Config["zfs.pool_name"] == "" {
+			s.pool.Config["zfs.pool_name"] = zpoolName
+		}
+
+		f, err := os.Create(vdev)
+		if err != nil {
+			return fmt.Errorf("Failed to open %s: %s", vdev, err)
+		}
+		defer f.Close()
+
+		err = f.Chmod(0600)
+		if err != nil {
+			return fmt.Errorf("Failed to chmod %s: %s", vdev, err)
+		}
+
+		size, err := shared.ParseByteSizeString(s.pool.Config["size"])
+		if err != nil {
+			return err
+		}
+		err = f.Truncate(size)
+		if err != nil {
+			return fmt.Errorf("Failed to create sparse file %s: %s", vdev, err)
+		}
+
+		if err := zfsPoolCreate(zpoolName, vdev); err != nil {
+			return err
+		}
+	} else {
+		// Unset size property since it doesn't make sense.
+		s.pool.Config["size"] = ""
+
+		if filepath.IsAbs(vdev) {
+			if !shared.IsBlockdevPath(vdev) {
+				return fmt.Errorf("custom loop file locations are not supported")
+			}
+
+			if s.pool.Config["zfs.pool_name"] == "" {
+				s.pool.Config["zfs.pool_name"] = zpoolName
+			}
+
+			// This is a block device. Note, that we do not store the
+			// block device path or UUID or PARTUUID or similar in
+			// the database. All of those might change or might be
+			// used in a special way (For example, zfs uses a single
+			// UUID in a multi-device pool for all devices.). The
+			// safest way is to just store the name of the zfs pool
+			// we create.
+			s.pool.Config["source"] = zpoolName
+			if err := zfsPoolCreate(zpoolName, vdev); err != nil {
+				return err
+			}
+		} else {
+			if s.pool.Config["zfs.pool_name"] != "" {
+				return fmt.Errorf("invalid combination of \"source\" and \"zfs.pool_name\" property")
+			}
+			s.pool.Config["zfs.pool_name"] = vdev
+			s.dataset = vdev
+
+			if strings.Contains(vdev, "/") {
+				if !zfsFilesystemEntityExists(vdev, "") {
+					if err := zfsPoolCreate("", vdev); err != nil {
+						return err
+					}
+				} else {
+					if err := zfsPoolVolumeSet(vdev, "", "mountpoint", "none"); err != nil {
+						return err
+					}
+				}
+			} else {
+				err := zfsPoolCheck(vdev)
+				if err != nil {
+					return err
+				}
+
+				subvols, err := zfsPoolListSubvolumes(zpoolName, vdev)
+				if err != nil {
+					return err
+				}
+
+				if len(subvols) > 0 {
+					return fmt.Errorf("Provided ZFS pool (or dataset) isn't empty")
+				}
+
+				if err := zfsPoolVolumeSet(vdev, "", "mountpoint", "none"); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	// Create default dummy datasets to avoid zfs races during container
+	// creation.
+	poolName := s.getOnDiskPoolName()
+	dataset := fmt.Sprintf("%s/containers", poolName)
+	msg, err := zfsPoolVolumeCreate(dataset, "mountpoint=none")
+	if err != nil {
+		logger.Errorf("failed to create containers dataset: %s", msg)
+		return err
+	}
+
+	fixperms := shared.VarPath("storage-pools", s.pool.Name, "containers")
+	err = os.MkdirAll(fixperms, containersDirMode)
+	if err != nil && !os.IsNotExist(err) {
+		return err
+	}
+
+	err = os.Chmod(fixperms, containersDirMode)
+	if err != nil {
+		logger.Warnf("failed to chmod \"%s\" to \"0%s\": %s", fixperms, strconv.FormatInt(int64(containersDirMode), 8), err)
+	}
+
+	dataset = fmt.Sprintf("%s/images", poolName)
+	msg, err = zfsPoolVolumeCreate(dataset, "mountpoint=none")
+	if err != nil {
+		logger.Errorf("failed to create images dataset: %s", msg)
+		return err
+	}
+
+	fixperms = shared.VarPath("storage-pools", s.pool.Name, "images")
+	err = os.MkdirAll(fixperms, imagesDirMode)
+	if err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	err = os.Chmod(fixperms, imagesDirMode)
+	if err != nil {
+		logger.Warnf("failed to chmod \"%s\" to \"0%s\": %s", fixperms, strconv.FormatInt(int64(imagesDirMode), 8), err)
+	}
+
+	dataset = fmt.Sprintf("%s/custom", poolName)
+	msg, err = zfsPoolVolumeCreate(dataset, "mountpoint=none")
+	if err != nil {
+		logger.Errorf("failed to create custom dataset: %s", msg)
+		return err
+	}
+
+	fixperms = shared.VarPath("storage-pools", s.pool.Name, "custom")
+	err = os.MkdirAll(fixperms, customDirMode)
+	if err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	err = os.Chmod(fixperms, customDirMode)
+	if err != nil {
+		logger.Warnf("failed to chmod \"%s\" to \"0%s\": %s", fixperms, strconv.FormatInt(int64(customDirMode), 8), err)
+	}
+
+	dataset = fmt.Sprintf("%s/deleted", poolName)
+	msg, err = zfsPoolVolumeCreate(dataset, "mountpoint=none")
+	if err != nil {
+		logger.Errorf("failed to create deleted dataset: %s", msg)
+		return err
+	}
+
+	dataset = fmt.Sprintf("%s/snapshots", poolName)
+	msg, err = zfsPoolVolumeCreate(dataset, "mountpoint=none")
+	if err != nil {
+		logger.Errorf("failed to create snapshots dataset: %s", msg)
+		return err
+	}
+
+	fixperms = shared.VarPath("storage-pools", s.pool.Name, "snapshots")
+	err = os.MkdirAll(fixperms, snapshotsDirMode)
+	if err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	err = os.Chmod(fixperms, snapshotsDirMode)
+	if err != nil {
+		logger.Warnf("failed to chmod \"%s\" to \"0%s\": %s", fixperms, strconv.FormatInt(int64(snapshotsDirMode), 8), err)
+	}
+
+	return nil
+}
+
 func (s *storageZfs) StoragePoolDelete() error {
 	logger.Infof("Deleting ZFS storage pool \"%s\".", s.pool.Name)
 
diff --git a/lxd/storage_zfs_utils.go b/lxd/storage_zfs_utils.go
index 423b3b6a6..77a86e910 100644
--- a/lxd/storage_zfs_utils.go
+++ b/lxd/storage_zfs_utils.go
@@ -6,7 +6,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -63,198 +62,25 @@ func zfsPoolCheck(pool string) error {
 	return nil
 }
 
-func (s *storageZfs) zfsPoolCreate() error {
-	zpoolName := s.getOnDiskPoolName()
-	vdev := s.pool.Config["source"]
-	if vdev == "" {
-		vdev = filepath.Join(shared.VarPath("disks"), fmt.Sprintf("%s.img", s.pool.Name))
-		s.pool.Config["source"] = vdev
-
-		if s.pool.Config["zfs.pool_name"] == "" {
-			s.pool.Config["zfs.pool_name"] = zpoolName
-		}
-
-		f, err := os.Create(vdev)
-		if err != nil {
-			return fmt.Errorf("Failed to open %s: %s", vdev, err)
-		}
-		defer f.Close()
-
-		err = f.Chmod(0600)
-		if err != nil {
-			return fmt.Errorf("Failed to chmod %s: %s", vdev, err)
-		}
-
-		size, err := shared.ParseByteSizeString(s.pool.Config["size"])
-		if err != nil {
-			return err
-		}
-		err = f.Truncate(size)
-		if err != nil {
-			return fmt.Errorf("Failed to create sparse file %s: %s", vdev, err)
-		}
-
+func zfsPoolCreate(pool string, vdev string) error {
+	var output string
+	var err error
+	if pool == "" {
 		output, err := shared.RunCommand(
-			"zpool",
-			"create", zpoolName, vdev,
-			"-f", "-m", "none", "-O", "compression=on")
+			"zfs", "create", "-p", "-o", "mountpoint=none", vdev)
 		if err != nil {
-			return fmt.Errorf("Failed to create the ZFS pool: %s", output)
+			logger.Errorf("zfs create failed: %s.", output)
+			return fmt.Errorf("Failed to create ZFS filesystem: %s", output)
 		}
 	} else {
-		// Unset size property since it doesn't make sense.
-		s.pool.Config["size"] = ""
-
-		if filepath.IsAbs(vdev) {
-			if !shared.IsBlockdevPath(vdev) {
-				return fmt.Errorf("custom loop file locations are not supported")
-			}
-
-			if s.pool.Config["zfs.pool_name"] == "" {
-				s.pool.Config["zfs.pool_name"] = zpoolName
-			}
-
-			// This is a block device. Note, that we do not store the
-			// block device path or UUID or PARTUUID or similar in
-			// the database. All of those might change or might be
-			// used in a special way (For example, zfs uses a single
-			// UUID in a multi-device pool for all devices.). The
-			// safest way is to just store the name of the zfs pool
-			// we create.
-			s.pool.Config["source"] = zpoolName
-			output, err := shared.RunCommand(
-				"zpool",
-				"create", zpoolName, vdev,
-				"-f", "-m", "none", "-O", "compression=on")
-			if err != nil {
-				return fmt.Errorf("Failed to create the ZFS pool: %s", output)
-			}
-		} else {
-			if s.pool.Config["zfs.pool_name"] != "" {
-				return fmt.Errorf("invalid combination of \"source\" and \"zfs.pool_name\" property")
-			}
-			s.pool.Config["zfs.pool_name"] = vdev
-			s.dataset = vdev
-
-			if strings.Contains(vdev, "/") {
-				if !zfsFilesystemEntityExists(vdev, "") {
-					output, err := shared.RunCommand(
-						"zfs",
-						"create",
-						"-p",
-						"-o",
-						"mountpoint=none",
-						vdev)
-					if err != nil {
-						logger.Errorf("zfs create failed: %s.", output)
-						return fmt.Errorf("Failed to create ZFS filesystem: %s", output)
-					}
-				} else {
-					if err := zfsPoolVolumeSet(vdev, "", "mountpoint", "none"); err != nil {
-						return err
-					}
-				}
-			} else {
-				err := zfsPoolCheck(vdev)
-				if err != nil {
-					return err
-				}
-
-				subvols, err := zfsPoolListSubvolumes(zpoolName, vdev)
-				if err != nil {
-					return err
-				}
-
-				if len(subvols) > 0 {
-					return fmt.Errorf("Provided ZFS pool (or dataset) isn't empty")
-				}
-
-				if err := zfsPoolVolumeSet(vdev, "", "mountpoint", "none"); err != nil {
-					return err
-				}
-			}
+		output, err = shared.RunCommand(
+			"zpool", "create", pool, vdev, "-f", "-m", "none", "-O", "compression=on")
+		if err != nil {
+			logger.Errorf("zfs create failed: %s.", output)
+			return fmt.Errorf("Failed to create the ZFS pool: %s", output)
 		}
 	}
 
-	// Create default dummy datasets to avoid zfs races during container
-	// creation.
-	poolName := s.getOnDiskPoolName()
-	dataset := fmt.Sprintf("%s/containers", poolName)
-	msg, err := zfsPoolVolumeCreate(dataset, "mountpoint=none")
-	if err != nil {
-		logger.Errorf("failed to create containers dataset: %s", msg)
-		return err
-	}
-
-	fixperms := shared.VarPath("storage-pools", s.pool.Name, "containers")
-	err = os.MkdirAll(fixperms, containersDirMode)
-	if err != nil && !os.IsNotExist(err) {
-		return err
-	}
-
-	err = os.Chmod(fixperms, containersDirMode)
-	if err != nil {
-		logger.Warnf("failed to chmod \"%s\" to \"0%s\": %s", fixperms, strconv.FormatInt(int64(containersDirMode), 8), err)
-	}
-
-	dataset = fmt.Sprintf("%s/images", poolName)
-	msg, err = zfsPoolVolumeCreate(dataset, "mountpoint=none")
-	if err != nil {
-		logger.Errorf("failed to create images dataset: %s", msg)
-		return err
-	}
-
-	fixperms = shared.VarPath("storage-pools", s.pool.Name, "images")
-	err = os.MkdirAll(fixperms, imagesDirMode)
-	if err != nil && !os.IsNotExist(err) {
-		return err
-	}
-	err = os.Chmod(fixperms, imagesDirMode)
-	if err != nil {
-		logger.Warnf("failed to chmod \"%s\" to \"0%s\": %s", fixperms, strconv.FormatInt(int64(imagesDirMode), 8), err)
-	}
-
-	dataset = fmt.Sprintf("%s/custom", poolName)
-	msg, err = zfsPoolVolumeCreate(dataset, "mountpoint=none")
-	if err != nil {
-		logger.Errorf("failed to create custom dataset: %s", msg)
-		return err
-	}
-
-	fixperms = shared.VarPath("storage-pools", s.pool.Name, "custom")
-	err = os.MkdirAll(fixperms, customDirMode)
-	if err != nil && !os.IsNotExist(err) {
-		return err
-	}
-	err = os.Chmod(fixperms, customDirMode)
-	if err != nil {
-		logger.Warnf("failed to chmod \"%s\" to \"0%s\": %s", fixperms, strconv.FormatInt(int64(customDirMode), 8), err)
-	}
-
-	dataset = fmt.Sprintf("%s/deleted", poolName)
-	msg, err = zfsPoolVolumeCreate(dataset, "mountpoint=none")
-	if err != nil {
-		logger.Errorf("failed to create deleted dataset: %s", msg)
-		return err
-	}
-
-	dataset = fmt.Sprintf("%s/snapshots", poolName)
-	msg, err = zfsPoolVolumeCreate(dataset, "mountpoint=none")
-	if err != nil {
-		logger.Errorf("failed to create snapshots dataset: %s", msg)
-		return err
-	}
-
-	fixperms = shared.VarPath("storage-pools", s.pool.Name, "snapshots")
-	err = os.MkdirAll(fixperms, snapshotsDirMode)
-	if err != nil && !os.IsNotExist(err) {
-		return err
-	}
-	err = os.Chmod(fixperms, snapshotsDirMode)
-	if err != nil {
-		logger.Warnf("failed to chmod \"%s\" to \"0%s\": %s", fixperms, strconv.FormatInt(int64(snapshotsDirMode), 8), err)
-	}
-
 	return nil
 }