[lxc-devel] [lxd/master] storage: lock Container{U, M}ount for zfs, lvm

brauner on Github lxc-bot at linuxcontainers.org
Wed Feb 15 16:40:33 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170215/629bda37/attachment.bin>
-------------- next part --------------
From 53c312d1f985d94c6ccd0807744f42482730259d Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 15 Feb 2017 17:38:53 +0100
Subject: [PATCH] storage: lock Container{U,M}ount for zfs, lvm

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage.go       |  4 +--
 lxd/storage_btrfs.go | 14 +++++-----
 lxd/storage_lvm.go   | 72 +++++++++++++++++++++++++++++++++++++++------------
 lxd/storage_zfs.go   | 73 +++++++++++++++++++++++++++++++++++++++-------------
 4 files changed, 119 insertions(+), 44 deletions(-)

diff --git a/lxd/storage.go b/lxd/storage.go
index f90cc74..9023031 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -22,8 +22,8 @@ import (
 	log "gopkg.in/inconshreveable/log15.v2"
 )
 
-var imageCreationInPool = map[string]chan bool{}
-var imageCreationInPoolLock sync.Mutex
+var lxdStorageLockMap = map[string]chan bool{}
+var lxdStorageLock sync.Mutex
 
 /* Some interesting filesystems */
 const (
diff --git a/lxd/storage_btrfs.go b/lxd/storage_btrfs.go
index d847f34..ebcecdb 100644
--- a/lxd/storage_btrfs.go
+++ b/lxd/storage_btrfs.go
@@ -571,23 +571,23 @@ func (s *storageBtrfs) ContainerCreateFromImage(container container, fingerprint
 	// ${LXD_DIR}/images/<fingerprint>
 	imageMntPoint := getImageMountPoint(s.pool.Name, fingerprint)
 	imageStoragePoolLockID := fmt.Sprintf("%s/%s", s.pool.Name, fingerprint)
-	imageCreationInPoolLock.Lock()
-	if waitChannel, ok := imageCreationInPool[imageStoragePoolLockID]; ok {
-		imageCreationInPoolLock.Unlock()
+	lxdStorageLock.Lock()
+	if waitChannel, ok := lxdStorageLockMap[imageStoragePoolLockID]; ok {
+		lxdStorageLock.Unlock()
 		if _, ok := <-waitChannel; ok {
 			shared.LogWarnf("Value transmitted over image lock semaphore?")
 		}
 	} else {
-		imageCreationInPool[imageStoragePoolLockID] = make(chan bool)
+		lxdStorageLockMap[imageStoragePoolLockID] = make(chan bool)
 		var imgerr error
 		if !shared.PathExists(imageMntPoint) || !s.isBtrfsPoolVolume(imageMntPoint) {
 			imgerr = s.ImageCreate(fingerprint)
 		}
-		if waitChannel, ok := imageCreationInPool[imageStoragePoolLockID]; ok {
+		if waitChannel, ok := lxdStorageLockMap[imageStoragePoolLockID]; ok {
 			close(waitChannel)
-			delete(imageCreationInPool, imageStoragePoolLockID)
+			delete(lxdStorageLockMap, imageStoragePoolLockID)
 		}
-		imageCreationInPoolLock.Unlock()
+		lxdStorageLock.Unlock()
 		if imgerr != nil {
 			return imgerr
 		}
diff --git a/lxd/storage_lvm.go b/lxd/storage_lvm.go
index 4afda74..7ae48ce 100644
--- a/lxd/storage_lvm.go
+++ b/lxd/storage_lvm.go
@@ -547,23 +547,23 @@ func (s *storageLvm) ContainerCreateFromImage(container container, fingerprint s
 	imageLvmDevPath := getLvmDevPath(s.pool.Name, storagePoolVolumeApiEndpointImages, fingerprint)
 
 	imageStoragePoolLockID := fmt.Sprintf("%s/%s", s.pool.Name, fingerprint)
-	imageCreationInPoolLock.Lock()
-	if waitChannel, ok := imageCreationInPool[imageStoragePoolLockID]; ok {
-		imageCreationInPoolLock.Unlock()
+	lxdStorageLock.Lock()
+	if waitChannel, ok := lxdStorageLockMap[imageStoragePoolLockID]; ok {
+		lxdStorageLock.Unlock()
 		if _, ok := <-waitChannel; ok {
 			shared.LogWarnf("Value transmitted over image lock semaphore?")
 		}
 	} else {
-		imageCreationInPool[imageStoragePoolLockID] = make(chan bool)
+		lxdStorageLockMap[imageStoragePoolLockID] = make(chan bool)
 		var imgerr error
 		if !shared.PathExists(imageMntPoint) || !shared.PathExists(imageLvmDevPath) {
 			imgerr = s.ImageCreate(fingerprint)
 		}
-		if waitChannel, ok := imageCreationInPool[imageStoragePoolLockID]; ok {
+		if waitChannel, ok := lxdStorageLockMap[imageStoragePoolLockID]; ok {
 			close(waitChannel)
-			delete(imageCreationInPool, imageStoragePoolLockID)
+			delete(lxdStorageLockMap, imageStoragePoolLockID)
 		}
-		imageCreationInPoolLock.Unlock()
+		lxdStorageLock.Unlock()
 		if imgerr != nil {
 			return imgerr
 		}
@@ -759,31 +759,69 @@ func (s *storageLvm) ContainerMount(name string, path string) (bool, error) {
 	mountOptions := s.volume.Config["block.mount_options"]
 	containerMntPoint := getContainerMountPoint(s.pool.Name, name)
 
-	if shared.IsMountPoint(containerMntPoint) {
+	containerMountLockID := fmt.Sprintf("mount/%s/%s", s.pool.Name, name)
+	lxdStorageLock.Lock()
+	if waitChannel, ok := lxdStorageLockMap[containerMountLockID]; ok {
+		lxdStorageLock.Unlock()
+		if _, ok := <-waitChannel; ok {
+			shared.LogWarnf("Value transmitted over image lock semaphore?")
+		}
+		// Give the benefit of the doubt and assume that the other
+		// thread actually succeeded in mounting the storage volume.
 		return false, nil
 	}
 
-	err := tryMount(containerLvmPath, containerMntPoint, lvFsType, 0, mountOptions)
-	if err != nil {
-		return false, fmt.Errorf("Error mounting snapshot LV path='%s': %s", path, err)
+	lxdStorageLockMap[containerMountLockID] = make(chan bool)
+	var imgerr error
+	ourMount := false
+	if !shared.IsMountPoint(containerMntPoint) {
+		imgerr = tryMount(containerLvmPath, containerMntPoint, lvFsType, 0, mountOptions)
+		ourMount = true
+	}
+	if waitChannel, ok := lxdStorageLockMap[containerMountLockID]; ok {
+		close(waitChannel)
+		delete(lxdStorageLockMap, containerMountLockID)
+	}
+	lxdStorageLock.Unlock()
+	if imgerr != nil {
+		return false, imgerr
 	}
 
-	return true, nil
+	return ourMount, nil
 }
 
 func (s *storageLvm) ContainerUmount(name string, path string) (bool, error) {
 	containerMntPoint := getContainerMountPoint(s.pool.Name, name)
 
-	if !shared.IsMountPoint(containerMntPoint) {
+	containerUmountLockID := fmt.Sprintf("umount/%s/%s", s.pool.Name, name)
+	lxdStorageLock.Lock()
+	if waitChannel, ok := lxdStorageLockMap[containerUmountLockID]; ok {
+		lxdStorageLock.Unlock()
+		if _, ok := <-waitChannel; ok {
+			shared.LogWarnf("Value transmitted over image lock semaphore?")
+		}
+		// Give the benefit of the doubt and assume that the other
+		// thread actually succeeded in unmounting the storage volume.
 		return false, nil
 	}
 
-	err := tryUnmount(containerMntPoint, 0)
-	if err != nil {
-		return false, fmt.Errorf("failed to unmount container path '%s': %s", path, err)
+	lxdStorageLockMap[containerUmountLockID] = make(chan bool)
+	var imgerr error
+	ourUmount := false
+	if shared.IsMountPoint(containerMntPoint) {
+		imgerr = tryUnmount(containerMntPoint, 0)
+		ourUmount = true
+	}
+	if waitChannel, ok := lxdStorageLockMap[containerUmountLockID]; ok {
+		close(waitChannel)
+		delete(lxdStorageLockMap, containerUmountLockID)
+	}
+	lxdStorageLock.Unlock()
+	if imgerr != nil {
+		return false, imgerr
 	}
 
-	return true, nil
+	return ourUmount, nil
 }
 
 func (s *storageLvm) ContainerRename(container container, newContainerName string) error {
diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 0e83884..0ad6eda 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -398,33 +398,70 @@ func (s *storageZfs) ContainerMount(name string, path string) (bool, error) {
 	fs := fmt.Sprintf("containers/%s", name)
 	containerPoolVolumeMntPoint := getContainerMountPoint(s.pool.Name, name)
 
-	// Just in case the container filesystem got unmounted
-	if shared.IsMountPoint(containerPoolVolumeMntPoint) {
+	containerMountLockID := fmt.Sprintf("mount/%s/%s", s.pool.Name, name)
+	lxdStorageLock.Lock()
+	if waitChannel, ok := lxdStorageLockMap[containerMountLockID]; ok {
+		lxdStorageLock.Unlock()
+		if _, ok := <-waitChannel; ok {
+			shared.LogWarnf("Value transmitted over image lock semaphore?")
+		}
+		// Give the benefit of the doubt and assume that the other
+		// thread actually succeeded in mounting the storage volume.
 		return false, nil
 	}
 
-	err := s.zfsPoolVolumeMount(fs)
-	if err != nil {
-		return false, err
+	lxdStorageLockMap[containerMountLockID] = make(chan bool)
+	var imgerr error
+	ourMount := false
+	if !shared.IsMountPoint(containerPoolVolumeMntPoint) {
+		imgerr = s.zfsPoolVolumeMount(fs)
+		ourMount = true
+	}
+	if waitChannel, ok := lxdStorageLockMap[containerMountLockID]; ok {
+		close(waitChannel)
+		delete(lxdStorageLockMap, containerMountLockID)
+	}
+	lxdStorageLock.Unlock()
+	if imgerr != nil {
+		return false, imgerr
 	}
 
-	return true, nil
+	return ourMount, nil
 }
 
 func (s *storageZfs) ContainerUmount(name string, path string) (bool, error) {
 	fs := fmt.Sprintf("containers/%s", name)
 	containerPoolVolumeMntPoint := getContainerMountPoint(s.pool.Name, name)
 
-	if !shared.IsMountPoint(containerPoolVolumeMntPoint) {
+	containerUmountLockID := fmt.Sprintf("umount/%s/%s", s.pool.Name, name)
+	lxdStorageLock.Lock()
+	if waitChannel, ok := lxdStorageLockMap[containerUmountLockID]; ok {
+		lxdStorageLock.Unlock()
+		if _, ok := <-waitChannel; ok {
+			shared.LogWarnf("Value transmitted over image lock semaphore?")
+		}
+		// Give the benefit of the doubt and assume that the other
+		// thread actually succeeded in unmounting the storage volume.
 		return false, nil
 	}
 
-	err := s.zfsPoolVolumeUmount(fs)
-	if err != nil {
-		return false, err
+	lxdStorageLockMap[containerUmountLockID] = make(chan bool)
+	var imgerr error
+	ourUmount := false
+	if shared.IsMountPoint(containerPoolVolumeMntPoint) {
+		imgerr = s.zfsPoolVolumeUmount(fs)
+		ourUmount = true
+	}
+	if waitChannel, ok := lxdStorageLockMap[containerUmountLockID]; ok {
+		close(waitChannel)
+		delete(lxdStorageLockMap, containerUmountLockID)
+	}
+	lxdStorageLock.Unlock()
+	if imgerr != nil {
+		return false, imgerr
 	}
 
-	return true, nil
+	return ourUmount, nil
 }
 
 // Things we do have to care about
@@ -477,23 +514,23 @@ func (s *storageZfs) ContainerCreateFromImage(container container, fingerprint s
 	fsImage := fmt.Sprintf("images/%s", fingerprint)
 
 	imageStoragePoolLockID := fmt.Sprintf("%s/%s", s.pool.Name, fingerprint)
-	imageCreationInPoolLock.Lock()
-	if waitChannel, ok := imageCreationInPool[imageStoragePoolLockID]; ok {
-		imageCreationInPoolLock.Unlock()
+	lxdStorageLock.Lock()
+	if waitChannel, ok := lxdStorageLockMap[imageStoragePoolLockID]; ok {
+		lxdStorageLock.Unlock()
 		if _, ok := <-waitChannel; ok {
 			shared.LogWarnf("Value transmitted over image lock semaphore?")
 		}
 	} else {
-		imageCreationInPool[imageStoragePoolLockID] = make(chan bool)
+		lxdStorageLockMap[imageStoragePoolLockID] = make(chan bool)
 		var imgerr error
 		if !s.zfsPoolVolumeExists(fsImage) {
 			imgerr = s.ImageCreate(fingerprint)
 		}
-		if waitChannel, ok := imageCreationInPool[imageStoragePoolLockID]; ok {
+		if waitChannel, ok := lxdStorageLockMap[imageStoragePoolLockID]; ok {
 			close(waitChannel)
-			delete(imageCreationInPool, imageStoragePoolLockID)
+			delete(lxdStorageLockMap, imageStoragePoolLockID)
 		}
-		imageCreationInPoolLock.Unlock()
+		lxdStorageLock.Unlock()
 		if imgerr != nil {
 			return imgerr
 		}


More information about the lxc-devel mailing list