Skip to content

Commit

Permalink
lxc/instance/drivers: Close files opened during qemu.conf creation
Browse files Browse the repository at this point in the history
  • Loading branch information
@hamistao
hamistao committed May 16, 2024
1 parent 52aecaf commit a6c22cd
Showing 1 changed file with 36 additions and 24 deletions.
60 changes: 36 additions & 24 deletions lxd/instance/drivers/driver_qemu.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1468,12 +1468,18 @@ func (d *qemu) start(stateful bool, op *operationlock.InstanceOperation) error {
}

// Generate the QEMU configuration.
confFile, monHooks, err := d.generateQemuConfigFile(cpuInfo, mountInfo, qemuBus, vsockFD, devConfs, &fdFiles)
confFile, monHooks, openFiles, err := d.generateQemuConfigFile(cpuInfo, mountInfo, qemuBus, vsockFD, devConfs, &fdFiles)
if err != nil {
op.Done(err)
return err
}

defer func() {
for _, file := range openFiles {
file.Close()
}
}()

// Start QEMU.
qemuCmd := []string{
"--",
Expand Down Expand Up @@ -3099,22 +3105,23 @@ func (d *qemu) deviceBootPriorities(base int) (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 (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePools.MountInfo, busName string, vsockFD int, devConfs []*deviceConfig.RunConfig, fdFiles *[]*os.File) (string, []monitorHook, error) {
func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePools.MountInfo, busName string, vsockFD int, devConfs []*deviceConfig.RunConfig, fdFiles *[]*os.File) (string, []monitorHook, []*os.File, error) {
var monHooks []monitorHook
var openFiles []*os.File

cfg := qemuBase(&qemuBaseOpts{d.Architecture()})

err := d.addCPUMemoryConfig(&cfg, cpuInfo)
if err != nil {
return "", nil, err
return "", nil, nil, err
}

// Parse raw.qemu.
rawOptions := []string{}
if d.expandedConfig["raw.qemu"] != "" {
rawOptions, err = shellquote.Split(d.expandedConfig["raw.qemu"])
if err != nil {
return "", nil, err
return "", nil, nil, err
}
}

Expand All @@ -3126,7 +3133,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
// This is so the QEMU process can still read/write the file after it has dropped its user privs.
nvRAMFile, err := os.Open(d.nvramPath())
if err != nil {
return "", nil, fmt.Errorf("Failed opening NVRAM file: %w", err)
return "", nil, nil, fmt.Errorf("Failed opening NVRAM file: %w", err)
}

// Determine expected firmware.
Expand All @@ -3146,7 +3153,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
}

if vmfCode == "" {
return "", nil, fmt.Errorf("Unable to locate matching firmware: %+v", firmwares)
return "", nil, nil, fmt.Errorf("Unable to locate matching firmware: %+v", firmwares)
}

// As 2MB firmware was deprecated in the LXD snap we have to regenerate NVRAM for VMs which used the 2MB one.
Expand All @@ -3156,7 +3163,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
if shared.InSnap() && (isOVMF2MB || isOVMFCSM) {
err = d.setupNvram()
if err != nil {
return "", nil, err
return "", nil, nil, err
}

// force to use a top-priority firmware
Expand All @@ -3170,7 +3177,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo

fwPath := d.fwPath(vmfCode)
if fwPath == "" {
return "", nil, fmt.Errorf("Unable to locate the file for firmware %q", vmfCode)
return "", nil, nil, fmt.Errorf("Unable to locate the file for firmware %q", vmfCode)
}

driveFirmwareOpts := qemuDriveFirmwareOpts{
Expand Down Expand Up @@ -3240,7 +3247,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
// Existing vsock ID from volatile.
vsockID, err := d.getVsockID()
if err != nil {
return "", nil, err
return "", nil, nil, err
}

devBus, devAddr, multi = bus.allocate(busFunctionGroupGeneric)
Expand Down Expand Up @@ -3323,7 +3330,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
if shared.IsTrue(d.expandedConfig["security.sev"]) {
sevOpts, err := d.setupSEV(fdFiles)
if err != nil {
return "", nil, err
return "", nil, nil, err
}

if sevOpts != nil {
Expand All @@ -3346,9 +3353,11 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
// Trickery to handle paths > 107 chars.
configSockDir, err := os.Open(filepath.Dir(originalConfigSockPath))
if err != nil {
return "", nil, fmt.Errorf("Couldn't open device socket directory %q: %s", filepath.Dir(originalConfigSockPath), err)
return "", nil, nil, fmt.Errorf("Couldn't open device socket directory %q: %s", filepath.Dir(originalConfigSockPath), err)
}

openFiles = append(openFiles, configSockDir)

configSockPath := fmt.Sprintf("/proc/self/fd/%d/%s", d.addFileDescriptor(fdFiles, configSockDir), filepath.Base(originalConfigSockPath))

if shared.PathExists(configSockPath) {
Expand Down Expand Up @@ -3397,7 +3406,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo

bootIndexes, err := d.deviceBootPriorities(base)
if err != nil {
return "", nil, fmt.Errorf("Error calculating boot indexes: %w", err)
return "", nil, nil, fmt.Errorf("Error calculating boot indexes: %w", err)
}

// Record the mounts we are going to do inside the VM using the agent.
Expand Down Expand Up @@ -3442,13 +3451,13 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
if drive.TargetPath == "/" {
monHook, err = d.addRootDriveConfig(qemuDev, mountInfo, bootIndexes, drive)
} else if drive.FSType == "9p" {
err = d.addDriveDirConfig(&cfg, bus, fdFiles, &agentMounts, drive)
err = d.addDriveDirConfig(&cfg, bus, fdFiles, openFiles, &agentMounts, drive)
} else {
monHook, err = d.addDriveConfig(qemuDev, bootIndexes, drive)
}

if err != nil {
return "", nil, fmt.Errorf("Failed setting up disk device %q: %w", drive.DevName, err)
return "", nil, nil, fmt.Errorf("Failed setting up disk device %q: %w", drive.DevName, err)
}

if monHook != nil {
Expand Down Expand Up @@ -3476,7 +3485,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo

monHook, err := d.addNetDevConfig(bus.name, qemuDev, bootIndexes, runConf.NetworkInterface)
if err != nil {
return "", nil, err
return "", nil, nil, err
}

monHooks = append(monHooks, monHook)
Expand All @@ -3486,23 +3495,23 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
if len(runConf.GPUDevice) > 0 {
err = d.addGPUDevConfig(&cfg, bus, runConf.GPUDevice)
if err != nil {
return "", nil, err
return "", nil, nil, err
}
}

// Add PCI device.
if len(runConf.PCIDevice) > 0 {
err = d.addPCIDevConfig(&cfg, bus, runConf.PCIDevice)
if err != nil {
return "", nil, err
return "", nil, nil, err
}
}

// Add USB devices.
for _, usbDev := range runConf.USBDevice {
monHook, err := d.addUSBDeviceConfig(usbDev)
if err != nil {
return "", nil, err
return "", nil, nil, err
}

monHooks = append(monHooks, monHook)
Expand All @@ -3512,7 +3521,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
if len(runConf.TPMDevice) > 0 {
err = d.addTPMDeviceConfig(&cfg, runConf.TPMDevice)
if err != nil {
return "", nil, err
return "", nil, nil, err
}
}
}
Expand All @@ -3521,7 +3530,7 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
if d.architecture == osarch.ARCH_64BIT_INTEL_X86 {
err = d.addVmgenDeviceConfig(&cfg, d.localConfig["volatile.uuid.generation"])
if err != nil {
return "", nil, err
return "", nil, nil, err
}
}

Expand All @@ -3533,21 +3542,22 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
// Write the agent mount config.
agentMountJSON, err := json.Marshal(agentMounts)
if err != nil {
return "", nil, fmt.Errorf("Failed marshalling agent mounts to JSON: %w", err)
return "", nil, nil, fmt.Errorf("Failed marshalling agent mounts to JSON: %w", err)
}

agentMountFile := filepath.Join(d.Path(), "config", "agent-mounts.json")
err = os.WriteFile(agentMountFile, agentMountJSON, 0400)
if err != nil {
return "", nil, fmt.Errorf("Failed writing agent mounts file: %w", err)
return "", nil, nil, fmt.Errorf("Failed writing agent mounts file: %w", err)
}

// process any user-specified overrides
cfg = qemuRawCfgOverride(cfg, d.expandedConfig)
// Write the config file to disk.
sb := qemuStringifyCfg(cfg...)
configPath := filepath.Join(d.LogPath(), "qemu.conf")
return configPath, monHooks, os.WriteFile(configPath, []byte(sb.String()), 0640)

return configPath, monHooks, openFiles, os.WriteFile(configPath, []byte(sb.String()), 0640)
}

// addCPUMemoryConfig adds the qemu config required for setting the number of virtualised CPUs and memory.
Expand Down Expand Up @@ -3720,7 +3730,7 @@ func (d *qemu) addRootDriveConfig(qemuDev map[string]string, mountInfo *storageP
}

// addDriveDirConfig adds the qemu config required for adding a supplementary drive directory share.
func (d *qemu) addDriveDirConfig(cfg *[]cfgSection, bus *qemuBus, fdFiles *[]*os.File, agentMounts *[]instancetype.VMAgentMount, driveConf deviceConfig.MountEntryItem) error {
func (d *qemu) addDriveDirConfig(cfg *[]cfgSection, bus *qemuBus, fdFiles *[]*os.File, openFiles []*os.File, agentMounts *[]instancetype.VMAgentMount, driveConf deviceConfig.MountEntryItem) error {
mountTag := fmt.Sprintf("lxd_%s", driveConf.DevName)

agentMount := instancetype.VMAgentMount{
Expand Down Expand Up @@ -3769,6 +3779,8 @@ func (d *qemu) addDriveDirConfig(cfg *[]cfgSection, bus *qemuBus, fdFiles *[]*os
return fmt.Errorf("Couldn't open device socket directory %q: %s", filepath.Dir(virtiofsdSockPath), err)
}

openFiles = append(openFiles, virtiofsdSockDir)

Check failure on line 3782 in lxd/instance/drivers/driver_qemu.go

View workflow job for this annotation

GitHub Actions / Code 

ineffectual assignment to openFiles (ineffassign)

deviceSockPath := fmt.Sprintf("/proc/self/fd/%d/%s", d.addFileDescriptor(fdFiles, virtiofsdSockDir), filepath.Base(virtiofsdSockPath))

// Add virtio-fs device as this will be preferred over 9p.
Expand Down

0 comments on commit a6c22cd

Please sign in to comment.