The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6988
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
From 2d725c47f1359fcd86cd19c9147a0a0ca2aa8f65 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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 {
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
