From 3edd98ef0f97da3f19c58a1d3965fcd0faff0b3f Mon Sep 17 00:00:00 2001 From: Michael 'ASAP' Weinrich Date: Sat, 21 Dec 2024 13:35:16 -0800 Subject: [PATCH] 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 --- internal/server/device/device_utils_disk.go | 44 +++++++----- .../server/instance/drivers/driver_qemu.go | 68 +++---------------- 2 files changed, 37 insertions(+), 75 deletions(-) diff --git a/internal/server/device/device_utils_disk.go b/internal/server/device/device_utils_disk.go index 832d6821492..291fca801dd 100644 --- 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 index fb43917ca96..295b203a7e0 100644 --- 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")