The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6721
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) ===
Adds support for adding disk devices to VMs:
Usage:
```
lxc config device add v1 sdb disk source=/dev/sdb
lxc config device add v1 sdb disk source=/home/my.iso
```
From 54159ecc9bc523ad087ae7b29dfbcd660d8cdbd9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
