[lxc-devel] [lxd/master] Handle immutable/append-only files on removal

[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/20200317/3e54862c/attachment-0001.bin>
-------------- next part --------------
From fa89288a986fad8f049f2ec81d1925f6ea8bd27e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 17 Mar 2020 18:37:31 -0400
Subject: [PATCH 1/3] lxd/storage/utils: Add missing comments
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/utils.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lxd/storage/drivers/utils.go b/lxd/storage/drivers/utils.go
index 430f9673b9..2fcbf98acb 100644
--- a/lxd/storage/drivers/utils.go
+++ b/lxd/storage/drivers/utils.go
@@ -43,6 +43,7 @@ func wipeDirectory(path string) error {
 	return nil
 }
 
+// forceUnmount unmounts stacked mounts until no mountpoint remains.
 func forceUnmount(path string) (bool, error) {
 	unmounted := false
 
@@ -66,6 +67,7 @@ func forceUnmount(path string) (bool, error) {
 	}
 }
 
+// mountReadOnly performs a read-only bind-mount.
 func mountReadOnly(srcPath string, dstPath string) (bool, error) {
 	// Check if already mounted.
 	if shared.IsMountPoint(dstPath) {
@@ -88,6 +90,7 @@ func mountReadOnly(srcPath string, dstPath string) (bool, error) {
 	return true, nil
 }
 
+// sameMount checks if two paths are on the same mountpoint.
 func sameMount(srcPath string, dstPath string) bool {
 	// Get the source vfs path information
 	var srcFsStat unix.Statfs_t
@@ -171,6 +174,7 @@ func TryUnmount(path string, flags int) error {
 	return nil
 }
 
+// tryExists waits up to 10s for a file to exist.
 func tryExists(path string) bool {
 	// Attempt 20 checks over 10s
 	for i := 0; i < 20; i++ {
@@ -184,10 +188,12 @@ func tryExists(path string) bool {
 	return false
 }
 
+// fsUUID returns the filesystem UUID for the given block path.
 func fsUUID(path string) (string, error) {
 	return shared.RunCommand("blkid", "-s", "UUID", "-o", "value", path)
 }
 
+// hasFilesystem checks if a given path is backed by a specified filesystem.
 func hasFilesystem(path string, fsType int64) bool {
 	fs := unix.Statfs_t{}
 
@@ -595,6 +601,7 @@ func UnshiftBtrfsRootfs(path string, diskIdmap *idmap.IdmapSet) error {
 	return shiftBtrfsRootfs(path, diskIdmap, false)
 }
 
+// shiftBtrfsRootfs shiftfs a filesystem that main include read-only subvolumes.
 func shiftBtrfsRootfs(path string, diskIdmap *idmap.IdmapSet, shift bool) error {
 	var err error
 	roSubvols := []string{}
@@ -660,6 +667,7 @@ func BTRFSSubVolumesGet(path string) ([]string, error) {
 	return result, nil
 }
 
+// btrfsIsSubvolume checks if a given path is a subvolume.
 func btrfsIsSubVolume(subvolPath string) bool {
 	fs := unix.Stat_t{}
 	err := unix.Lstat(subvolPath, &fs)

From 926d96d5f8ee41be2c2f14353a0effb81c3282c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 17 Mar 2020 18:46:38 -0400
Subject: [PATCH 2/3] lxd/storage/utils: Add forceRemoveAll
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/utils.go | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/lxd/storage/drivers/utils.go b/lxd/storage/drivers/utils.go
index 2fcbf98acb..a0752f7416 100644
--- a/lxd/storage/drivers/utils.go
+++ b/lxd/storage/drivers/utils.go
@@ -43,6 +43,20 @@ func wipeDirectory(path string) error {
 	return nil
 }
 
+// forceRemoveAll wipes a path including any immutable/non-append files.
+func forceRemoveAll(path string) error {
+	err := os.RemoveAll(path)
+	if err != nil {
+		shared.RunCommand("chattr", "-ai", "-R", path)
+		err = os.RemoveAll(path)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // forceUnmount unmounts stacked mounts until no mountpoint remains.
 func forceUnmount(path string) (bool, error) {
 	unmounted := false

From d2500dcb9f1e09478a44ad335d833a61df6a2b52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 17 Mar 2020 18:46:51 -0400
Subject: [PATCH 3/3] lxd/storage/dir: Use forceRemoveAll
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This clears the append-only and immutable flags on whatever is left
after a normal RemoveAll, then calls RemoveAll again.

Closes #6998

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/storage/drivers/driver_dir_volumes.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go
index 5f00eb7a2c..5160ffe8bd 100644
--- a/lxd/storage/drivers/driver_dir_volumes.go
+++ b/lxd/storage/drivers/driver_dir_volumes.go
@@ -169,7 +169,7 @@ func (d *dir) DeleteVolume(vol Volume, op *operations.Operation) error {
 	}
 
 	// Remove the volume from the storage device.
-	err = os.RemoveAll(volPath)
+	err = forceRemoveAll(volPath)
 	if err != nil && !os.IsNotExist(err) {
 		return errors.Wrapf(err, "Failed to remove '%s'", volPath)
 	}
@@ -350,7 +350,7 @@ func (d *dir) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) err
 	snapPath := snapVol.MountPath()
 
 	// Remove the snapshot from the storage device.
-	err := os.RemoveAll(snapPath)
+	err := forceRemoveAll(snapPath)
 	if err != nil && !os.IsNotExist(err) {
 		return errors.Wrapf(err, "Failed to remove '%s'", snapPath)
 	}