Skip to content

feat(storage): config support to reuse file systems#1575

Merged
joseivanlopez merged 4 commits intoagama-project:masterfrom
joseivanlopez:storage-reuse-filesystem
Sep 6, 2024
Merged

feat(storage): config support to reuse file systems#1575
joseivanlopez merged 4 commits intoagama-project:masterfrom
joseivanlopez:storage-reuse-filesystem

Conversation

@joseivanlopez
Copy link
Copy Markdown
Contributor

@joseivanlopez joseivanlopez commented Sep 4, 2024

Allow using reuseIfPossible for a filesystem section in the storage JSON config. The auto_storage document proposes to have a reuse property, but reuseIfPossible was used instead, see #1575 (review):

If reuseIfPossible is true, then Agama will try to reuse the file system, if any. In that case, some other properties from the filesystem section would be ignored.

Example:

{
  "search": "/dev/vda1",
  "filesystem": {
    "reuseIfPossible": true,
    "path": "/logs",
    "type": "xfs",
    "mkfsOptions": ["rw"]
  }
}

The properties type and mkfsOptions would be considered by Agama only if the file system is not finally reused.

@coveralls
Copy link
Copy Markdown

coveralls commented Sep 4, 2024

Coverage Status

coverage: 71.801% (-0.002%) from 71.803%
when pulling a5b7c46 on joseivanlopez:storage-reuse-filesystem
into f38e15b on openSUSE:master.

@joseivanlopez joseivanlopez force-pushed the storage-reuse-filesystem branch from 3083d2b to dafe319 Compare September 5, 2024 06:36
"additionalProperties": false,
"properties": {
"reuse": {
"reuseIfPossible": {
Copy link
Copy Markdown
Contributor Author

@joseivanlopez joseivanlopez Sep 5, 2024

Choose a reason for hiding this comment

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

The document auto_storage proposes to have a reuse property, but reuseIfPossible was used because the following reasons:

a) A file system cannot be reused if the partition is going to be created (search is not specified).
b) A file system cannot be reused if the device is going to be encrypted.
c) A file system cannot be reused if the encryption layer is not going to be reused too.
d) A file system cannot be reused if the device is not currently formatted.

Validating the JSON config to avoid the usage of reuse in a, b and c would make the schema definition much more complex with very little benefit. Moreover, d cannot be validated at schema level because it requires information about the system.

The property was called reuseIfPossible to indicate that Agama will try reusing the existing file system if it is actually possible.

@joseivanlopez joseivanlopez force-pushed the storage-reuse-filesystem branch from dafe319 to a5b7c46 Compare September 5, 2024 10:30
@joseivanlopez joseivanlopez marked this pull request as ready for review September 5, 2024 10:30
require "agama/storage/config"
require "y2storage/agama_proposal"

# @param config [Agama::Storage::Configs::Drive, Agama::Storage::Configs::Partition]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These methods look like a hint that we need (or will shortly need) shared helpers or factories for creating config objects and similar stuff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Totally agree.

ancorgs

This comment was marked as resolved.

@joseivanlopez joseivanlopez force-pushed the storage-reuse-filesystem branch from a5b7c46 to 5477989 Compare September 6, 2024 09:01
Copy link
Copy Markdown
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

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

LGTM

@joseivanlopez joseivanlopez merged commit d6a691a into agama-project:master Sep 6, 2024
@imobachgs imobachgs mentioned this pull request Sep 20, 2024
imobachgs added a commit that referenced this pull request Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants