From f1524a720062fd9cc491b2e442be34cd42e91bc3 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Tue, 17 Mar 2026 16:02:24 -0400 Subject: [PATCH 1/4] mantle/platform/{qemu,qemuiso}: remove mutex around RenderUserData RenderUserData() operates on read-only shared state (rconf, bf.baseopts) and local variables, making it safe for concurrent calls without synchronization. No other platform cluster (AWS, GCP, Azure, DO, ESX, OpenStack) uses a mutex around RenderUserData either. Remove the mu sync.Mutex field from both the qemu and qemuiso Cluster structs and the corresponding Lock/Unlock calls around RenderUserData, along with the now-unused sync import. This code was originally introduced in 67b9c0d (which was a large commit) and most likely was an artifact of the development process.. OR maybe it was needed at one time, but current analysis seems to think not. Assisted-By: --- mantle/platform/machine/qemu/cluster.go | 9 --------- mantle/platform/machine/qemuiso/cluster.go | 10 ---------- 2 files changed, 19 deletions(-) diff --git a/mantle/platform/machine/qemu/cluster.go b/mantle/platform/machine/qemu/cluster.go index 37ffcdafbc..309737b676 100644 --- a/mantle/platform/machine/qemu/cluster.go +++ b/mantle/platform/machine/qemu/cluster.go @@ -20,8 +20,6 @@ import ( "path/filepath" "strconv" "strings" - - "sync" "time" "github.com/pborman/uuid" @@ -40,7 +38,6 @@ type Cluster struct { *platform.BaseCluster flight *flight - mu sync.Mutex tearingDown bool } @@ -66,16 +63,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() journal, err := platform.NewJournal(dir) if err != nil { diff --git a/mantle/platform/machine/qemuiso/cluster.go b/mantle/platform/machine/qemuiso/cluster.go index b14df90ff7..35d2c07cae 100644 --- a/mantle/platform/machine/qemuiso/cluster.go +++ b/mantle/platform/machine/qemuiso/cluster.go @@ -19,8 +19,6 @@ import ( "os" "path/filepath" "strconv" - - "sync" "time" "github.com/pborman/uuid" @@ -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) { @@ -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() { From fe1a6be5250f320a9deb5350fa8d293728a74fe7 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Tue, 17 Mar 2026 16:01:30 -0400 Subject: [PATCH 2/4] mantle/platform/qemu: fix data race on tearingDown boolean The tearingDown field is written in Destroy() from the main goroutine and read from a background goroutine spawned in NewMachineWithQemuOptions() that waits for the QEMU process to exit. This unsynchronized access constitutes a data race detectable by Go's race detector. Switch tearingDown from bool to sync/atomic.Bool and use Store()/Load() to elminate the race. Assisted-By: --- mantle/platform/machine/qemu/cluster.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mantle/platform/machine/qemu/cluster.go b/mantle/platform/machine/qemu/cluster.go index 309737b676..9bd6169a8e 100644 --- a/mantle/platform/machine/qemu/cluster.go +++ b/mantle/platform/machine/qemu/cluster.go @@ -20,6 +20,7 @@ import ( "path/filepath" "strconv" "strings" + "sync/atomic" "time" "github.com/pborman/uuid" @@ -38,7 +39,8 @@ type Cluster struct { *platform.BaseCluster flight *flight - tearingDown bool + // Use atomic.Bool to prevent race conditions + tearingDown atomic.Bool } func (qc *Cluster) NewMachine(userdata *conf.UserData) (platform.Machine, error) { @@ -240,7 +242,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) } }() @@ -249,7 +251,7 @@ 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) } From a05e7054f745468f96a10a6aafec76559a984626 Mon Sep 17 00:00:00 2001 From: Nikita Dubrovskii Date: Fri, 14 Nov 2025 14:42:00 +0100 Subject: [PATCH 3/4] mantle/kola/qemu: use new function for ignition config processing This will render the userdata and write the resulting Ignition file out to the working `dir`. Co-authored-by: Dusty Mabe --- mantle/platform/machine/qemu/cluster.go | 38 ++++++++++++++++--------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/mantle/platform/machine/qemu/cluster.go b/mantle/platform/machine/qemu/cluster.go index 9bd6169a8e..e6d7f16813 100644 --- a/mantle/platform/machine/qemu/cluster.go +++ b/mantle/platform/machine/qemu/cluster.go @@ -65,7 +65,7 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl return nil, err } - conf, err := qc.RenderUserData(userdata, map[string]string{}) + config, configPath, err := qc.RenderUserDataAndWriteIgnitionFileToDir(userdata, dir) if err != nil { return nil, err } @@ -88,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 != "" { @@ -255,3 +244,24 @@ func (qc *Cluster) Destroy() { qc.BaseCluster.Destroy() qc.flight.DelCluster(qc) } + +func (qc *Cluster) RenderUserDataAndWriteIgnitionFileToDir(userdata *conf.UserData, dir string) (*conf.Conf, string, error) { + var config *conf.Conf + var configPath string + var err error + config, err = qc.RenderUserData(userdata, map[string]string{}) + if err != nil { + return nil, "", err + } + 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 +} From 86a5b31615d3ae8441e3651fecc86317c34bb2db Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Tue, 17 Mar 2026 17:48:16 -0400 Subject: [PATCH 4/4] mantle/kola/qemu: allow for caller to provide rendered configs This is one step towards merging in `kola testiso` into `kola run` [1] since the testiso tests usually provide a config directly rather than the userdata that then needs to be rendered. [1] https://github.com/coreos/coreos-assembler/issues/3989 Co-authored-by: Nikita Dubrovskii --- mantle/platform/machine/qemu/cluster.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/mantle/platform/machine/qemu/cluster.go b/mantle/platform/machine/qemu/cluster.go index e6d7f16813..d05b59bb58 100644 --- a/mantle/platform/machine/qemu/cluster.go +++ b/mantle/platform/machine/qemu/cluster.go @@ -245,13 +245,23 @@ func (qc *Cluster) Destroy() { qc.flight.DelCluster(qc) } -func (qc *Cluster) RenderUserDataAndWriteIgnitionFileToDir(userdata *conf.UserData, dir string) (*conf.Conf, string, error) { +func (qc *Cluster) RenderUserDataAndWriteIgnitionFileToDir(userdata any, dir string) (*conf.Conf, string, error) { var config *conf.Conf var configPath string var err error - config, err = qc.RenderUserData(userdata, map[string]string{}) - if err != nil { - return nil, "", err + // 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() {