From e0b9ac936f136a5a03a40283b911bc4802d98bef Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Mon, 21 Jul 2025 14:08:15 +0200 Subject: all: simplify subsystem revision updates Don't specify the subsystem revision in the dashboard config and instead let it be nested in the registered subsystems. This reduces the amount of the manual work needed to switch syzbot to a newer subsystem list. --- dashboard/app/api.go | 7 +++++-- dashboard/app/app_test.go | 6 +++--- dashboard/app/config.go | 2 -- dashboard/app/subsystem.go | 8 ++------ dashboard/app/util_test.go | 7 +------ pkg/subsystem/list.go | 28 +++++++++++++++++++++----- pkg/subsystem/lists/linux.go | 2 +- pkg/subsystem/service.go | 11 +++++----- pkg/subsystem/service_test.go | 2 +- tools/syz-query-subsystems/generator.go | 9 ++++++--- tools/syz-query-subsystems/query_subsystems.go | 14 +++++++------ 11 files changed, 55 insertions(+), 41 deletions(-) diff --git a/dashboard/app/api.go b/dashboard/app/api.go index 4a9b03171..c1d77881c 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -876,10 +876,13 @@ func reportCrash(c context.Context, build *Build, req *dashapi.Crash) (*Bug, err log.Infof(c, "not saving crash for %q", bug.Title) } + subsystemService := getNsConfig(c, ns).Subsystems.Service + newSubsystems := []*subsystem.Subsystem{} // Recalculate subsystems on the first saved crash and on the first saved repro, // unless a user has already manually specified them. - calculateSubsystems := save && + calculateSubsystems := subsystemService != nil && + save && !bug.hasUserSubsystems() && (bug.NumCrashes == 0 || bug.ReproLevel == ReproLevelNone && reproLevel != ReproLevelNone) @@ -910,7 +913,7 @@ func reportCrash(c context.Context, build *Build, req *dashapi.Crash) (*Bug, err bug.HasReport = true } if calculateSubsystems { - bug.SetAutoSubsystems(c, newSubsystems, now, getNsConfig(c, ns).Subsystems.Revision) + bug.SetAutoSubsystems(c, newSubsystems, now, subsystemService.Revision) } bug.increaseCrashStats(now) bug.HappenedOn = mergeString(bug.HappenedOn, build.Manager) diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go index 445fcf785..693ca61ee 100644 --- a/dashboard/app/app_test.go +++ b/dashboard/app/app_test.go @@ -128,7 +128,7 @@ var testConfig = &GlobalConfig{ }, }, Subsystems: SubsystemsConfig{ - Service: subsystem.MustMakeService(testSubsystems), + Service: subsystem.MustMakeService(testSubsystems, 0), }, }, "test2": { @@ -326,7 +326,7 @@ var testConfig = &GlobalConfig{ }, RetestRepros: true, Subsystems: SubsystemsConfig{ - Service: subsystem.MustMakeService(testSubsystems), + Service: subsystem.MustMakeService(testSubsystems, 0), Redirect: map[string]string{ "oldSubsystem": "subsystemA", }, @@ -517,7 +517,7 @@ var testConfig = &GlobalConfig{ }, }, Subsystems: SubsystemsConfig{ - Service: subsystem.MustMakeService(testSubsystems), + Service: subsystem.MustMakeService(testSubsystems, 0), Reminder: &BugListReportingConfig{ SourceReporting: "public", BugsInReport: 6, diff --git a/dashboard/app/config.go b/dashboard/app/config.go index 409b99ce5..8c9d9fb6e 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -200,8 +200,6 @@ type DiscussionEmailConfig struct { type SubsystemsConfig struct { // If Service is set, dashboard will use it to infer and recalculate subsystems. Service *subsystem.Service - // If all existing subsystem labels must be recalculated, increase this integer. - Revision int // Periodic per-subsystem reminders about open bugs. Reminder *BugListReportingConfig // Maps old subsystem names to new ones. diff --git a/dashboard/app/subsystem.go b/dashboard/app/subsystem.go index 293f67481..c49bc081b 100644 --- a/dashboard/app/subsystem.go +++ b/dashboard/app/subsystem.go @@ -28,7 +28,7 @@ func reassignBugSubsystems(c context.Context, ns string, count int) error { return err } log.Infof(c, "updating subsystems for %d bugs in %#v", len(keys), ns) - rev := getNsConfig(c, ns).Subsystems.Revision + rev := service.Revision for i, bugKey := range keys { if bugs[i].hasUserSubsystems() { // It might be that the user-set subsystem no longer exists. @@ -54,7 +54,7 @@ func reassignBugSubsystems(c context.Context, ns string, count int) error { func bugsToUpdateSubsystems(c context.Context, ns string, count int) ([]*Bug, []*db.Key, error) { now := timeNow(c) - rev := getSubsystemRevision(c, ns) + rev := getSubsystemService(c, ns).Revision queries := []*db.Query{ // If revision has been updated, first update open bugs. db.NewQuery("Bug"). @@ -194,10 +194,6 @@ func getSubsystemService(c context.Context, ns string) *subsystem.Service { return getNsConfig(c, ns).Subsystems.Service } -func getSubsystemRevision(c context.Context, ns string) int { - return getNsConfig(c, ns).Subsystems.Revision -} - func subsystemListURL(c context.Context, ns string) string { if getNsConfig(c, ns).Subsystems.Service == nil { return "" diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index 2040f879e..f403d8f2b 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -216,12 +216,7 @@ func (c *Ctx) setSubsystems(ns string, list []*subsystem.Subsystem, rev int) { c.transformContext = func(c context.Context) context.Context { newConfig := replaceNamespaceConfig(c, ns, func(cfg *Config) *Config { ret := *cfg - ret.Subsystems.Revision = rev - if list == nil { - ret.Subsystems.Service = nil - } else { - ret.Subsystems.Service = subsystem.MustMakeService(list) - } + ret.Subsystems.Service = subsystem.MustMakeService(list, rev) return &ret }) return contextWithConfig(c, newConfig) diff --git a/pkg/subsystem/list.go b/pkg/subsystem/list.go index d512a3fe1..c3650e7f9 100644 --- a/pkg/subsystem/list.go +++ b/pkg/subsystem/list.go @@ -3,6 +3,8 @@ package subsystem +import "fmt" + // In general, it's not correct to assume that subsystems are only determined by target.OS, // because subsystems are related not to the user interface of the OS kernel, but rather to // the OS kernel implementation. @@ -13,20 +15,36 @@ package subsystem // Therefore, subsystem lists have to be a completely different entity. var ( - lists = make(map[string][]*Subsystem) + lists = make(map[string]registeredSubsystem) ) -func RegisterList(name string, list []*Subsystem) { +type registeredSubsystem struct { + list []*Subsystem + revision int +} + +func RegisterList(name string, list []*Subsystem, revision int) { if _, ok := lists[name]; ok { panic(name + " subsystem list already exists!") } - lists[name] = list + lists[name] = registeredSubsystem{ + list: list, + revision: revision, + } } func GetList(name string) []*Subsystem { - return lists[name] + info, ok := lists[name] + if !ok { + panic(fmt.Sprintf("list %q is not registered", name)) + } + return info.list } func ListService(name string) *Service { - return MustMakeService(lists[name]) + info, ok := lists[name] + if !ok { + panic(fmt.Sprintf("list %q is not registered", name)) + } + return MustMakeService(info.list, info.revision) } diff --git a/pkg/subsystem/lists/linux.go b/pkg/subsystem/lists/linux.go index e94fdb9fb..274db98c0 100644 --- a/pkg/subsystem/lists/linux.go +++ b/pkg/subsystem/lists/linux.go @@ -6,7 +6,7 @@ package lists import . "github.com/google/syzkaller/pkg/subsystem" func init() { - RegisterList("linux", subsystems_linux()) + RegisterList("linux", subsystems_linux(), 20250720) } // The subsystem list: diff --git a/pkg/subsystem/service.go b/pkg/subsystem/service.go index 24f86dd54..a921cdfed 100644 --- a/pkg/subsystem/service.go +++ b/pkg/subsystem/service.go @@ -9,22 +9,20 @@ import ( type Service struct { *Extractor + Revision int perName map[string]*Subsystem perParent map[*Subsystem][]*Subsystem } -func MustMakeService(list []*Subsystem) *Service { - if len(list) == 0 { - panic("the subsystem list is empty") - } - service, err := MakeService(list) +func MustMakeService(list []*Subsystem, revision int) *Service { + service, err := MakeService(list, revision) if err != nil { panic(fmt.Sprintf("service creation failed: %s", err)) } return service } -func MakeService(list []*Subsystem) (*Service, error) { +func MakeService(list []*Subsystem, revision int) (*Service, error) { extractor := MakeExtractor(list) perName := map[string]*Subsystem{} perParent := map[*Subsystem][]*Subsystem{} @@ -42,6 +40,7 @@ func MakeService(list []*Subsystem) (*Service, error) { } return &Service{ Extractor: extractor, + Revision: revision, perName: perName, perParent: perParent, }, nil diff --git a/pkg/subsystem/service_test.go b/pkg/subsystem/service_test.go index 2a95b9558..d45a62806 100644 --- a/pkg/subsystem/service_test.go +++ b/pkg/subsystem/service_test.go @@ -14,6 +14,6 @@ func TestServiceChildren(t *testing.T) { parent := &Subsystem{Name: "parent"} childA := &Subsystem{Name: "childA", Parents: []*Subsystem{parent}} childB := &Subsystem{Name: "childB", Parents: []*Subsystem{parent}} - service := MustMakeService([]*Subsystem{unrelated, parent, childA, childB}) + service := MustMakeService([]*Subsystem{unrelated, parent, childA, childB}, 1) assert.ElementsMatch(t, service.Children(parent), []*Subsystem{childA, childB}) } diff --git a/tools/syz-query-subsystems/generator.go b/tools/syz-query-subsystems/generator.go index 058c63d2a..a7db17d3f 100644 --- a/tools/syz-query-subsystems/generator.go +++ b/tools/syz-query-subsystems/generator.go @@ -15,9 +15,10 @@ import ( "github.com/google/syzkaller/pkg/serializer" "github.com/google/syzkaller/pkg/subsystem" + "github.com/google/syzkaller/pkg/vcs" ) -func generateSubsystemsFile(name string, list []*subsystem.Subsystem, commitInfo string) ([]byte, error) { +func generateSubsystemsFile(name string, list []*subsystem.Subsystem, commit *vcs.Commit) ([]byte, error) { // Set names first -- we'll need them for filling in the Parents array. objToName := map[*subsystem.Subsystem]string{} for _, entry := range list { @@ -31,7 +32,8 @@ func generateSubsystemsFile(name string, list []*subsystem.Subsystem, commitInfo // Prepare the template data. vars := &templateVars{ Name: name, - CommitInfo: commitInfo, + CommitInfo: fmt.Sprintf(`Commit %s, "%.32s"`, commit.Hash, commit.Title), + Version: commit.Date.Year()*10000 + int(commit.Date.Month())*100 + commit.Date.Day(), Hierarchy: hierarchyList(list), } for _, entry := range list { @@ -126,6 +128,7 @@ type templateSubsystem struct { type templateVars struct { Name string + Version int CommitInfo string List []*templateSubsystem Hierarchy []string @@ -141,7 +144,7 @@ package lists import . "github.com/google/syzkaller/pkg/subsystem" func init() { - RegisterList("{{.Name}}", subsystems_{{.Name}}()) + RegisterList("{{.Name}}", subsystems_{{.Name}}(), {{.Version}}) } // The subsystem list: diff --git a/tools/syz-query-subsystems/query_subsystems.go b/tools/syz-query-subsystems/query_subsystems.go index 41a4a7a74..8f0fbae76 100644 --- a/tools/syz-query-subsystems/query_subsystems.go +++ b/tools/syz-query-subsystems/query_subsystems.go @@ -58,7 +58,10 @@ func main() { if err = osutil.MkdirAll(folder); err != nil { tool.Failf("failed to create %s: %v", folder, err) } - commitInfo := determineCommitInfo(*flagKernelRepo) + commitInfo, err := determineCommitInfo(*flagKernelRepo) + if err != nil { + tool.Failf("failed to fetch commit info: %v", err) + } code, err := generateSubsystemsFile(*flagName, list, commitInfo) if err != nil { tool.Failf("failed to generate code: %s", err) @@ -95,15 +98,14 @@ func prepareFilter() func(*subsystem.Subsystem) bool { } } -func determineCommitInfo(dir string) string { - // Best effort only. +func determineCommitInfo(dir string) (*vcs.Commit, error) { repo, err := vcs.NewRepo(*flagOS, "", dir, vcs.OptPrecious, vcs.OptDontSandbox) if err != nil { - return fmt.Sprintf("failed to open repo: %v", err) + return nil, fmt.Errorf("failed to open repo: %w", err) } commit, err := repo.Commit(vcs.HEAD) if err != nil { - return fmt.Sprintf("failed to get HEAD commit: %v", err) + return nil, fmt.Errorf("failed to get HEAD commit: %w", err) } - return fmt.Sprintf(`Commit %s, "%.32s"`, commit.Hash, commit.Title) + return commit, err } -- cgit mrf-deployment