Skip to content
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.

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

many: handlers and backend for kernel-modules components #13512

Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 2 additions & 0 deletions overlord/snapstate/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ type managerBackend interface {
StartServices(svcs []*snap.AppInfo, disabledSvcs []string, meter progress.Meter, tm timings.Measurer) error
StopServices(svcs []*snap.AppInfo, reason snap.ServiceStopReason, meter progress.Meter, tm timings.Measurer) error
QueryDisabledServices(info *snap.Info, pb progress.Meter) ([]string, error)
SetupKernelModulesComponent(cpi snap.ContainerPlaceInfo, cref naming.ComponentRef, kernelVersion string, meter progress.Meter) (err error)
UndoSetupKernelModulesComponent(cpi snap.ContainerPlaceInfo, cref naming.ComponentRef, kernelVersion string, meter progress.Meter) error

// the undoers for install
UndoSetupSnap(s snap.PlaceInfo, typ snap.Type, installRecord *backend.InstallRecord, dev snap.Device, meter progress.Meter) error
Expand Down
8 changes: 8 additions & 0 deletions overlord/snapstate/backend/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,11 @@ func MockMkdirAllChown(f func(string, os.FileMode, sys.UserID, sys.GroupID) erro
mkdirAllChown = old
}
}

func MockRunDepmod(f func(baseDir, kernelVersion string) error) func() {
old := runDepmod
runDepmod = f
return func() {
runDepmod = old
}
}
217 changes: 217 additions & 0 deletions overlord/snapstate/backend/kernel_modules_components.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2024 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

package backend

import (
"fmt"
"os"
"path/filepath"

"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/logger"
"github.com/snapcore/snapd/osutil"
"github.com/snapcore/snapd/progress"
"github.com/snapcore/snapd/snap"
"github.com/snapcore/snapd/snap/naming"
"github.com/snapcore/snapd/systemd"
)

type NoKernelDriversError struct {
cref naming.ComponentRef
kernelVersion string
}

func (e NoKernelDriversError) Error() string {
return fmt.Sprintf("%s does not contain firmware or components for %s",
e.cref, e.kernelVersion)
}

func cleanupMount(mountDir string, meter progress.Meter) error {
mountDir = filepath.Join(dirs.GlobalRootDir, mountDir)
// this also ensures that the mount unit stops
if err := removeMountUnit(mountDir, meter); err != nil {
return err
}

if err := os.RemoveAll(mountDir); err != nil {
return err
}

return nil
Comment on lines +53 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: could be shortened if you wanted to

Suggested change
if err := os.RemoveAll(mountDir); err != nil {
return err
}
return nil
return os.RemoveAll(mountDir)

}

type kernelModulesCleanupParts struct {
compMountDir string
modulesMountDir string
rerunDepmod bool
}

func cleanupKernelModulesSetup(parts *kernelModulesCleanupParts, kernelVersion string, meter progress.Meter) error {
if parts.modulesMountDir != "" {
if err := cleanupMount(parts.modulesMountDir, meter); err != nil {
return err
}
if parts.rerunDepmod {
if err := runDepmod("/usr", kernelVersion); err != nil {
return err
}
}
}

if parts.compMountDir != "" {
if err := cleanupMount(parts.compMountDir, meter); err != nil {
return err
}
}

return nil
}

func checkKernelModulesCompContent(mountDir, kernelVersion string) (bool, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

there isn't much in the way of checking, sounds more like discoverKernelModulesComContent?

hasModules := osutil.IsDirectory(filepath.Join(mountDir, "modules", kernelVersion))
hasFirmware := osutil.IsDirectory(filepath.Join(mountDir, "firmware"))
return hasModules, hasFirmware
}

func componentMountPoint(componentName, kernelVersion string) string {
return filepath.Join("/run/mnt/kernel-modules/", kernelVersion, componentName)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still wonder if it would be beneficial or not to include the revision in this one

}

func modulesMountPoint(componentName, kernelVersion string) string {
return filepath.Join("/usr/lib/modules", kernelVersion, "updates", componentName)
}

// SetupKernelModulesComponent creates and starts mount units for
// kernel-modules components.
func (b Backend) SetupKernelModulesComponent(cpi snap.ContainerPlaceInfo, cref naming.ComponentRef, kernelVersion string, meter progress.Meter) (err error) {
var sysd systemd.Systemd
if b.preseed {
sysd = systemd.NewEmulationMode(dirs.GlobalRootDir)
} else {
sysd = systemd.New(systemd.SystemMode, meter)
}

// Restore state if something goes wrong
var cleanOnFailure kernelModulesCleanupParts
defer func() {
if err == nil {
return
}
if err := cleanupKernelModulesSetup(&cleanOnFailure, kernelVersion, meter); err != nil {
logger.Noticef("while cleaning up a failed kernel-modules set-up: %v", err)
}
}()
Comment on lines +112 to +120
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we keep doing this here? or should we make it a problem of the caller, which might decide to do a switch back instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point as indeed in some cases we want to restore the old state and we might not want to do previously a clean-up.


// Check that the kernel-modules component really has
// something we must mount on early boot.
hasModules, hasFirmware := checkKernelModulesCompContent(cpi.MountDir(), kernelVersion)
if !hasModules && !hasFirmware {
return &NoKernelDriversError{cref: cref, kernelVersion: kernelVersion}
}

// Mount the component itself (we need it early, so the mount in /snap cannot
// be used).
componentMount := componentMountPoint(cref.ComponentName, kernelVersion)
addMountUnitOptions := &systemd.MountUnitOptions{
MountUnitType: systemd.BeforeDriversLoadMountUnit,
Lifetime: systemd.Persistent,
Description: fmt.Sprintf("Mount unit for kernel-modules component %s", cref),
What: cpi.MountFile(),
Where: componentMount,
Fstype: "squashfs",
Options: []string{"nodev,ro,x-gdu.hide,x-gvfs-hide"},
}
_, err = sysd.EnsureMountUnitFileWithOptions(addMountUnitOptions)
if err != nil {
return fmt.Errorf("cannot create mount in %q: %w", componentMount, err)
}
cleanOnFailure.compMountDir = componentMount

if hasModules {
// systemd automatically works out dependencies on the "what"
// path too so this mount happens after the component one.
modulesDir := modulesMountPoint(cref.ComponentName, kernelVersion)
addMountUnitOptions = &systemd.MountUnitOptions{
MountUnitType: systemd.BeforeDriversLoadMountUnit,
Lifetime: systemd.Persistent,
Description: fmt.Sprintf("Mount unit for modules from %s", cref.String()),
What: filepath.Join(componentMount, "modules", kernelVersion),
Where: modulesDir,
Fstype: "none",
Options: []string{"bind"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

could we record somewhere a discussion of why using a bind mount and not a symlink for this?

}
_, err = sysd.EnsureMountUnitFileWithOptions(addMountUnitOptions)
if err != nil {
return fmt.Errorf("cannot create mount in %q: %w", modulesDir, err)
}
cleanOnFailure.modulesMountDir = modulesDir

// Rebuild modinfo files
if err := runDepmod("/usr", kernelVersion); err != nil {
return err
}
cleanOnFailure.rerunDepmod = true
}

if hasFirmware {
// TODO create recursively symlinks in
// /usr/lib/firmware/updates while checking for conflicts with
// existing files.
}

return nil
}

var runDepmod = runDepmodImpl

func runDepmodImpl(baseDir, kernelVersion string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we have at least some tests using testutil.MockCmd to test this?

logger.Debugf("running depmod on %q for kernel %s", baseDir, kernelVersion)
stdout, stderr, err := osutil.RunSplitOutput("depmod", "-b", baseDir, kernelVersion)
Copy link
Contributor

Choose a reason for hiding this comment

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

btw. do you need to pass -e to get more useful error information maybe?

logger.Debugf("depmod stderr:\n%s\n\ndepmod stdout:\n%s",
string(stderr), string(stdout))
if err != nil {
return osutil.OutputErrCombine(stdout, stderr, err)
}
return nil
}

// UndoSetupKernelModulesComponent undoes the work of SetupKernelModulesComponent
func (b Backend) UndoSetupKernelModulesComponent(cpi snap.ContainerPlaceInfo, cref naming.ComponentRef, kernelVersion string, meter progress.Meter) error {
hasModules, hasFirmware := checkKernelModulesCompContent(cpi.MountDir(), kernelVersion)
var partsToClean kernelModulesCleanupParts
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably good to have a comment about why rerunDepmod is not set

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a comment a bit below related to depmod - not 100% sure if that is what you meant though.

if hasFirmware {
// TODO remove recursively symlinks in
// /usr/lib/firmware/updates (set var in kernelModulesCleanupParts)
}

// Remove created mount units
if hasModules {
partsToClean.modulesMountDir =
modulesMountPoint(cref.ComponentName, kernelVersion)
partsToClean.rerunDepmod = true
}

if hasModules || hasFirmware {
partsToClean.compMountDir =
componentMountPoint(cref.ComponentName, kernelVersion)
}

return cleanupKernelModulesSetup(&partsToClean, kernelVersion, meter)
}
Loading
Loading