[lxc-devel] [lxd/master] VM: Add support for pool volumes attach for disk directory share
tomponline on Github
lxc-bot at linuxcontainers.org
Thu Mar 5 11:40:41 UTC 2020
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/20200305/3ab1dcda/attachment-0001.bin>
-------------- next part --------------
From 2d725c47f1359fcd86cd19c9147a0a0ca2aa8f65 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Mar 2020 11:36:40 +0000
Subject: [PATCH 1/6] lxd/device/disk: Adds mountPoolVolume function
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/disk.go | 70 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 7b8dc60c19..fc01f1b748 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -634,6 +634,76 @@ func (d *disk) generateLimits(runConf *deviceConfig.RunConfig) error {
return nil
}
+// mountPoolVolume mounts the pool volume specified in d.config["source"] from pool specified in d.config["pool"]
+// and return the mount path. If the instance type is container volume will be shifted if needed.
+func (d *disk) mountPoolVolume(reverter *revert.Reverter) (string, error) {
+ // Deal with mounting storage volumes created via the storage api. Extract the name of the storage volume
+ // that we are supposed to attach. We assume that the only syntactically valid ways of specifying a
+ // storage volume are:
+ // - <volume_name>
+ // - <type>/<volume_name>
+ // Currently, <type> must either be empty or "custom".
+ // We do not yet support instance mounts.
+ if filepath.IsAbs(d.config["source"]) {
+ return "", fmt.Errorf(`When the "pool" property is set "source" must specify the name of a volume, not a path`)
+ }
+
+ volumeTypeName := ""
+ volumeName := filepath.Clean(d.config["source"])
+ slash := strings.Index(volumeName, "/")
+ if (slash > 0) && (len(volumeName) > slash) {
+ // Extract volume name.
+ volumeName = d.config["source"][(slash + 1):]
+ // Extract volume type.
+ volumeTypeName = d.config["source"][:slash]
+ }
+
+ var srcPath string
+
+ switch volumeTypeName {
+ case db.StoragePoolVolumeTypeNameContainer:
+ return "", fmt.Errorf("Using instance storage volumes is not supported")
+ case "":
+ // We simply received the name of a storage volume.
+ volumeTypeName = db.StoragePoolVolumeTypeNameCustom
+ fallthrough
+ case db.StoragePoolVolumeTypeNameCustom:
+ srcPath = shared.VarPath("storage-pools", d.config["pool"], volumeTypeName, volumeName)
+ case db.StoragePoolVolumeTypeNameImage:
+ return "", fmt.Errorf("Using image storage volumes is not supported")
+ default:
+ return "", fmt.Errorf("Unknown storage type prefix %q found", volumeTypeName)
+ }
+
+ volumeType, err := storagePools.VolumeTypeNameToType(volumeTypeName)
+ if err != nil {
+ return "", err
+ }
+
+ pool, err := storagePools.GetPoolByName(d.state, d.config["pool"])
+ if err != nil {
+ return "", err
+ }
+
+ ourMount, err := pool.MountCustomVolume(volumeName, nil)
+ if err != nil {
+ return "", errors.Wrapf(err, "Failed mounting storage volume %q of type %q on storage pool %q", volumeName, volumeTypeName, pool.Name())
+ }
+
+ if ourMount {
+ reverter.Add(func() { pool.UnmountCustomVolume(volumeName, nil) })
+ }
+
+ if d.inst.Type() == instancetype.Container {
+ err = d.storagePoolVolumeAttachShift(pool.Name(), volumeName, volumeType)
+ if err != nil {
+ return "", errors.Wrapf(err, "Failed shifting storage volume %q of type %q on storage pool %q", volumeName, volumeTypeName, pool.Name())
+ }
+ }
+
+ return srcPath, nil
+}
+
// createDevice creates a disk device mount on host.
func (d *disk) createDevice() (string, error) {
revert := revert.New()
From 4dc23ca9ee09ad6a4d68e0d73de1f9741a9ec152 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Mar 2020 11:37:19 +0000
Subject: [PATCH 2/6] lxd/device/disk: Error message quoting
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/disk.go | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index fc01f1b748..2548adce94 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -778,7 +778,7 @@ func (d *disk) createDevice() (string, error) {
// Map the RBD.
rbdPath, err := diskCephRbdMap(clusterName, userName, poolName, volumeName)
if err != nil {
- msg := fmt.Sprintf("Could not mount map Ceph RBD: %s.", err)
+ msg := fmt.Sprintf("Could not mount map Ceph RBD: %v", err)
if !isRequired {
// Will fail the PathExists test below.
logger.Warn(msg)
@@ -877,7 +877,7 @@ func (d *disk) createDevice() (string, error) {
if !isRequired {
return "", nil
}
- return "", fmt.Errorf("Source path %s doesn't exist for device %s", srcPath, d.name)
+ return "", fmt.Errorf("Source path %q doesn't exist for device %q", srcPath, d.name)
}
// Create the devices directory if missing.
@@ -1176,7 +1176,7 @@ func (d *disk) postStop() error {
go func() {
err := diskCephRbdUnmap(v["ceph_rbd"])
if err != nil {
- logger.Errorf("Failed to unmap RBD volume '%s' for '%s': %v", v["ceph_rbd"], project.Instance(d.inst.Project(), d.inst.Name()), err)
+ logger.Errorf("Failed to unmap RBD volume %q for %q: %v", v["ceph_rbd"], project.Instance(d.inst.Project(), d.inst.Name()), err)
}
}()
}
@@ -1242,7 +1242,7 @@ func (d *disk) getDiskLimits() (map[string]diskBlockLimit, error) {
if !shared.PathExists(source) {
// Require that device is mounted before resolving block device if required.
if d.isRequired(dev) {
- return nil, fmt.Errorf("Block device path doesn't exist: %s", source)
+ return nil, fmt.Errorf("Block device path doesn't exist %q", source)
}
continue // Do not resolve block device if device isn't mounted.
@@ -1276,7 +1276,7 @@ func (d *disk) getDiskLimits() (map[string]diskBlockLimit, error) {
}
if blockStr == "" {
- return nil, fmt.Errorf("Block device doesn't support quotas: %s", block)
+ return nil, fmt.Errorf("Block device doesn't support quotas %q", block)
}
if blockLimits[blockStr] == nil {
@@ -1436,7 +1436,7 @@ func (d *disk) getParentBlocks(path string) ([]string, error) {
output, err := shared.RunCommand("zpool", "status", "-P", "-L", poolName)
if err != nil {
- return nil, fmt.Errorf("Failed to query zfs filesystem information for %s: %v", dev[1], err)
+ return nil, fmt.Errorf("Failed to query zfs filesystem information for %q: %v", dev[1], err)
}
header := true
@@ -1484,13 +1484,13 @@ func (d *disk) getParentBlocks(path string) ([]string, error) {
}
if len(devices) == 0 {
- return nil, fmt.Errorf("Unable to find backing block for zfs pool: %s", poolName)
+ return nil, fmt.Errorf("Unable to find backing block for zfs pool %q", poolName)
}
} else if fs == "btrfs" && shared.PathExists(dev[1]) {
// Accessible btrfs filesystems
output, err := shared.RunCommand("btrfs", "filesystem", "show", dev[1])
if err != nil {
- return nil, fmt.Errorf("Failed to query btrfs filesystem information for %s: %v", dev[1], err)
+ return nil, fmt.Errorf("Failed to query btrfs filesystem information for %q: %v", dev[1], err)
}
for _, line := range strings.Split(output, "\n") {
@@ -1515,7 +1515,7 @@ func (d *disk) getParentBlocks(path string) ([]string, error) {
devices = append(devices, fmt.Sprintf("%d:%d", major, minor))
} else {
- return nil, fmt.Errorf("Invalid block device: %s", dev[1])
+ return nil, fmt.Errorf("Invalid block device %q", dev[1])
}
return devices, nil
From b6047c27480e410e23987b2d04b2d98c100fbb4b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Mar 2020 11:37:44 +0000
Subject: [PATCH 3/6] lxd/device/disk: Adds pool volume support for VMs
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/disk.go | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 2548adce94..b61b6d58c5 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -361,6 +361,7 @@ func (d *disk) startContainer() (*deviceConfig.RunConfig, error) {
// startVM starts the disk device for a virtual machine instance.
func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
runConf := deviceConfig.RunConfig{}
+ isRequired := d.isRequired(d.config)
if shared.IsRootDiskDevice(d.config) {
runConf.Mounts = []deviceConfig.MountEntryItem{
@@ -392,11 +393,29 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
srcPath := shared.HostPath(d.config["source"])
- // This is a normal disk device, image or injected 9p directory share.
+ // Mount the pool volume and update srcPath to mount path.
+ if d.config["pool"] != "" {
+ var err error
+ srcPath, err = d.mountPoolVolume(revert)
+ if err != nil {
+ if !isRequired {
+ // Leave to the pathExists check below.
+ logger.Warn(err.Error())
+ } else {
+ return nil, err
+ }
+ }
+ }
+
if !shared.PathExists(srcPath) {
- return nil, fmt.Errorf("Cannot find disk source %q", srcPath)
+ if isRequired {
+ return nil, fmt.Errorf("Source path %q doesn't exist for device %q", srcPath, d.name)
+ }
+
+ return &runConf, nil
}
+ // Default to block device or image file passthrough first.
mount := deviceConfig.MountEntryItem{
DevPath: srcPath,
DevName: d.name,
From abf0410f43a33f2173241a69aa38ee5f4a9f911b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Mar 2020 11:38:12 +0000
Subject: [PATCH 4/6] lxd/device/disk: Switches createDevice to use
d.mountPoolVolume for containers
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/disk.go | 70 +++-------------------------------------------
1 file changed, 4 insertions(+), 66 deletions(-)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index b61b6d58c5..811dbedd38 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -816,74 +816,12 @@ func (d *disk) createDevice() (string, error) {
isFile = false
}
} else {
- // Deal with mounting storage volumes created via the storage api. Extract the name
- // of the storage volume that we are supposed to attach. We assume that the only
- // syntactically valid ways of specifying a storage volume are:
- // - <volume_name>
- // - <type>/<volume_name>
- // Currently, <type> must either be empty or "custom".
- // We do not yet support container mounts.
-
- if filepath.IsAbs(d.config["source"]) {
- return "", fmt.Errorf("When the \"pool\" property is set \"source\" must specify the name of a volume, not a path")
- }
-
- volumeTypeName := ""
- volumeName := filepath.Clean(d.config["source"])
- slash := strings.Index(volumeName, "/")
- if (slash > 0) && (len(volumeName) > slash) {
- // Extract volume name.
- volumeName = d.config["source"][(slash + 1):]
- // Extract volume type.
- volumeTypeName = d.config["source"][:slash]
- }
-
- switch volumeTypeName {
- case db.StoragePoolVolumeTypeNameContainer:
- return "", fmt.Errorf("Using container storage volumes is not supported")
- case "":
- // We simply received the name of a storage volume.
- volumeTypeName = db.StoragePoolVolumeTypeNameCustom
- fallthrough
- case db.StoragePoolVolumeTypeNameCustom:
- srcPath = shared.VarPath("storage-pools", d.config["pool"], volumeTypeName, volumeName)
- case db.StoragePoolVolumeTypeNameImage:
- return "", fmt.Errorf("Using image storage volumes is not supported")
- default:
- return "", fmt.Errorf("Unknown storage type prefix %q found", volumeTypeName)
- }
-
- volumeType, err := storagePools.VolumeTypeNameToType(volumeTypeName)
- if err != nil {
- return "", err
- }
-
- pool, err := storagePools.GetPoolByName(d.state, d.config["pool"])
- if err != nil {
- return "", err
- }
-
- // Mount and prepare the volume to be attached.
- err = func() error {
- ourMount, err := pool.MountCustomVolume(volumeName, nil)
- if err != nil {
- return errors.Wrapf(err, "Could not mount storage volume %q of type %q on storage pool %q", volumeName, volumeTypeName, d.config["pool"])
- }
-
- if ourMount {
- revert.Add(func() { pool.UnmountCustomVolume(volumeName, nil) })
- }
-
- err = d.storagePoolVolumeAttachPrepare(pool.Name(), volumeName, volumeType)
- if err != nil {
- return errors.Wrapf(err, "Could not attach storage volume %q of type %q on storage pool %q", volumeName, volumeTypeName, d.config["pool"])
- }
-
- return nil
- }()
+ // Mount the pool volume.
+ var err error
+ srcPath, err = d.mountPoolVolume(revert)
if err != nil {
if !isRequired {
- // Will fail the PathExists test below.
+ // Leave to the pathExists check below.
logger.Warn(err.Error())
} else {
return "", err
From 3b507c5f705471ea3535b714a9b095cb404bc8e6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Mar 2020 11:38:38 +0000
Subject: [PATCH 5/6] lxd/device/disk: Renames storagePoolVolumeAttachPrepare
to storagePoolVolumeAttachShift
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/disk.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 811dbedd38..d9717498ab 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -878,7 +878,7 @@ func (d *disk) createDevice() (string, error) {
return devPath, nil
}
-func (d *disk) storagePoolVolumeAttachPrepare(poolName string, volumeName string, volumeType int) error {
+func (d *disk) storagePoolVolumeAttachShift(poolName string, volumeName string, volumeType int) error {
// Load the DB records.
poolID, pool, err := d.state.Cluster.StoragePoolGet(poolName)
if err != nil {
From 3a8fca5195cea739d1dba79d99888aecf6361b2b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Mar 2020 11:39:43 +0000
Subject: [PATCH 6/6] lxd/device/disk: Ensures custom pool volumes are
unmounted on VM device stop
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/disk.go | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index d9717498ab..bcbbeca2ec 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -1088,12 +1088,16 @@ func (d *disk) stopVM() (*deviceConfig.RunConfig, error) {
os.Remove(filepath.Join(d.inst.DevicesPath(), fmt.Sprintf("%s.sock", d.name)))
}
- return &deviceConfig.RunConfig{}, nil
+ runConf := deviceConfig.RunConfig{
+ PostHooks: []func() error{d.postStop},
+ }
+
+ return &runConf, nil
}
// postStop is run after the device is removed from the instance.
func (d *disk) postStop() error {
- // Check if pool-specific action should be taken.
+ // Check if pool-specific action should be taken to unmount custom volume.
if d.config["pool"] != "" {
pool, err := storagePools.GetPoolByName(d.state, d.config["pool"])
if err != nil {
More information about the lxc-devel
mailing list