From 396762795c62bb3cd83f04187440f8b66994231f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 12 Feb 2026 16:10:33 +0100 Subject: [PATCH 01/10] storage/types: remove Save() and StorageConfig() AFAICT these have been added at some point to allow storage.conf rewriting but that has never been implemented anywhere. As such drop these as the concept of writing to a signle default file becomes very questionable with the new incoming config loading as it uses many files not just one. Signed-off-by: Paul Holzinger --- storage/types/options.go | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/storage/types/options.go b/storage/types/options.go index 8af7c9c405..5b99e17788 100644 --- a/storage/types/options.go +++ b/storage/types/options.go @@ -509,39 +509,3 @@ func Options() (StoreOptions, error) { defaultStoreOptionsOnce.Do(loadDefaultStoreOptions) return defaultStoreOptions, loadDefaultStoreOptionsErr } - -// Save overwrites the tomlConfig in storage.conf with the given conf -func Save(conf TomlConfig) error { - configFile, err := DefaultConfigFile() - if err != nil { - return err - } - - if err = os.Remove(configFile); !os.IsNotExist(err) && err != nil { - return err - } - - f, err := os.Create(configFile) - if err != nil { - return err - } - - return toml.NewEncoder(f).Encode(conf) -} - -// StorageConfig is used to retrieve the storage.conf toml in order to overwrite it -func StorageConfig() (*TomlConfig, error) { - config := new(TomlConfig) - - configFile, err := DefaultConfigFile() - if err != nil { - return nil, err - } - - _, err = toml.DecodeFile(configFile, &config) - if err != nil { - return nil, err - } - - return config, nil -} From 01c928778e4dcb89908ff375dbf9ce40f45139a1 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 17 Feb 2026 19:41:04 +0100 Subject: [PATCH 02/10] storage/types: rework config files parsing Use the new pkg/configfile to allow parsing multiple files including drop ins per our design doc[1]. I went with the hard way of removing a lot of complexity and public APIs to start with a much more simple approach. Now there are only public functions left, LoadStoreOptions() which loads all files every time it gets called, this could be used for callers who like to re-read the files and a later point. Otherwsie the main existing DefaultStoreOptions() which caches the result on first call so we do not load the same files multiple times. Then a lot of this custom code to load rootless options were dropped. I think that should generally be fine to there might be subtile things I miss. One point is the choice of the rootless storage driver. The new code leaves that empty, the store load path already has logic to get a default driver. One important note is the handling of rootless_storage_path, which we read before graphroot now. So if graphroot is set in any configfile then rootless_storage_path is ignored. Overall we should deprecate rootless_storage_path as both the runroot and graphroot already uses the env expand code. And with the new configfile code users who only want to set the storage path of rootless users should create /etc/containers/storage.rootless.conf.d/99-myconf.conf with the graphroot set. That file will not be read by the rootful process. And to set a rootful default use /etc/containers/storage.rootful.conf.d/99-myconf.conf. [1] https://github.com/containers/podman/blob/7e78e842a05b4c924d9adae263bbdab340ab031f/contrib/design-docs/config-file-parsing.md Signed-off-by: Paul Holzinger --- storage/cmd/containers-storage/config.go | 13 +- storage/store.go | 22 +- storage/types/options.go | 405 +++++------------------ storage/types/options_bsd.go | 11 - storage/types/options_darwin.go | 8 - storage/types/options_linux.go | 44 --- storage/types/options_test.go | 124 +------ storage/types/options_windows.go | 11 - storage/types/storage_test.conf | 4 +- storage/types/utils.go | 58 ---- storage/types/utils_test.go | 44 --- 11 files changed, 105 insertions(+), 639 deletions(-) delete mode 100644 storage/types/utils_test.go diff --git a/storage/cmd/containers-storage/config.go b/storage/cmd/containers-storage/config.go index 7cc2a2d600..cb76a90bd9 100644 --- a/storage/cmd/containers-storage/config.go +++ b/storage/cmd/containers-storage/config.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "os" "go.podman.io/storage" "go.podman.io/storage/pkg/mflag" @@ -9,15 +10,15 @@ import ( ) func config(flags *mflag.FlagSet, action string, m storage.Store, args []string) (int, error) { - options, err := types.DefaultStoreOptions() - if err != nil { - return 1, fmt.Errorf("default: %+v", err) - } if len(args) > 0 { - if err = types.ReloadConfigurationFileIfNeeded(args[0], &options); err != nil { - return 1, fmt.Errorf("reload: %+v", err) + if err := os.Setenv("CONTAINERS_STORAGE_CONF", args[0]); err != nil { + return 1, fmt.Errorf("setenv: %w", err) } } + options, err := types.DefaultStoreOptions() + if err != nil { + return 1, fmt.Errorf("load default options: %w", err) + } return outputJSON(options) } diff --git a/storage/store.go b/storage/store.go index 3d8ea50759..6fc1f4645b 100644 --- a/storage/store.go +++ b/storage/store.go @@ -807,7 +807,7 @@ type store struct { // return // } func GetStore(options types.StoreOptions) (Store, error) { - defaultOpts, err := types.Options() + defaultOpts, err := types.DefaultStoreOptions() if err != nil { return nil, err } @@ -3923,27 +3923,9 @@ const AutoUserNsMaxSize = 65536 // creating a user namespace. const RootAutoUserNsUser = "containers" -// SetDefaultConfigFilePath sets the default configuration to the specified path, and loads the file. -// Deprecated: Use types.SetDefaultConfigFilePath, which can return an error. -func SetDefaultConfigFilePath(path string) { - _ = types.SetDefaultConfigFilePath(path) -} - -// DefaultConfigFile returns the path to the storage config file used -func DefaultConfigFile() (string, error) { - return types.DefaultConfigFile() -} - -// ReloadConfigurationFile parses the specified configuration file and overrides -// the configuration in storeOptions. -// Deprecated: Use types.ReloadConfigurationFile, which can return an error. -func ReloadConfigurationFile(configFile string, storeOptions *types.StoreOptions) { - _ = types.ReloadConfigurationFile(configFile, storeOptions) -} - // GetDefaultMountOptions returns the default mountoptions defined in container/storage func GetDefaultMountOptions() ([]string, error) { - defaultStoreOptions, err := types.Options() + defaultStoreOptions, err := types.DefaultStoreOptions() if err != nil { return nil, err } diff --git a/storage/types/options.go b/storage/types/options.go index 5b99e17788..48aeb8e6e9 100644 --- a/storage/types/options.go +++ b/storage/types/options.go @@ -1,19 +1,15 @@ package types import ( - "errors" "fmt" "os" "path/filepath" - "slices" "strings" "sync" - "time" - "github.com/BurntSushi/toml" "github.com/sirupsen/logrus" cfg "go.podman.io/storage/pkg/config" - "go.podman.io/storage/pkg/fileutils" + "go.podman.io/storage/pkg/configfile" "go.podman.io/storage/pkg/homedir" "go.podman.io/storage/pkg/idtools" "go.podman.io/storage/pkg/unshare" @@ -39,89 +35,6 @@ const ( storageConfEnv = "CONTAINERS_STORAGE_CONF" ) -var ( - defaultStoreOptionsOnce sync.Once - loadDefaultStoreOptionsErr error - once sync.Once - storeOptions StoreOptions - storeError error - defaultConfigFileSet bool - // defaultConfigFile path to the system wide storage.conf file - defaultConfigFile = SystemConfigFile - // DefaultStoreOptions is a reasonable default set of options. - defaultStoreOptions StoreOptions -) - -func loadDefaultStoreOptions() { - defaultStoreOptions.GraphDriverName = "" - - setDefaults := func() { - // reload could set values to empty for run and graph root if config does not contains anything - if defaultStoreOptions.RunRoot == "" { - defaultStoreOptions.RunRoot = defaultRunRoot - } - if defaultStoreOptions.GraphRoot == "" { - defaultStoreOptions.GraphRoot = defaultGraphRoot - } - } - setDefaults() - - if path, ok := os.LookupEnv(storageConfEnv); ok { - defaultOverrideConfigFile = path - if err := ReloadConfigurationFileIfNeeded(path, &defaultStoreOptions); err != nil { - loadDefaultStoreOptionsErr = err - return - } - setDefaults() - return - } - - if path, ok := os.LookupEnv("XDG_CONFIG_HOME"); ok { - homeConfigFile := filepath.Join(path, "containers", "storage.conf") - if err := fileutils.Exists(homeConfigFile); err == nil { - // user storage.conf in XDG_CONFIG_HOME if it exists - defaultOverrideConfigFile = homeConfigFile - } else { - if !os.IsNotExist(err) { - loadDefaultStoreOptionsErr = err - return - } - } - } - - err := fileutils.Exists(defaultOverrideConfigFile) - if err == nil { - // The DefaultConfigFile() function returns the path - // of the used storage.conf file, by returning defaultConfigFile - // If override exists containers/storage uses it by default. - defaultConfigFile = defaultOverrideConfigFile - if err := ReloadConfigurationFileIfNeeded(defaultOverrideConfigFile, &defaultStoreOptions); err != nil { - loadDefaultStoreOptionsErr = err - return - } - setDefaults() - return - } - - if !os.IsNotExist(err) { - logrus.Warningf("Attempting to use %s, %v", defaultConfigFile, err) - } - if err := ReloadConfigurationFileIfNeeded(defaultConfigFile, &defaultStoreOptions); err != nil && !errors.Is(err, os.ErrNotExist) { - loadDefaultStoreOptionsErr = err - return - } - setDefaults() -} - -// loadStoreOptions returns the default storage ops for containers -func loadStoreOptions() (StoreOptions, error) { - storageConf, err := DefaultConfigFile() - if err != nil { - return defaultStoreOptions, err - } - return loadStoreOptionsFromConfFile(storageConf) -} - // usePerUserStorage returns whether the user private storage must be used. // We cannot simply use the unshare.IsRootless() condition, because // that checks only if the current process needs a user namespace to @@ -132,96 +45,14 @@ func usePerUserStorage() bool { return unshare.IsRootless() && unshare.GetRootlessUID() != 0 } -// loadStoreOptionsFromConfFile is an internal implementation detail of DefaultStoreOptions to allow testing. -// Everyone but the tests this is intended for should only call loadStoreOptions, never this function. -func loadStoreOptionsFromConfFile(storageConf string) (StoreOptions, error) { - var ( - defaultRootlessRunRoot string - defaultRootlessGraphRoot string - err error - ) - - defaultStoreOptionsOnce.Do(loadDefaultStoreOptions) - if loadDefaultStoreOptionsErr != nil { - return StoreOptions{}, loadDefaultStoreOptionsErr - } - storageOpts := defaultStoreOptions - if usePerUserStorage() { - storageOpts, err = getRootlessStorageOpts(storageOpts) - if err != nil { - return storageOpts, err - } - } - err = fileutils.Exists(storageConf) - if err != nil && !os.IsNotExist(err) { - return storageOpts, err - } - if err == nil && !defaultConfigFileSet { - defaultRootlessRunRoot = storageOpts.RunRoot - defaultRootlessGraphRoot = storageOpts.GraphRoot - storageOpts = StoreOptions{} - reloadConfigurationFileIfNeeded(storageConf, &storageOpts) - // If the file did not specify a graphroot or runroot, - // set sane defaults so we don't try and use root-owned - // directories - if storageOpts.RunRoot == "" { - storageOpts.RunRoot = defaultRootlessRunRoot - } - if storageOpts.GraphRoot == "" { - if storageOpts.RootlessStoragePath != "" { - storageOpts.GraphRoot = storageOpts.RootlessStoragePath - } else { - storageOpts.GraphRoot = defaultRootlessGraphRoot - } - } - } - if storageOpts.RunRoot == "" { - return storageOpts, fmt.Errorf("runroot must be set") - } - rootlessUID := unshare.GetRootlessUID() - runRoot, err := expandEnvPath(storageOpts.RunRoot, rootlessUID) - if err != nil { - return storageOpts, err - } - storageOpts.RunRoot = runRoot - - if storageOpts.GraphRoot == "" { - return storageOpts, fmt.Errorf("graphroot must be set") - } - graphRoot, err := expandEnvPath(storageOpts.GraphRoot, rootlessUID) - if err != nil { - return storageOpts, err - } - storageOpts.GraphRoot = graphRoot - - if storageOpts.RootlessStoragePath != "" { - storagePath, err := expandEnvPath(storageOpts.RootlessStoragePath, rootlessUID) - if err != nil { - return storageOpts, err - } - storageOpts.RootlessStoragePath = storagePath - } - - if storageOpts.ImageStore != "" && storageOpts.ImageStore == storageOpts.GraphRoot { - return storageOpts, fmt.Errorf("imagestore %s must either be not set or be a different than graphroot", storageOpts.ImageStore) - } - - return storageOpts, nil -} - -// UpdateOptions should be called iff container engine received a SIGHUP, -// otherwise use DefaultStoreOptions -func UpdateStoreOptions() (StoreOptions, error) { - storeOptions, storeError = loadStoreOptions() - return storeOptions, storeError -} +// defaultStoreOptions is kept private so external callers can not reassign the value +var defaultStoreOptions = sync.OnceValues(func() (StoreOptions, error) { + return LoadStoreOptions(LoadOptions{}) +}) // DefaultStoreOptions returns the default storage ops for containers func DefaultStoreOptions() (StoreOptions, error) { - once.Do(func() { - storeOptions, storeError = loadStoreOptions() - }) - return storeOptions, storeError + return defaultStoreOptions() } // StoreOptions is used for passing initialization options to GetStore(), for @@ -237,9 +68,6 @@ type StoreOptions struct { // Image Store is the alternate location of image store if a location // separate from the container store is required. ImageStore string `json:"imagestore,omitempty"` - // RootlessStoragePath is the storage path for rootless users - // default $HOME/.local/share/containers/storage - RootlessStoragePath string `toml:"rootless_storage_path"` // If the driver is not specified, the best suited driver will be picked // either from GraphDriverPriority, if specified, or from the platform // dependent priority list (in that order). @@ -272,163 +100,57 @@ type StoreOptions struct { TransientStore bool `json:"transient_store,omitempty"` } -// isRootlessDriver returns true if the given storage driver is valid for containers running as non root -func isRootlessDriver(driver string) bool { - validDrivers := map[string]bool{ - "btrfs": true, - "overlay": true, - "overlay2": true, - "vfs": true, - } - return validDrivers[driver] -} - -// getRootlessStorageOpts returns the storage opts for containers running as non root -func getRootlessStorageOpts(systemOpts StoreOptions) (StoreOptions, error) { - var opts StoreOptions - - rootlessUID := unshare.GetRootlessUID() - +// setDefaultRootlessStoreOptions sets the storage opts for containers running as non root +func setDefaultRootlessStoreOptions(opts *StoreOptions) error { dataDir, err := homedir.GetDataHome() if err != nil { - return opts, err + return err } rootlessRuntime, err := homedir.GetRuntimeDir() if err != nil { - return opts, err + return err } + opts.GraphRoot = filepath.Join(dataDir, "containers", "storage") opts.RunRoot = filepath.Join(rootlessRuntime, "containers") - if err := os.MkdirAll(opts.RunRoot, 0o700); err != nil { - return opts, fmt.Errorf("unable to make rootless runtime: %w", err) - } - opts.PullOptions = systemOpts.PullOptions - if systemOpts.RootlessStoragePath != "" { - opts.GraphRoot, err = expandEnvPath(systemOpts.RootlessStoragePath, rootlessUID) - if err != nil { - return opts, err - } - } else { - opts.GraphRoot = filepath.Join(dataDir, "containers", "storage") - } - - if driver := systemOpts.GraphDriverName; isRootlessDriver(driver) { - opts.GraphDriverName = driver - } - if driver := os.Getenv("STORAGE_DRIVER"); driver != "" { - opts.GraphDriverName = driver - } - if opts.GraphDriverName == overlay2 { - logrus.Warnf("Switching default driver from overlay2 to the equivalent overlay driver") - opts.GraphDriverName = overlayDriver - } - - // If the configuration file was explicitly set, then copy all the options - // present. - if defaultConfigFileSet { - opts.GraphDriverOptions = systemOpts.GraphDriverOptions - opts.ImageStore = systemOpts.ImageStore - } else if opts.GraphDriverName == overlayDriver { - for _, o := range systemOpts.GraphDriverOptions { - if strings.Contains(o, "ignore_chown_errors") { - opts.GraphDriverOptions = append(opts.GraphDriverOptions, o) - break - } - } - } - if opts.GraphDriverName == "" { - if len(systemOpts.GraphDriverPriority) == 0 { - dirEntries, err := os.ReadDir(opts.GraphRoot) - if err == nil { - for _, entry := range dirEntries { - if name, ok := strings.CutSuffix(entry.Name(), "-images"); ok { - opts.GraphDriverName = name - break - } - } - } - - if opts.GraphDriverName == "" { - if canUseRootlessOverlay() { - opts.GraphDriverName = overlayDriver - } else { - opts.GraphDriverName = "vfs" - } - } - } else { - opts.GraphDriverPriority = systemOpts.GraphDriverPriority - } - } - - if os.Getenv("STORAGE_OPTS") != "" { - opts.GraphDriverOptions = slices.AppendSeq(opts.GraphDriverOptions, strings.SplitSeq(os.Getenv("STORAGE_OPTS"), ",")) - } - - return opts, nil + return nil } -var prevReloadConfig = struct { - storeOptions *StoreOptions - mod time.Time - mutex sync.Mutex - configFile string -}{} - -// SetDefaultConfigFilePath sets the default configuration to the specified path -func SetDefaultConfigFilePath(path string) error { - defaultConfigFile = path - defaultConfigFileSet = true - return ReloadConfigurationFileIfNeeded(defaultConfigFile, &defaultStoreOptions) +type LoadOptions struct { + // RootForImplicitAbsolutePaths is the path to an alternate root + // If not "", prefixed to any absolute paths used by default in the package. + // NOTE: This does NOT affect paths starting by $HOME or environment variables paths. + RootForImplicitAbsolutePaths string } -func ReloadConfigurationFileIfNeeded(configFile string, storeOptions *StoreOptions) error { - prevReloadConfig.mutex.Lock() - defer prevReloadConfig.mutex.Unlock() +func LoadStoreOptions(opts LoadOptions) (StoreOptions, error) { + config := new(TomlConfig) - fi, err := os.Stat(configFile) + rootlessUID := unshare.GetRootlessUID() + err := configfile.ParseTOML(config, &configfile.File{ + Name: "storage", + Extension: "conf", + EnvironmentName: storageConfEnv, + UserId: rootlessUID, + RootForImplicitAbsolutePaths: opts.RootForImplicitAbsolutePaths, + }) if err != nil { - return err + return StoreOptions{}, err } - mtime := fi.ModTime() - if prevReloadConfig.storeOptions != nil && mtime.Equal(prevReloadConfig.mod) && prevReloadConfig.configFile == configFile { - *storeOptions = *prevReloadConfig.storeOptions - return nil + storeOptions := StoreOptions{ + GraphRoot: defaultGraphRoot, + RunRoot: defaultRunRoot, } - - if err := ReloadConfigurationFile(configFile, storeOptions); err != nil { - return err - } - - cOptions := *storeOptions - prevReloadConfig.storeOptions = &cOptions - prevReloadConfig.mod = mtime - prevReloadConfig.configFile = configFile - return nil -} - -// ReloadConfigurationFile parses the specified configuration file and overrides -// the configuration in storeOptions. -func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) error { - config := new(TomlConfig) - - meta, err := toml.DecodeFile(configFile, &config) - if err == nil { - keys := meta.Undecoded() - if len(keys) > 0 { - logrus.Warningf("Failed to decode the keys %q from %q", keys, configFile) - } - } else { - if !os.IsNotExist(err) { - logrus.Warningf("Failed to read %s %v\n", configFile, err.Error()) - return err + if usePerUserStorage() { + err := setDefaultRootlessStoreOptions(&storeOptions) + if err != nil { + return StoreOptions{}, err } } - // Clear storeOptions of previous settings - *storeOptions = StoreOptions{} if config.Storage.Driver != "" { storeOptions.GraphDriverName = config.Storage.Driver } @@ -442,20 +164,13 @@ func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) erro } storeOptions.GraphDriverPriority = config.Storage.DriverPriority if storeOptions.GraphDriverName == "" && len(storeOptions.GraphDriverPriority) == 0 { - logrus.Warnf("The storage 'driver' option should be set in %s. A driver was picked automatically.", configFile) - } - if config.Storage.RunRoot != "" { - storeOptions.RunRoot = config.Storage.RunRoot - } - if config.Storage.GraphRoot != "" { - storeOptions.GraphRoot = config.Storage.GraphRoot + logrus.Warn("The storage 'driver' option should be set in storage.conf. A driver was picked automatically") } + if config.Storage.ImageStore != "" { storeOptions.ImageStore = config.Storage.ImageStore } - if config.Storage.RootlessStoragePath != "" { - storeOptions.RootlessStoragePath = config.Storage.RootlessStoragePath - } + for _, s := range config.Storage.Options.AdditionalImageStores { storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.imagestore=%s", config.Storage.Driver, s)) } @@ -502,10 +217,44 @@ func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) erro if len(storeOptions.GraphDriverOptions) == 1 && storeOptions.GraphDriverOptions[0] == "" { storeOptions.GraphDriverOptions = nil } - return nil -} -func Options() (StoreOptions, error) { - defaultStoreOptionsOnce.Do(loadDefaultStoreOptions) - return defaultStoreOptions, loadDefaultStoreOptionsErr + if config.Storage.RunRoot != "" { + runRoot, err := expandEnvPath(config.Storage.RunRoot, rootlessUID) + if err != nil { + return storeOptions, err + } + storeOptions.RunRoot = runRoot + } + if storeOptions.RunRoot == "" { + return storeOptions, fmt.Errorf("runroot must be set") + } + + // FIXME: Should this be before or after the graphroot setting? + // Now it is before which means any graphroot setting overwrites the + // rootless_storage_path, so if the main config has both set only graphroot is used. + if config.Storage.RootlessStoragePath != "" && usePerUserStorage() { + storagePath, err := expandEnvPath(config.Storage.RootlessStoragePath, rootlessUID) + if err != nil { + return storeOptions, err + } + storeOptions.GraphRoot = storagePath + } + + if config.Storage.GraphRoot != "" { + graphRoot, err := expandEnvPath(config.Storage.GraphRoot, rootlessUID) + if err != nil { + return storeOptions, err + } + storeOptions.GraphRoot = graphRoot + } + + if storeOptions.GraphRoot == "" { + return storeOptions, fmt.Errorf("graphroot must be set") + } + + if storeOptions.ImageStore != "" && storeOptions.ImageStore == storeOptions.GraphRoot { + return storeOptions, fmt.Errorf("imagestore %s must either be not set or be a different than graphroot", storeOptions.ImageStore) + } + + return storeOptions, nil } diff --git a/storage/types/options_bsd.go b/storage/types/options_bsd.go index 040fdc797d..a93ebc3412 100644 --- a/storage/types/options_bsd.go +++ b/storage/types/options_bsd.go @@ -7,15 +7,4 @@ const ( // for rootless path is constructed via getRootlessStorageOpts defaultRunRoot string = "/var/run/containers/storage" defaultGraphRoot string = "/var/db/containers/storage" - SystemConfigFile = "/usr/local/share/containers/storage.conf" ) - -// defaultConfigFile path to the system wide storage.conf file -var ( - defaultOverrideConfigFile = "/usr/local/etc/containers/storage.conf" -) - -// canUseRootlessOverlay returns true if the overlay driver can be used for rootless containers -func canUseRootlessOverlay() bool { - return false -} diff --git a/storage/types/options_darwin.go b/storage/types/options_darwin.go index 27ba6a061d..1f70b9c7a2 100644 --- a/storage/types/options_darwin.go +++ b/storage/types/options_darwin.go @@ -5,12 +5,4 @@ const ( // for rootless path is constructed via getRootlessStorageOpts defaultRunRoot string = "/run/containers/storage" defaultGraphRoot string = "/var/lib/containers/storage" - SystemConfigFile = "/usr/share/containers/storage.conf" ) - -var defaultOverrideConfigFile = "/etc/containers/storage.conf" - -// canUseRootlessOverlay returns true if the overlay driver can be used for rootless containers -func canUseRootlessOverlay() bool { - return false -} diff --git a/storage/types/options_linux.go b/storage/types/options_linux.go index 09cbae54b6..1f70b9c7a2 100644 --- a/storage/types/options_linux.go +++ b/storage/types/options_linux.go @@ -1,52 +1,8 @@ package types -import ( - "os/exec" - "strconv" - "strings" - - "golang.org/x/sys/unix" -) - const ( // these are default path for run and graph root for rootful users // for rootless path is constructed via getRootlessStorageOpts defaultRunRoot string = "/run/containers/storage" defaultGraphRoot string = "/var/lib/containers/storage" - SystemConfigFile = "/usr/share/containers/storage.conf" ) - -// defaultConfigFile path to the system wide storage.conf file -var ( - defaultOverrideConfigFile = "/etc/containers/storage.conf" -) - -// canUseRootlessOverlay returns true if the overlay driver can be used for rootless containers -func canUseRootlessOverlay() bool { - // we check first for fuse-overlayfs since it is cheaper. - if path, _ := exec.LookPath("fuse-overlayfs"); path != "" { - return true - } - - // We cannot use overlay.SupportsNativeOverlay since canUseRootlessOverlay is called by Podman - // before we enter the user namespace and the driver we pick here is written in the podman database. - // Checking the kernel version is usually not a good idea since the feature could be back-ported, e.g. RHEL - // but this is just an heuristic and on RHEL we always install the storage.conf file. - // native overlay for rootless was added upstream in 5.13 (at least the first version that we support), so check - // that the kernel is >= 5.13. - var uts unix.Utsname - if err := unix.Uname(&uts); err == nil { - parts := strings.Split(string(uts.Release[:]), ".") - major, _ := strconv.Atoi(parts[0]) - if major >= 6 { - return true - } - if major == 5 && len(parts) > 1 { - minor, _ := strconv.Atoi(parts[1]) - if minor >= 13 { - return true - } - } - } - return false -} diff --git a/storage/types/options_test.go b/storage/types/options_test.go index b0dd98269c..cdf6e6131d 100644 --- a/storage/types/options_test.go +++ b/storage/types/options_test.go @@ -2,9 +2,8 @@ package types import ( "bytes" - "fmt" "os" - "path/filepath" + "strconv" "strings" "testing" @@ -14,116 +13,27 @@ import ( "gotest.tools/v3/assert" ) -func TestGetRootlessStorageOpts(t *testing.T) { - envDriver, envDriverSet := os.LookupEnv("STORAGE_DRIVER") - os.Unsetenv("STORAGE_DRIVER") - - const vfsDriver = "vfs" - - t.Run("systemDriver=", func(t *testing.T) { - systemOpts := StoreOptions{} - - td := t.TempDir() - home := filepath.Join(td, "unset-driver-home") - runhome := filepath.Join(td, "unset-driver-runhome") - defer os.RemoveAll(home) - defer os.RemoveAll(runhome) - - systemOpts.GraphRoot = home - systemOpts.RunRoot = runhome - storageOpts, err := getRootlessStorageOpts(systemOpts) - - assert.NilError(t, err) - expectedDriver := vfsDriver - if canUseRootlessOverlay() { - expectedDriver = overlayDriver - } - assert.Equal(t, storageOpts.GraphDriverName, expectedDriver) - }) - - t.Run("systemDriver=btrfs", func(t *testing.T) { - systemOpts := StoreOptions{} - systemOpts.GraphDriverName = "btrfs" - storageOpts, err := getRootlessStorageOpts(systemOpts) - assert.NilError(t, err) - assert.Equal(t, storageOpts.GraphDriverName, "btrfs") - }) - - t.Run("systemDriver=overlay", func(t *testing.T) { - systemOpts := StoreOptions{} - systemOpts.GraphDriverName = overlayDriver - storageOpts, err := getRootlessStorageOpts(systemOpts) - assert.NilError(t, err) - assert.Equal(t, storageOpts.GraphDriverName, overlayDriver) - }) - - t.Run("systemDriver=overlay2", func(t *testing.T) { - systemOpts := StoreOptions{} - systemOpts.GraphDriverName = "overlay2" - storageOpts, err := getRootlessStorageOpts(systemOpts) - assert.NilError(t, err) - assert.Equal(t, storageOpts.GraphDriverName, overlayDriver) - }) - - t.Run("systemDriver=vfs", func(t *testing.T) { - systemOpts := StoreOptions{} - systemOpts.GraphDriverName = vfsDriver - storageOpts, err := getRootlessStorageOpts(systemOpts) - assert.NilError(t, err) - assert.Equal(t, storageOpts.GraphDriverName, vfsDriver) - }) - - t.Run("systemDriver=zfs", func(t *testing.T) { - systemOpts := StoreOptions{} - systemOpts.GraphDriverName = "zfs" - storageOpts, err := getRootlessStorageOpts(systemOpts) - assert.NilError(t, err) - assert.Assert(t, storageOpts.GraphDriverName == overlayDriver || storageOpts.GraphDriverName == vfsDriver, fmt.Sprintf("The rootless driver should be set to 'overlay' or 'vfs' not '%v'", storageOpts.GraphDriverName)) - }) - - t.Run("STORAGE_DRIVER=btrfs", func(t *testing.T) { - t.Setenv("STORAGE_DRIVER", "btrfs") - systemOpts := StoreOptions{} - systemOpts.GraphDriverName = vfsDriver - storageOpts, err := getRootlessStorageOpts(systemOpts) - assert.NilError(t, err) - assert.Equal(t, storageOpts.GraphDriverName, "btrfs") - }) - - t.Run("STORAGE_DRIVER=zfs", func(t *testing.T) { - t.Setenv("STORAGE_DRIVER", "zfs") - systemOpts := StoreOptions{} - systemOpts.GraphDriverName = vfsDriver - storageOpts, err := getRootlessStorageOpts(systemOpts) - assert.NilError(t, err) - assert.Equal(t, storageOpts.GraphDriverName, "zfs") - }) - - if envDriverSet { - os.Setenv("STORAGE_DRIVER", envDriver) - } else { - os.Unsetenv("STORAGE_DRIVER") - } -} - -func TestGetRootlessStorageOpts2(t *testing.T) { - opts := StoreOptions{ - RootlessStoragePath: "/$HOME/$UID/containers/storage", - } - expectedPath := filepath.Join(os.Getenv("HOME"), fmt.Sprintf("%d", unshare.GetRootlessUID()), "containers/storage") - storageOpts, err := getRootlessStorageOpts(opts) - assert.NilError(t, err) - assert.Equal(t, storageOpts.GraphRoot, expectedPath) -} - -func TestReloadConfigurationFile(t *testing.T) { +func TestInvalidKeyFile(t *testing.T) { + t.Setenv(storageConfEnv, "./storage_broken.conf") content := bytes.NewBufferString("") logrus.SetOutput(content) + defer logrus.SetOutput(os.Stderr) + logrus.SetLevel(logrus.DebugLevel) + defer logrus.SetLevel(logrus.InfoLevel) var storageOpts StoreOptions - err := ReloadConfigurationFile("./storage_broken.conf", &storageOpts) + storageOpts, err := LoadStoreOptions(LoadOptions{}) require.NoError(t, err) assert.Equal(t, storageOpts.RunRoot, "/run/containers/test") - logrus.SetOutput(os.Stderr) assert.Equal(t, strings.Contains(content.String(), "Failed to decode the keys [\\\"foo\\\" \\\"storage.options.graphroot\\\"] from \\\"./storage_broken.conf\\\"\""), true) } + +func TestLoadStoreOptions(t *testing.T) { + t.Setenv(storageConfEnv, "./storage_test.conf") + var storageOpts StoreOptions + storageOpts, err := LoadStoreOptions(LoadOptions{}) + require.NoError(t, err) + + assert.Equal(t, storageOpts.RunRoot, "/run/"+strconv.Itoa(unshare.GetRootlessUID())+"/containers/storage") + assert.Equal(t, storageOpts.GraphRoot, os.Getenv("HOME")+"/"+strconv.Itoa(unshare.GetRootlessUID())+"/containers/storage") +} diff --git a/storage/types/options_windows.go b/storage/types/options_windows.go index 99a67ff210..1f70b9c7a2 100644 --- a/storage/types/options_windows.go +++ b/storage/types/options_windows.go @@ -5,15 +5,4 @@ const ( // for rootless path is constructed via getRootlessStorageOpts defaultRunRoot string = "/run/containers/storage" defaultGraphRoot string = "/var/lib/containers/storage" - SystemConfigFile = "/usr/share/containers/storage.conf" ) - -// defaultConfigFile path to the system wide storage.conf file -var ( - defaultOverrideConfigFile = "/etc/containers/storage.conf" -) - -// canUseRootlessOverlay returns true if the overlay driver can be used for rootless containers -func canUseRootlessOverlay() bool { - return false -} diff --git a/storage/types/storage_test.conf b/storage/types/storage_test.conf index 761b3a795d..be074b942e 100644 --- a/storage/types/storage_test.conf +++ b/storage/types/storage_test.conf @@ -8,14 +8,14 @@ driver = "" # Temporary storage location -runroot = "$HOME/$UID/containers/storage" +runroot = "/run/$UID/containers/storage" # Primary Read/Write location of container storage graphroot = "$HOME/$UID/containers/storage" # Storage path for rootless users # -rootless_storage_path = "$HOME/$UID/containers/storage" +rootless_storage_path = "$HOME/$UID/rootless/storage" [storage.options] # Storage options to be passed to underlying storage drivers diff --git a/storage/types/utils.go b/storage/types/utils.go index fd25eaa075..d19959ee4b 100644 --- a/storage/types/utils.go +++ b/storage/types/utils.go @@ -1,15 +1,10 @@ package types import ( - "errors" "os" "path/filepath" "strconv" "strings" - - "github.com/sirupsen/logrus" - "go.podman.io/storage/pkg/fileutils" - "go.podman.io/storage/pkg/homedir" ) func expandEnvPath(path string, rootlessUID int) (string, error) { @@ -22,56 +17,3 @@ func expandEnvPath(path string, rootlessUID int) (string, error) { } return newpath, nil } - -func DefaultConfigFile() (string, error) { - if defaultConfigFileSet { - return defaultConfigFile, nil - } - - if path, ok := os.LookupEnv(storageConfEnv); ok { - return path, nil - } - if !usePerUserStorage() { - if err := fileutils.Exists(defaultOverrideConfigFile); err == nil { - return defaultOverrideConfigFile, nil - } - return defaultConfigFile, nil - } - - if configHome := os.Getenv("XDG_CONFIG_HOME"); configHome != "" { - return filepath.Join(configHome, "containers/storage.conf"), nil - } - home := homedir.Get() - if home == "" { - return "", errors.New("cannot determine user's homedir") - } - return filepath.Join(home, ".config/containers/storage.conf"), nil -} - -func reloadConfigurationFileIfNeeded(configFile string, storeOptions *StoreOptions) { - prevReloadConfig.mutex.Lock() - defer prevReloadConfig.mutex.Unlock() - - fi, err := os.Stat(configFile) - if err != nil { - if !os.IsNotExist(err) { - logrus.Warningf("Failed to read %s %v\n", configFile, err.Error()) - } - return - } - - mtime := fi.ModTime() - if prevReloadConfig.storeOptions != nil && mtime.Equal(prevReloadConfig.mod) && prevReloadConfig.configFile == configFile { - *storeOptions = *prevReloadConfig.storeOptions - return - } - - if err := ReloadConfigurationFile(configFile, storeOptions); err != nil { - logrus.Warningf("Failed to reload %q %v\n", configFile, err) - return - } - - prevReloadConfig.storeOptions = storeOptions - prevReloadConfig.mod = mtime - prevReloadConfig.configFile = configFile -} diff --git a/storage/types/utils_test.go b/storage/types/utils_test.go deleted file mode 100644 index fb77d33240..0000000000 --- a/storage/types/utils_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package types - -import ( - "fmt" - "os" - "path/filepath" - "testing" - - "go.podman.io/storage/pkg/unshare" - "gotest.tools/v3/assert" -) - -func TestDefaultStoreOpts(t *testing.T) { - if !usePerUserStorage() { - t.Skip() - } - storageOpts, err := loadStoreOptionsFromConfFile("./storage_test.conf") - expectedPath := filepath.Join(os.Getenv("HOME"), fmt.Sprintf("%d", unshare.GetRootlessUID()), "containers/storage") - - assert.NilError(t, err) - assert.Equal(t, storageOpts.RunRoot, expectedPath) - assert.Equal(t, storageOpts.GraphRoot, expectedPath) - assert.Equal(t, storageOpts.RootlessStoragePath, expectedPath) -} - -func TestStorageConfOverrideEnvironmentDefaultConfigFileRootless(t *testing.T) { - t.Setenv("CONTAINERS_STORAGE_CONF", "default_override_test.conf") - defaultFile, err := DefaultConfigFile() - - expectedPath := "default_override_test.conf" - - assert.NilError(t, err) - assert.Equal(t, defaultFile, expectedPath) -} - -func TestStorageConfOverrideEnvironmentDefaultConfigFileRoot(t *testing.T) { - t.Setenv("CONTAINERS_STORAGE_CONF", "default_override_test.conf") - defaultFile, err := DefaultConfigFile() - - expectedPath := "default_override_test.conf" - - assert.NilError(t, err) - assert.Equal(t, defaultFile, expectedPath) -} From 46f6083544a3d23bc9522c171085c78fc54cc279 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 19 Feb 2026 16:46:14 +0100 Subject: [PATCH 03/10] Revert "Ignore failure to ignore thinpool keys" This reverts commit 9d71cd1fe792cb7a70f31b3919a950b83f247cd8. This was only added to avoid warning with older storage.conf that have that key. This problem is now close to two years old so all storage.conf should have been updated in the meantime and if they are not the current log level is only debug in pkg/configfile and no longer a wanring so it will not spam users either way. Signed-off-by: Paul Holzinger --- storage/pkg/config/config.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/storage/pkg/config/config.go b/storage/pkg/config/config.go index bf350c43cc..534d6d20a1 100644 --- a/storage/pkg/config/config.go +++ b/storage/pkg/config/config.go @@ -96,9 +96,6 @@ type OptionsConfig struct { // Btrfs container options to be handed to btrfs drivers Btrfs struct{ BtrfsOptionsConfig } `toml:"btrfs,omitempty"` - // Thinpool container options to be handed to thinpool drivers (NOP) - Thinpool struct{} `toml:"thinpool,omitempty"` - // Overlay container options to be handed to overlay drivers Overlay struct{ OverlayOptionsConfig } `toml:"overlay,omitempty"` From f0716cedd30ad0fc0ade2e7d2724f9236483f4e1 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 19 Feb 2026 17:02:26 +0100 Subject: [PATCH 04/10] move common/internal/attributedstring into storage/pkg/configfile The motivation is that the new configwork should use that to allow appending values in all files. As such move it into the existing configfile package so it can be reused, this does mean the package is no longer internal but it cannot be as we use it in common which is a different module. Signed-off-by: Paul Holzinger --- common/go.mod | 2 +- common/libnetwork/cni/cni_suite_test.go | 4 +- .../netavark/netavark_suite_test.go | 4 +- common/libnetwork/pasta/pasta_linux_test.go | 4 +- common/pkg/config/config.go | 64 +++++++++---------- common/pkg/config/default.go | 28 ++++---- .../pkg/configfile}/slice.go | 2 +- .../pkg/configfile}/slice_test.go | 2 +- 8 files changed, 55 insertions(+), 55 deletions(-) rename {common/internal/attributedstring => storage/pkg/configfile}/slice.go (99%) rename {common/internal/attributedstring => storage/pkg/configfile}/slice_test.go (99%) diff --git a/common/go.mod b/common/go.mod index db932fbbdc..1de85d7b22 100644 --- a/common/go.mod +++ b/common/go.mod @@ -5,7 +5,6 @@ module go.podman.io/common go 1.24.6 require ( - github.com/BurntSushi/toml v1.6.0 github.com/checkpoint-restore/checkpointctl v1.5.0 github.com/checkpoint-restore/go-criu/v8 v8.2.0 github.com/containerd/platforms v1.0.0-rc.2 @@ -57,6 +56,7 @@ require ( cyphar.com/go-pathrs v0.2.1 // indirect dario.cat/mergo v1.0.2 // indirect github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c // indirect + github.com/BurntSushi/toml v1.6.0 // indirect github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/Microsoft/go-winio v0.6.2 // indirect github.com/VividCortex/ewma v1.2.0 // indirect diff --git a/common/libnetwork/cni/cni_suite_test.go b/common/libnetwork/cni/cni_suite_test.go index d7bca07b9b..1fe7e5394f 100644 --- a/common/libnetwork/cni/cni_suite_test.go +++ b/common/libnetwork/cni/cni_suite_test.go @@ -9,10 +9,10 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "go.podman.io/common/internal/attributedstring" "go.podman.io/common/libnetwork/cni" "go.podman.io/common/libnetwork/types" "go.podman.io/common/pkg/config" + "go.podman.io/storage/pkg/configfile" ) var cniPluginDirs = []string{ @@ -32,7 +32,7 @@ func getNetworkInterface(cniConfDir string) (types.ContainerNetwork, error) { CNIConfigDir: cniConfDir, Config: &config.Config{ Network: config.NetworkConfig{ - CNIPluginDirs: attributedstring.NewSlice(cniPluginDirs), + CNIPluginDirs: configfile.NewSlice(cniPluginDirs), }, }, }) diff --git a/common/libnetwork/netavark/netavark_suite_test.go b/common/libnetwork/netavark/netavark_suite_test.go index 61c9d5f246..8590b32a52 100644 --- a/common/libnetwork/netavark/netavark_suite_test.go +++ b/common/libnetwork/netavark/netavark_suite_test.go @@ -12,11 +12,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" gomegaTypes "github.com/onsi/gomega/types" - "go.podman.io/common/internal/attributedstring" "go.podman.io/common/libnetwork/netavark" "go.podman.io/common/libnetwork/types" "go.podman.io/common/libnetwork/util" "go.podman.io/common/pkg/config" + "go.podman.io/storage/pkg/configfile" "go.podman.io/storage/pkg/unshare" ) @@ -55,7 +55,7 @@ func getNetworkInterfaceWithPlugins(confDir string, pluginDirs []string) (types. NetworkRunDir: confDir, Config: &config.Config{ Network: config.NetworkConfig{ - NetavarkPluginDirs: attributedstring.NewSlice(pluginDirs), + NetavarkPluginDirs: configfile.NewSlice(pluginDirs), }, }, }) diff --git a/common/libnetwork/pasta/pasta_linux_test.go b/common/libnetwork/pasta/pasta_linux_test.go index 7f0e96a68f..96d7c568b9 100644 --- a/common/libnetwork/pasta/pasta_linux_test.go +++ b/common/libnetwork/pasta/pasta_linux_test.go @@ -4,14 +4,14 @@ import ( "testing" "github.com/stretchr/testify/assert" - "go.podman.io/common/internal/attributedstring" "go.podman.io/common/libnetwork/types" "go.podman.io/common/pkg/config" + "go.podman.io/storage/pkg/configfile" ) func makeSetupOptions(configArgs, extraArgs []string, ports []types.PortMapping) *SetupOptions { return &SetupOptions{ - Config: &config.Config{Network: config.NetworkConfig{PastaOptions: attributedstring.NewSlice(configArgs)}}, + Config: &config.Config{Network: config.NetworkConfig{PastaOptions: configfile.NewSlice(configArgs)}}, Netns: "netns123", ExtraOptions: extraArgs, Ports: ports, diff --git a/common/pkg/config/config.go b/common/pkg/config/config.go index b434e011de..4fb28dfb5d 100644 --- a/common/pkg/config/config.go +++ b/common/pkg/config/config.go @@ -13,8 +13,8 @@ import ( units "github.com/docker/go-units" selinux "github.com/opencontainers/selinux/go-selinux" "github.com/sirupsen/logrus" - "go.podman.io/common/internal/attributedstring" "go.podman.io/common/libnetwork/types" + "go.podman.io/storage/pkg/configfile" "go.podman.io/storage/pkg/fileutils" "go.podman.io/storage/pkg/homedir" "go.podman.io/storage/pkg/unshare" @@ -65,17 +65,17 @@ type Config struct { // containers global options for containers tools. type ContainersConfig struct { // Devices to add to all containers - Devices attributedstring.Slice `toml:"devices,omitempty"` + Devices configfile.Slice `toml:"devices,omitempty"` // Volumes to add to all containers - Volumes attributedstring.Slice `toml:"volumes,omitempty"` + Volumes configfile.Slice `toml:"volumes,omitempty"` // ApparmorProfile is the apparmor profile name which is used as the // default for the runtime. ApparmorProfile string `toml:"apparmor_profile,omitempty"` // Annotation to add to all containers - Annotations attributedstring.Slice `toml:"annotations,omitempty"` + Annotations configfile.Slice `toml:"annotations,omitempty"` // BaseHostsFile is the path to a hosts file, the entries from this file // are added to the containers hosts file. As special value "image" is @@ -92,7 +92,7 @@ type ContainersConfig struct { // CgroupConf entries specifies a list of cgroup files to write to and their values. For example // "memory.high=1073741824" sets the memory.high limit to 1GB. - CgroupConf attributedstring.Slice `toml:"cgroup_conf,omitempty"` + CgroupConf configfile.Slice `toml:"cgroup_conf,omitempty"` // When no hostname is set for a container, use the container's name, with // characters not valid for a hostname removed, as the hostname instead of @@ -102,25 +102,25 @@ type ContainersConfig struct { ContainerNameAsHostName bool `toml:"container_name_as_hostname,omitempty"` // Capabilities to add to all containers. - DefaultCapabilities attributedstring.Slice `toml:"default_capabilities,omitempty"` + DefaultCapabilities configfile.Slice `toml:"default_capabilities,omitempty"` // Sysctls to add to all containers. - DefaultSysctls attributedstring.Slice `toml:"default_sysctls,omitempty"` + DefaultSysctls configfile.Slice `toml:"default_sysctls,omitempty"` // DefaultUlimits specifies the default ulimits to apply to containers - DefaultUlimits attributedstring.Slice `toml:"default_ulimits,omitempty"` + DefaultUlimits configfile.Slice `toml:"default_ulimits,omitempty"` // DefaultMountsFile is the path to the default mounts file for testing DefaultMountsFile string `toml:"-"` // DNSServers set default DNS servers. - DNSServers attributedstring.Slice `toml:"dns_servers,omitempty"` + DNSServers configfile.Slice `toml:"dns_servers,omitempty"` // DNSOptions set default DNS options. - DNSOptions attributedstring.Slice `toml:"dns_options,omitempty"` + DNSOptions configfile.Slice `toml:"dns_options,omitempty"` // DNSSearches set default DNS search domains. - DNSSearches attributedstring.Slice `toml:"dns_searches,omitempty"` + DNSSearches configfile.Slice `toml:"dns_searches,omitempty"` // EnableKeyring tells the container engines whether to create // a kernel keyring for use within the container @@ -137,7 +137,7 @@ type ContainersConfig struct { EnableLabeledUsers bool `toml:"label_users,omitempty"` // Env is the environment variable list for container process. - Env attributedstring.Slice `toml:"env,omitempty"` + Env configfile.Slice `toml:"env,omitempty"` // EnvHost Pass all host environment variables into the container. EnvHost bool `toml:"env_host,omitempty"` @@ -185,7 +185,7 @@ type ContainersConfig struct { LogTag string `toml:"log_tag,omitempty"` // Mount to add to all containers - Mounts attributedstring.Slice `toml:"mounts,omitempty"` + Mounts configfile.Slice `toml:"mounts,omitempty"` // NetNS indicates how to create a network namespace for the container NetNS string `toml:"netns,omitempty"` @@ -263,15 +263,15 @@ type EngineConfig struct { // ConmonEnvVars are environment variables to pass to the Conmon binary // when it is launched. - ConmonEnvVars attributedstring.Slice `toml:"conmon_env_vars,omitempty"` + ConmonEnvVars configfile.Slice `toml:"conmon_env_vars,omitempty"` // ConmonPath is the path to the Conmon binary used for managing containers. // The first path pointing to a valid file will be used. - ConmonPath attributedstring.Slice `toml:"conmon_path,omitempty"` + ConmonPath configfile.Slice `toml:"conmon_path,omitempty"` // ConmonRsPath is the path to the Conmon-rs binary used for managing containers. // The first path pointing to a valid file will be used. - ConmonRsPath attributedstring.Slice `toml:"conmonrs_path,omitempty"` + ConmonRsPath configfile.Slice `toml:"conmonrs_path,omitempty"` // CompatAPIEnforceDockerHub enforces using docker.io for completing // short names in Podman's compatibility REST API. Note that this will @@ -283,7 +283,7 @@ type EngineConfig struct { // compose command. The first found provider is used for execution. // Can be an absolute and relative path or a (file) name. Make sure to // expand the return items via `os.ExpandEnv`. - ComposeProviders attributedstring.Slice `toml:"compose_providers,omitempty"` + ComposeProviders configfile.Slice `toml:"compose_providers,omitempty"` // ComposeWarningLogs emits logs on each invocation of the compose // command indicating that an external compose provider is being @@ -306,7 +306,7 @@ type EngineConfig struct { EnablePortReservation bool `toml:"enable_port_reservation,omitempty"` // Environment variables to be used when running the container engine (e.g., Podman, Buildah). For example "http_proxy=internal.proxy.company.com" - Env attributedstring.Slice `toml:"env,omitempty"` + Env configfile.Slice `toml:"env,omitempty"` // EventsLogFilePath is where the events log is stored. EventsLogFilePath string `toml:"events_logfile_path,omitempty"` @@ -335,17 +335,17 @@ type EngineConfig struct { // HelperBinariesDir is a list of directories which are used to search for // helper binaries. - HelperBinariesDir attributedstring.Slice `toml:"helper_binaries_dir,omitempty"` + HelperBinariesDir configfile.Slice `toml:"helper_binaries_dir,omitempty"` // configuration files. When the same filename is present in // multiple directories, the file in the directory listed last in // this slice takes precedence. - HooksDir attributedstring.Slice `toml:"hooks_dir,omitempty"` + HooksDir configfile.Slice `toml:"hooks_dir,omitempty"` // Location of CDI configuration files. These define mounts devices and // other configs according to the CDI spec. In particular this is used // for GPU passthrough. - CdiSpecDirs attributedstring.Slice `toml:"cdi_spec_dirs,omitempty"` + CdiSpecDirs configfile.Slice `toml:"cdi_spec_dirs,omitempty"` // ImageBuildFormat (DEPRECATED) indicates the default image format to // building container images. Should use ImageDefaultFormat @@ -404,7 +404,7 @@ type EngineConfig struct { // NetworkCmdOptions is the default options to pass to the slirp4netns binary. // For example "allow_host_loopback=true" - NetworkCmdOptions attributedstring.Slice `toml:"network_cmd_options,omitempty"` + NetworkCmdOptions configfile.Slice `toml:"network_cmd_options,omitempty"` // NoPivotRoot sets whether to set no-pivot-root in the OCI runtime. NoPivotRoot bool `toml:"no_pivot_root,omitempty"` @@ -455,7 +455,7 @@ type EngineConfig struct { ActiveService string `toml:"active_service,omitempty"` // Add existing instances with requested compression algorithms to manifest list - AddCompression attributedstring.Slice `toml:"add_compression,omitempty"` + AddCompression configfile.Slice `toml:"add_compression,omitempty"` // ServiceDestinations mapped by service Names ServiceDestinations map[string]Destination `toml:"service_destinations,omitempty"` @@ -467,19 +467,19 @@ type EngineConfig struct { // The first path pointing to a valid file will be used This is used only // when there are no OCIRuntime/OCIRuntimes defined. It is used only to be // backward compatible with older versions of Podman. - RuntimePath attributedstring.Slice `toml:"runtime_path,omitempty"` + RuntimePath configfile.Slice `toml:"runtime_path,omitempty"` // RuntimeSupportsJSON is the list of the OCI runtimes that support // --format=json. - RuntimeSupportsJSON attributedstring.Slice `toml:"runtime_supports_json,omitempty"` + RuntimeSupportsJSON configfile.Slice `toml:"runtime_supports_json,omitempty"` // RuntimeSupportsNoCgroups is a list of OCI runtimes that support // running containers without CGroups. - RuntimeSupportsNoCgroups attributedstring.Slice `toml:"runtime_supports_nocgroup,omitempty"` + RuntimeSupportsNoCgroups configfile.Slice `toml:"runtime_supports_nocgroup,omitempty"` // RuntimeSupportsKVM is a list of OCI runtimes that support // KVM separation for containers. - RuntimeSupportsKVM attributedstring.Slice `toml:"runtime_supports_kvm,omitempty"` + RuntimeSupportsKVM configfile.Slice `toml:"runtime_supports_kvm,omitempty"` // SetOptions contains a subset of config options. It's used to indicate if // a given option has either been set by the user or by the parsed @@ -591,10 +591,10 @@ type NetworkConfig struct { NetworkBackend string `toml:"network_backend,omitempty"` // CNIPluginDirs is where CNI plugin binaries are stored. - CNIPluginDirs attributedstring.Slice `toml:"cni_plugin_dirs,omitempty"` + CNIPluginDirs configfile.Slice `toml:"cni_plugin_dirs,omitempty"` // NetavarkPluginDirs is a list of directories which contain netavark plugins. - NetavarkPluginDirs attributedstring.Slice `toml:"netavark_plugin_dirs,omitempty"` + NetavarkPluginDirs configfile.Slice `toml:"netavark_plugin_dirs,omitempty"` // FirewallDriver is the firewall driver to be used FirewallDriver string `toml:"firewall_driver,omitempty"` @@ -630,14 +630,14 @@ type NetworkConfig struct { // PastaOptions contains a default list of pasta(1) options that should // be used when running pasta. - PastaOptions attributedstring.Slice `toml:"pasta_options,omitempty"` + PastaOptions configfile.Slice `toml:"pasta_options,omitempty"` // DefaultHostIPs is the default host IPs to bind published container ports // to when no host IP is explicitly specified in the -p flag (e.g., -p 80:80). // If empty, the default behavior is to bind to all interfaces (0.0.0.0). // If multiple IPs are specified, separate port mapping for each of the specified // IP would be created. - DefaultHostIPs attributedstring.Slice `toml:"default_host_ips,omitempty"` + DefaultHostIPs configfile.Slice `toml:"default_host_ips,omitempty"` } type SubnetPool struct { @@ -688,7 +688,7 @@ type MachineConfig struct { // User to use for rootless podman when init-ing a podman machine VM User string `toml:"user,omitempty"` // Volumes are host directories mounted into the VM by default. - Volumes attributedstring.Slice `toml:"volumes,omitempty"` + Volumes configfile.Slice `toml:"volumes,omitempty"` // Provider is the virtualization provider used to run podman-machine VM Provider string `toml:"provider,omitempty"` // Rosetta is the flag to enable Rosetta in the podman-machine VM on Apple Silicon diff --git a/common/pkg/config/default.go b/common/pkg/config/default.go index ee4dd63791..f34396acd4 100644 --- a/common/pkg/config/default.go +++ b/common/pkg/config/default.go @@ -12,9 +12,9 @@ import ( "github.com/opencontainers/selinux/go-selinux" "github.com/sirupsen/logrus" - "go.podman.io/common/internal/attributedstring" nettypes "go.podman.io/common/libnetwork/types" "go.podman.io/common/pkg/apparmor" + "go.podman.io/storage/pkg/configfile" "go.podman.io/storage/pkg/fileutils" "go.podman.io/storage/pkg/homedir" "go.podman.io/storage/pkg/unshare" @@ -232,20 +232,20 @@ func defaultConfig() (*Config, error) { return &Config{ Containers: ContainersConfig{ - Annotations: attributedstring.Slice{}, + Annotations: configfile.Slice{}, ApparmorProfile: DefaultApparmorProfile, BaseHostsFile: "", CgroupNS: "private", Cgroups: getDefaultCgroupsMode(), - DNSOptions: attributedstring.Slice{}, - DNSSearches: attributedstring.Slice{}, - DNSServers: attributedstring.Slice{}, - DefaultCapabilities: attributedstring.NewSlice(DefaultCapabilities), - DefaultSysctls: attributedstring.Slice{}, - Devices: attributedstring.Slice{}, + DNSOptions: configfile.Slice{}, + DNSSearches: configfile.Slice{}, + DNSServers: configfile.Slice{}, + DefaultCapabilities: configfile.NewSlice(DefaultCapabilities), + DefaultSysctls: configfile.Slice{}, + Devices: configfile.Slice{}, EnableKeyring: true, EnableLabeling: selinuxEnabled(), - Env: attributedstring.NewSlice(defaultContainerEnv), + Env: configfile.NewSlice(defaultContainerEnv), EnvHost: false, HTTPProxy: true, IPCNS: "shareable", @@ -253,7 +253,7 @@ func defaultConfig() (*Config, error) { InitPath: "", LogDriver: defaultLogDriver(), LogSizeMax: DefaultLogSizeMax, - Mounts: attributedstring.Slice{}, + Mounts: configfile.Slice{}, NetNS: "private", NoHosts: false, PidNS: "private", @@ -263,7 +263,7 @@ func defaultConfig() (*Config, error) { UTSNS: "private", Umask: "0022", UserNSSize: DefaultUserNSSize, // Deprecated - Volumes: attributedstring.Slice{}, + Volumes: configfile.Slice{}, }, Network: NetworkConfig{ FirewallDriver: "", @@ -272,8 +272,8 @@ func defaultConfig() (*Config, error) { DefaultSubnetPools: DefaultSubnetPools, DefaultRootlessNetworkCmd: "pasta", DNSBindPort: 0, - CNIPluginDirs: attributedstring.NewSlice(DefaultCNIPluginDirs), - NetavarkPluginDirs: attributedstring.NewSlice(DefaultNetavarkPluginDirs), + CNIPluginDirs: configfile.NewSlice(DefaultCNIPluginDirs), + NetavarkPluginDirs: configfile.NewSlice(DefaultNetavarkPluginDirs), }, Engine: *defaultEngineConfig, Secrets: defaultSecretConfig(), @@ -303,7 +303,7 @@ func defaultMachineConfig() MachineConfig { Image: "docker://quay.io/podman/machine-os", Memory: 2048, User: getDefaultMachineUser(), - Volumes: attributedstring.NewSlice(getDefaultMachineVolumes()), + Volumes: configfile.NewSlice(getDefaultMachineVolumes()), Rosetta: true, } } diff --git a/common/internal/attributedstring/slice.go b/storage/pkg/configfile/slice.go similarity index 99% rename from common/internal/attributedstring/slice.go rename to storage/pkg/configfile/slice.go index 298d468d5b..62ea8fe75a 100644 --- a/common/internal/attributedstring/slice.go +++ b/storage/pkg/configfile/slice.go @@ -1,4 +1,4 @@ -package attributedstring +package configfile import ( "bytes" diff --git a/common/internal/attributedstring/slice_test.go b/storage/pkg/configfile/slice_test.go similarity index 99% rename from common/internal/attributedstring/slice_test.go rename to storage/pkg/configfile/slice_test.go index 734fb79d44..40a1adcc3c 100644 --- a/common/internal/attributedstring/slice_test.go +++ b/storage/pkg/configfile/slice_test.go @@ -1,4 +1,4 @@ -package attributedstring +package configfile import ( "bytes" From 0aea2befca8b98c90a310696e33916c50a1f337f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 19 Feb 2026 18:00:08 +0100 Subject: [PATCH 05/10] storage.conf: use custom slice type to allow appending Use the custom Slice type we have had in cotnainers.conf also for the storage.conf arrays. With drop in files users should have a way to append and not just replace array values. Signed-off-by: Paul Holzinger --- storage/pkg/config/config.go | 6 ++++-- storage/types/options.go | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/storage/pkg/config/config.go b/storage/pkg/config/config.go index 534d6d20a1..6e7766bf0d 100644 --- a/storage/pkg/config/config.go +++ b/storage/pkg/config/config.go @@ -3,6 +3,8 @@ package config import ( "fmt" "os" + + "go.podman.io/storage/pkg/configfile" ) type BtrfsOptionsConfig struct { @@ -53,7 +55,7 @@ type OptionsConfig struct { // AdditionalImagesStores is the location of additional read/only // Image stores. Usually used to access Networked File System // for shared image content - AdditionalImageStores []string `toml:"additionalimagestores,omitempty"` + AdditionalImageStores configfile.Slice `toml:"additionalimagestores,omitempty"` // ImageStore is the location of image store which is separated from the // container store. Usually this is not recommended unless users wants @@ -65,7 +67,7 @@ type OptionsConfig struct { // for shared image content // This API is experimental and can be changed without bumping the // major version number. - AdditionalLayerStores []string `toml:"additionallayerstores,omitempty"` + AdditionalLayerStores configfile.Slice `toml:"additionallayerstores,omitempty"` // Size Size string `toml:"size,omitempty"` diff --git a/storage/types/options.go b/storage/types/options.go index 48aeb8e6e9..305ca92731 100644 --- a/storage/types/options.go +++ b/storage/types/options.go @@ -19,7 +19,7 @@ import ( type TomlConfig struct { Storage struct { Driver string `toml:"driver,omitempty"` - DriverPriority []string `toml:"driver_priority,omitempty"` + DriverPriority configfile.Slice `toml:"driver_priority,omitempty"` RunRoot string `toml:"runroot,omitempty"` ImageStore string `toml:"imagestore,omitempty"` GraphRoot string `toml:"graphroot,omitempty"` @@ -162,7 +162,7 @@ func LoadStoreOptions(opts LoadOptions) (StoreOptions, error) { logrus.Warnf("Switching default driver from overlay2 to the equivalent overlay driver") storeOptions.GraphDriverName = overlayDriver } - storeOptions.GraphDriverPriority = config.Storage.DriverPriority + storeOptions.GraphDriverPriority = config.Storage.DriverPriority.Values if storeOptions.GraphDriverName == "" && len(storeOptions.GraphDriverPriority) == 0 { logrus.Warn("The storage 'driver' option should be set in storage.conf. A driver was picked automatically") } @@ -171,10 +171,10 @@ func LoadStoreOptions(opts LoadOptions) (StoreOptions, error) { storeOptions.ImageStore = config.Storage.ImageStore } - for _, s := range config.Storage.Options.AdditionalImageStores { + for _, s := range config.Storage.Options.AdditionalImageStores.Values { storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.imagestore=%s", config.Storage.Driver, s)) } - for _, s := range config.Storage.Options.AdditionalLayerStores { + for _, s := range config.Storage.Options.AdditionalLayerStores.Values { storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.additionallayerstore=%s", config.Storage.Driver, s)) } if config.Storage.Options.Size != "" { From 81cf9d106cbcbe83c8743253b89ed96e562873c6 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 4 Mar 2026 16:44:09 +0100 Subject: [PATCH 06/10] storage: rewrite option parsing Overhaul option parsing to not require driver prefix option names. When creating a driver it makes little sense to require all callers to send in option with the driver name prefixed. As such chnage the driver parsing so it accepts just the option name or the existing name. syntax. With that we can have a better option parsing out of the storage.conf file because we no longer need the full driver name already while parsing the file. Even when in practise the actual driver name mind not be known until way later when we init the graphdriver. Now we still need the driver.optname syntax so that the config file can pass down options for different drivers without knowing the actual driver that will be used. So the option parsing in the drivers now should ignore options meant for a different driver to not cause hard errors when a config file sets a vfs and overlay option at the same time. Also I know the commit is a bit large but I was not able to split this in a useful way to would actualyl have working induvidual commits and tests. Signed-off-by: Paul Holzinger --- storage/drivers/btrfs/btrfs.go | 10 +- storage/drivers/driver.go | 12 ++ storage/drivers/overlay/overlay.go | 5 +- storage/drivers/overlay/overlay_test.go | 184 ++++++++++++++++++++++++ storage/drivers/vfs/driver.go | 13 +- storage/drivers/zfs/zfs.go | 11 +- storage/pkg/config/config.go | 118 ++++++--------- storage/pkg/config/config_test.go | 125 +++------------- storage/types/options.go | 19 +-- 9 files changed, 300 insertions(+), 197 deletions(-) diff --git a/storage/drivers/btrfs/btrfs.go b/storage/drivers/btrfs/btrfs.go index aba898ed55..2aec0cb7d0 100644 --- a/storage/drivers/btrfs/btrfs.go +++ b/storage/drivers/btrfs/btrfs.go @@ -102,18 +102,22 @@ func parseOptions(opt []string) (btrfsOptions, bool, error) { return options, userDiskQuota, err } key = strings.ToLower(key) + key = strings.TrimPrefix(key, "btrfs.") switch key { - case "btrfs.min_space": + case "min_space": minSpace, err := units.RAMInBytes(val) if err != nil { return options, userDiskQuota, err } userDiskQuota = true options.minSpace = uint64(minSpace) - case "btrfs.mountopt": + case "mountopt": return options, userDiskQuota, fmt.Errorf("btrfs driver does not support mount options") default: - return options, userDiskQuota, fmt.Errorf("unknown option %s (%q)", key, option) + // do not error for options meant for another storage driver + if !graphdriver.IsDriverPrefixedOption(key) { + return options, userDiskQuota, fmt.Errorf("unknown option %s (%q)", key, option) + } } } return options, userDiskQuota, nil diff --git a/storage/drivers/driver.go b/storage/drivers/driver.go index 38706dc999..f54a26a7d3 100644 --- a/storage/drivers/driver.go +++ b/storage/drivers/driver.go @@ -547,3 +547,15 @@ func driverPut(driver ProtoDriver, id string, mainErr *error) { } } } + +// IsDriverPrefixedOption checks for options with the syntax of .. +// It only validates the driver name and not optname. +func IsDriverPrefixedOption(opt string) bool { + name, _, ok := strings.Cut(opt, ".") + if ok { + if _, found := drivers[name]; found { + return true + } + } + return false +} diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index 00974c890c..2aa00aaeff 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -594,7 +594,10 @@ func parseOptions(options []string) (*overlayOptions, error) { m := os.FileMode(mask) o.forceMask = &m default: - return nil, fmt.Errorf("overlay: unknown option %s", key) + // do not error for options meant for another storage driver + if !graphdriver.IsDriverPrefixedOption(trimkey) { + return nil, fmt.Errorf("overlay: unknown option %s", key) + } } } return o, nil diff --git a/storage/drivers/overlay/overlay_test.go b/storage/drivers/overlay/overlay_test.go index d1e96e7f14..2c165827c2 100644 --- a/storage/drivers/overlay/overlay_test.go +++ b/storage/drivers/overlay/overlay_test.go @@ -4,15 +4,19 @@ package overlay import ( "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" graphdriver "go.podman.io/storage/drivers" "go.podman.io/storage/drivers/graphtest" + "go.podman.io/storage/drivers/quota" "go.podman.io/storage/pkg/archive" "go.podman.io/storage/pkg/idtools" "go.podman.io/storage/pkg/reexec" + + _ "go.podman.io/storage/drivers/vfs" // needed by parseOptions ) const driverName = "overlay" @@ -180,3 +184,183 @@ func BenchmarkDiff20Layers(b *testing.B) { func BenchmarkRead20Layers(b *testing.B) { graphtest.DriverBenchDeepLayerRead(b, 20, driverName) } + +func Test_parseOptions(t *testing.T) { + var ( + sharedMask = os.FileMode(0o755) + privateMask = os.FileMode(0o700) + customMask = os.FileMode(0o644) + ) + + tmpDir := t.TempDir() + tmpFile := filepath.Join(tmpDir, "test-file") + + f, err := os.Create(tmpFile) + require.NoError(t, err) + require.NoError(t, f.Close()) + + tests := []struct { + name string // description of this test case + // Named input parameters for target function. + options []string + want *overlayOptions + wantErr string + }{ + { + name: "no opts", + options: []string{}, + want: &overlayOptions{}, + }, + { + name: "mountopt", + options: []string{"mountopt=abc"}, + want: &overlayOptions{mountOptions: "abc"}, + }, + { + name: "overlay prefix handling", + options: []string{"overlay.mountopt=def"}, + want: &overlayOptions{mountOptions: "def"}, + }, + { + name: "overlay2 prefix handling", + options: []string{"overlay2.mountopt=ghi"}, + want: &overlayOptions{mountOptions: "ghi"}, + }, + { + name: "size", + options: []string{"size=50kb"}, + want: &overlayOptions{quota: quota.Quota{Size: 51200}}, + }, + { + name: "size - invalid", + options: []string{"size=abc"}, + wantErr: "invalid size", + }, + { + name: "inodes", + options: []string{"inodes=1024"}, + want: &overlayOptions{quota: quota.Quota{Inodes: 1024}}, + }, + { + name: "inodes - invalid", + options: []string{"inodes=abc"}, + wantErr: "invalid syntax", + }, + { + name: "override_kernel_check", + options: []string{"override_kernel_check=true"}, + want: &overlayOptions{}, // Has no effect other than log output + }, + { + name: "imagestore - valid directory", + options: []string{"imagestore=" + tmpDir}, + want: &overlayOptions{imageStores: []string{tmpDir}}, + }, + { + name: "imagestore - invalid (relative path)", + options: []string{"imagestore=./relative/path"}, + wantErr: "is not absolute", + }, + { + name: "imagestore - invalid (file instead of dir)", + options: []string{"imagestore=" + tmpFile}, + wantErr: "must be a directory", + }, + { + name: "additionallayerstore - valid directory", + options: []string{"additionallayerstore=" + tmpDir}, + want: &overlayOptions{layerStores: []additionalLayerStore{{path: tmpDir, withReference: false}}}, + }, + { + name: "additionallayerstore - with ref", + options: []string{"additionallayerstore=" + tmpDir + ":ref"}, + want: &overlayOptions{layerStores: []additionalLayerStore{{path: tmpDir, withReference: true}}}, + }, + { + name: "additionallayerstore - ref used twice", + options: []string{"additionallayerstore=" + tmpDir + ":ref:ref"}, + wantErr: "contains \"ref\" option twice", + }, + { + name: "additionallayerstore - unknown option", + options: []string{"additionallayerstore=" + tmpDir + ":unknown"}, + wantErr: "contains unknown option \"unknown\"", + }, + { + name: "mount_program - valid file", + options: []string{"mount_program=" + tmpFile}, + want: &overlayOptions{mountProgram: tmpFile}, + }, + { + name: "mount_program - missing file", + options: []string{"mount_program=/does/not/exist"}, + wantErr: "can't stat program", + }, + { + name: "use_composefs", + options: []string{"use_composefs=true"}, + want: &overlayOptions{useComposefs: true}, + }, + { + name: "use_composefs - invalid", + options: []string{"use_composefs=notabool"}, + wantErr: "invalid syntax", + }, + { + name: "skip_mount_home", + options: []string{"skip_mount_home=true"}, + want: &overlayOptions{skipMountHome: true}, + }, + { + name: "ignore_chown_errors", + options: []string{"ignore_chown_errors=true"}, + want: &overlayOptions{ignoreChownErrors: true}, + }, + { + name: "force_mask - shared", + options: []string{"force_mask=shared"}, + want: &overlayOptions{forceMask: &sharedMask}, + }, + { + name: "force_mask - private", + options: []string{"force_mask=private"}, + want: &overlayOptions{forceMask: &privateMask}, + }, + { + name: "force_mask - custom octal", + options: []string{"force_mask=644"}, + want: &overlayOptions{forceMask: &customMask}, + }, + { + name: "force_mask - invalid syntax", + options: []string{"force_mask=hello"}, + wantErr: "invalid syntax", + }, + { + name: "unknown option", + options: []string{"unknown=value"}, + wantErr: "unknown option unknown", + }, + { + name: "unknown overlay prefix option", + options: []string{"overlay.123=value"}, + wantErr: "unknown option overlay.123", + }, + { + name: "other driver option should not error", + options: []string{"vfs.unknown=value"}, + want: &overlayOptions{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, gotErr := parseOptions(tt.options) + if tt.wantErr != "" { + require.ErrorContains(t, gotErr, tt.wantErr) + return + } + require.NoError(t, gotErr) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/storage/drivers/vfs/driver.go b/storage/drivers/vfs/driver.go index b90c2046cf..50515712e6 100644 --- a/storage/drivers/vfs/driver.go +++ b/storage/drivers/vfs/driver.go @@ -51,13 +51,14 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) return nil, err } key = strings.ToLower(key) + key = strings.TrimPrefix(key, "vfs.") switch key { - case "vfs.imagestore", ".imagestore": + case "imagestore": d.additionalHomes = slices.AppendSeq(d.additionalHomes, strings.SplitSeq(val, ",")) continue - case "vfs.mountopt": + case "mountopt": return nil, fmt.Errorf("vfs driver does not support mount options") - case ".ignore_chown_errors", "vfs.ignore_chown_errors": + case "ignore_chown_errors": logrus.Debugf("vfs: ignore_chown_errors=%s", val) var err error d.ignoreChownErrors, err = strconv.ParseBool(val) @@ -65,7 +66,11 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) return nil, err } default: - return nil, fmt.Errorf("vfs driver does not support %s options", key) + // do not error for options meant for another storage driver + if !graphdriver.IsDriverPrefixedOption(key) { + return nil, fmt.Errorf("vfs driver does not support %s options", key) + } + } } diff --git a/storage/drivers/zfs/zfs.go b/storage/drivers/zfs/zfs.go index b994278bb2..925c9f810b 100644 --- a/storage/drivers/zfs/zfs.go +++ b/storage/drivers/zfs/zfs.go @@ -128,13 +128,18 @@ func parseOptions(opt []string) (zfsOptions, error) { return options, err } key = strings.ToLower(key) + key = strings.TrimPrefix(key, "zfs.") switch key { - case "zfs.fsname": + case "fsname": options.fsName = val - case "zfs.mountopt": + case "mountopt": options.mountOptions = val default: - return options, fmt.Errorf("unknown option %s", key) + // do not error for options meant for another storage driver + if !graphdriver.IsDriverPrefixedOption(key) { + return options, fmt.Errorf("unknown option %s", key) + } + } } return options, nil diff --git a/storage/pkg/config/config.go b/storage/pkg/config/config.go index 6e7766bf0d..3480771ed3 100644 --- a/storage/pkg/config/config.go +++ b/storage/pkg/config/config.go @@ -125,78 +125,54 @@ type OptionsConfig struct { } // GetGraphDriverOptions returns the driver specific options -func GetGraphDriverOptions(driverName string, options OptionsConfig) []string { +func GetGraphDriverOptions(options OptionsConfig) []string { var doptions []string - switch driverName { - case "btrfs": - if options.Btrfs.MinSpace != "" { - return append(doptions, fmt.Sprintf("%s.min_space=%s", driverName, options.Btrfs.MinSpace)) - } - if options.Btrfs.Size != "" { - doptions = append(doptions, fmt.Sprintf("%s.size=%s", driverName, options.Btrfs.Size)) - } else if options.Size != "" { - doptions = append(doptions, fmt.Sprintf("%s.size=%s", driverName, options.Size)) - } - - case "overlay", "overlay2": - // Specify whether composefs must be used to mount the data layers - if options.Overlay.IgnoreChownErrors != "" { - doptions = append(doptions, fmt.Sprintf("%s.ignore_chown_errors=%s", driverName, options.Overlay.IgnoreChownErrors)) - } else if options.IgnoreChownErrors != "" { - doptions = append(doptions, fmt.Sprintf("%s.ignore_chown_errors=%s", driverName, options.IgnoreChownErrors)) - } - if options.Overlay.MountProgram != "" { - doptions = append(doptions, fmt.Sprintf("%s.mount_program=%s", driverName, options.Overlay.MountProgram)) - } else if options.MountProgram != "" { - doptions = append(doptions, fmt.Sprintf("%s.mount_program=%s", driverName, options.MountProgram)) - } - if options.Overlay.MountOpt != "" { - doptions = append(doptions, fmt.Sprintf("%s.mountopt=%s", driverName, options.Overlay.MountOpt)) - } else if options.MountOpt != "" { - doptions = append(doptions, fmt.Sprintf("%s.mountopt=%s", driverName, options.MountOpt)) - } - if options.Overlay.Size != "" { - doptions = append(doptions, fmt.Sprintf("%s.size=%s", driverName, options.Overlay.Size)) - } else if options.Size != "" { - doptions = append(doptions, fmt.Sprintf("%s.size=%s", driverName, options.Size)) - } - if options.Overlay.Inodes != "" { - doptions = append(doptions, fmt.Sprintf("%s.inodes=%s", driverName, options.Overlay.Inodes)) - } - if options.Overlay.SkipMountHome != "" { - doptions = append(doptions, fmt.Sprintf("%s.skip_mount_home=%s", driverName, options.Overlay.SkipMountHome)) - } else if options.SkipMountHome != "" { - doptions = append(doptions, fmt.Sprintf("%s.skip_mount_home=%s", driverName, options.SkipMountHome)) - } - if options.Overlay.ForceMask != "" { - doptions = append(doptions, fmt.Sprintf("%s.force_mask=%s", driverName, options.Overlay.ForceMask)) - } else if options.ForceMask != 0 { - doptions = append(doptions, fmt.Sprintf("%s.force_mask=%s", driverName, options.ForceMask)) - } - if options.Overlay.UseComposefs != "" { - doptions = append(doptions, fmt.Sprintf("%s.use_composefs=%s", driverName, options.Overlay.UseComposefs)) - } - case "vfs": - if options.Vfs.IgnoreChownErrors != "" { - doptions = append(doptions, fmt.Sprintf("%s.ignore_chown_errors=%s", driverName, options.Vfs.IgnoreChownErrors)) - } else if options.IgnoreChownErrors != "" { - doptions = append(doptions, fmt.Sprintf("%s.ignore_chown_errors=%s", driverName, options.IgnoreChownErrors)) - } - - case "zfs": - if options.Zfs.Name != "" { - doptions = append(doptions, fmt.Sprintf("%s.fsname=%s", driverName, options.Zfs.Name)) - } - if options.Zfs.MountOpt != "" { - doptions = append(doptions, fmt.Sprintf("%s.mountopt=%s", driverName, options.Zfs.MountOpt)) - } else if options.MountOpt != "" { - doptions = append(doptions, fmt.Sprintf("%s.mountopt=%s", driverName, options.MountOpt)) - } - if options.Zfs.Size != "" { - doptions = append(doptions, fmt.Sprintf("%s.size=%s", driverName, options.Zfs.Size)) - } else if options.Size != "" { - doptions = append(doptions, fmt.Sprintf("%s.size=%s", driverName, options.Size)) - } + if options.Btrfs.MinSpace != "" { + return append(doptions, fmt.Sprintf("btrfs.min_space=%s", options.Btrfs.MinSpace)) } + if options.Btrfs.Size != "" { + doptions = append(doptions, fmt.Sprintf("btrfs.size=%s", options.Btrfs.Size)) + } + + // Specify whether composefs must be used to mount the data layers + if options.Overlay.IgnoreChownErrors != "" { + doptions = append(doptions, fmt.Sprintf("overlay.ignore_chown_errors=%s", options.Overlay.IgnoreChownErrors)) + } + if options.Overlay.MountProgram != "" { + doptions = append(doptions, fmt.Sprintf("overlay.mount_program=%s", options.Overlay.MountProgram)) + } + if options.Overlay.MountOpt != "" { + doptions = append(doptions, fmt.Sprintf("overlay.mountopt=%s", options.Overlay.MountOpt)) + } + if options.Overlay.Size != "" { + doptions = append(doptions, fmt.Sprintf("overlay.size=%s", options.Overlay.Size)) + } + if options.Overlay.Inodes != "" { + doptions = append(doptions, fmt.Sprintf("overlay.inodes=%s", options.Overlay.Inodes)) + } + if options.Overlay.SkipMountHome != "" { + doptions = append(doptions, fmt.Sprintf("overlay.skip_mount_home=%s", options.Overlay.SkipMountHome)) + } + if options.Overlay.ForceMask != "" { + doptions = append(doptions, fmt.Sprintf("overlay.force_mask=%s", options.Overlay.ForceMask)) + } + if options.Overlay.UseComposefs != "" { + doptions = append(doptions, fmt.Sprintf("overlay.use_composefs=%s", options.Overlay.UseComposefs)) + } + + if options.Vfs.IgnoreChownErrors != "" { + doptions = append(doptions, fmt.Sprintf("vfs.ignore_chown_errors=%s", options.Vfs.IgnoreChownErrors)) + } + + if options.Zfs.Name != "" { + doptions = append(doptions, fmt.Sprintf("zfs.fsname=%s", options.Zfs.Name)) + } + if options.Zfs.MountOpt != "" { + doptions = append(doptions, fmt.Sprintf("zfs.mountopt=%s", options.Zfs.MountOpt)) + } + if options.Zfs.Size != "" { + doptions = append(doptions, fmt.Sprintf("zfs.size=%s", options.Zfs.Size)) + } + return doptions } diff --git a/storage/pkg/config/config_test.go b/storage/pkg/config/config_test.go index 477abb53ff..965ff53249 100644 --- a/storage/pkg/config/config_test.go +++ b/storage/pkg/config/config_test.go @@ -27,14 +27,14 @@ func TestBtrfsOptions(t *testing.T) { doptions []string options OptionsConfig ) - doptions = GetGraphDriverOptions("btrfs", options) + doptions = GetGraphDriverOptions(options) if len(doptions) != 0 { t.Fatalf("Expected 0 options, got %v", doptions) } // Make sure legacy mountopt still works options = OptionsConfig{} options.Btrfs.MinSpace = s100 - doptions = GetGraphDriverOptions("btrfs", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 0 options, got %v", doptions) } @@ -43,17 +43,9 @@ func TestBtrfsOptions(t *testing.T) { } options = OptionsConfig{} - options.Size = s200 - doptions = GetGraphDriverOptions("btrfs", options) - if len(doptions) == 0 { - t.Fatalf("Expected 0 options, got %v", doptions) - } - if !searchOptions(doptions, s200) { - t.Fatalf("Expected to find size %q options, got %v", s200, doptions) - } // Make sure Btrfs.Size takes precedence options.Btrfs.Size = s100 - doptions = GetGraphDriverOptions("btrfs", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 0 options, got %v", doptions) } @@ -67,54 +59,24 @@ func TestOverlayOptions(t *testing.T) { doptions []string options OptionsConfig ) - doptions = GetGraphDriverOptions("overlay", options) - if len(doptions) != 0 { - t.Fatalf("Expected 0 options, got %v", doptions) - } - options.Vfs.IgnoreChownErrors = trueString - doptions = GetGraphDriverOptions("overlay", options) + doptions = GetGraphDriverOptions(options) if len(doptions) != 0 { t.Fatalf("Expected 0 options, got %v", doptions) } options.Overlay.IgnoreChownErrors = trueString - doptions = GetGraphDriverOptions("overlay", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 1 options, got %v", doptions) } options.Overlay.IgnoreChownErrors = "false" - doptions = GetGraphDriverOptions("overlay", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 0 options, got %v", doptions) } - // Make sure legacy IgnoreChownErrors still works - options = OptionsConfig{} - options.IgnoreChownErrors = trueString - doptions = GetGraphDriverOptions("overlay", options) - if len(doptions) == 0 { - t.Fatalf("Expected 1 options, got %v", doptions) - } - // Make sure legacy mountopt still works - options = OptionsConfig{} - options.MountOpt = foobar - doptions = GetGraphDriverOptions("overlay", options) - if len(doptions) == 0 { - t.Fatalf("Expected 0 options, got %v", doptions) - } - if !searchOptions(doptions, "mountopt=foobar") { - t.Fatalf("Expected to find 'foobar' options, got %v", doptions) - } - - // Make sure Overlay ignores other drivers mountpoints takes presedence - options.Zfs.MountOpt = nodev - doptions = GetGraphDriverOptions("overlay", options) - if searchOptions(doptions, "mountopt=nodev") { - t.Fatalf("Expected to find 'nodev' options, got %v", doptions) - } - // Make sure OverlayMountOpt takes precedence options.Overlay.MountOpt = nodev - doptions = GetGraphDriverOptions("overlay", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 0 options, got %v", doptions) } @@ -122,17 +84,8 @@ func TestOverlayOptions(t *testing.T) { t.Fatalf("Expected to find 'nodev' options, got %v", doptions) } - // Make sure mount_program takes precedence - options.MountProgram = "/usr/bin/root_overlay" - doptions = GetGraphDriverOptions("overlay", options) - if len(doptions) == 0 { - t.Fatalf("Expected 0 options, got %v", doptions) - } - if !searchOptions(doptions, "mount_program=/usr/bin/root_overlay") { - t.Fatalf("Expected to find 'root_overlay' options, got %v", doptions) - } options.Overlay.MountProgram = "/usr/bin/fuse_overlay" - doptions = GetGraphDriverOptions("overlay", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 0 options, got %v", doptions) } @@ -140,7 +93,7 @@ func TestOverlayOptions(t *testing.T) { t.Fatalf("Expected to find 'fuse_overlay' options, got %v", doptions) } options.Overlay.SkipMountHome = "true" - doptions = GetGraphDriverOptions("overlay", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 0 options, got %v", doptions) } @@ -149,7 +102,7 @@ func TestOverlayOptions(t *testing.T) { } options.Overlay.UseComposefs = "true" - doptions = GetGraphDriverOptions("overlay", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected > 0 options, got %v", doptions) } @@ -157,28 +110,9 @@ func TestOverlayOptions(t *testing.T) { t.Fatalf("Expected to find 'use_composefs' options, got %v", doptions) } - // Make sure legacy mountopt still works - options = OptionsConfig{} - options.SkipMountHome = "true" - doptions = GetGraphDriverOptions("overlay", options) - if len(doptions) == 0 { - t.Fatalf("Expected 0 options, got %v", doptions) - } - if !searchOptions(doptions, "skip_mount_home") { - t.Fatalf("Expected to find 'skip_mount_home' options, got %v", doptions) - } - - options.Size = s200 - doptions = GetGraphDriverOptions("overlay", options) - if len(doptions) == 0 { - t.Fatalf("Expected 0 options, got %v", doptions) - } - if !searchOptions(doptions, s200) { - t.Fatalf("Expected to find size %q options, got %v", s200, doptions) - } // Make sure Overlay.Size takes precedence options.Overlay.Size = s100 - doptions = GetGraphDriverOptions("overlay", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 0 options, got %v", doptions) } @@ -192,24 +126,17 @@ func TestVfsOptions(t *testing.T) { doptions []string options OptionsConfig ) - doptions = GetGraphDriverOptions("vfs", options) + doptions = GetGraphDriverOptions(options) if len(doptions) != 0 { t.Fatalf("Expected 0 options, got %v", doptions) } options.Overlay.IgnoreChownErrors = trueString - doptions = GetGraphDriverOptions("vfs", options) - if len(doptions) != 0 { - t.Fatalf("Expected 0 options, got %v", doptions) - } - options.Vfs.IgnoreChownErrors = trueString - doptions = GetGraphDriverOptions("vfs", options) - if len(doptions) == 0 { + doptions = GetGraphDriverOptions(options) + if len(doptions) != 1 { t.Fatalf("Expected 1 options, got %v", doptions) } - // Make sure legacy IgnoreChownErrors still works - options = OptionsConfig{} - options.IgnoreChownErrors = trueString - doptions = GetGraphDriverOptions("vfs", options) + options.Vfs.IgnoreChownErrors = trueString + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 1 options, got %v", doptions) } @@ -220,38 +147,24 @@ func TestZfsOptions(t *testing.T) { doptions []string options OptionsConfig ) - doptions = GetGraphDriverOptions("zfs", options) + doptions = GetGraphDriverOptions(options) if len(doptions) != 0 { t.Fatalf("Expected 0 options, got %v", doptions) } // Make sure legacy mountopt still works options = OptionsConfig{} options.Zfs.Name = foobar - doptions = GetGraphDriverOptions("zfs", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 0 options, got %v", doptions) } if !searchOptions(doptions, options.Zfs.Name) { t.Fatalf("Expected to find 'foobar' options, got %v", doptions) } - // Make sure Zfs ignores other drivers mountpoints takes presedence - options.Overlay.MountOpt = nodev - doptions = GetGraphDriverOptions("zfs", options) - if searchOptions(doptions, "mountopt=nodev") { - t.Fatalf("Expected Not to find 'nodev' options, got %v", doptions) - } - options.Size = s200 - doptions = GetGraphDriverOptions("zfs", options) - if len(doptions) == 0 { - t.Fatalf("Expected 0 options, got %v", doptions) - } - if !searchOptions(doptions, s200) { - t.Fatalf("Expected to find size %q options, got %v", s200, doptions) - } // Make sure Zfs.Size takes precedence options.Zfs.Size = s100 - doptions = GetGraphDriverOptions("zfs", options) + doptions = GetGraphDriverOptions(options) if len(doptions) == 0 { t.Fatalf("Expected 0 options, got %v", doptions) } diff --git a/storage/types/options.go b/storage/types/options.go index 305ca92731..b43fb014b8 100644 --- a/storage/types/options.go +++ b/storage/types/options.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" "sync" @@ -172,28 +173,28 @@ func LoadStoreOptions(opts LoadOptions) (StoreOptions, error) { } for _, s := range config.Storage.Options.AdditionalImageStores.Values { - storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.imagestore=%s", config.Storage.Driver, s)) + storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, "imagestore="+s) } for _, s := range config.Storage.Options.AdditionalLayerStores.Values { - storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.additionallayerstore=%s", config.Storage.Driver, s)) + storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, "additionallayerstore="+s) } if config.Storage.Options.Size != "" { - storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.size=%s", config.Storage.Driver, config.Storage.Options.Size)) + storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, "size="+config.Storage.Options.Size) } if config.Storage.Options.MountProgram != "" { - storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.mount_program=%s", config.Storage.Driver, config.Storage.Options.MountProgram)) + storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, " mount_program="+config.Storage.Options.MountProgram) } if config.Storage.Options.SkipMountHome != "" { - storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.skip_mount_home=%s", config.Storage.Driver, config.Storage.Options.SkipMountHome)) + storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, "skip_mount_home="+config.Storage.Options.SkipMountHome) } if config.Storage.Options.IgnoreChownErrors != "" { - storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.ignore_chown_errors=%s", config.Storage.Driver, config.Storage.Options.IgnoreChownErrors)) + storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, "ignore_chown_errors="+config.Storage.Options.IgnoreChownErrors) } if config.Storage.Options.ForceMask != 0 { - storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.force_mask=%o", config.Storage.Driver, config.Storage.Options.ForceMask)) + storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, "force_mask="+strconv.FormatUint(uint64(config.Storage.Options.ForceMask), 8)) } if config.Storage.Options.MountOpt != "" { - storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.mountopt=%s", config.Storage.Driver, config.Storage.Options.MountOpt)) + storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, "mountopt="+config.Storage.Options.MountOpt) } storeOptions.RootAutoNsUser = config.Storage.Options.RootAutoUsernsUser if config.Storage.Options.AutoUsernsMinSize > 0 { @@ -209,7 +210,7 @@ func LoadStoreOptions(opts LoadOptions) (StoreOptions, error) { storeOptions.DisableVolatile = config.Storage.Options.DisableVolatile storeOptions.TransientStore = config.Storage.TransientStore - storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, cfg.GetGraphDriverOptions(storeOptions.GraphDriverName, config.Storage.Options)...) + storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, cfg.GetGraphDriverOptions(config.Storage.Options)...) if opts, ok := os.LookupEnv("STORAGE_OPTS"); ok { storeOptions.GraphDriverOptions = strings.Split(opts, ",") From cf9476527e2a58bb99de42533753242e00e6829e Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 4 Mar 2026 17:54:55 +0100 Subject: [PATCH 07/10] storage: no longer warn when the config file has no driver set The option logic was chnaged in the prior commit to handle the case without driver name properly. Based on the comment in [1] the reason we had this warning was only because of this special option handling. [1] https://github.com/containers/storage/pull/486 Signed-off-by: Paul Holzinger --- storage/types/options.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/storage/types/options.go b/storage/types/options.go index b43fb014b8..f76a0c7590 100644 --- a/storage/types/options.go +++ b/storage/types/options.go @@ -164,9 +164,6 @@ func LoadStoreOptions(opts LoadOptions) (StoreOptions, error) { storeOptions.GraphDriverName = overlayDriver } storeOptions.GraphDriverPriority = config.Storage.DriverPriority.Values - if storeOptions.GraphDriverName == "" && len(storeOptions.GraphDriverPriority) == 0 { - logrus.Warn("The storage 'driver' option should be set in storage.conf. A driver was picked automatically") - } if config.Storage.ImageStore != "" { storeOptions.ImageStore = config.Storage.ImageStore From ba3e5fca9e788a09d4687d1c6bf7c1512d52fb4f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 5 Mar 2026 17:45:04 +0100 Subject: [PATCH 08/10] storage: fix test to pass proper driver option Using the dot without a driver name is not valid, while the old parsing code accepted that it was due the incorrect passing of options. Any use of that syntax seems more of a mistake to me so lets not try to support that any longer. Signed-off-by: Paul Holzinger --- storage/tests/idmaps.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/tests/idmaps.bats b/storage/tests/idmaps.bats index a340360f72..254fd1df9f 100644 --- a/storage/tests/idmaps.bats +++ b/storage/tests/idmaps.bats @@ -1040,7 +1040,7 @@ load helpers echo "$output" [ "$status" -eq 0 ] - run storage --graph=$TESTDIR/newstore --storage-opt=.imagestore=$TESTDIR/imagestore --debug=false create-image $layer + run storage --graph=$TESTDIR/newstore --storage-opt=imagestore=$TESTDIR/imagestore --debug=false create-image $layer echo "$output" [ "$status" -eq 0 ] image="$output" @@ -1050,7 +1050,7 @@ load helpers gidrange[$i]=$((($RANDOM+32767)*65536)) done - run storage --graph=$TESTDIR/newstore --storage-opt=.imagestore=$TESTDIR/imagestore --debug=false create-container --uidmap 0:${uidrange[0]}:$(($n+1)) --gidmap 0:${gidrange[0]}:$(($n+1)) $image + run storage --graph=$TESTDIR/newstore --storage-opt=imagestore=$TESTDIR/imagestore --debug=false create-container --uidmap 0:${uidrange[0]}:$(($n+1)) --gidmap 0:${gidrange[0]}:$(($n+1)) $image echo "$output" [ "$status" -eq 0 ] [ "$output" != "" ] From 827c6f3323b00ab75bb4ec4ecc0f691c0f089a61 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 5 Mar 2026 17:59:31 +0100 Subject: [PATCH 09/10] storage: comment out some defaults from storage.conf With the new config parsing graphroot/runroot in the main file will be used by all process root and rootless so it makes no sense to set them in the default config file as rootless would try to use them. The library already picks the right defaults so there is no need to set them. If one would want to set them for only root they should go into the new storage.rootful.conf.d/ directory. Note I kept mountopt for now as this is wanted per commit cf28ef61f867 ("Add default mount options to pass to drivers") and I don't think the library uses that default internally. I think we should change that eventually but for now I like to not make this even bigger than it already is. Signed-off-by: Paul Holzinger --- storage/storage.conf | 11 +++++------ storage/storage.conf-freebsd | 11 +++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/storage/storage.conf b/storage/storage.conf index 2fff0cecf2..421b999adb 100644 --- a/storage/storage.conf +++ b/storage/storage.conf @@ -13,11 +13,11 @@ # The "storage" table contains all of the server options. [storage] -# Default storage driver, must be set for proper operation. -driver = "overlay" +# Default storage driver. Optional. +# driver = "overlay" # Temporary storage location -runroot = "/run/containers/storage" +# runroot = "/run/containers/storage" # Priority list for the storage drivers that will be tested one # after the other to pick the storage driver if it is not defined. @@ -29,7 +29,7 @@ runroot = "/run/containers/storage" # following commands: # semanage fcontext -a -e /var/lib/containers/storage /NEWSTORAGEPATH # restorecon -R -v /NEWSTORAGEPATH -graphroot = "/var/lib/containers/storage" +# graphroot = "/var/lib/containers/storage" # Optional alternate location of image store if a location separate from the # container store is required. If set, it must be different than graphroot. @@ -51,8 +51,7 @@ graphroot = "/var/lib/containers/storage" # AdditionalImageStores is used to pass paths to additional Read/Only image stores # Must be comma separated list. -additionalimagestores = [ -] +# additionalimagestores = [] # Options controlling how storage is populated when pulling images. [storage.options.pull_options] diff --git a/storage/storage.conf-freebsd b/storage/storage.conf-freebsd index 4181aa6555..3b42dcff8b 100644 --- a/storage/storage.conf-freebsd +++ b/storage/storage.conf-freebsd @@ -13,14 +13,14 @@ # The "container storage" table contains all of the server options. [storage] -# Default Storage Driver, Must be set for proper operation. -driver = "zfs" +# Default storage driver. Optional. +# driver = "zfs" # Temporary storage location -runroot = "/var/run/containers/storage" +# runroot = "/var/run/containers/storage" # Primary Read/Write location of container storage -graphroot = "/var/db/containers/storage" +# graphroot = "/var/db/containers/storage" # Optional value for image storage location # If set, it must be different than graphroot. @@ -36,8 +36,7 @@ graphroot = "/var/db/containers/storage" # AdditionalImageStores is used to pass paths to additional Read/Only image stores # Must be comma separated list. -additionalimagestores = [ -] +# additionalimagestores = [] # Root-auto-userns-user is a user name which can be used to look up one or more UID/GID # ranges in the /etc/subuid and /etc/subgid file. These ranges will be partitioned From 6d5d416cf642b7a183ff3ae7390a1bc81eee3b50 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 5 Mar 2026 18:36:59 +0100 Subject: [PATCH 10/10] storage: update storage.conf docs Update the docs based on the new parsing behavior. Note as far as the new drop in and file precedence works I plan to add a new man page and then have all the man pages link to that instead of duplication how the ordering works in each man page. Signed-off-by: Paul Holzinger --- storage/docs/containers-storage.conf.5.md | 37 ++++++++++++++--------- storage/storage.conf | 5 --- storage/storage.conf-freebsd | 5 --- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/storage/docs/containers-storage.conf.5.md b/storage/docs/containers-storage.conf.5.md index bc4d5d0b26..bf8a48e01b 100644 --- a/storage/docs/containers-storage.conf.5.md +++ b/storage/docs/containers-storage.conf.5.md @@ -26,15 +26,12 @@ The `storage` table supports the following options: **driver**="" Copy On Write (COW) container storage driver. Valid drivers are "overlay", "vfs", "btrfs", and "zfs". Some drivers (for example, "zfs" and "btrfs") may not work if your kernel lacks support for the filesystem. -This field is required to guarantee proper operation. Valid rootless drivers are "btrfs", "overlay", and "vfs". -Rootless users default to the driver defined in the system configuration when possible. -When the system configuration uses an unsupported rootless driver, rootless users default to "overlay" if available, otherwise "vfs". **graphroot**="" - container storage graph dir (default: "/var/lib/containers/storage") + container storage graph dir (default: `/var/lib/containers/storage` or as rootless `$XDG_DATA_HOME/containers/storage`/`$HOME/.local/share/containers/storage` depending on if `$XDG_DATA_HOME` is set) Default directory to store all writable content created by container storage programs. -The rootless graphroot path supports environment variable substitutions (ie. `$HOME/containers/storage`). +The graphroot path supports environment variable substitutions (ie. `$HOME/containers/storage`). When changing the graphroot location on an SELINUX system, ensure the labeling matches the default locations labels with the following commands: ``` @@ -48,10 +45,9 @@ In rootless mode you would set # semanage fcontext -a -e $HOME/.local/share/containers NEWSTORAGEPATH $ restorecon -R -v /NEWSTORAGEPATH ``` -**rootless_storage_path**="$HOME/.local/share/containers/storage" - Storage path for rootless users. By default the graphroot for rootless users is set to `$XDG_DATA_HOME/containers/storage`, if XDG_DATA_HOME is set. Otherwise `$HOME/.local/share/containers/storage` is used. This field can be used if administrators need to change the storage location for all users. The rootless storage path supports environment variable substitutions (ie. `$HOME/containers/storage`) -A common use case for this field is to provide a local storage directory when user home directories are NFS-mounted (podman does not support container storage over NFS). +**rootless_storage_path**="" + Deprecated, use **graphroot** instead. To switch the paths for all the rootless users, put the config file with **graphroot** under the `storage.rootless.conf.d/` directory. **imagestore**="" The image storage path (the default is assumed to be the same as `graphroot`). Path of the imagestore, which is different from `graphroot`. By default, images in the storage library are stored in the `graphroot`. If `imagestore` is provided, newly pulled images will be stored in the `imagestore` location. All other storage continues to be stored in the `graphroot`. When using the `overlay` driver, images previously stored in the `graphroot` remain accessible. Internally, the storage library mounts `graphroot` as an `additionalImageStore` to allow this behavior. @@ -61,8 +57,8 @@ A common use case for the `imagestore` field is users who need to split filesyst Imagestore, if set, must be different from `graphroot`. **runroot**="" - container storage run dir (default: "/run/containers/storage") -Default directory to store all temporary writable content created by container storage programs. The rootless runroot path supports environment variable substitutions (ie. `$HOME/containers/storage`) + container storage run dir (default: `/run/containers/storage` or as rootless `$XDG_RUNTIME_DIR/containers`/`/tmp/storage-run-$UID/containers` depending on if $XDG_RUNTIME_DIR is set) +Default directory to store all temporary writable content created by container storage programs. This path should be on a tmpfs. The path supports environment variable substitutions (ie. `$HOME/containers/storage`). **driver_priority**=[] Priority list for the storage drivers that will be tested one after the other to pick the storage driver if it is not defined. The first storage driver in this list that can be used, will be picked as the new one and all subsequent ones will not be tried. If all drivers in this list are not viable, then **all** known drivers will be tried and the first working one will be picked. @@ -295,11 +291,24 @@ This is a way to prevent xfs_quota management from conflicting with containers/s ## FILES -Distributions often provide a `/usr/share/containers/storage.conf` file to define default storage configuration. Administrators can override this file by creating `/etc/containers/storage.conf` to specify their own configuration. Likewise rootless users can create a storage.conf file to override the system storage.conf files. Files should be stored in the `$XDG_CONFIG_HOME/containers/storage.conf` file. If `$XDG_CONFIG_HOME` is not set then the file `$HOME/.config/containers/storage.conf` is used. +The following search locations are used: -Note: The storage.conf file overrides all other storage.conf files. Container -engines run by users with a storage.conf file in their home directory do not -use options in the system storage.conf files. +- /usr/share/containers/storage.conf +- /usr/share/containers/storage.conf.d/ +- /usr/share/containers/storage.rootful.conf.d/ (only when UID == 0) +- /usr/share/containers/storage.rootless.conf.d/ (only when UID > 0) +- /usr/share/containers/storage.rootless.conf.d// (only when UID > 0) + +- /etc/containers/storage.conf +- /etc/containers/storage.conf.d/ +- /etc/containers/storage.rootful.conf.d/ (only when UID == 0) +- /etc/containers/storage.rootless.conf.d/ (only when UID > 0) +- /etc/containers/storage.rootless.conf.d// (only when UID > 0) + +- $XDG_CONFIG_HOME/containers/storage.conf +- $XDG_CONFIG_HOME/containers/storage.conf.d/ + +If $XDG_CONFIG_HOME is empty then it uses $HOME/.config. The lookup in the home directory is also done as root. /etc/projects - XFS persistent project root definition /etc/projid - XFS project name mapping file diff --git a/storage/storage.conf b/storage/storage.conf index 421b999adb..d5c6fd51e9 100644 --- a/storage/storage.conf +++ b/storage/storage.conf @@ -4,11 +4,6 @@ # container/storage library do not inherit fields from other storage.conf # files. # -# Note: The storage.conf file overrides other storage.conf files based on this precedence: -# /usr/containers/storage.conf -# /etc/containers/storage.conf -# $HOME/.config/containers/storage.conf -# $XDG_CONFIG_HOME/containers/storage.conf (if XDG_CONFIG_HOME is set) # See man 5 containers-storage.conf for more information # The "storage" table contains all of the server options. [storage] diff --git a/storage/storage.conf-freebsd b/storage/storage.conf-freebsd index 3b42dcff8b..f33e17a8c0 100644 --- a/storage/storage.conf-freebsd +++ b/storage/storage.conf-freebsd @@ -4,11 +4,6 @@ # container/storage library do not inherit fields from other storage.conf # files. # -# Note: The storage.conf file overrides other storage.conf files based on this precedence: -# /usr/local/share/containers/storage.conf -# /usr/local/etc/containers/storage.conf -# $HOME/.config/containers/storage.conf -# $XDG_CONFIG_HOME/containers/storage.conf (If XDG_CONFIG_HOME is set) # See man 5 containers-storage.conf for more information # The "container storage" table contains all of the server options. [storage]