[lxc-devel] [lxd/master] Improve idmap comparisons

[stgraber on Github]

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/20190412/dec07cab/attachment.bin>
-------------- next part --------------
From 7c6ce1495f3a418ac82d027bfddc1b7fdad0174d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Fri, 12 Apr 2019 15:25:30 -0400
Subject: [PATCH 1/3] lxd/storage: Fix error message on differing maps
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.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/storage.go b/lxd/storage.go
index a9894198d1..bf409e7770 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -478,7 +478,7 @@ func storagePoolVolumeAttachInit(s *state.State, poolName string, volumeName str
 				}
 
 				if !reflect.DeepEqual(nextIdmap, ctNextIdmap) {
-					return nil, fmt.Errorf("Idmaps of container %v and storage volume %v are not identical", ctNextIdmap, nextIdmap)
+					return nil, fmt.Errorf("Idmaps of container %v and storage volume %v are not identical", ctName, volumeName)
 				}
 			}
 		} else if len(volumeUsedBy) == 1 {

From 61349c60d3c97bb4c8df0e32dee9e5281ed84c4d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Fri, 12 Apr 2019 15:19:44 -0400
Subject: [PATCH 2/3] shared/idmap: Add comparison function
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>
---
 shared/idmap/idmapset_linux.go | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/shared/idmap/idmapset_linux.go b/shared/idmap/idmapset_linux.go
index 617f43acfb..1e9eee1e12 100644
--- a/shared/idmap/idmapset_linux.go
+++ b/shared/idmap/idmapset_linux.go
@@ -264,6 +264,34 @@ type IdmapSet struct {
 	Idmap []IdmapEntry
 }
 
+func (m *IdmapSet) Equals(other *IdmapSet) bool {
+	// Get comparable maps
+	expandSortIdmap := func(input *IdmapSet) IdmapSet {
+		if input == nil {
+			input = &IdmapSet{}
+		}
+
+		newEntries := []IdmapEntry{}
+
+		for _, entry := range input.Idmap {
+			if entry.Isuid && entry.Isgid {
+				newEntries = append(newEntries, IdmapEntry{true, false, entry.Hostid, entry.Nsid, entry.Maprange})
+				newEntries = append(newEntries, IdmapEntry{false, true, entry.Hostid, entry.Nsid, entry.Maprange})
+			} else {
+				newEntries = append(newEntries, entry)
+			}
+		}
+
+		output := IdmapSet{Idmap: newEntries}
+		sort.Sort(output)
+
+		return output
+	}
+
+	// Actually compare
+	return reflect.DeepEqual(expandSortIdmap(m), expandSortIdmap(other))
+}
+
 func (m IdmapSet) Len() int {
 	return len(m.Idmap)
 }

From b7b68cf350698246f019ed302e39c7a3b97962f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Fri, 12 Apr 2019 15:20:01 -0400
Subject: [PATCH 3/3] lxd: Use idmap.Equals
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/container_lxc.go | 2 +-
 lxd/storage.go       | 7 +++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index a25923748e..98ad0fefaa 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -2011,7 +2011,7 @@ func (c *containerLXC) startCommon() (string, error) {
 		return "", errors.Wrap(err, "Set last ID map")
 	}
 
-	if !reflect.DeepEqual(nextIdmap, diskIdmap) && !(diskIdmap == nil && c.state.OS.Shiftfs) {
+	if !nextIdmap.Equals(diskIdmap) && !(diskIdmap == nil && c.state.OS.Shiftfs) {
 		if shared.IsTrue(c.expandedConfig["security.protection.shift"]) {
 			return "", fmt.Errorf("Container is protected against filesystem shifting")
 		}
diff --git a/lxd/storage.go b/lxd/storage.go
index bf409e7770..2e07d53039 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"reflect"
 	"sync"
 	"sync/atomic"
 
@@ -452,7 +451,7 @@ func storagePoolVolumeAttachInit(s *state.State, poolName string, volumeName str
 		return nil, err
 	}
 
-	if !reflect.DeepEqual(nextIdmap, lastIdmap) {
+	if !nextIdmap.Equals(lastIdmap) {
 		logger.Debugf("Shifting storage volume")
 		volumeUsedBy, err := storagePoolVolumeUsedByContainersGet(s,
 			"default", volumeName, volumeTypeName)
@@ -477,7 +476,7 @@ func storagePoolVolumeAttachInit(s *state.State, poolName string, volumeName str
 					return nil, fmt.Errorf("Failed to retrieve idmap of container")
 				}
 
-				if !reflect.DeepEqual(nextIdmap, ctNextIdmap) {
+				if !nextIdmap.Equals(ctNextIdmap) {
 					return nil, fmt.Errorf("Idmaps of container %v and storage volume %v are not identical", ctName, volumeName)
 				}
 			}
@@ -766,7 +765,7 @@ func resetContainerDiskIdmap(container container, srcIdmap *idmap.IdmapSet) erro
 		dstIdmap = new(idmap.IdmapSet)
 	}
 
-	if !reflect.DeepEqual(srcIdmap, dstIdmap) {
+	if !srcIdmap.Equals(dstIdmap) {
 		var jsonIdmap string
 		if srcIdmap != nil {
 			idmapBytes, err := json.Marshal(srcIdmap.Idmap)