incusd/instance/drivers: say less about rbd to qemu

QEMU interfaces with librados for providing Ceph RBD support and as such already has quite robust cluster discovery abilities. Instead of telling QEMU everything it could possibly want to know and risking telling it something wrong, tell it what it needs to know (user, pool, image). Then QEMU can almost certainly do a better and faster job of collecting the necessary information for initiating a cluster connection. Signed-off-by: Michael 'ASAP' Weinrich <[email protected]>
lxc · Dec 21, 2024 · 3edd98e · 3edd98e
1 parent f7620df
commit 3edd98e
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 75 deletions.
diff --git a/internal/server/device/device_utils_disk.go b/internal/server/device/device_utils_disk.go
@@ -30,25 +30,36 @@ const RBDFormatPrefix = "rbd"
 // RBDFormatSeparator is the field separate used in disk paths for RBD devices.
 const RBDFormatSeparator = " "
 
-// DiskParseRBDFormat parses an rbd formatted string, and returns the pool name, volume name, and list of options.
-func DiskParseRBDFormat(rbd string) (string, string, []string, error) {
-	if !strings.HasPrefix(rbd, fmt.Sprintf("%s%s", RBDFormatPrefix, RBDFormatSeparator)) {
-		return "", "", nil, fmt.Errorf("Invalid rbd format, missing prefix")
-	}
-
-	fields := strings.SplitN(rbd, RBDFormatSeparator, 3)
-	if len(fields) != 3 {
-		return "", "", nil, fmt.Errorf("Invalid rbd format, invalid number of fields")
-	}
-
-	opts := fields[2]
+// DiskParseRBDFormat parses an rbd formatted string, and returns the pool name, volume name, and map of options.
+func DiskParseRBDFormat(rbd string) (pool string, volume string, opts map[string]string, err error) {
+	// FIXME: This does not handle escaped strings
+	// Remove and check the prefix
+	prefix, rbd, _ := strings.Cut(rbd, RBDFormatSeparator)
+	if prefix != RBDFormatPrefix {
+		return "", "", nil, fmt.Errorf("Invalid rbd format, wrong prefix: %q", prefix)
+	}
+
+	// Split the path and options
+	path, rawOpts, _ := strings.Cut(rbd, RBDFormatSeparator)
+
+	// Check for valid RBD path
+	pool, volume, validPath := strings.Cut(path, "/")
+	if !validPath {
+		return "", "", nil, fmt.Errorf("Invalid rbd format, missing pool and/or volume: %q", path)
+	}
+
+	// Parse options
+	opts = make(map[string]string)
+	for _, o := range strings.Split(rawOpts, ":") {
+		k, v, isValid := strings.Cut(o, "=")
+		if !isValid {
+			return "", "", nil, fmt.Errorf("Invalid rbd format, bad option: %q", o)
+		}
 
-	fields = strings.SplitN(fields[1], "/", 2)
-	if len(fields) != 2 {
-		return "", "", nil, fmt.Errorf("Invalid rbd format, invalid pool or volume")
+		opts[k] = v
 	}
 
-	return fields[0], fields[1], strings.Split(opts, ":"), nil
+	return pool, volume, opts, nil
 }
 
 // DiskGetRBDFormat returns a rbd formatted string with the given values.
@@ -59,7 +70,6 @@ func DiskGetRBDFormat(clusterName string, userName string, poolName string, volu
 	opts := []string{
 		fmt.Sprintf("id=%s", optEscaper.Replace(userName)),
 		fmt.Sprintf("pool=%s", optEscaper.Replace(poolName)),
-		fmt.Sprintf("conf=/etc/ceph/%s.conf", optEscaper.Replace(clusterName)),
 	}
 
 	return fmt.Sprintf("%s%s%s/%s%s%s", RBDFormatPrefix, RBDFormatSeparator, optEscaper.Replace(poolName), optEscaper.Replace(volumeName), RBDFormatSeparator, strings.Join(opts, ":"))

diff --git a/internal/server/instance/drivers/driver_qemu.go b/internal/server/instance/drivers/driver_qemu.go
@@ -4148,7 +4148,7 @@ func (d *qemu) addDriveConfig(qemuDev map[string]any, bootIndexes map[string]int
 	} else if isRBDImage {
 		blockDev["driver"] = "rbd"
 
-		_, volName, opts, err := device.DiskParseRBDFormat(driveConf.DevPath)
+		poolName, volName, opts, err := device.DiskParseRBDFormat(driveConf.DevPath)
 		if err != nil {
 			return nil, fmt.Errorf("Failed parsing rbd string: %w", err)
 		}
@@ -4173,68 +4173,20 @@ func (d *qemu) addDriveConfig(qemuDev map[string]any, bootIndexes map[string]int
 		vol := storageDrivers.NewVolume(nil, "", volumeType, rbdContentType, volumeName, nil, nil)
 		rbdImageName := storageDrivers.CephGetRBDImageName(vol, "", false)
 
-		// Parse the options (ceph credentials).
-		userName := storageDrivers.CephDefaultUser
-		clusterName := storageDrivers.CephDefaultCluster
-		poolName := ""
-
-		for _, option := range opts {
-			fields := strings.Split(option, "=")
-			if len(fields) != 2 {
-				return nil, fmt.Errorf("Unexpected volume rbd option %q", option)
-			}
-
-			if fields[0] == "id" {
-				userName = fields[1]
-			} else if fields[0] == "pool" {
-				poolName = fields[1]
-			} else if fields[0] == "conf" {
-				baseName := filepath.Base(fields[1])
-				clusterName = strings.TrimSuffix(baseName, ".conf")
+		// scan & pass through options
+		blockDev["pool"] = poolName
+		blockDev["image"] = rbdImageName
+		for key, val := range opts {
+			// We use 'id' where qemu uses 'user'
+			if key == "id" {
+				blockDev["user"] = val
+			} else {
+				blockDev[key] = val
 			}
 		}
 
-		if poolName == "" {
-			return nil, fmt.Errorf("Missing pool name")
-		}
-
 		// The aio option isn't available when using the rbd driver.
 		delete(blockDev, "aio")
-		blockDev["pool"] = poolName
-		blockDev["image"] = rbdImageName
-		blockDev["user"] = userName
-		blockDev["server"] = []map[string]string{}
-
-		// Derference ceph config path.
-		cephConfPath := fmt.Sprintf("/etc/ceph/%s.conf", clusterName)
-		target, err := filepath.EvalSymlinks(cephConfPath)
-		if err == nil {
-			cephConfPath = target
-		}
-
-		blockDev["conf"] = cephConfPath
-
-		// Setup the Ceph cluster config (monitors and keyring).
-		monitors, err := storageDrivers.CephMonitors(clusterName)
-		if err != nil {
-			return nil, err
-		}
-
-		for _, monitor := range monitors {
-			idx := strings.LastIndex(monitor, ":")
-			host := monitor[:idx]
-			port := monitor[idx+1:]
-
-			blockDev["server"] = append(blockDev["server"].([]map[string]string), map[string]string{
-				"host": strings.Trim(host, "[]"),
-				"port": port,
-			})
-		}
-
-		rbdSecret, err = storageDrivers.CephKeyring(clusterName, userName)
-		if err != nil {
-			return nil, err
-		}
 	}
 
 	readonly := slices.Contains(driveConf.Opts, "ro")