[lxc-devel] [lxd/master] VM: 9p disk device shares

tomponline on Github lxc-bot at linuxcontainers.org
Tue Mar 3 11:26:46 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/20200303/5ef166ec/attachment-0001.bin>
-------------- next part --------------
From 349316b7f8ba28cd4da262cfeb69d0d51e3b37ed Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 3 Mar 2020 08:08:59 +0000
Subject: [PATCH 1/9] lxd/device/disk: Validation error message quoting
 consistency

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

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index c3c2c3a52d..80a8844a55 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -105,19 +105,19 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 	}
 
 	if d.config["required"] != "" && d.config["optional"] != "" {
-		return fmt.Errorf("Cannot use both \"required\" and deprecated \"optional\" properties at the same time")
+		return fmt.Errorf(`Cannot use both "required" and deprecated "optional" properties at the same time`)
 	}
 
 	if d.config["source"] == "" && d.config["path"] != "/" {
-		return fmt.Errorf("Disk entry is missing the required \"source\" property")
+		return fmt.Errorf(`Disk entry is missing the required "source" property`)
 	}
 
 	if d.config["path"] == "/" && d.config["source"] != "" {
-		return fmt.Errorf("Root disk entry may not have a \"source\" property set")
+		return fmt.Errorf(`Root disk entry may not have a "source" property set`)
 	}
 
 	if d.config["path"] == "/" && d.config["pool"] == "" {
-		return fmt.Errorf("Root disk entry must have a \"pool\" property set")
+		return fmt.Errorf(`Root disk entry must have a "pool" property set`)
 	}
 
 	if d.config["size"] != "" && d.config["path"] != "/" {
@@ -129,7 +129,7 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 	}
 
 	if !(strings.HasPrefix(d.config["source"], "ceph:") || strings.HasPrefix(d.config["source"], "cephfs:")) && (d.config["ceph.cluster_name"] != "" || d.config["ceph.user_name"] != "") {
-		return fmt.Errorf("Invalid options ceph.cluster_name/ceph.user_name for source: %s", d.config["source"])
+		return fmt.Errorf("Invalid options ceph.cluster_name/ceph.user_name for source %q", d.config["source"])
 	}
 
 	// Check no other devices also have the same path as us. Use LocalDevices for this check so
@@ -153,12 +153,12 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 	// for the existence of "source" when "pool" is empty.
 	if d.config["pool"] == "" && d.config["source"] != "" && d.config["source"] != diskSourceCloudInit && d.isRequired(d.config) && !shared.PathExists(shared.HostPath(d.config["source"])) &&
 		!strings.HasPrefix(d.config["source"], "ceph:") && !strings.HasPrefix(d.config["source"], "cephfs:") {
-		return fmt.Errorf("Missing source '%s' for disk '%s'", d.config["source"], d.name)
+		return fmt.Errorf("Missing source %q for disk %q", d.config["source"], d.name)
 	}
 
 	if d.config["pool"] != "" {
 		if d.config["shift"] != "" {
-			return fmt.Errorf("The \"shift\" property cannot be used with custom storage volumes")
+			return fmt.Errorf(`The "shift" property cannot be used with custom storage volumes`)
 		}
 
 		if filepath.IsAbs(d.config["source"]) {
@@ -167,7 +167,7 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 
 		_, err := d.state.Cluster.StoragePoolGetID(d.config["pool"])
 		if err != nil {
-			return fmt.Errorf("The \"%s\" storage pool doesn't exist", d.config["pool"])
+			return fmt.Errorf("The %q storage pool doesn't exist", d.config["pool"])
 		}
 
 		// Only check storate volume is available if we are validating an instance device and not a profile

From 94501ea6097f2d4471b99303674b82dc37f9b270 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 3 Mar 2020 08:15:50 +0000
Subject: [PATCH 2/9] lxd/device/disk: Adds support for adding directory source
 for VM 9p sharing

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

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 80a8844a55..fb68703c35 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -88,15 +88,7 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 		"ceph.cluster_name": shared.IsAny,
 		"ceph.user_name":    shared.IsAny,
 		"boot.priority":     shared.IsUint32,
-	}
-
-	// 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 instConf.Type() == instancetype.Any {
-		rules["path"] = shared.IsAny
-	} else if instConf.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
+		"path":              shared.IsAny,
 	}
 
 	err := d.config.Validate(rules)
@@ -393,21 +385,23 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
 		}
 		return &runConf, nil
 	} else if d.config["source"] != "" {
-		// This is a normal disk device or image.
+		// This is a normal disk device, image or injected 9p directory share.
 		if !shared.PathExists(shared.HostPath(d.config["source"])) {
 			return nil, fmt.Errorf("Cannot find disk source")
 		}
 
-		if shared.IsDir(shared.HostPath(d.config["source"])) {
-			return nil, fmt.Errorf("Only block devices and disk images can be attached to VMs")
+		mount := deviceConfig.MountEntryItem{
+			DevPath: shared.HostPath(d.config["source"]),
+			DevName: d.name,
 		}
 
-		runConf.Mounts = []deviceConfig.MountEntryItem{
-			{
-				DevPath: shared.HostPath(d.config["source"]),
-				DevName: d.name,
-			},
+		// If the source being added is a directory, then we will be using 9p directory sharing to mount
+		// the directory inside the VM, as such we need to indicate to the VM the target path to mount to.
+		if shared.IsDir(shared.HostPath(d.config["source"])) {
+			mount.TargetPath = d.config["path"]
 		}
+
+		runConf.Mounts = []deviceConfig.MountEntryItem{mount}
 		return &runConf, nil
 	}
 

From a803980eefb712842ed11168078b2a1c3391828c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 3 Mar 2020 10:38:31 +0000
Subject: [PATCH 3/9] lxd/instance/drivers/driver/qemu/templates: Adds 9p
 directory disk device template

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

diff --git a/lxd/instance/drivers/driver_qemu_templates.go b/lxd/instance/drivers/driver_qemu_templates.go
index 63e525d19e..8b2e599e44 100644
--- a/lxd/instance/drivers/driver_qemu_templates.go
+++ b/lxd/instance/drivers/driver_qemu_templates.go
@@ -200,6 +200,23 @@ fsdev = "qemu_config"
 mount_tag = "config"
 `))
 
+// Devices use "lxd_" prefix indicating that this is a internally named device.
+var qemuDriveDir = template.Must(template.New("qemuDriveDir").Parse(`
+# {{.devName}} drive
+[fsdev "lxd_{{.devName}}"]
+fsdriver = "proxy"
+path = "{{.devPath}}"
+sock_fd = "{{.proxyFD}}"
+{{- if .readonly}}
+readonly = "on"
+{{- end}}
+
+[device "dev-lxd_{{.devName}}"]
+driver = "virtio-9p-pci"
+fsdev = "lxd_{{.devName}}"
+mount_tag = "lxd_{{.devName}}"
+`))
+
 // Devices use "lxd_" prefix indicating that this is a user named device.
 var qemuDrive = template.Must(template.New("qemuDrive").Parse(`
 # {{.devName}} drive

From 2495e687d333220607d500b346c811a6d9d22045 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 3 Mar 2020 10:40:46 +0000
Subject: [PATCH 4/9] lxd/device/disk: Adds support for disk 9p directory share

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

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index fb68703c35..25379fd8ad 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -399,6 +399,11 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
 		// the directory inside the VM, as such we need to indicate to the VM the target path to mount to.
 		if shared.IsDir(shared.HostPath(d.config["source"])) {
 			mount.TargetPath = d.config["path"]
+			mount.FSType = "9p"
+
+			if shared.IsTrue(d.config["readonly"]) {
+				mount.Opts = append(mount.Opts, "ro")
+			}
 		}
 
 		runConf.Mounts = []deviceConfig.MountEntryItem{mount}

From 3e90874139802e34c53832191b4200c2dd62499a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 3 Mar 2020 10:43:47 +0000
Subject: [PATCH 5/9] lxd/instance/drivers/driver/qemu: Removes unused
 architecture var

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

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 67e8a4dc66..613265640b 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -1699,12 +1699,11 @@ func (vm *qemu) addDriveConfig(sb *strings.Builder, bootIndexes map[string]int,
 	}
 
 	return qemuDrive.Execute(sb, map[string]interface{}{
-		"architecture": vm.architectureName,
-		"devName":      driveConf.DevName,
-		"devPath":      driveConf.DevPath,
-		"bootIndex":    bootIndexes[driveConf.DevName],
-		"cacheMode":    cacheMode,
-		"aioMode":      aioMode,
+		"devName":   driveConf.DevName,
+		"devPath":   driveConf.DevPath,
+		"bootIndex": bootIndexes[driveConf.DevName],
+		"cacheMode": cacheMode,
+		"aioMode":   aioMode,
 	})
 }
 

From f055168b4072ec9b404c4acf0af9f7b8489984a9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 3 Mar 2020 10:41:31 +0000
Subject: [PATCH 6/9] lxd/instance/drivers/driver/qemu: Adds support for
 passing through unix socket FD to qemu

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

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 613265640b..c657a054a9 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -781,13 +781,41 @@ func (vm *qemu) Start(stateful bool) error {
 
 	// Open any extra files and pass their file handles to qemu command.
 	for _, file := range fdFiles {
-		f, err := os.OpenFile(file, os.O_RDWR, 0)
+		info, err := os.Stat(file)
 		if err != nil {
-			err = errors.Wrapf(err, "Error opening exta file %q", file)
+			err = errors.Wrapf(err, "Error detecting file type %q", file)
 			op.Done(err)
 			return err
 		}
-		defer f.Close() // Close file after qemu has started.
+
+		var f *os.File
+		mode := info.Mode()
+		if mode&os.ModeSocket != 0 {
+			c, err := vm.openUnixSocket(file)
+			if err != nil {
+				err = errors.Wrapf(err, "Error opening socket file %q", file)
+				op.Done(err)
+				return err
+			}
+
+			f, err = c.File()
+			if err != nil {
+				err = errors.Wrapf(err, "Error getting socket file descriptor %q", file)
+				op.Done(err)
+				return err
+			}
+			defer c.Close()
+			defer f.Close() // Close file after qemu has started.
+		} else {
+			f, err = os.OpenFile(file, os.O_RDWR, 0)
+			if err != nil {
+				err = errors.Wrapf(err, "Error opening exta file %q", file)
+				op.Done(err)
+				return err
+			}
+			defer f.Close() // Close file after qemu has started.
+		}
+
 		cmd.ExtraFiles = append(cmd.ExtraFiles, f)
 	}
 

From 4b55d454786fe07e5decf22885a98f90ba7148c6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 3 Mar 2020 10:41:53 +0000
Subject: [PATCH 7/9] lxd/instance/drivers/driver/qemu: Adds openUnixSocket
 function

This is used for when passing unix socket FDs to qemu.

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

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index c657a054a9..90b0660691 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -927,6 +927,21 @@ func (vm *qemu) Start(stateful bool) error {
 	return nil
 }
 
+// openUnixSocket connects to a UNIX socket and returns the connection.
+func (vm *qemu) openUnixSocket(sockPath string) (*net.UnixConn, error) {
+	addr, err := net.ResolveUnixAddr("unix", sockPath)
+	if err != nil {
+		return nil, err
+	}
+
+	c, err := net.DialUnix("unix", nil, addr)
+	if err != nil {
+		return nil, err
+	}
+
+	return c, nil
+}
+
 func (vm *qemu) setupNvram() error {
 	// No UEFI nvram for ppc64le.
 	if vm.architecture == osarch.ARCH_64BIT_POWERPC_LITTLE_ENDIAN {

From 979fa9c67182259e875b956e919948b98a2d956f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 3 Mar 2020 10:43:13 +0000
Subject: [PATCH 8/9] lxd/instance/drivers/driver/qemu: Adds addFileDescriptor
 function

Keeps the logic of file descriptor counting inside a single function, as will now be used in multiple places.

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

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 90b0660691..0ee127be87 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -1680,6 +1680,14 @@ func (vm *qemu) addConfDriveConfig(sb *strings.Builder) error {
 	})
 }
 
+// addFileDescriptor adds a file path to the list of files to open and pass file descriptor to qemu.
+// Returns the file descriptor number that qemu will receive.
+func (vm *qemu) addFileDescriptor(fdFiles *[]string, filePath string) int {
+	// Append the tap device file path to the list of files to be opened and passed to qemu.
+	*fdFiles = append(*fdFiles, filePath)
+	return 2 + len(*fdFiles) // Use 2+fdFiles count, as first user file descriptor is 3.
+}
+
 // addRootDriveConfig adds the qemu config required for adding the root drive.
 func (vm *qemu) addRootDriveConfig(sb *strings.Builder, bootIndexes map[string]int, rootDriveConf deviceConfig.MountEntryItem) error {
 	if rootDriveConf.TargetPath != "/" {
@@ -1790,8 +1798,7 @@ func (vm *qemu) addNetDevConfig(sb *strings.Builder, nicIndex int, bootIndexes m
 		}
 
 		// Append the tap device file path to the list of files to be opened and passed to qemu.
-		*fdFiles = append(*fdFiles, fmt.Sprintf("/dev/tap%d", ifindex))
-		tplFields["tapFD"] = 2 + len(*fdFiles) // Use 2+fdFiles count, as first user file descriptor is 3.
+		tplFields["tapFD"] = vm.addFileDescriptor(fdFiles, fmt.Sprintf("/dev/tap%d", ifindex))
 		tpl = qemuNetdevTapFD
 	} else if shared.PathExists(fmt.Sprintf("/sys/class/net/%s/tun_flags", nicName)) {
 		// Detect TAP (via TUN driver) device.

From 1c7f29495faa056b9e92198bf4f1881bfa3fc6aa Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 3 Mar 2020 10:42:30 +0000
Subject: [PATCH 9/9] lxd/instance/drivers/driver/qemu: Adds addDriveDirConfig
 function

This is used to generate qemu config for 9p directory disk shares.

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

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 0ee127be87..9f74ad5a4b 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -688,7 +688,7 @@ func (vm *qemu) Start(stateful bool) error {
 	// Define a set of files to open and pass their file descriptors to qemu command.
 	fdFiles := make([]string, 0)
 
-	confFile, err := vm.generateQemuConfigFile(devConfs, &fdFiles)
+	confFile, err := vm.generateQemuConfigFile(devConfs, &fdFiles, revert)
 	if err != nil {
 		op.Done(err)
 		return err
@@ -1510,7 +1510,7 @@ func (vm *qemu) deviceBootPriorities() (map[string]int, error) {
 
 // generateQemuConfigFile writes the qemu config file and returns its location.
 // It writes the config file inside the VM's log path.
-func (vm *qemu) generateQemuConfigFile(devConfs []*deviceConfig.RunConfig, fdFiles *[]string) (string, error) {
+func (vm *qemu) generateQemuConfigFile(devConfs []*deviceConfig.RunConfig, fdFiles *[]string, reverter *revert.Reverter) (string, error) {
 	var sb *strings.Builder = &strings.Builder{}
 
 	err := qemuBase.Execute(sb, map[string]interface{}{
@@ -1564,6 +1564,8 @@ func (vm *qemu) generateQemuConfigFile(devConfs []*deviceConfig.RunConfig, fdFil
 			for _, drive := range runConf.Mounts {
 				if drive.TargetPath == "/" {
 					err = vm.addRootDriveConfig(sb, bootIndexes, drive)
+				} else if drive.FSType == "9p" {
+					err = vm.addDriveDirConfig(sb, drive, fdFiles, reverter)
 				} else {
 					err = vm.addDriveConfig(sb, bootIndexes, drive)
 				}
@@ -1721,6 +1723,40 @@ func (vm *qemu) addRootDriveConfig(sb *strings.Builder, bootIndexes map[string]i
 	return vm.addDriveConfig(sb, bootIndexes, driveConf)
 }
 
+// addDriveDirConfig adds the qemu config required for adding a supplementary drive directory share.
+func (vm *qemu) addDriveDirConfig(sb *strings.Builder, driveConf deviceConfig.MountEntryItem, fdFiles *[]string, reverter *revert.Reverter) error {
+	// Start the virtfs-proxy-helper process as root so that when the VM process is started
+	// as an unprivileged user, we can still share directories that process cannot access.
+	sockPath := filepath.Join(vm.DevicesPath(), fmt.Sprintf("%s.sock", driveConf.DevName))
+	os.Remove(sockPath)
+	_, err := shared.RunCommand("virtfs-proxy-helper", "-u", "0", "-g", "0", "-s", sockPath, "-p", driveConf.DevPath)
+	if err != nil {
+		return err
+	}
+
+	// If qemu fails to start, we should connect to the socket and then disconnect to stop the proxy process.
+	reverter.Add(func() {
+		if shared.PathExists(sockPath) {
+			s, err := vm.openUnixSocket(sockPath)
+			if err != nil {
+				return
+			}
+
+			s.Close()
+		}
+	})
+
+	proxyFD := vm.addFileDescriptor(fdFiles, sockPath)
+
+	return qemuDriveDir.Execute(sb, map[string]interface{}{
+		"devName":    driveConf.DevName,
+		"devPath":    driveConf.DevPath,
+		"targetPath": driveConf.TargetPath,
+		"readonly":   shared.StringInSlice("ro", driveConf.Opts),
+		"proxyFD":    proxyFD,
+	})
+}
+
 // addDriveConfig adds the qemu config required for adding a supplementary drive.
 func (vm *qemu) addDriveConfig(sb *strings.Builder, bootIndexes map[string]int, driveConf deviceConfig.MountEntryItem) error {
 	// Use native kernel async IO and O_DIRECT by default.


More information about the lxc-devel mailing list