[lxc-devel] [lxd/master] Wrap errors in drivers package

stgraber on Github lxc-bot at linuxcontainers.org
Tue Jan 14 02:02:31 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200113/de5e2a1e/attachment-0001.bin>
-------------- next part --------------
From 1bd1e47831434df19de54b7047fef6b8a695f055 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 13 Jan 2020 20:21:38 -0500
Subject: [PATCH 1/2] lxd/storage/drivers: Use errors.Wrap
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/storage/drivers/driver_btrfs.go          |  9 +++++----
 lxd/storage/drivers/driver_btrfs_volumes.go  |  4 +++-
 lxd/storage/drivers/driver_cephfs_volumes.go |  4 +++-
 lxd/storage/drivers/driver_common.go         |  4 +++-
 lxd/storage/drivers/driver_dir_volumes.go    |  4 +++-
 lxd/storage/drivers/utils.go                 | 13 ++++++-------
 6 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/lxd/storage/drivers/driver_btrfs.go b/lxd/storage/drivers/driver_btrfs.go
index e51abbd710..c2c553fdbb 100644
--- a/lxd/storage/drivers/driver_btrfs.go
+++ b/lxd/storage/drivers/driver_btrfs.go
@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 
 	"github.com/lxc/lxd/lxd/migration"
@@ -89,19 +90,19 @@ func (d *btrfs) Create() error {
 
 		err = createSparseFile(d.config["source"], size)
 		if err != nil {
-			return fmt.Errorf("Failed to create the sparse file: %v", err)
+			return errors.Wrap(err, "Failed to create the sparse file")
 		}
 
 		// Format the file.
 		_, err = makeFSType(d.config["source"], "btrfs", &mkfsOptions{Label: d.name})
 		if err != nil {
-			return fmt.Errorf("Failed to format sparse file: %v", err)
+			return errors.Wrap(err, "Failed to format sparse file")
 		}
 	} else if shared.IsBlockdevPath(d.config["source"]) {
 		// Format the block device.
 		_, err := makeFSType(d.config["source"], "btrfs", &mkfsOptions{Label: d.name})
 		if err != nil {
-			return fmt.Errorf("Failed to format block device: %v", err)
+			return errors.Wrap(err, "Failed to format block device")
 		}
 
 		// Record the UUID as the source.
@@ -121,7 +122,7 @@ func (d *btrfs) Create() error {
 			// Existing btrfs subvolume.
 			subvols, err := d.getSubvolumes(hostPath)
 			if err != nil {
-				return fmt.Errorf("Could not determine if existing btrfs subvolume is empty: %v", err)
+				return errors.Wrap(err, "Could not determine if existing btrfs subvolume is empty")
 			}
 
 			// Check that the provided subvolume is empty.
diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go
index 8b567fee7b..25f9c7b27a 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"github.com/lxc/lxd/lxd/migration"
 	"github.com/lxc/lxd/lxd/operations"
 	"github.com/lxc/lxd/lxd/revert"
@@ -402,7 +404,7 @@ func (d *btrfs) SetVolumeQuota(vol Volume, size string, op *operations.Operation
 			var output string
 			output, err = shared.RunCommand("btrfs", "subvolume", "show", volPath)
 			if err != nil {
-				return fmt.Errorf("Failed to get subvol information: %v", err)
+				return errors.Wrap(err, "Failed to get subvol information")
 			}
 
 			id := ""
diff --git a/lxd/storage/drivers/driver_cephfs_volumes.go b/lxd/storage/drivers/driver_cephfs_volumes.go
index c49b8ee17b..bf4f106e4d 100644
--- a/lxd/storage/drivers/driver_cephfs_volumes.go
+++ b/lxd/storage/drivers/driver_cephfs_volumes.go
@@ -7,6 +7,8 @@ import (
 	"path/filepath"
 	"strconv"
 
+	"github.com/pkg/errors"
+
 	"github.com/lxc/lxd/lxd/migration"
 	"github.com/lxc/lxd/lxd/operations"
 	"github.com/lxc/lxd/lxd/rsync"
@@ -515,7 +517,7 @@ func (d *cephfs) RestoreVolume(vol Volume, snapshotName string, op *operations.O
 	bwlimit := d.config["rsync.bwlimit"]
 	output, err := rsync.LocalCopy(cephSnapPath, vol.MountPath(), bwlimit, false)
 	if err != nil {
-		return fmt.Errorf("Failed to rsync volume: %s: %s", string(output), err)
+		return errors.Wrapf(err, "Failed to rsync volume: %s", string(output))
 	}
 
 	return nil
diff --git a/lxd/storage/drivers/driver_common.go b/lxd/storage/drivers/driver_common.go
index 6f8f9e88c4..9362528c8e 100644
--- a/lxd/storage/drivers/driver_common.go
+++ b/lxd/storage/drivers/driver_common.go
@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"github.com/lxc/lxd/lxd/migration"
 	"github.com/lxc/lxd/lxd/operations"
 	"github.com/lxc/lxd/lxd/rsync"
@@ -60,7 +62,7 @@ func (d *common) validateVolume(vol Volume, driverRules map[string]func(value st
 		checkedFields[k] = struct{}{} //Mark field as checked.
 		err := validator(vol.config[k])
 		if err != nil {
-			return fmt.Errorf("Invalid value for volume option %s: %v", k, err)
+			return errors.Wrapf(err, "Invalid value for volume option %s", k)
 		}
 	}
 
diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go
index 30ab4892db..a1a9d761bd 100644
--- a/lxd/storage/drivers/driver_dir_volumes.go
+++ b/lxd/storage/drivers/driver_dir_volumes.go
@@ -5,6 +5,8 @@ import (
 	"io"
 	"os"
 
+	"github.com/pkg/errors"
+
 	"github.com/lxc/lxd/lxd/migration"
 	"github.com/lxc/lxd/lxd/operations"
 	"github.com/lxc/lxd/lxd/revert"
@@ -374,7 +376,7 @@ func (d *dir) RestoreVolume(vol Volume, snapshotName string, op *operations.Oper
 	bwlimit := d.config["rsync.bwlimit"]
 	_, err := rsync.LocalCopy(srcPath, volPath, bwlimit, true)
 	if err != nil {
-		return fmt.Errorf("Failed to rsync volume: %s", err)
+		return errors.Wrap(err, "Failed to rsync volume")
 	}
 
 	return nil
diff --git a/lxd/storage/drivers/utils.go b/lxd/storage/drivers/utils.go
index 781e5a6c56..9151fd43d6 100644
--- a/lxd/storage/drivers/utils.go
+++ b/lxd/storage/drivers/utils.go
@@ -265,18 +265,18 @@ func deleteParentSnapshotDirIfEmpty(poolName string, volType VolumeType, volName
 func createSparseFile(filePath string, sizeBytes int64) error {
 	f, err := os.Create(filePath)
 	if err != nil {
-		return fmt.Errorf("Failed to open %s: %s", filePath, err)
+		return errors.Wrapf(err, "Failed to open %s", filePath)
 	}
 	defer f.Close()
 
 	err = f.Chmod(0600)
 	if err != nil {
-		return fmt.Errorf("Failed to chmod %s: %s", filePath, err)
+		return errors.Wrapf(err, "Failed to chmod %s", filePath)
 	}
 
 	err = f.Truncate(sizeBytes)
 	if err != nil {
-		return fmt.Errorf("Failed to create sparse file %s: %s", filePath, err)
+		return errors.Wrapf(err, "Failed to create sparse file %s", filePath)
 	}
 
 	return nil
@@ -297,7 +297,7 @@ func ensureVolumeBlockFile(vol Volume, path string) error {
 	if shared.PathExists(path) {
 		_, err = shared.RunCommand("qemu-img", "resize", "-f", "raw", path, fmt.Sprintf("%d", blockSizeBytes))
 		if err != nil {
-			return fmt.Errorf("Failed resizing disk image %s to size %s: %v", path, blockSize, err)
+			return errors.Wrapf(err, "Failed resizing disk image %s to size %s", path, blockSize)
 		}
 	} else {
 		// If path doesn't exist, then there has been no filler function
@@ -305,7 +305,7 @@ func ensureVolumeBlockFile(vol Volume, path string) error {
 		// volume (use for PXE booting a VM).
 		_, err = shared.RunCommand("qemu-img", "create", "-f", "raw", path, fmt.Sprintf("%d", blockSizeBytes))
 		if err != nil {
-			return fmt.Errorf("Failed creating disk image %s as size %s: %v", path, blockSize, err)
+			return errors.Wrapf(err, "Failed creating disk image %s as size %s", path, blockSize)
 		}
 	}
 
@@ -463,8 +463,7 @@ func growFileSystem(fsType string, devPath string, vol Volume) error {
 		}
 
 		if err != nil {
-			errorMsg := fmt.Sprintf(`Could not extend underlying %s filesystem for "%s": %s`, fsType, devPath, msg)
-			return fmt.Errorf(errorMsg)
+			return fmt.Errorf(`Could not extend underlying %s filesystem for "%s": %s`, fsType, devPath, msg)
 		}
 
 		return nil

From 47de8a8596ddd70d2c4ef1f458139ee14611fb66 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 13 Jan 2020 21:01:12 -0500
Subject: [PATCH 2/2] lxd/storage/drivers: Wrap os/ioutil calls
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This also adds checks for IsNotExist on Remove/RemoveAll errors.

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/storage/drivers/driver_btrfs.go          | 14 ++++----
 lxd/storage/drivers/driver_btrfs_utils.go    |  6 ++--
 lxd/storage/drivers/driver_btrfs_volumes.go  | 26 +++++++-------
 lxd/storage/drivers/driver_cephfs.go         | 20 ++++++-----
 lxd/storage/drivers/driver_cephfs_utils.go   |  6 ++--
 lxd/storage/drivers/driver_cephfs_volumes.go | 36 ++++++++++----------
 lxd/storage/drivers/driver_common.go         | 10 +++---
 lxd/storage/drivers/driver_dir_volumes.go    |  8 ++---
 lxd/storage/drivers/driver_zfs.go            | 10 +++---
 lxd/storage/drivers/driver_zfs_volumes.go    | 21 ++++++------
 lxd/storage/drivers/utils.go                 | 17 ++++++---
 lxd/storage/drivers/volume.go                |  6 ++--
 12 files changed, 97 insertions(+), 83 deletions(-)

diff --git a/lxd/storage/drivers/driver_btrfs.go b/lxd/storage/drivers/driver_btrfs.go
index c2c553fdbb..e569167028 100644
--- a/lxd/storage/drivers/driver_btrfs.go
+++ b/lxd/storage/drivers/driver_btrfs.go
@@ -145,8 +145,8 @@ func (d *btrfs) Create() error {
 
 				// Delete the current directory to replace by subvolume.
 				err := os.Remove(cleanSource)
-				if err != nil {
-					return err
+				if err != nil && !os.IsNotExist(err) {
+					return errors.Wrapf(err, "Failed to remove '%s'", cleanSource)
 				}
 			}
 
@@ -211,17 +211,15 @@ func (d *btrfs) Delete(op *operations.Operation) error {
 		// And re-create as an empty directory to make the backend happy.
 		err = os.Mkdir(GetPoolMountPath(d.name), 0700)
 		if err != nil {
-			return err
+			return errors.Wrapf(err, "Failed to create directory '%s'", GetPoolMountPath(d.name))
 		}
 	}
 
 	// Delete any loop file we may have used.
 	loopPath := filepath.Join(shared.VarPath("disks"), fmt.Sprintf("%s.img", d.name))
-	if shared.PathExists(loopPath) {
-		err = os.Remove(loopPath)
-		if err != nil {
-			return err
-		}
+	err = os.Remove(loopPath)
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "Failed to remove '%s'", loopPath)
 	}
 
 	return nil
diff --git a/lxd/storage/drivers/driver_btrfs_utils.go b/lxd/storage/drivers/driver_btrfs_utils.go
index 5f759dd3c6..deb3e02ae8 100644
--- a/lxd/storage/drivers/driver_btrfs_utils.go
+++ b/lxd/storage/drivers/driver_btrfs_utils.go
@@ -11,10 +11,12 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/pkg/errors"
+	"golang.org/x/sys/unix"
+
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/ioprogress"
 	"github.com/lxc/lxd/shared/logger"
-	"golang.org/x/sys/unix"
 )
 
 // Errors
@@ -367,7 +369,7 @@ func (d *btrfs) receiveSubvolume(path string, targetPath string, conn io.ReadWri
 	// And move it to the target path.
 	err = os.Rename(receivedSnapshot, targetPath)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to rename '%s' to '%s'", receivedSnapshot, targetPath)
 	}
 
 	return nil
diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go
index 25f9c7b27a..354325a137 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -110,13 +110,13 @@ func (d *btrfs) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData i
 	// Create a temporary directory to unpack the backup into.
 	unpackDir, err := ioutil.TempDir(GetVolumeMountPath(d.name, vol.volType, ""), vol.name)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "Failed to create temporary directory under '%s'", GetVolumeMountPath(d.name, vol.volType, ""))
 	}
 	defer os.RemoveAll(unpackDir)
 
 	err = os.Chmod(unpackDir, 0100)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "Failed to chmod '%s'", unpackDir)
 	}
 
 	// Find the compression algorithm used for backup source data.
@@ -154,7 +154,7 @@ func (d *btrfs) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData i
 		// Open the backup.
 		feeder, err := os.Open(filepath.Join(unpackDir, "snapshots", fmt.Sprintf("%s.bin", snapName)))
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, errors.Wrapf(err, "Failed to open '%s'", filepath.Join(unpackDir, "snapshots", fmt.Sprintf("%s.bin", snapName)))
 		}
 		defer feeder.Close()
 
@@ -168,7 +168,7 @@ func (d *btrfs) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData i
 	// Open the backup.
 	feeder, err := os.Open(filepath.Join(unpackDir, "container.bin"))
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "Failed to open '%s'", filepath.Join(unpackDir, "container.bin"))
 	}
 	defer feeder.Close()
 
@@ -280,13 +280,13 @@ func (d *btrfs) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser, v
 	// Create a temporary directory which will act as the parent directory of the received ro snapshot.
 	tmpVolumesMountPoint, err := ioutil.TempDir(instancesPath, vol.name)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to create temporary directory under '%s'", instancesPath)
 	}
 	defer os.RemoveAll(tmpVolumesMountPoint)
 
 	err = os.Chmod(tmpVolumesMountPoint, 0100)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to chmod '%s'", tmpVolumesMountPoint)
 	}
 
 	wrapper := migration.ProgressWriter(op, "fs_progress", vol.name)
@@ -521,13 +521,13 @@ func (d *btrfs) MigrateVolume(vol Volume, conn io.ReadWriteCloser, volSrcArgs *m
 	// Create a temporary directory which will act as the parent directory of the read-only snapshot.
 	tmpVolumesMountPoint, err := ioutil.TempDir(instancesPath, vol.name)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to create temporary directory under '%s'", instancesPath)
 	}
 	defer os.RemoveAll(tmpVolumesMountPoint)
 
 	err = os.Chmod(tmpVolumesMountPoint, 0100)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to chmod '%s'", tmpVolumesMountPoint)
 	}
 
 	// Make read-only snapshot of the subvolume as writable subvolumes cannot be sent.
@@ -579,7 +579,7 @@ func (d *btrfs) BackupVolume(vol Volume, targetPath string, optimized bool, snap
 		// Create the file.
 		fd, err := os.OpenFile(file, os.O_RDWR|os.O_CREATE, 0644)
 		if err != nil {
-			return err
+			return errors.Wrapf(err, "Failed to open '%s'", file)
 		}
 		defer fd.Close()
 
@@ -607,7 +607,7 @@ func (d *btrfs) BackupVolume(vol Volume, targetPath string, optimized bool, snap
 		if len(volSnapshots) > 0 {
 			err = os.MkdirAll(snapshotsPath, 0711)
 			if err != nil {
-				return err
+				return errors.Wrapf(err, "Failed to create directory '%s'", snapshotsPath)
 			}
 		}
 
@@ -640,13 +640,13 @@ func (d *btrfs) BackupVolume(vol Volume, targetPath string, optimized bool, snap
 
 	tmpContainerMntPoint, err := ioutil.TempDir(containersPath, vol.name)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to create temporary directory under '%s'", containersPath)
 	}
 	defer os.RemoveAll(tmpContainerMntPoint)
 
 	err = os.Chmod(tmpContainerMntPoint, 0100)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to chmod '%s'", tmpContainerMntPoint)
 	}
 
 	// Create the read-only snapshot.
@@ -726,7 +726,7 @@ func (d *btrfs) RestoreVolume(vol Volume, snapshotName string, op *operations.Op
 	backupSubvolume := fmt.Sprintf("%s%s", vol.MountPath(), tmpVolSuffix)
 	err := os.Rename(vol.MountPath(), backupSubvolume)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to rename '%s' to '%s'", vol.MountPath(), backupSubvolume)
 	}
 
 	// Setup revert logic.
diff --git a/lxd/storage/drivers/driver_cephfs.go b/lxd/storage/drivers/driver_cephfs.go
index 6af9b1ee3c..bbeacc4478 100644
--- a/lxd/storage/drivers/driver_cephfs.go
+++ b/lxd/storage/drivers/driver_cephfs.go
@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"github.com/lxc/lxd/lxd/migration"
 	"github.com/lxc/lxd/lxd/operations"
 	"github.com/lxc/lxd/shared"
@@ -103,20 +105,20 @@ func (d *cephfs) Create() error {
 	// Create a temporary mountpoint.
 	mountPath, err := ioutil.TempDir("", "lxd_cephfs_")
 	if err != nil {
-		return err
+		return errors.Wrap(err, "Failed to create temporary directory under")
 	}
 	defer os.RemoveAll(mountPath)
 
 	err = os.Chmod(mountPath, 0700)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to chmod '%s'", mountPath)
 	}
 
 	mountPoint := filepath.Join(mountPath, "mount")
 
 	err = os.Mkdir(mountPoint, 0700)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to create directory '%s'", mountPoint)
 	}
 
 	// Get the credentials and host.
@@ -136,7 +138,7 @@ func (d *cephfs) Create() error {
 	// Create the path if missing.
 	err = os.MkdirAll(filepath.Join(mountPoint, fsPath), 0755)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to create directory '%s'", filepath.Join(mountPoint, fsPath))
 	}
 
 	// Check that the existing path is empty.
@@ -161,19 +163,19 @@ func (d *cephfs) Delete(op *operations.Operation) error {
 	// Create a temporary mountpoint.
 	mountPath, err := ioutil.TempDir("", "lxd_cephfs_")
 	if err != nil {
-		return err
+		return errors.Wrap(err, "Failed to create temporary directory under")
 	}
 	defer os.RemoveAll(mountPath)
 
 	err = os.Chmod(mountPath, 0700)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to chmod '%s'", mountPath)
 	}
 
 	mountPoint := filepath.Join(mountPath, "mount")
 	err = os.Mkdir(mountPoint, 0700)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to create directory '%s'", mountPoint)
 	}
 
 	// Get the credentials and host.
@@ -201,8 +203,8 @@ func (d *cephfs) Delete(op *operations.Operation) error {
 		// Delete the path itself.
 		if fsPath != "" && fsPath != "/" {
 			err = os.Remove(filepath.Join(mountPoint, fsPath))
-			if err != nil {
-				return err
+			if err != nil && !os.IsNotExist(err) {
+				return errors.Wrapf(err, "Failed to remove directory '%s'", filepath.Join(mountPoint, fsPath))
 			}
 		}
 	}
diff --git a/lxd/storage/drivers/driver_cephfs_utils.go b/lxd/storage/drivers/driver_cephfs_utils.go
index 638bdfe68f..31b5d1d572 100644
--- a/lxd/storage/drivers/driver_cephfs_utils.go
+++ b/lxd/storage/drivers/driver_cephfs_utils.go
@@ -6,6 +6,8 @@ import (
 	"os"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"github.com/lxc/lxd/shared"
 )
 
@@ -24,7 +26,7 @@ func (d *cephfs) getConfig(clusterName string, userName string) ([]string, strin
 	// Parse the CEPH configuration.
 	cephConf, err := os.Open(fmt.Sprintf("/etc/ceph/%s.conf", clusterName))
 	if err != nil {
-		return nil, "", err
+		return nil, "", errors.Wrapf(err, "Failed to open '%s", fmt.Sprintf("/etc/ceph/%s.conf", clusterName))
 	}
 
 	cephMon := []string{}
@@ -59,7 +61,7 @@ func (d *cephfs) getConfig(clusterName string, userName string) ([]string, strin
 	// Parse the CEPH keyring.
 	cephKeyring, err := os.Open(fmt.Sprintf("/etc/ceph/%v.client.%v.keyring", clusterName, userName))
 	if err != nil {
-		return nil, "", err
+		return nil, "", errors.Wrapf(err, "Failed to open '%s", fmt.Sprintf("/etc/ceph/%v.client.%v.keyring", clusterName, userName))
 	}
 
 	var cephSecret string
diff --git a/lxd/storage/drivers/driver_cephfs_volumes.go b/lxd/storage/drivers/driver_cephfs_volumes.go
index bf4f106e4d..746a8ce0c3 100644
--- a/lxd/storage/drivers/driver_cephfs_volumes.go
+++ b/lxd/storage/drivers/driver_cephfs_volumes.go
@@ -249,8 +249,8 @@ func (d *cephfs) DeleteVolume(vol Volume, op *operations.Operation) error {
 
 	// Remove the volume from the storage device.
 	err = os.RemoveAll(volPath)
-	if err != nil {
-		return err
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "Failed to delete '%s'", volPath)
 	}
 
 	// Although the volume snapshot directory should already be removed, lets remove it here
@@ -258,8 +258,8 @@ func (d *cephfs) DeleteVolume(vol Volume, op *operations.Operation) error {
 	snapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, vol.name)
 
 	err = os.RemoveAll(snapshotDir)
-	if err != nil {
-		return err
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "Failed to delete '%s'", snapshotDir)
 	}
 
 	return nil
@@ -360,7 +360,7 @@ func (d *cephfs) RenameVolume(vol Volume, newName string, op *operations.Operati
 		// Remove the new snapshot directory if we are reverting.
 		if len(revertPaths) > 0 {
 			snapshotDir := GetVolumeSnapshotDir(d.name, vol.volType, newName)
-			err = os.RemoveAll(snapshotDir)
+			os.RemoveAll(snapshotDir)
 		}
 	}()
 
@@ -372,7 +372,7 @@ func (d *cephfs) RenameVolume(vol Volume, newName string, op *operations.Operati
 
 		err = os.Rename(srcSnapshotDir, targetSnapshotDir)
 		if err != nil {
-			return err
+			return errors.Wrapf(err, "Failed to rename '%s' to '%s'", srcSnapshotDir, targetSnapshotDir)
 		}
 
 		revertPaths = append(revertPaths, volRevert{
@@ -401,7 +401,7 @@ func (d *cephfs) RenameVolume(vol Volume, newName string, op *operations.Operati
 		// Update the symlink.
 		err = os.Symlink(newCephSnapPath, newPath)
 		if err != nil {
-			return err
+			return errors.Wrapf(err, "Failed to symlink '%s' to '%s'", newCephSnapPath, newPath)
 		}
 
 		revertPaths = append(revertPaths, volRevert{
@@ -415,7 +415,7 @@ func (d *cephfs) RenameVolume(vol Volume, newName string, op *operations.Operati
 	newPath := GetVolumeMountPath(d.name, vol.volType, newName)
 	err = os.Rename(oldPath, newPath)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to rename '%s' to '%s'", oldPath, newPath)
 	}
 
 	revertPaths = append(revertPaths, volRevert{
@@ -451,7 +451,7 @@ func (d *cephfs) CreateVolumeSnapshot(snapVol Volume, op *operations.Operation)
 
 	err := os.Mkdir(cephSnapPath, 0711)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to create directory '%s'", cephSnapPath)
 	}
 
 	// Create the parent directory.
@@ -464,7 +464,7 @@ func (d *cephfs) CreateVolumeSnapshot(snapVol Volume, op *operations.Operation)
 	targetPath := snapVol.MountPath()
 	err = os.Symlink(cephSnapPath, targetPath)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to symlink '%s' to '%s'", cephSnapPath, targetPath)
 	}
 
 	return nil
@@ -479,15 +479,15 @@ func (d *cephfs) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation)
 	cephSnapPath := filepath.Join(sourcePath, ".snap", snapName)
 
 	err := os.Remove(cephSnapPath)
-	if err != nil {
-		return err
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "Failed to remove '%s'", cephSnapPath)
 	}
 
 	// Remove the symlink.
 	snapPath := snapVol.MountPath()
 	err = os.Remove(snapPath)
-	if err != nil {
-		return err
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "Failed to remove '%s'", snapPath)
 	}
 
 	return nil
@@ -532,20 +532,20 @@ func (d *cephfs) RenameVolumeSnapshot(snapVol Volume, newSnapshotName string, op
 
 	err := os.Rename(oldCephSnapPath, newCephSnapPath)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to rename '%s' to '%s'", oldCephSnapPath, newCephSnapPath)
 	}
 
 	// Re-generate the snapshot symlink.
 	oldPath := snapVol.MountPath()
 	err = os.Remove(oldPath)
-	if err != nil {
-		return err
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "Failed to remove '%s'", oldPath)
 	}
 
 	newPath := GetVolumeMountPath(d.name, snapVol.volType, GetSnapshotVolumeName(parentName, newSnapshotName))
 	err = os.Symlink(newCephSnapPath, newPath)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to symlink '%s' to '%s'", newCephSnapPath, newPath)
 	}
 
 	return nil
diff --git a/lxd/storage/drivers/driver_common.go b/lxd/storage/drivers/driver_common.go
index 9362528c8e..8506ce9af9 100644
--- a/lxd/storage/drivers/driver_common.go
+++ b/lxd/storage/drivers/driver_common.go
@@ -159,7 +159,7 @@ func (d *common) vfsRenameVolume(vol Volume, newVolName string, op *operations.O
 
 	err := os.Rename(srcVolumePath, dstVolumePath)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to rename '%s' to '%s'", srcVolumePath, dstVolumePath)
 	}
 
 	revertRename := true
@@ -178,7 +178,7 @@ func (d *common) vfsRenameVolume(vol Volume, newVolName string, op *operations.O
 	if shared.PathExists(srcSnapshotDir) {
 		err = os.Rename(srcSnapshotDir, dstSnapshotDir)
 		if err != nil {
-			return err
+			return errors.Wrapf(err, "Failed to rename '%s' to '%s'", srcSnapshotDir, dstSnapshotDir)
 		}
 	}
 
@@ -198,7 +198,7 @@ func (d *common) vfsVolumeSnapshots(vol Volume, op *operations.Operation) ([]str
 			return snapshots, nil
 		}
 
-		return nil, err
+		return nil, errors.Wrapf(err, "Failed to list directory '%s'", snapshotDir)
 	}
 
 	for _, ent := range ents {
@@ -225,7 +225,7 @@ func (d *common) vfsRenameVolumeSnapshot(snapVol Volume, newSnapshotName string,
 
 	err := os.Rename(oldPath, newPath)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to rename '%s' to '%s'", oldPath, newPath)
 	}
 
 	return nil
@@ -308,7 +308,7 @@ func (d *common) vfsBackupVolume(vol Volume, targetPath string, snapshots bool,
 		if len(snapshots) > 0 {
 			err = os.MkdirAll(snapshotsPath, 0711)
 			if err != nil {
-				return err
+				return errors.Wrapf(err, "Failed to create directory '%s'", snapshotsPath)
 			}
 		}
 
diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go
index a1a9d761bd..78ac040f25 100644
--- a/lxd/storage/drivers/driver_dir_volumes.go
+++ b/lxd/storage/drivers/driver_dir_volumes.go
@@ -169,8 +169,8 @@ func (d *dir) DeleteVolume(vol Volume, op *operations.Operation) error {
 
 	// Remove the volume from the storage device.
 	err = os.RemoveAll(volPath)
-	if err != nil {
-		return err
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "Failed to remove '%s'", volPath)
 	}
 
 	// Although the volume snapshot directory should already be removed, lets remove it here
@@ -331,8 +331,8 @@ func (d *dir) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) err
 
 	// Remove the snapshot from the storage device.
 	err := os.RemoveAll(snapPath)
-	if err != nil {
-		return err
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "Failed to remove '%s'", snapPath)
 	}
 
 	parentName, _, _ := shared.InstanceGetParentAndSnapshotName(snapVol.name)
diff --git a/lxd/storage/drivers/driver_zfs.go b/lxd/storage/drivers/driver_zfs.go
index 11f1774213..47b9e0b5b0 100644
--- a/lxd/storage/drivers/driver_zfs.go
+++ b/lxd/storage/drivers/driver_zfs.go
@@ -8,6 +8,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"github.com/lxc/lxd/lxd/migration"
 	"github.com/lxc/lxd/lxd/operations"
 	"github.com/lxc/lxd/lxd/util"
@@ -265,11 +267,9 @@ func (d *zfs) Delete(op *operations.Operation) error {
 
 	// Delete any loop file we may have used
 	loopPath := filepath.Join(shared.VarPath("disks"), fmt.Sprintf("%s.img", d.name))
-	if shared.PathExists(loopPath) {
-		err = os.Remove(loopPath)
-		if err != nil {
-			return err
-		}
+	err = os.Remove(loopPath)
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrapf(err, "Failed to remove '%s'", loopPath)
 	}
 
 	return nil
diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index 17a34a4c47..656ffc162c 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/pborman/uuid"
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 
 	"github.com/lxc/lxd/lxd/migration"
@@ -213,13 +214,13 @@ func (d *zfs) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData io.
 	// Create a temporary directory to unpack the backup into.
 	unpackDir, err := ioutil.TempDir(GetVolumeMountPath(d.name, vol.volType, ""), vol.name)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "Failed to create temporary directory under '%s'", GetVolumeMountPath(d.name, vol.volType, ""))
 	}
 	defer os.RemoveAll(unpackDir)
 
 	err = os.Chmod(unpackDir, 0100)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "Failed to chmod '%s'", unpackDir)
 	}
 
 	// Find the compression algorithm used for backup source data.
@@ -255,7 +256,7 @@ func (d *zfs) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData io.
 		// Open the backup.
 		feeder, err := os.Open(filepath.Join(unpackDir, "snapshots", fmt.Sprintf("%s.bin", snapName)))
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, errors.Wrapf(err, "Failed to open '%s'", filepath.Join(unpackDir, "snapshots", fmt.Sprintf("%s.bin", snapName)))
 		}
 		defer feeder.Close()
 
@@ -270,7 +271,7 @@ func (d *zfs) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData io.
 	// Open the backup.
 	feeder, err := os.Open(filepath.Join(unpackDir, "container.bin"))
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "Failed to open '%s'", filepath.Join(unpackDir, "container.bin"))
 	}
 	defer feeder.Close()
 
@@ -624,13 +625,13 @@ func (d *zfs) DeleteVolume(vol Volume, op *operations.Operation) error {
 		// Delete the mountpoint if present.
 		err := os.Remove(vol.MountPath())
 		if err != nil && !os.IsNotExist(err) {
-			return err
+			return errors.Wrapf(err, "Failed to remove '%s'", vol.MountPath())
 		}
 
 		// Delete the snapshot storage.
 		err = os.RemoveAll(GetVolumeSnapshotDir(d.name, vol.volType, vol.name))
 		if err != nil && !os.IsNotExist(err) {
-			return err
+			return errors.Wrapf(err, "Failed to remove '%s'", GetVolumeSnapshotDir(d.name, vol.volType, vol.name))
 		}
 	}
 
@@ -805,7 +806,7 @@ func (d *zfs) GetVolumeDiskPath(vol Volume) (string, error) {
 	// List all the device nodes.
 	entries, err := ioutil.ReadDir("/dev")
 	if err != nil {
-		return "", err
+		return "", errors.Wrap(err, "Failed to read /dev")
 	}
 
 	for _, entry := range entries {
@@ -1061,7 +1062,7 @@ func (d *zfs) BackupVolume(vol Volume, targetPath string, optimized bool, snapsh
 		// Create the file.
 		fd, err := os.OpenFile(file, os.O_RDWR|os.O_CREATE, 0644)
 		if err != nil {
-			return err
+			return errors.Wrapf(err, "Failed to open '%s'", file)
 		}
 		defer fd.Close()
 
@@ -1089,7 +1090,7 @@ func (d *zfs) BackupVolume(vol Volume, targetPath string, optimized bool, snapsh
 		if len(volSnapshots) > 0 {
 			err = os.MkdirAll(snapshotsPath, 0711)
 			if err != nil {
-				return err
+				return errors.Wrapf(err, "Failed to create directory '%s'", snapshotsPath)
 			}
 		}
 
@@ -1205,7 +1206,7 @@ func (d *zfs) DeleteVolumeSnapshot(vol Volume, op *operations.Operation) error {
 	// Delete the mountpoint.
 	err = os.Remove(vol.MountPath())
 	if err != nil && !os.IsNotExist(err) {
-		return err
+		return errors.Wrapf(err, "Failed to remove '%s'", vol.MountPath())
 	}
 
 	// Remove the parent snapshot directory if this is the last snapshot being removed.
diff --git a/lxd/storage/drivers/utils.go b/lxd/storage/drivers/utils.go
index 9151fd43d6..7e0cd66b3f 100644
--- a/lxd/storage/drivers/utils.go
+++ b/lxd/storage/drivers/utils.go
@@ -25,14 +25,16 @@ func wipeDirectory(path string) error {
 		if os.IsNotExist(err) {
 			return nil
 		}
+
+		return errors.Wrapf(err, "Failed to list directory '%s'", path)
 	}
 
 	// Individually wipe all entries.
 	for _, entry := range entries {
 		entryPath := filepath.Join(path, entry.Name())
 		err := os.RemoveAll(entryPath)
-		if err != nil {
-			return err
+		if err != nil && !os.IsNotExist(err) {
+			return errors.Wrapf(err, "Failed to remove '%s'", entryPath)
 		}
 	}
 
@@ -232,7 +234,12 @@ func createParentSnapshotDirIfMissing(poolName string, volType VolumeType, volNa
 
 	// If it's missing, create it.
 	if !shared.PathExists(snapshotsPath) {
-		return os.Mkdir(snapshotsPath, 0700)
+		err := os.Mkdir(snapshotsPath, 0700)
+		if err != nil {
+			return errors.Wrapf(err, "Failed to create directory '%s'", snapshotsPath)
+		}
+
+		return nil
 	}
 
 	return nil
@@ -252,8 +259,8 @@ func deleteParentSnapshotDirIfEmpty(poolName string, volType VolumeType, volName
 
 		if isEmpty {
 			err := os.Remove(snapshotsPath)
-			if err != nil {
-				return err
+			if err != nil && !os.IsNotExist(err) {
+				return errors.Wrapf(err, "Failed to remove '%s'", snapshotsPath)
 			}
 		}
 	}
diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go
index d479c9d030..71127996b4 100644
--- a/lxd/storage/drivers/volume.go
+++ b/lxd/storage/drivers/volume.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/pkg/errors"
+
 	"github.com/lxc/lxd/lxd/operations"
 	"github.com/lxc/lxd/lxd/storage/locking"
 	"github.com/lxc/lxd/shared"
@@ -128,7 +130,7 @@ func (v Volume) EnsureMountPath() error {
 	// Create volume's mount path, with any created directories set to 0711.
 	err := os.Mkdir(volPath, 0711)
 	if err != nil && !os.IsExist(err) {
-		return err
+		return errors.Wrapf(err, "Failed to create directory '%s'", volPath)
 	}
 
 	// Set very restrictive mode 0100 for non-custom and non-image volumes.
@@ -140,7 +142,7 @@ func (v Volume) EnsureMountPath() error {
 	// Set mode of actual volume's mount path.
 	err = os.Chmod(volPath, mode)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "Failed to chmod '%s'", volPath)
 	}
 
 	return nil


More information about the lxc-devel mailing list