[lxc-devel] [lxd/master] Storage: Removes readonly option from snapshotSubvolume()
tomponline on Github
lxc-bot at linuxcontainers.org
Thu May 7 14:08:33 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1106 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200507/9659d713/attachment.bin>
-------------- next part --------------
From 1b1a4c9abdbe61428b63707a531356ffb7e7cb03 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 7 May 2020 14:47:22 +0100
Subject: [PATCH 1/3] lxd/storage/drivers/driver/btrfs/utils: Adds
setSubvolumeReadonlyProperty function
Properly detect running in user namespace and ignores setting readonly.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_btrfs_utils.go | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/lxd/storage/drivers/driver_btrfs_utils.go b/lxd/storage/drivers/driver_btrfs_utils.go
index 5b30f043c4..88ba8fd6db 100644
--- a/lxd/storage/drivers/driver_btrfs_utils.go
+++ b/lxd/storage/drivers/driver_btrfs_utils.go
@@ -390,3 +390,15 @@ func (d *btrfs) volumeSize(vol Volume) string {
return size
}
+
+// setSubvolumeReadonlyProperty sets the readonly property on the subvolume to true or false.
+func (d *btrfs) setSubvolumeReadonlyProperty(path string, readonly bool) error {
+ // Silently ignore requests to set subvolume to readonly if running in a user namespace as it would mean
+ // we cannot undo this action.
+ if readonly && d.state.OS.RunningInUserNS {
+ return nil
+ }
+
+ _, err := shared.RunCommand("btrfs", "property", "set", "-ts", path, "ro", fmt.Sprintf("%t", readonly))
+ return err
+}
From 624cfb2690d4c1a7bfb82e457bdd18b5eed3025e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 7 May 2020 14:47:07 +0100
Subject: [PATCH 2/3] lxd/storag/drivers/driver/btrfs/volumes: Removes readonly
argument from snapshotSubvolume
Readonly argument was only selectively applied which was confusing in some situations.
The caller is now expected to set the subvolume(s) they want to readonly in the specific situation they need.
Updates comments.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_btrfs_utils.go | 48 +++++++----------------
1 file changed, 15 insertions(+), 33 deletions(-)
diff --git a/lxd/storage/drivers/driver_btrfs_utils.go b/lxd/storage/drivers/driver_btrfs_utils.go
index 88ba8fd6db..e6efa42902 100644
--- a/lxd/storage/drivers/driver_btrfs_utils.go
+++ b/lxd/storage/drivers/driver_btrfs_utils.go
@@ -86,18 +86,11 @@ func (d *btrfs) getSubvolumes(path string) ([]string, error) {
return result, nil
}
-func (d *btrfs) snapshotSubvolume(path string, dest string, readonly bool, recursion bool) error {
+// snapshotSubvolume creates a snapshot of the specified path at the dest supplied. If recursion is true and
+// sub volumes are found below the path then they are created at the relative location in dest.
+func (d *btrfs) snapshotSubvolume(path string, dest string, recursion bool) error {
// Single subvolume deletion.
snapshot := func(path string, dest string) error {
- if readonly && !d.state.OS.RunningInUserNS {
- _, err := shared.RunCommand("btrfs", "subvolume", "snapshot", "-r", path, dest)
- if err != nil {
- return err
- }
-
- return nil
- }
-
_, err := shared.RunCommand("btrfs", "subvolume", "snapshot", path, dest)
if err != nil {
return err
@@ -106,43 +99,32 @@ func (d *btrfs) snapshotSubvolume(path string, dest string, readonly bool, recur
return nil
}
+ // First snapshot the root.
+ err := snapshot(path, dest)
+ if err != nil {
+ return err
+ }
+
// Now snapshot all subvolumes of the root.
if recursion {
// Get the subvolumes list.
- subsubvols, err := d.getSubvolumes(path)
+ subSubVols, err := d.getSubvolumes(path)
if err != nil {
return err
}
- sort.Sort(sort.StringSlice(subsubvols))
+ sort.Sort(sort.StringSlice(subSubVols))
- if len(subsubvols) > 0 && readonly {
- // Creating subvolumes requires the parent to be writable.
- readonly = false
- }
+ for _, subSubVol := range subSubVols {
+ subSubVolSnapPath := filepath.Join(dest, subSubVol)
- // First snapshot the root.
- err = snapshot(path, dest)
- if err != nil {
- return err
- }
-
- for _, subsubvol := range subsubvols {
// Clear the target for the subvol to use.
- os.Remove(filepath.Join(dest, subsubvol))
+ os.Remove(subSubVolSnapPath)
- err := snapshot(filepath.Join(path, subsubvol), filepath.Join(dest, subsubvol))
+ err := snapshot(filepath.Join(path, subSubVol), subSubVolSnapPath)
if err != nil {
return err
}
}
-
- return nil
- }
-
- // Handle standalone volume.
- err := snapshot(path, dest)
- if err != nil {
- return err
}
return nil
From 2f97c85c677d294f9b6d1a3b2b7dccb590102c10 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 7 May 2020 14:48:13 +0100
Subject: [PATCH 3/3] lxd/storage/drivers/driver/btrfs:
d.setSubvolumeReadonlyProperty and d.snapshotSubvolume usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_btrfs_utils.go | 2 +-
lxd/storage/drivers/driver_btrfs_volumes.go | 51 +++++++++++++++++----
2 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/lxd/storage/drivers/driver_btrfs_utils.go b/lxd/storage/drivers/driver_btrfs_utils.go
index e6efa42902..bf48422b45 100644
--- a/lxd/storage/drivers/driver_btrfs_utils.go
+++ b/lxd/storage/drivers/driver_btrfs_utils.go
@@ -140,7 +140,7 @@ func (d *btrfs) deleteSubvolume(path string, recursion bool) error {
}
// Attempt to make the subvolume writable.
- shared.RunCommand("btrfs", "property", "set", path, "ro", "false")
+ d.setSubvolumeReadonlyProperty(path, false)
// Temporarily change ownership & mode to help with nesting.
os.Chmod(path, 0700)
diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go
index 275fc5d97d..f9d00d61e9 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -94,8 +94,8 @@ func (d *btrfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Op
// Attempt to mark image read-only.
if vol.volType == VolumeTypeImage {
- _, err = shared.RunCommand("btrfs", "property", "set", volPath, "ro", "true")
- if err != nil && !d.state.OS.RunningInUserNS {
+ err = d.setSubvolumeReadonlyProperty(volPath, true)
+ if err != nil {
return err
}
}
@@ -229,7 +229,7 @@ func (d *btrfs) CreateVolumeFromBackup(vol Volume, snapshots []string, srcData i
defer d.deleteSubvolume(filepath.Join(unpackDir, ".backup"), true)
// Re-create the writable subvolume.
- err = d.snapshotSubvolume(filepath.Join(unpackDir, ".backup"), vol.MountPath(), false, false)
+ err = d.snapshotSubvolume(filepath.Join(unpackDir, ".backup"), vol.MountPath(), false)
if err != nil {
return nil, nil, err
}
@@ -244,7 +244,7 @@ func (d *btrfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bo
defer revert.Fail()
// Recursively copy the main volume.
- err := d.snapshotSubvolume(srcVol.MountPath(), vol.MountPath(), false, true)
+ err := d.snapshotSubvolume(srcVol.MountPath(), vol.MountPath(), true)
if err != nil {
return err
}
@@ -295,7 +295,12 @@ func (d *btrfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bo
srcSnapshot := GetVolumeMountPath(d.name, srcVol.volType, GetSnapshotVolumeName(srcVol.name, snapName))
dstSnapshot := GetVolumeMountPath(d.name, vol.volType, GetSnapshotVolumeName(vol.name, snapName))
- err = d.snapshotSubvolume(srcSnapshot, dstSnapshot, true, false)
+ err = d.snapshotSubvolume(srcSnapshot, dstSnapshot, false)
+ if err != nil {
+ return err
+ }
+
+ err = d.setSubvolumeReadonlyProperty(dstSnapshot, true)
if err != nil {
return err
}
@@ -651,12 +656,17 @@ func (d *btrfs) MigrateVolume(vol Volume, conn io.ReadWriteCloser, volSrcArgs *m
// Make read-only snapshot of the subvolume as writable subvolumes cannot be sent.
migrationSendSnapshot := filepath.Join(tmpVolumesMountPoint, ".migration-send")
- err = d.snapshotSubvolume(vol.MountPath(), migrationSendSnapshot, true, false)
+ err = d.snapshotSubvolume(vol.MountPath(), migrationSendSnapshot, false)
if err != nil {
return err
}
defer d.deleteSubvolume(migrationSendSnapshot, true)
+ err = d.setSubvolumeReadonlyProperty(migrationSendSnapshot, true)
+ if err != nil {
+ return err
+ }
+
// Setup progress tracking.
var wrapper *ioprogress.ProgressTracker
if volSrcArgs.TrackProgress {
@@ -706,10 +716,16 @@ func (d *btrfs) BackupVolume(vol Volume, tarWriter *instancewriter.InstanceTarWr
// Create the read-only snapshot.
mountPath := vol.MountPath()
- err = d.snapshotSubvolume(sourcePath, mountPath, true, true)
+ err = d.snapshotSubvolume(sourcePath, mountPath, true)
if err != nil {
return err
}
+
+ err = d.setSubvolumeReadonlyProperty(mountPath, true)
+ if err != nil {
+ return err
+ }
+
d.logger.Debug("Created read-only backup snapshot", log.Ctx{"sourcePath": sourcePath, "path": mountPath})
defer d.deleteSubvolume(mountPath, true)
}
@@ -813,12 +829,17 @@ func (d *btrfs) BackupVolume(vol Volume, tarWriter *instancewriter.InstanceTarWr
// Create the read-only snapshot.
targetVolume := fmt.Sprintf("%s/.backup", tmpInstanceMntPoint)
- err = d.snapshotSubvolume(sourceVolume, targetVolume, true, true)
+ err = d.snapshotSubvolume(sourceVolume, targetVolume, true)
if err != nil {
return err
}
defer d.deleteSubvolume(targetVolume, true)
+ err = d.setSubvolumeReadonlyProperty(targetVolume, true)
+ if err != nil {
+ return err
+ }
+
// Dump the container to a file.
fileName := "container.bin"
if vol.volType == VolumeTypeVM {
@@ -850,7 +871,17 @@ func (d *btrfs) CreateVolumeSnapshot(snapVol Volume, op *operations.Operation) e
return err
}
- return d.snapshotSubvolume(srcPath, snapPath, true, true)
+ err = d.snapshotSubvolume(srcPath, snapPath, true)
+ if err != nil {
+ return err
+ }
+
+ err = d.setSubvolumeReadonlyProperty(snapPath, true)
+ if err != nil {
+ return err
+ }
+
+ return nil
}
// DeleteVolumeSnapshot removes a snapshot from the storage device. The volName and snapshotName
@@ -920,7 +951,7 @@ func (d *btrfs) RestoreVolume(vol Volume, snapshotName string, op *operations.Op
// Restore the snapshot.
source := GetVolumeMountPath(d.name, vol.volType, GetSnapshotVolumeName(vol.name, snapshotName))
- err = d.snapshotSubvolume(source, vol.MountPath(), false, true)
+ err = d.snapshotSubvolume(source, vol.MountPath(), true)
if err != nil {
return err
}
More information about the lxc-devel
mailing list