Skip to content

mantle/platform/qemu: remove mutex; protect tearingDown; add helper function #4486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dustymabe merged 4 commits into from
Mar 18, 2026
+39 −36
Merged

mantle/platform/qemu: remove mutex; protect tearingDown; add helper function #4486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f1524a7
mantle/platform/{qemu,qemuiso}: remove mutex around RenderUserData
dustymabe Mar 17, 2026
fe1a6be
mantle/platform/qemu: fix data race on tearingDown boolean
dustymabe Mar 17, 2026
a05e705
mantle/kola/qemu: use new function for ignition config processing
nikita-dubrovskii Nov 14, 2025
86a5b31
mantle/kola/qemu: allow for caller to provide rendered configs
dustymabe Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 39 additions & 26 deletions mantle/platform/machine/qemu/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ import (
"path/filepath"
"strconv"
"strings"

"sync"
"sync/atomic"
"time"

"github.com/pborman/uuid"
Expand All @@ -40,8 +39,8 @@ type Cluster struct {
*platform.BaseCluster
flight *flight

mu sync.Mutex
tearingDown bool
// Use atomic.Bool to prevent race conditions
tearingDown atomic.Bool
}

func (qc *Cluster) NewMachine(userdata *conf.UserData) (platform.Machine, error) {
Expand All @@ -66,16 +65,10 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
return nil, err
}

// hacky solution for cloud config ip substitution
// NOTE: escaping is not supported
qc.mu.Lock()

conf, err := qc.RenderUserData(userdata, map[string]string{})
config, configPath, err := qc.RenderUserDataAndWriteIgnitionFileToDir(userdata, dir)
if err != nil {
qc.mu.Unlock()
return nil, err
}
qc.mu.Unlock()

journal, err := platform.NewJournal(dir)
if err != nil {
Expand All @@ -95,23 +88,12 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
}

if qc.flight.opts.SecureExecution {
if err := builder.SetSecureExecution(qc.flight.opts.SecureExecutionIgnitionPubKey, qc.flight.opts.SecureExecutionHostKey, conf); err != nil {
if err := builder.SetSecureExecution(qc.flight.opts.SecureExecutionIgnitionPubKey, qc.flight.opts.SecureExecutionHostKey, config); err != nil {
return nil, err
}
}

var confPath string
if conf.IsIgnition() {
confPath = filepath.Join(dir, "ignition.json")
if err := conf.WriteFile(confPath); err != nil {
return nil, err
}
} else if conf.IsEmpty() {
} else {
return nil, fmt.Errorf("qemu only supports Ignition or empty configs")
}

builder.ConfigFile = confPath
builder.ConfigFile = configPath
defer builder.Close()
builder.UUID = qm.id
if qc.flight.opts.Arch != "" {
Expand Down Expand Up @@ -249,7 +231,7 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
// it knows to stop the test once QEMU dies.
go func() {
err := inst.Wait()
if err != nil && !qc.tearingDown {
if err != nil && !qc.tearingDown.Load() {
plog.Errorf("QEMU process finished abnormally: %v", err)
}
}()
Expand All @@ -258,7 +240,38 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
}

func (qc *Cluster) Destroy() {
qc.tearingDown = true
qc.tearingDown.Store(true)
qc.BaseCluster.Destroy()
qc.flight.DelCluster(qc)
}

func (qc *Cluster) RenderUserDataAndWriteIgnitionFileToDir(userdata any, dir string) (*conf.Conf, string, error) {
var config *conf.Conf
var configPath string
var err error
// Some callers provide the config directly rather than something
// that needs to be rendered. Render the userdata into a config
// if needed.
switch c := userdata.(type) {
case *conf.UserData:
config, err = qc.RenderUserData(c, map[string]string{})
if err != nil {
return nil, "", err
}
case *conf.Conf:
config = c // Already rendered. Just pass through what was provided.
default:
return nil, "", fmt.Errorf("unknown config pointer type: %T", c)
}
if config != nil {
if config.IsIgnition() {
configPath = filepath.Join(dir, "ignition.json")
if err = config.WriteFile(configPath); err != nil {
return nil, "", err
}
} else if !config.IsEmpty() {
return nil, "", fmt.Errorf("qemu only supports Ignition or empty configs")
}
}
return config, configPath, nil
Comment on lines +266 to +276
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic to handle the rendered configuration can be simplified to improve readability by using early returns and reducing nesting. This makes the control flow flatter and easier to follow.

if config == nil {
	return nil, "", nil
}

if !config.IsIgnition() {
	if !config.IsEmpty() {
		return nil, "", fmt.Errorf("qemu only supports Ignition or empty configs")
	}
	return config, "", nil
}

configPath := filepath.Join(dir, "ignition.json")
if err := config.WriteFile(configPath); err != nil {
	return nil, "", err
}
return config, configPath, nil

}
10 changes: 0 additions & 10 deletions mantle/platform/machine/qemuiso/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import (
"os"
"path/filepath"
"strconv"

"sync"
"time"

"github.com/pborman/uuid"
Expand All @@ -38,8 +36,6 @@ import (
type Cluster struct {
*platform.BaseCluster
flight *flight

mu sync.Mutex
}

func (qc *Cluster) NewMachine(userdata *conf.UserData) (platform.Machine, error) {
Expand All @@ -66,16 +62,10 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
return nil, err
}

// hacky solution for cloud config ip substitution
// NOTE: escaping is not supported
qc.mu.Lock()

conf, err := qc.RenderUserData(userdata, map[string]string{})
if err != nil {
qc.mu.Unlock()
return nil, err
}
qc.mu.Unlock()

var confPath string
if conf.IsIgnition() {
Expand Down
Loading