[lxc-devel] [lxd/master] [RFC] btrfs improvements

brauner on Github lxc-bot at linuxcontainers.org
Sun Mar 5 20:08:52 UTC 2017


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/20170305/bfe037ee/attachment.bin>
-------------- next part --------------
From 92a386c356dcaa9e31efd3aa7ebd92b311477079 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sun, 5 Mar 2017 13:25:05 +0100
Subject: [PATCH 1/2] btrfs: correctly handle loop-backed pools

We allow users to create loop-backed btrfs pools even it they are on a btrfs
filesystem already. However, until recently StoragePoolMount() did not correctly
handle this case. This commit ensure that mounting a loop-backed storage pool on
a btrfs filesystem works correctly.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_btrfs.go | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lxd/storage_btrfs.go b/lxd/storage_btrfs.go
index d1b3b1d..eb77824 100644
--- a/lxd/storage_btrfs.go
+++ b/lxd/storage_btrfs.go
@@ -341,8 +341,13 @@ func (s *storageBtrfs) StoragePoolMount() (bool, error) {
 	}
 
 	mountSource := source
+	isBlockDev := shared.IsBlockdevPath(source)
 	if filepath.IsAbs(source) {
-		if !shared.IsBlockdevPath(source) && s.d.BackingFs != "btrfs" {
+		loopFilePath := shared.VarPath("disks", s.pool.Name+".img")
+		if !isBlockDev && source == loopFilePath {
+			// If source == "${LXD_DIR}"/disks/{pool_name} it is a
+			// loop file we're dealing with.
+			//
 			// Since we mount the loop device LO_FLAGS_AUTOCLEAR is
 			// fine since the loop device will be kept around for as
 			// long as the mount exists.
@@ -352,9 +357,11 @@ func (s *storageBtrfs) StoragePoolMount() (bool, error) {
 			}
 			mountSource = loopF.Name()
 			defer loopF.Close()
-		} else {
+		} else if !isBlockDev && s.d.BackingFs == "btrfs" {
+			// User is using btrfs subvolume as pool.
 			return false, nil
 		}
+		// User is using block device path.
 	} else {
 		// Try to lookup the disk device by UUID but don't fail. If we
 		// don't find one this might just mean we have been given the

From ab64dd7aa13d26e11ae18019925bdaf07bf468f7 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sun, 5 Mar 2017 20:02:33 +0100
Subject: [PATCH 2/2] btrfs: handle custom subvolume paths

This improves handling:

1. lxc storage create pool1 source=/path/to/subvolume
- create /path/to/subvolume if it doesn't already exist and bind-mount onto
  ${LXD_DIR}/storage-pools/pool1

2. lxc storage create pool1 source=${LXD_DIR}/storage-pools/pool1
  - create subvolume if LXD is located on btrfs

3. lxc storage create pool1 source=/path-somewhere-under-${LXD_DIR}-but-not-under-storage-pools-subdir
  - This is considered invalid (As it might mess with LXD's on-disk layout).
    The following error message is given:

	chb at conventiont|~/source/go/src/github.com/lxc/lxd|2017-03-05/btrfs_improvements *$%>
	> lxc storage create pool1 btrfs source=/home/chb/mnt/l1/storage-pools/
	error: BTRFS subvolumes requests in LXD directory "/home/chb/mnt/l1" are only valid under "/home/chb/mnt/l1/storage-pools"
	(e.g. source=/home/chb/mnt/l1/storage-pools/pool1)

    I really wouldn't want to allow users to create subvolumes under say e.g.
    ${LXD_DIR}/containers or the like. This will just cause trouble.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_btrfs.go | 76 +++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 57 insertions(+), 19 deletions(-)

diff --git a/lxd/storage_btrfs.go b/lxd/storage_btrfs.go
index eb77824..c563c47 100644
--- a/lxd/storage_btrfs.go
+++ b/lxd/storage_btrfs.go
@@ -138,13 +138,32 @@ func (s *storageBtrfs) StoragePoolCreate() error {
 					return fmt.Errorf("Failed to create the BTRFS pool: %s", output)
 				}
 			} else {
-				if isBtrfsSubVolume(source) || s.d.BackingFs == "btrfs" {
+				if isBtrfsSubVolume(source) {
+					subvols, err := btrfsSubVolumesGet(source)
+					if err != nil {
+						return fmt.Errorf("Could not determine if existing BTRFS subvolume ist empty: %s.", err)
+					}
+					if len(subvols) > 0 {
+						return fmt.Errorf("Requested BTRFS subvolume exists but is not empty.")
+					}
+				} else {
+					cleanSource := filepath.Clean(source)
+					lxdDir := shared.VarPath()
+					poolMntPoint := getStoragePoolMountPoint(s.pool.Name)
+					if shared.PathExists(source) && !isOnBtrfs(source) {
+						return fmt.Errorf("Existing path is neither a BTRFS subvolume nor does it reside on a BTRFS filesystem.")
+					} else if strings.HasPrefix(cleanSource, lxdDir) {
+						if cleanSource != poolMntPoint {
+							return fmt.Errorf("BTRFS subvolumes requests in LXD directory \"%s\" are only valid under \"%s\"\n(e.g. source=%s)", shared.VarPath(), shared.VarPath("storage-pools"), poolMntPoint)
+						} else if s.d.BackingFs != "btrfs" {
+							return fmt.Errorf("Creation of BTRFS subvolume requested but \"%s\" does not reside on BTRFS filesystem.", source)
+						}
+					}
+
 					err := btrfsSubVolumeCreate(source)
 					if err != nil {
 						return err
 					}
-				} else {
-					return fmt.Errorf("Custom loop file locations are not supported.")
 				}
 			}
 		} else {
@@ -274,11 +293,14 @@ func (s *storageBtrfs) StoragePoolDelete() error {
 		shared.LogDebugf(msg)
 	} else {
 		var err error
-		if s.d.BackingFs == "btrfs" {
-			err = btrfsSubVolumesDelete(source)
-		} else {
+		cleanSource := filepath.Clean(source)
+		sourcePath := shared.VarPath("disks", s.pool.Name)
+		loopFilePath := sourcePath + ".img"
+		if cleanSource == loopFilePath {
 			// This is a loop file --> simply remove it.
 			err = os.Remove(source)
+		} else {
+			err = btrfsSubVolumesDelete(source)
 		}
 		if err != nil {
 			return err
@@ -340,9 +362,12 @@ func (s *storageBtrfs) StoragePoolMount() (bool, error) {
 		return false, nil
 	}
 
+	mountFlags := uintptr(0)
 	mountSource := source
 	isBlockDev := shared.IsBlockdevPath(source)
 	if filepath.IsAbs(source) {
+		cleanSource := filepath.Clean(source)
+		poolMntPoint := getStoragePoolMountPoint(s.pool.Name)
 		loopFilePath := shared.VarPath("disks", s.pool.Name+".img")
 		if !isBlockDev && source == loopFilePath {
 			// If source == "${LXD_DIR}"/disks/{pool_name} it is a
@@ -357,8 +382,10 @@ func (s *storageBtrfs) StoragePoolMount() (bool, error) {
 			}
 			mountSource = loopF.Name()
 			defer loopF.Close()
-		} else if !isBlockDev && s.d.BackingFs == "btrfs" {
-			// User is using btrfs subvolume as pool.
+		} else if !isBlockDev && cleanSource != poolMntPoint {
+			mountSource = source
+			mountFlags = syscall.MS_BIND
+		} else if !isBlockDev && cleanSource == poolMntPoint && s.d.BackingFs == "btrfs" {
 			return false, nil
 		}
 		// User is using block device path.
@@ -382,7 +409,7 @@ func (s *storageBtrfs) StoragePoolMount() (bool, error) {
 	}
 
 	// This is a block device.
-	err := syscall.Mount(mountSource, poolMntPoint, "btrfs", 0, btrfsMntOptions)
+	err := syscall.Mount(mountSource, poolMntPoint, "btrfs", mountFlags, btrfsMntOptions)
 	if err != nil {
 		return false, err
 	}
@@ -1313,7 +1340,8 @@ func (s *storageBtrfs) ImageUmount(fingerprint string) (bool, error) {
 func btrfsSubVolumeCreate(subvol string) error {
 	parentDestPath := filepath.Dir(subvol)
 	if !shared.PathExists(parentDestPath) {
-		if err := os.MkdirAll(parentDestPath, 0711); err != nil {
+		err := os.MkdirAll(parentDestPath, 0711)
+		if err != nil {
 			return err
 		}
 	}
@@ -1324,11 +1352,8 @@ func btrfsSubVolumeCreate(subvol string) error {
 		"create",
 		subvol).CombinedOutput()
 	if err != nil {
-		return fmt.Errorf(
-			"btrfs subvolume create failed, subvol=%s, output%s",
-			subvol,
-			string(output),
-		)
+		shared.LogErrorf("Failed to create BTRFS subvolume \"%s\": %s.", subvol, string(output))
+		return err
 	}
 
 	return nil
@@ -1526,10 +1551,8 @@ func (s *storageBtrfs) btrfsPoolVolumesSnapshot(source string, dest string, read
 	return nil
 }
 
-/*
- * isBtrfsSubVolume returns true if the given Path is a btrfs subvolume
- * else false.
- */
+// isBtrfsSubVolume returns true if the given Path is a btrfs subvolume else
+// false.
 func isBtrfsSubVolume(subvolPath string) bool {
 	fs := syscall.Stat_t{}
 	err := syscall.Lstat(subvolPath, &fs)
@@ -1545,6 +1568,21 @@ func isBtrfsSubVolume(subvolPath string) bool {
 	return true
 }
 
+func isOnBtrfs(path string) bool {
+	fs := syscall.Statfs_t{}
+
+	err := syscall.Statfs(path, &fs)
+	if err != nil {
+		return false
+	}
+
+	if fs.Type != filesystemSuperMagicBtrfs {
+		return false
+	}
+
+	return true
+}
+
 func btrfsSubVolumesGet(path string) ([]string, error) {
 	result := []string{}
 


More information about the lxc-devel mailing list