[lxc-devel] [lxd/master] VM: Disk device

tomponline on Github lxc-bot at linuxcontainers.org
Thu Jan 16 11:58:08 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 474 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200116/c1c2e0eb/attachment-0001.bin>
-------------- next part --------------
From 54159ecc9bc523ad087ae7b29dfbcd660d8cdbd9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 16 Jan 2020 11:52:29 +0000
Subject: [PATCH 1/9] lxd/container: Improves error messages in
 instanceValidDevices

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index 91332d2514..ef02d21221 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -123,7 +123,7 @@ func instanceValidDevices(state *state.State, cluster *db.Cluster, instanceType
 	for name, config := range devices {
 		_, err := device.New(inst, state, name, config, nil, nil)
 		if err != nil {
-			return err
+			return errors.Wrapf(err, "Device validation failed %q", name)
 		}
 
 	}
@@ -132,7 +132,7 @@ func instanceValidDevices(state *state.State, cluster *db.Cluster, instanceType
 	if expanded {
 		_, _, err := shared.GetRootDiskDevice(devices.CloneNative())
 		if err != nil {
-			return errors.Wrap(err, "Detect root disk device")
+			return errors.Wrap(err, "Failed detecting root disk device")
 		}
 	}
 

From db64e3bdbee07808d734ef8ce02ff85465625ba4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 16 Jan 2020 11:52:47 +0000
Subject: [PATCH 2/9] lxd/container: instance.ValidDevices usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/container.go b/lxd/container.go
index ef02d21221..b912804b1c 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -776,7 +776,7 @@ func instanceCreateInternal(s *state.State, args db.InstanceArgs) (instance.Inst
 	}
 
 	// Validate container devices with the supplied container name and devices.
-	err = instanceValidDevices(s, s.Cluster, args.Type, args.Name, args.Devices, false)
+	err = instance.ValidDevices(s, s.Cluster, args.Type, args.Name, args.Devices, false)
 	if err != nil {
 		return nil, errors.Wrap(err, "Invalid devices")
 	}

From 6432867e3d09c80a409023d8d81ef78096634ee2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 16 Jan 2020 11:53:03 +0000
Subject: [PATCH 3/9] lxd/container/lxc: instance.ValidDevices usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_lxc.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index d5ce352882..90b6f49508 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -195,7 +195,7 @@ func containerLXCCreate(s *state.State, args db.InstanceArgs) (instance.Instance
 		return nil, err
 	}
 
-	err = instanceValidDevices(s, s.Cluster, c.Type(), c.Name(), c.expandedDevices, true)
+	err = instance.ValidDevices(s, s.Cluster, c.Type(), c.Name(), c.expandedDevices, true)
 	if err != nil {
 		c.Delete()
 		logger.Error("Failed creating container", ctxMap)
@@ -3905,7 +3905,7 @@ func (c *containerLXC) Update(args db.InstanceArgs, userRequested bool) error {
 	}
 
 	// Validate the new devices without using expanded devices validation (expensive checks disabled).
-	err = instanceValidDevices(c.state, c.state.Cluster, c.Type(), c.Name(), args.Devices, false)
+	err = instance.ValidDevices(c.state, c.state.Cluster, c.Type(), c.Name(), args.Devices, false)
 	if err != nil {
 		return errors.Wrap(err, "Invalid devices")
 	}
@@ -4096,7 +4096,7 @@ func (c *containerLXC) Update(args db.InstanceArgs, userRequested bool) error {
 	}
 
 	// Do full expanded validation of the devices diff.
-	err = instanceValidDevices(c.state, c.state.Cluster, c.Type(), c.Name(), c.expandedDevices, true)
+	err = instance.ValidDevices(c.state, c.state.Cluster, c.Type(), c.Name(), c.expandedDevices, true)
 	if err != nil {
 		return errors.Wrap(err, "Invalid expanded devices")
 	}

From a085847c141db8330418f40eb749a5a8d65043db Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 16 Jan 2020 11:53:27 +0000
Subject: [PATCH 4/9] lxd/device/config/devices: Improves error messages

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/config/devices.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lxd/device/config/devices.go b/lxd/device/config/devices.go
index fc91d73218..f6ec31c1e4 100644
--- a/lxd/device/config/devices.go
+++ b/lxd/device/config/devices.go
@@ -27,7 +27,7 @@ func (device Device) Validate(rules map[string]func(value string) error) error {
 		checkedFields[k] = struct{}{} //Mark field as checked.
 		err := validator(device[k])
 		if err != nil {
-			return fmt.Errorf("Invalid value for device option %s: %v", k, err)
+			return fmt.Errorf("Invalid value for device option %q: %v", k, err)
 		}
 	}
 
@@ -47,7 +47,7 @@ func (device Device) Validate(rules map[string]func(value string) error) error {
 			continue
 		}
 
-		return fmt.Errorf("Invalid device option: %s", k)
+		return fmt.Errorf("Invalid device option %q", k)
 	}
 
 	return nil

From a28827139be5d7d2ac9f6513d9f02747a85d3eb1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 16 Jan 2020 11:53:58 +0000
Subject: [PATCH 5/9] lxd/device/disk: Adds support for VM disk devices

Usage: lxc config device add v1 sdb disk source=/dev/sdb

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/disk.go | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index a963ece965..95299b1977 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -15,6 +15,7 @@ import (
 	"github.com/lxc/lxd/lxd/cgroup"
 	"github.com/lxc/lxd/lxd/db"
 	deviceConfig "github.com/lxc/lxd/lxd/device/config"
+	"github.com/lxc/lxd/lxd/instance"
 	"github.com/lxc/lxd/lxd/instance/instancetype"
 	storagePools "github.com/lxc/lxd/lxd/storage"
 	"github.com/lxc/lxd/lxd/util"
@@ -83,9 +84,12 @@ func (d *disk) validateConfig() error {
 		"ceph.user_name":    shared.IsAny,
 	}
 
-	// VMs can have a special cloud-init config drive attached with no path. Don't check instance type here
-	// to allow mixed instance type profiles to use this setting. We will check at device start for VM type.
-	if d.config["source"] != diskSourceCloudInit {
+	// VMs don't use the "path" property, but containers need it, so if we are validating a profile that can
+	// be used for all instance types, we must allow any value.
+	if d.inst.Name() == instance.ProfileValidationName {
+		rules["path"] = shared.IsAny
+	} else if d.inst.Type() == instancetype.Container || d.config["path"] == "/" {
+		// If we are validating a container or the root device is being validated, then require the value.
 		rules["path"] = shared.IsNotEmpty
 	}
 
@@ -129,10 +133,10 @@ func (d *disk) validateConfig() error {
 	// this can still be cleanly removed.
 	pathCount := 0
 	for _, devConfig := range d.inst.LocalDevices() {
-		if devConfig["type"] == "disk" && devConfig["path"] == d.config["path"] {
+		if devConfig["type"] == "disk" && d.config["path"] != "" && devConfig["path"] == d.config["path"] {
 			pathCount++
 			if pathCount > 1 {
-				return fmt.Errorf("More than one disk device uses the same path: %s", d.config["path"])
+				return fmt.Errorf("More than one disk device uses the same path %q", d.config["path"])
 
 			}
 		}
@@ -161,10 +165,10 @@ func (d *disk) validateConfig() error {
 		}
 
 		// Only check storate volume is available if we are validating an instance device
-		// and not a profile device (check for non-empty instance name), and we have least
+		// and not a profile device (check for ProfileValidationName name), and we have least
 		// one expanded device (this is so we only do this expensive check after devices
 		// have been expanded).
-		if d.inst.Name() != "" && len(d.inst.ExpandedDevices()) > 0 && d.config["source"] != "" && d.config["path"] != "/" {
+		if d.inst.Name() != instance.ProfileValidationName && len(d.inst.ExpandedDevices()) > 0 && d.config["source"] != "" && d.config["path"] != "/" {
 			isAvailable, err := d.state.Cluster.StorageVolumeIsAvailable(d.config["pool"], d.config["source"])
 			if err != nil {
 				return fmt.Errorf("Check if volume is available: %v", err)
@@ -359,12 +363,11 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
 	runConf := deviceConfig.RunConfig{}
 
 	if shared.IsRootDiskDevice(d.config) {
+		// The root disk device is special as it is given the first device ID and boot order in VM config.
 		runConf.RootFS.Path = d.config["path"]
 		return &runConf, nil
-	}
-
-	// This is a virtual disk source that can be attached to a VM to provide cloud-init config.
-	if d.config["source"] == diskSourceCloudInit {
+	} else if d.config["source"] == diskSourceCloudInit {
+		// This is a special virtual disk source that can be attached to a VM to provide cloud-init config.
 		isoPath, err := d.generateVMConfigDrive()
 		if err != nil {
 			return nil, err
@@ -377,6 +380,18 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
 			},
 		}
 		return &runConf, nil
+	} else if d.config["source"] != "" {
+		// This is a normal disk device or image.
+		if !shared.PathExists(d.config["source"]) {
+			return nil, fmt.Errorf("Cannot find disk source")
+		}
+		runConf.Mounts = []deviceConfig.MountEntryItem{
+			{
+				DevPath:    d.config["source"],
+				TargetPath: d.name,
+			},
+		}
+		return &runConf, nil
 	}
 
 	return nil, fmt.Errorf("Disk type not supported for VMs")

From 65238f9388f0f25e4d25da17b21d33d60ad4d98e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 16 Jan 2020 11:54:53 +0000
Subject: [PATCH 6/9] lxd/instance/instance/interface: Comment ending
 consistency

Also removes out of date TODO comment.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/instance_interface.go | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/lxd/instance/instance_interface.go b/lxd/instance/instance_interface.go
index 60c9628653..8c9e284e47 100644
--- a/lxd/instance/instance_interface.go
+++ b/lxd/instance/instance_interface.go
@@ -14,9 +14,9 @@ import (
 	"github.com/lxc/lxd/shared/api"
 )
 
-// The Instance interface
+// The Instance interface.
 type Instance interface {
-	// Instance actions
+	// Instance actions.
 	Freeze() error
 	Shutdown(timeout time.Duration) error
 	Start(stateful bool) error
@@ -25,27 +25,25 @@ type Instance interface {
 
 	IsPrivileged() bool
 
-	// Snapshots & migration & backups
+	// Snapshots & migration & backups.
 	Restore(source Instance, stateful bool) error
 	Snapshots() ([]Instance, error)
 	Backups() ([]backup.Backup, error)
 	UpdateBackupFile() error
 
-	// Config handling
+	// Config handling.
 	Rename(newName string) error
-
-	// TODO rename db.InstanceArgs to db.InstanceArgs.
 	Update(newConfig db.InstanceArgs, userRequested bool) error
 
 	Delete() error
 	Export(w io.Writer, properties map[string]string) error
 
-	// Live configuration
+	// Live configuration.
 	CGroupGet(key string) (string, error)
 	CGroupSet(key string, value string) error
 	VolatileSet(changes map[string]string) error
 
-	// File handling
+	// File handling.
 	FileExists(path string) error
 	FilePull(srcpath string, dstpath string) (int64, int64, os.FileMode, string, []string, error)
 	FilePush(fileType string, srcpath string, dstpath string, uid int64, gid int64, mode int, write string) error
@@ -65,10 +63,10 @@ type Instance interface {
 	IsSnapshot() bool
 	IsStateful() bool
 
-	// Hooks
+	// Hooks.
 	DeviceEventHandler(*deviceConfig.RunConfig) error
 
-	// Properties
+	// Properties.
 	ID() int
 	Location() string
 	Project() string
@@ -88,7 +86,7 @@ type Instance interface {
 	ExpiryDate() time.Time
 	FillNetworkDevice(name string, m deviceConfig.Device) (deviceConfig.Device, error)
 
-	// Paths
+	// Paths.
 	Path() string
 	RootfsPath() string
 	TemplatesPath() string
@@ -98,13 +96,13 @@ type Instance interface {
 	LogPath() string
 	DevicesPath() string
 
-	// Storage
+	// Storage.
 	StoragePool() (string, error)
 
-	// Progress reporting
+	// Progress reporting.
 	SetOperation(op *operations.Operation)
 
-	// FIXME: Those should be internal functions
+	// FIXME: Those should be internal functions.
 	// Needed for migration for now.
 	StorageStart() (bool, error)
 	StorageStop() (bool, error)

From 2f765d04ed7cd97e509e1e7117e940307352fe08 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 16 Jan 2020 11:55:33 +0000
Subject: [PATCH 7/9] lxd/instance/qemu/vm/qemu: Fixes driver index loop bug

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/qemu/vm_qemu.go | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lxd/instance/qemu/vm_qemu.go b/lxd/instance/qemu/vm_qemu.go
index 9311b6189f..e5e7690fbc 100644
--- a/lxd/instance/qemu/vm_qemu.go
+++ b/lxd/instance/qemu/vm_qemu.go
@@ -1177,6 +1177,7 @@ backend = "pty"
 	vm.addConfDriveConfig(sb)
 
 	nicIndex := 0
+	driveIndex := 0
 	for _, runConf := range devConfs {
 		// Add root drive device.
 		if runConf.RootFS.Path != "" {
@@ -1188,11 +1189,9 @@ backend = "pty"
 
 		// Add drive devices.
 		if len(runConf.Mounts) > 0 {
-			driveIndex := 0
 			for _, drive := range runConf.Mounts {
-				// Increment so index starts at 1, as root drive uses index 0.
+				// Increment first so index starts at 1, as root drive uses index 0.
 				driveIndex++
-
 				vm.addDriveConfig(sb, driveIndex, drive)
 			}
 		}

From 1772ce40f39bc0af48a8577dad00b5c9dee9cd8a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 16 Jan 2020 11:56:14 +0000
Subject: [PATCH 8/9] lxd/instance/instance/utils: Introduces constant to
 indicate profile validation in instance name

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/instance_utils.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lxd/instance/instance_utils.go b/lxd/instance/instance_utils.go
index fa5709106f..3dc21d5537 100644
--- a/lxd/instance/instance_utils.go
+++ b/lxd/instance/instance_utils.go
@@ -32,6 +32,9 @@ import (
 	"github.com/lxc/lxd/shared/version"
 )
 
+// ProfileValidationName is an indicator that the instance is just being used for profile validation.
+const ProfileValidationName = ""
+
 // ValidDevices is linked from main.instanceValidDevices to validate device config. Currently
 // main.instanceValidDevices uses containerLXC internally and so cannot be moved from main package.
 var ValidDevices func(state *state.State, cluster *db.Cluster, instanceType instancetype.Type, instanceName string, devices deviceConfig.Devices, expanded bool) error

From 5fa9a6b0b21129d18749342298bdc81c6daa650c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 16 Jan 2020 11:56:40 +0000
Subject: [PATCH 9/9] lxd/profiles: Switches to use
 instance.ProfileValidationName during profile validation

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/profiles.go       | 4 ++--
 lxd/profiles_utils.go | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lxd/profiles.go b/lxd/profiles.go
index 8f2047454a..df37752ecb 100644
--- a/lxd/profiles.go
+++ b/lxd/profiles.go
@@ -109,9 +109,9 @@ func profilesPost(d *Daemon, r *http.Request) response.Response {
 		return response.BadRequest(err)
 	}
 
-	// Validate instance devices with an empty instanceName to indicate profile validation.
+	// Validate instance devices with an ProfileValidationName instanceName to indicate profile validation.
 	// At this point we don't know the instance type, so just use Container type for validation.
-	err = instanceValidDevices(d.State(), d.cluster, instancetype.Container, "", deviceConfig.NewDevices(req.Devices), false)
+	err = instance.ValidDevices(d.State(), d.cluster, instancetype.Container, instance.ProfileValidationName, deviceConfig.NewDevices(req.Devices), false)
 	if err != nil {
 		return response.BadRequest(err)
 	}
diff --git a/lxd/profiles_utils.go b/lxd/profiles_utils.go
index d949ab2976..3242167c7f 100644
--- a/lxd/profiles_utils.go
+++ b/lxd/profiles_utils.go
@@ -21,9 +21,9 @@ func doProfileUpdate(d *Daemon, project, name string, id int64, profile *api.Pro
 		return err
 	}
 
-	// Validate instance devices with an empty instanceName to indicate profile validation.
+	// Validate instance devices with ProfileValidationName name to indicate profile validation.
 	// At this point we don't know the instance type, so just use Container type for validation.
-	err = instanceValidDevices(d.State(), d.cluster, instancetype.Container, "", deviceConfig.NewDevices(req.Devices), false)
+	err = instance.ValidDevices(d.State(), d.cluster, instancetype.Container, instance.ProfileValidationName, deviceConfig.NewDevices(req.Devices), false)
 	if err != nil {
 		return err
 	}


More information about the lxc-devel mailing list