[lxc-devel] [lxd/master] Storage: Fixes GPT alternative header not being at the end of the VM block device
tomponline on Github
lxc-bot at linuxcontainers.org
Tue Feb 25 17:34:39 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1725 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200225/c90a01d9/attachment.bin>
-------------- next part --------------
From 24afe0a39d070855f3833f2d1c85319abf4937a9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 25 Feb 2020 12:39:58 +0000
Subject: [PATCH 1/6] lxd/storage/drivers/driver/zfs/volumes: Create block
volumes with volmode=none
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_zfs_volumes.go | 33 +++++++++++++++--------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index d8caa92c54..759ce07b7b 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -81,19 +81,19 @@ func (d *zfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
return err
}
+ // Use volmode=none so volume is invisible until mounted.
+ opts := []string{"volmode=none"}
+
loopPath := loopFilePath(d.name)
if d.config["source"] == loopPath {
// Create the volume dataset with sync disabled (to avoid kernel lockups when using a disk based pool).
- err = d.createVolume(d.dataset(vol, false), sizeBytes, "sync=disabled")
- if err != nil {
- return err
- }
- } else {
- // Create the volume dataset.
- err = d.createVolume(d.dataset(vol, false), sizeBytes)
- if err != nil {
- return err
- }
+ opts = append(opts, "sync=disabled")
+ }
+
+ // Create the volume dataset.
+ err = d.createVolume(d.dataset(vol, false), sizeBytes, opts...)
+ if err != nil {
+ return err
}
}
@@ -458,8 +458,19 @@ func (d *zfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool
}
}
} else {
+ args := []string{
+ "clone",
+ srcSnapshot,
+ d.dataset(vol, false),
+ }
+
+ if vol.contentType == ContentTypeBlock {
+ // Use volmode=none so volume is invisible until mounted.
+ args = append(args, "-o", "volmode=none")
+ }
+
// Clone the snapshot.
- _, err := shared.RunCommand("zfs", "clone", srcSnapshot, d.dataset(vol, false))
+ _, err := shared.RunCommand("zfs", args...)
if err != nil {
return err
}
From a5267929ac564d60959b36ee0be0c0f1840d8d2c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 25 Feb 2020 15:54:19 +0000
Subject: [PATCH 2/6] lxd/storage/drivers/driver/zfs/volumes: Use MountTask
with CreateVolume
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_zfs_volumes.go | 60 ++++++++++-------------
1 file changed, 26 insertions(+), 34 deletions(-)
diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index 759ce07b7b..597f827ae0 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -108,49 +108,41 @@ func (d *zfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
revert.Add(func() { d.DeleteVolume(fsVol, op) })
}
- // Mount the dataset.
- _, err := d.MountVolume(vol, op)
- if err != nil {
- return err
- }
+ err := vol.MountTask(func(mountPath string, op *operations.Operation) error {
+ // Run the volume filler function if supplied.
+ if filler != nil && filler.Fill != nil {
+ if vol.contentType == ContentTypeFS {
+ // Run the filler.
+ err := filler.Fill(vol.MountPath(), "")
+ if err != nil {
+ return err
+ }
+ } else {
+ // Get the device path.
+ devPath, err := d.GetVolumeDiskPath(vol)
+ if err != nil {
+ return err
+ }
- if vol.contentType == ContentTypeFS {
- // Set the permissions.
- err = vol.EnsureMountPath()
- if err != nil {
- d.UnmountVolume(vol, op)
- return err
+ // Run the filler.
+ err = filler.Fill(vol.MountPath(), devPath)
+ if err != nil {
+ return err
+ }
+ }
}
- }
- // Run the volume filler function if supplied.
- if filler != nil && filler.Fill != nil {
if vol.contentType == ContentTypeFS {
- // Run the filler.
- err = filler.Fill(vol.MountPath(), "")
- if err != nil {
- d.UnmountVolume(vol, op)
- return err
- }
- } else {
- // Get the device path.
- devPath, err := d.GetVolumeDiskPath(vol)
+ // Run EnsureMountPath again after mounting and filling to ensure the mount directory has
+ // the correct permissions set.
+ err := vol.EnsureMountPath()
if err != nil {
- d.UnmountVolume(vol, op)
- return err
- }
-
- // Run the filler.
- err = filler.Fill(vol.MountPath(), devPath)
- if err != nil {
- d.UnmountVolume(vol, op)
return err
}
}
- }
- // Unmount the volume.
- _, err = d.UnmountVolume(vol, op)
+ return nil
+ }, op)
if err != nil {
return err
}
From 8fbaf86ffd759825515827f2d0f4c15249cce91e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 25 Feb 2020 15:55:35 +0000
Subject: [PATCH 3/6] lxd/storage/drivers/zfs/volumes: Makes MountVolume and
UnmountVolume more thorough in detecting mounts
Improves debug logging too.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_zfs_volumes.go | 77 ++++++++++++++---------
1 file changed, 46 insertions(+), 31 deletions(-)
diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index 597f827ae0..f5c25be0e4 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -20,6 +20,7 @@ import (
"github.com/lxc/lxd/lxd/revert"
"github.com/lxc/lxd/shared"
"github.com/lxc/lxd/shared/ioprogress"
+ log "github.com/lxc/lxd/shared/log15"
"github.com/lxc/lxd/shared/units"
)
@@ -841,55 +842,64 @@ func (d *zfs) GetVolumeDiskPath(vol Volume) (string, error) {
// MountVolume simulates mounting a volume.
func (d *zfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
- // For VMs, also mount the filesystem dataset.
- if vol.IsVMBlock() {
- fsVol := vol.NewVMBlockFilesystemVolume()
- _, err := d.MountVolume(fsVol, op)
+ var err error
+ mountPath := vol.MountPath()
+ dataset := d.dataset(vol, false)
+
+ // Check if already mounted.
+ if vol.contentType == ContentTypeFS && !shared.IsMountPoint(mountPath) {
+ // Mount the dataset.
+ _, err = shared.RunCommand("zfs", "mount", dataset)
if err != nil {
return false, err
}
+
+ d.logger.Debug("Mounted ZFS dataset", log.Ctx{"dev": dataset, "path": mountPath})
+ return true, nil
}
- // For block devices, we make them appear.
+ var ourMountBlock, ourMountFs bool
+
if vol.contentType == ContentTypeBlock {
+ // Check if already active.
current, err := d.getDatasetProperty(d.dataset(vol, false), "volmode")
- if err != nil {
- return false, err
- }
+ if current != "dev" {
+ // Activate.
+ err = d.setDatasetProperties(d.dataset(vol, false), "volmode=dev")
+ if err != nil {
+ return false, err
+ }
- // Check if already active.
- if current == "dev" {
- return false, nil
+ // Wait half a second to give udev a chance to kick in.
+ time.Sleep(500 * time.Millisecond)
+
+ d.logger.Debug("Activated ZFS volume", log.Ctx{"dev": dataset})
+ ourMountBlock = true
}
+ }
- // Activate.
- err = d.setDatasetProperties(d.dataset(vol, false), "volmode=dev")
+ // For block devices, we make them appear.
+ if vol.IsVMBlock() {
+ // For VMs, also mount the filesystem dataset.
+ fsVol := vol.NewVMBlockFilesystemVolume()
+ ourMountFs, err = d.MountVolume(fsVol, op)
if err != nil {
return false, err
}
-
- // Wait half a second to give udev a chance to kick in.
- time.Sleep(500 * time.Millisecond)
-
- return true, nil
- }
-
- // Check if not already mounted.
- if shared.IsMountPoint(vol.MountPath()) {
- return false, nil
}
- // Mount the dataset.
- _, err := shared.RunCommand("zfs", "mount", d.dataset(vol, false))
- if err != nil {
- return false, err
+ // If we 'mounted' either block or filesystem volumes, this was our mount.
+ if ourMountFs || ourMountBlock {
+ return true, nil
}
- return true, nil
+ return false, nil
}
// UnmountVolume simulates unmounting a volume.
func (d *zfs) UnmountVolume(vol Volume, op *operations.Operation) (bool, error) {
+ dataset := d.dataset(vol, false)
+
// For VMs, also mount the filesystem dataset.
if vol.IsVMBlock() {
fsVol := vol.NewVMBlockFilesystemVolume()
@@ -901,25 +911,30 @@ func (d *zfs) UnmountVolume(vol Volume, op *operations.Operation) (bool, error)
// For block devices, we make them disappear.
if vol.contentType == ContentTypeBlock {
- err := d.setDatasetProperties(d.dataset(vol, false), "volmode=none")
+ err := d.setDatasetProperties(dataset, "volmode=none")
if err != nil {
return false, err
}
+ d.logger.Debug("Deactivated ZFS volume", log.Ctx{"dev": dataset})
+
return false, nil
}
// Check if still mounted.
- if !shared.IsMountPoint(vol.MountPath()) {
+ mountPath := vol.MountPath()
+ if !shared.IsMountPoint(mountPath) {
return false, nil
}
// Mount the dataset.
- _, err := shared.RunCommand("zfs", "unmount", d.dataset(vol, false))
+ _, err := shared.RunCommand("zfs", "unmount", dataset)
if err != nil {
return false, err
}
+ d.logger.Debug("Unmounted ZFS dataset", log.Ctx{"dev": dataset, "path": mountPath})
+
return true, nil
}
From 04f7290a66d9a8fe07eb7847019c3c807c81767f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 25 Feb 2020 15:56:42 +0000
Subject: [PATCH 4/6] lxd/storage/drivers/driver/lvm/volumes: Always ensure
mount path after mount in CreateVolume
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_lvm_volumes.go | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/lxd/storage/drivers/driver_lvm_volumes.go b/lxd/storage/drivers/driver_lvm_volumes.go
index 016d163e73..4e6ff272fb 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -50,8 +50,8 @@ func (d *lvm) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
revert.Add(func() { d.DeleteVolume(fsVol, op) })
}
- if filler != nil && filler.Fill != nil {
- err = vol.MountTask(func(mountPath string, op *operations.Operation) error {
+ err = vol.MountTask(func(mountPath string, op *operations.Operation) error {
+ if filler != nil && filler.Fill != nil {
if vol.contentType == ContentTypeFS {
d.logger.Debug("Running filler function", log.Ctx{"path": volPath})
err = filler.Fill(mountPath, "")
@@ -72,19 +72,21 @@ func (d *lvm) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
return err
}
}
+ }
- // Run EnsureMountPath again after mounting to ensure the mount directory has the correct
- // permissions set.
+ if vol.contentType == ContentTypeFS {
+ // Run EnsureMountPath again after mounting and filling to ensure the mount directory has
+ // the correct permissions set.
err = vol.EnsureMountPath()
if err != nil {
return err
}
-
- return nil
- }, op)
- if err != nil {
- return err
}
+
+ return nil
+ }, op)
+ if err != nil {
+ return err
}
revert.Success()
From 2309c22b3a27205f721e715fc04354eff8f62840 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 25 Feb 2020 14:56:18 +0000
Subject: [PATCH 5/6] lxd/storage/drivers/utils: Adds moveGPTAltHeader function
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/utils.go | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/lxd/storage/drivers/utils.go b/lxd/storage/drivers/utils.go
index 0a8a3aaa91..e783818541 100644
--- a/lxd/storage/drivers/utils.go
+++ b/lxd/storage/drivers/utils.go
@@ -5,8 +5,10 @@ import (
"io"
"io/ioutil"
"os"
+ "os/exec"
"path/filepath"
"strings"
+ "syscall"
"time"
"github.com/pkg/errors"
@@ -573,3 +575,26 @@ func copyDevice(inputPath, outputPath string) error {
func loopFilePath(poolName string) string {
return filepath.Join(shared.VarPath("disks"), fmt.Sprintf("%s.img", poolName))
}
+
+var errNonGPTDisk = fmt.Errorf("Non-GPT disk")
+
+// moveGPTAltHeader moves the GPT alternative header to the end of the disk device supplied.
+// If the device supplied is not detected as not being a GPT disk then no action is taken and nil is returned.
+func moveGPTAltHeader(devPath string) error {
+ _, err := shared.RunCommand("sgdisk", "--move-second-header", devPath)
+ runErr, ok := err.(shared.RunError)
+ if ok {
+ exitError, ok := runErr.Err.(*exec.ExitError)
+ if ok {
+ waitStatus := exitError.Sys().(syscall.WaitStatus)
+
+ // sgdisk manpage says exit status 3 means:
+ // "Non-GPT disk detected and no -g option, but operation requires a write action".
+ if waitStatus.ExitStatus() == 3 {
+ return errNonGPTDisk // Non-GPT disk specified.
+ }
+ }
+ }
+
+ return err
+}
From 395e07c7b949619badf05a4eba2412dd96287222 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 25 Feb 2020 16:26:14 +0000
Subject: [PATCH 6/6] lxd/storage/drivers/driver/zfs/volumes: Uses
moveGPTAltHeader during CreateVolume block filler
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_zfs_volumes.go | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index f5c25be0e4..f332bfe8df 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -130,6 +130,14 @@ func (d *zfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
if err != nil {
return err
}
+
+ // Move the GPT alt header to end of disk if needed.
+ err = moveGPTAltHeader(devPath)
+ if err == nil {
+ d.logger.Debug("Moved GPT alternative header to end of disk")
+ } else if err != nil && err != errNonGPTDisk {
+ return err
+ }
}
}
More information about the lxc-devel
mailing list