Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rust/agama-lib/share/examples/storage.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
}
},
"filesystem": {
"reuse": false,
"type": {
"btrfs": {
"snapshots": true
Expand Down Expand Up @@ -87,6 +86,7 @@
"ifNotFound": "skip"
},
"filesystem": {
"reuseIfPossible": true,
"type": "ext4",
"path": "/var/log"
}
Expand Down
6 changes: 3 additions & 3 deletions rust/agama-lib/share/profile.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -870,9 +870,9 @@
"type": "object",
"additionalProperties": false,
"properties": {
"reuse": {
"reuseIfPossible": {
Copy link
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.

"title": "Reuse file system",
"description": "Whether to reuse the existing file system (if any).",
"description": "Try to reuse the existing file system. In some cases the file system could not be reused, for example, if the device is re-encrypted.",
"type": "boolean",
"default": false
},
Expand All @@ -886,7 +886,7 @@
"path": {
"title": "Mount path",
"type": "string",
"examples": ["/dev/vda"]
"examples": ["/var/log"]
},
"mountBy": {
"title": "How to mount the device",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,20 @@ def initialize(filesystem_json)
def convert(default = nil)
default_config = default.dup || Configs::Filesystem.new

default_config.tap do |config|
mount_by = convert_mount_by
type = convert_type(config.type)
label = filesystem_json[:label]
mkfs_options = filesystem_json[:mkfsOptions]
values = {
reuse: filesystem_json[:reuseIfPossible],
label: filesystem_json[:label],
path: filesystem_json[:path],
mount_options: filesystem_json[:mountOptions],
mkfs_options: filesystem_json[:mkfsOptions],
mount_by: convert_mount_by,
type: convert_type(default_config.type)
}

config.path = filesystem_json[:path]
config.mount_options = filesystem_json[:mountOptions] || []
config.mount_by = mount_by if mount_by
config.type = type if type
config.label = label if label
config.mkfs_options = mkfs_options if mkfs_options
default_config.tap do |config|
values.each do |property, value|
config.public_send("#{property}=", value) unless value.nil?
end
end
end

Expand Down
5 changes: 5 additions & 0 deletions service/lib/agama/storage/configs/filesystem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ class Filesystem
# @return [Pathname] Object that represents the root path.
ROOT_PATH = Pathname.new("/").freeze

# @return [Boolean]
attr_accessor :reuse
alias_method :reuse?, :reuse

# @return [String, nil]
attr_accessor :path

Expand All @@ -48,6 +52,7 @@ class Filesystem
attr_accessor :mount_by

def initialize
@reuse = false
@mount_options = []
@mkfs_options = []
end
Expand Down
21 changes: 16 additions & 5 deletions service/lib/y2storage/proposal/agama_device_planner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,25 @@ def planned_devices(_setting)
private

# @param planned [Planned::Disk, Planned::Partition]
# @param settings [#found_device]
def configure_reuse(planned, settings)
device = settings.found_device
# @param config [Agama::Storage::Configs::Drive, Agama::Storage::Configs::Partition]
def configure_reuse(planned, config)
device = config.found_device
return unless device

planned.assign_reuse(device)
# TODO: Allow mounting without reformatting.
planned.reformat = true
planned.reformat = reformat?(device, config)
end

# Whether to reformat the device.
#
# @param device [Y2Storage::BlkDevice]
# @param config [Agama::Storage::Configs::Drive, Agama::Storage::Configs::Partition]
# @return [Boolean]
def reformat?(device, config)
return true if device.filesystem.nil?

# TODO: reformat if the encryption has to be created.
!config.filesystem&.reuse?
end

# @param planned [Planned::Disk, Planned::Partition]
Expand Down
6 changes: 6 additions & 0 deletions service/package/rubygem-agama-yast.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Thu Sep 5 17:35:04 UTC 2024 - José Iván López González <[email protected]>

- Storage: add support for reusing file systems in the storage
config (gh#openSUSE/agama#1575).

-------------------------------------------------------------------
Thu Sep 5 16:25:00 UTC 2024 - Lubos Kocman <[email protected]>

Expand Down
15 changes: 9 additions & 6 deletions service/test/agama/storage/config_conversions/from_json_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -211,18 +211,20 @@

let(:filesystem) do
{
path: "/",
type: "xfs",
label: "root",
mkfsOptions: ["version=2"],
mountOptions: ["rw"],
mountBy: "label"
reuseIfPossible: true,
path: "/",
type: "xfs",
label: "root",
mkfsOptions: ["version=2"],
mountOptions: ["rw"],
mountBy: "label"
}
end

it "uses the specified attributes" do
config = subject.convert
filesystem = config.drives.first.filesystem
expect(filesystem.reuse?).to eq true
expect(filesystem.path).to eq "/"
expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::XFS
expect(filesystem.label).to eq "root"
Expand All @@ -237,6 +239,7 @@
it "uses the default type and btrfs attributes for that path" do
config = subject.convert
filesystem = config.drives.first.filesystem
expect(filesystem.reuse?).to eq false
expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS
expect(filesystem.type.btrfs.snapshots).to eq true
expect(filesystem.type.btrfs.default_subvolume).to eq "@"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,8 @@
- disk:
name: "/dev/vdb"
size: 50 GiB

- disk:
name: "/dev/vdc"
size: 50 GiB
file_system: "ext4"
Loading
Loading