[lxc-devel] [lxd/master] lvm: resolve mount options properly

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 381 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170504/71a47b60/attachment.bin>
-------------- next part --------------
From be8bd339793f8b0a8f83ef4b8e83a6d8f468bcc7 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 4 May 2017 17:56:51 +0200
Subject: [PATCH 1/2] lvm: resolve mount options properly

Closes #3284.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_lvm.go | 51 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/lxd/storage_lvm.go b/lxd/storage_lvm.go
index 61849fc..1c87d17 100644
--- a/lxd/storage_lvm.go
+++ b/lxd/storage_lvm.go
@@ -228,16 +228,39 @@ type storageLvm struct {
 	storageShared
 }
 
-func (s *storageLvm) getLvmBlockMountOptions() string {
-	if s.volume.Config["block.mount_options"] != "" {
-		return s.volume.Config["block.mount_options"]
+func (s *storageLvm) getLvmBlockMountOptions() (uintptr, string) {
+	opts := s.volume.Config["block.mount_options"]
+
+	if opts == "" {
+		opts = s.pool.Config["volume.block.mount_options"]
+	}
+
+	if opts == "" {
+		return 0, "discard"
 	}
 
-	if s.pool.Config["volume.block.mount_options"] != "" {
-		return s.pool.Config["volume.block.mount_options"]
+	mountFlags := uintptr(0)
+	tmp := strings.SplitN(opts, ",", -1)
+	for i := 0; i < len(tmp); i++ {
+		opt := tmp[i]
+		do, ok := MountOptions[opt]
+		if !ok {
+			continue
+		}
+
+		if do.unset {
+			mountFlags &= ^do.flag
+		} else {
+			mountFlags |= do.flag
+		}
+
+		copy(tmp[i:], tmp[i+1:])
+		tmp[len(tmp)-1] = ""
+		tmp = tmp[:len(tmp)-1]
+		i--
 	}
 
-	return "discard"
+	return mountFlags, strings.Join(tmp, ",")
 }
 
 func (s *storageLvm) getLvmFilesystem() string {
@@ -784,7 +807,7 @@ func (s *storageLvm) StoragePoolVolumeMount() (bool, error) {
 
 	customPoolVolumeMntPoint := getStoragePoolVolumeMountPoint(s.pool.Name, s.volume.Name)
 	poolName := s.getOnDiskPoolName()
-	mountOptions := s.getLvmBlockMountOptions()
+	mountFlags, mountOptions := s.getLvmBlockMountOptions()
 	lvFsType := s.getLvmFilesystem()
 	volumeType, err := storagePoolVolumeTypeNameToAPIEndpoint(s.volume.Type)
 	if err != nil {
@@ -810,7 +833,7 @@ func (s *storageLvm) StoragePoolVolumeMount() (bool, error) {
 	var customerr error
 	ourMount := false
 	if !shared.IsMountPoint(customPoolVolumeMntPoint) {
-		customerr = tryMount(lvmVolumePath, customPoolVolumeMntPoint, lvFsType, 0, mountOptions)
+		customerr = tryMount(lvmVolumePath, customPoolVolumeMntPoint, lvFsType, mountFlags, mountOptions)
 		ourMount = true
 	}
 
@@ -1493,7 +1516,7 @@ func (s *storageLvm) ContainerMount(c container) (bool, error) {
 	lvFsType := s.getLvmFilesystem()
 	poolName := s.getOnDiskPoolName()
 	containerLvmPath := getLvmDevPath(poolName, storagePoolVolumeAPIEndpointContainers, containerLvmName)
-	mountOptions := s.getLvmBlockMountOptions()
+	mountFlags, mountOptions := s.getLvmBlockMountOptions()
 
 	containerMntPoint := getContainerMountPoint(s.pool.Name, name)
 	if shared.IsSnapshot(name) {
@@ -1518,7 +1541,7 @@ func (s *storageLvm) ContainerMount(c container) (bool, error) {
 	var mounterr error
 	ourMount := false
 	if !shared.IsMountPoint(containerMntPoint) {
-		mounterr = tryMount(containerLvmPath, containerMntPoint, lvFsType, 0, mountOptions)
+		mounterr = tryMount(containerLvmPath, containerMntPoint, lvFsType, mountFlags, mountOptions)
 		ourMount = true
 	}
 
@@ -1862,11 +1885,11 @@ func (s *storageLvm) ContainerSnapshotStart(container container) (bool, error) {
 	}
 
 	lvFsType := s.getLvmFilesystem()
-	mountOptions := s.getLvmBlockMountOptions()
+	mountFlags, mountOptions := s.getLvmBlockMountOptions()
 	containerMntPoint := getSnapshotMountPoint(s.pool.Name, containerName)
 
 	if !shared.IsMountPoint(containerMntPoint) {
-		err = tryMount(containerLvmPath, containerMntPoint, lvFsType, 0, mountOptions)
+		err = tryMount(containerLvmPath, containerMntPoint, lvFsType, mountFlags, mountOptions)
 		if err != nil {
 			return false, fmt.Errorf("Error mounting snapshot LV path='%s': %s", containerMntPoint, err)
 		}
@@ -2058,8 +2081,8 @@ func (s *storageLvm) ImageMount(fingerprint string) (bool, error) {
 
 	poolName := s.getOnDiskPoolName()
 	lvmVolumePath := getLvmDevPath(poolName, storagePoolVolumeAPIEndpointImages, fingerprint)
-	lvmMountOptions := s.getLvmBlockMountOptions()
-	err := tryMount(lvmVolumePath, imageMntPoint, lvmFstype, 0, lvmMountOptions)
+	mountFlags, mountOptions := s.getLvmBlockMountOptions()
+	err := tryMount(lvmVolumePath, imageMntPoint, lvmFstype, mountFlags, mountOptions)
 	if err != nil {
 		logger.Errorf(fmt.Sprintf("Error mounting image LV for unpacking: %s", err))
 		return false, fmt.Errorf("Error mounting image LV: %v", err)

From 8844cba69d65ecfed5afdc0233b3dea1010b8f1e Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 4 May 2017 18:02:56 +0200
Subject: [PATCH 2/2] test: add mount option test for lvm

Closes #3284.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 test/suites/storage.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/test/suites/storage.sh b/test/suites/storage.sh
index 2ddbb46..8b2038c 100644
--- a/test/suites/storage.sh
+++ b/test/suites/storage.sh
@@ -176,6 +176,8 @@ test_storage() {
       lxc storage create "lxdtest-$(basename "${LXD_DIR}")-valid-lvm-pool-config-pool22" lvm lvm.use_thinpool=true
       lxc storage create "lxdtest-$(basename "${LXD_DIR}")-valid-lvm-pool-config-pool23" lvm lvm.use_thinpool=true lvm.thinpool_name="lxdtest-$(basename "${LXD_DIR}")-valid-lvm-pool-config"
       lxc storage create "lxdtest-$(basename "${LXD_DIR}")-valid-lvm-pool-config-pool24" lvm rsync.bwlimit=1024
+      lxc storage create "lxdtest-$(basename "${LXD_DIR}")-valid-lvm-pool-config-pool25" lvm volume.block.mount_options="rw,strictatime,discard"
+      lxc storage set "lxdtest-$(basename "${LXD_DIR}")-valid-lvm-pool-config-pool25" volume.block.mount_options "rw,lazytime"
     fi
 
     # Set default storage pool for image import.
@@ -658,6 +660,9 @@ test_storage() {
 
       lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-valid-lvm-pool-config-pool24"
       vgremove -ff "lxdtest-$(basename "${LXD_DIR}")-non-thinpool-pool24" || true
+
+      lxc storage delete "lxdtest-$(basename "${LXD_DIR}")-valid-lvm-pool-config-pool25"
+      vgremove -ff "lxdtest-$(basename "${LXD_DIR}")-non-thinpool-pool25" || true
     fi
   )