From c472c4a6ddaaa22a077eeacf8d4f2a17d2046c5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 5 Sep 2024 07:30:47 +0100 Subject: [PATCH 1/4] schema: add reuse partition --- rust/agama-lib/share/examples/storage.json | 2 +- rust/agama-lib/share/profile.schema.json | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/agama-lib/share/examples/storage.json b/rust/agama-lib/share/examples/storage.json index 833768a83b..636cfac366 100644 --- a/rust/agama-lib/share/examples/storage.json +++ b/rust/agama-lib/share/examples/storage.json @@ -24,7 +24,6 @@ } }, "filesystem": { - "reuse": false, "type": { "btrfs": { "snapshots": true @@ -87,6 +86,7 @@ "ifNotFound": "skip" }, "filesystem": { + "reuseIfPossible": true, "type": "ext4", "path": "/var/log" } diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index e8dea9610c..25fce02c4d 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -870,9 +870,9 @@ "type": "object", "additionalProperties": false, "properties": { - "reuse": { + "reuseIfPossible": { "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 }, @@ -886,7 +886,7 @@ "path": { "title": "Mount path", "type": "string", - "examples": ["/dev/vda"] + "examples": ["/var/log"] }, "mountBy": { "title": "How to mount the device", From 36b0079aa4aa59679578f4db5a60764830347b3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 5 Sep 2024 07:32:18 +0100 Subject: [PATCH 2/4] storage: configure reuse file system from JSON config --- .../filesystem/from_json.rb | 24 ++++++++++--------- .../lib/agama/storage/configs/filesystem.rb | 5 ++++ .../config_conversions/from_json_test.rb | 15 +++++++----- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/service/lib/agama/storage/config_conversions/filesystem/from_json.rb b/service/lib/agama/storage/config_conversions/filesystem/from_json.rb index b3a6056f86..fdd61b7503 100644 --- a/service/lib/agama/storage/config_conversions/filesystem/from_json.rb +++ b/service/lib/agama/storage/config_conversions/filesystem/from_json.rb @@ -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 diff --git a/service/lib/agama/storage/configs/filesystem.rb b/service/lib/agama/storage/configs/filesystem.rb index 4b126ddd26..c47b2cd517 100644 --- a/service/lib/agama/storage/configs/filesystem.rb +++ b/service/lib/agama/storage/configs/filesystem.rb @@ -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 @@ -48,6 +52,7 @@ class Filesystem attr_accessor :mount_by def initialize + @reuse = false @mount_options = [] @mkfs_options = [] end diff --git a/service/test/agama/storage/config_conversions/from_json_test.rb b/service/test/agama/storage/config_conversions/from_json_test.rb index 4a736505e5..6ad9257b57 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -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" @@ -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 "@" From 012873713178d18f71da7fa44d61dac0c190fe73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 5 Sep 2024 07:34:29 +0100 Subject: [PATCH 3/4] storage: make proposal reuse file systems --- .../proposal/agama_device_planner.rb | 21 +- .../{partitioned_disk.yaml => disks.yaml} | 5 + service/test/y2storage/agama_proposal_test.rb | 230 ++++++++++++++++-- 3 files changed, 231 insertions(+), 25 deletions(-) rename service/test/fixtures/{partitioned_disk.yaml => disks.yaml} (86%) diff --git a/service/lib/y2storage/proposal/agama_device_planner.rb b/service/lib/y2storage/proposal/agama_device_planner.rb index 70818198bd..95c3815b31 100644 --- a/service/lib/y2storage/proposal/agama_device_planner.rb +++ b/service/lib/y2storage/proposal/agama_device_planner.rb @@ -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] diff --git a/service/test/fixtures/partitioned_disk.yaml b/service/test/fixtures/disks.yaml similarity index 86% rename from service/test/fixtures/partitioned_disk.yaml rename to service/test/fixtures/disks.yaml index bfe5219833..38dfaf5ffa 100644 --- a/service/test/fixtures/partitioned_disk.yaml +++ b/service/test/fixtures/disks.yaml @@ -21,3 +21,8 @@ - disk: name: "/dev/vdb" size: 50 GiB + +- disk: + name: "/dev/vdc" + size: 50 GiB + file_system: "ext4" diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index 1260e9e6bd..ccb133968b 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -24,6 +24,51 @@ require "agama/storage/config" require "y2storage/agama_proposal" +# @param config [Agama::Storage::Configs::Drive, Agama::Storage::Configs::Partition] +# @param name [String, nil] e.g., "/dev/vda" +# @param filesystem [String, nil] e.g., "xfs" +def block_device_config(config, name: nil, filesystem: nil) + if name + config.search = Agama::Storage::Configs::Search.new.tap do |search_config| + search_config.name = name + end + end + + if filesystem + config.filesystem = Agama::Storage::Configs::Filesystem.new.tap do |fs_config| + fs_config.type = Agama::Storage::Configs::FilesystemType.new.tap do |type_config| + type_config.fs_type = Y2Storage::Filesystems::Type.find(filesystem) + end + end + end + + config +end + +# @param name [String, nil] e.g., "/dev/vda" +# @param filesystem [String, nil] e.g., "xfs" +def drive_config(name: nil, filesystem: nil) + config = Agama::Storage::Configs::Drive.new + block_device_config(config, name: name, filesystem: filesystem) +end + +# @param name [String, nil] e.g., "/dev/vda" +# @param filesystem [String, nil] e.g., "xfs" +# @param size [Y2Storage::DiskSize] +def partition_config(name: nil, filesystem: nil, size: nil) + config = Agama::Storage::Configs::Partition.new + block_device_config(config, name: name, filesystem: filesystem) + + if size + config.size = Agama::Storage::Configs::Size.new.tap do |size_config| + size_config.min = size + size_config.max = size + end + end + + config +end + describe Y2Storage::AgamaProposal do include Agama::RSpec::StorageHelpers @@ -117,7 +162,7 @@ end context "when the config has 2 drives" do - let(:scenario) { "partitioned_disk.yaml" } + let(:scenario) { "disks.yaml" } let(:drives) { [drive0, drive1] } @@ -136,6 +181,42 @@ end end + context "when trying to reuse a file system from a drive" do + let(:scenario) { "disks.yaml" } + + let(:drives) { [drive] } + + let(:drive) do + drive_config(name: name, filesystem: "ext3").tap { |c| c.filesystem.reuse = true } + end + + context "if the drive is already formatted" do + let(:name) { "/dev/vdc" } + + it "reuses the file system" do + vdc = Y2Storage::StorageManager.instance.probed.find_by_name("/dev/vdc") + fs_sid = vdc.filesystem.sid + + devicegraph = proposal.propose + + filesystem = devicegraph.find_by_name("/dev/vdc").filesystem + expect(filesystem.sid).to eq(fs_sid) + end + end + + context "if the drive is not formatted" do + let(:name) { "/dev/vdb" } + + it "creates the file system" do + devicegraph = proposal.propose + + filesystem = devicegraph.find_by_name("/dev/vdb").filesystem + expect(filesystem).to_not be_nil + expect(filesystem.type).to eq(Y2Storage::Filesystems::Type::EXT3) + end + end + end + context "when a partition table type is specified for a drive" do let(:drive0) do Agama::Storage::Configs::Drive.new.tap do |drive| @@ -308,7 +389,7 @@ end context "when searching for an existent drive" do - let(:scenario) { "partitioned_disk.yaml" } + let(:scenario) { "disks.yaml" } before do drive0.search.name = "/dev/vdb" @@ -326,7 +407,7 @@ end context "when searching for any drive" do - let(:scenario) { "partitioned_disk.yaml" } + let(:scenario) { "disks.yaml" } let(:drives) { [drive0, drive1] } @@ -400,7 +481,7 @@ end context "when searching for an existent partition" do - let(:scenario) { "partitioned_disk.yaml" } + let(:scenario) { "disks.yaml" } let(:partitions0) { [root_partition, home_partition] } @@ -421,7 +502,7 @@ end context "when searching for any partition" do - let(:scenario) { "partitioned_disk.yaml" } + let(:scenario) { "disks.yaml" } let(:partitions0) { [root_partition, home_partition] } @@ -455,25 +536,17 @@ end end - def partition_config(name) - Agama::Storage::Configs::Partition.new.tap do |partition_config| - partition_config.search = Agama::Storage::Configs::Search.new.tap do |search_config| - search_config.name = name - end - end - end - context "forcing to delete some partitions" do - let(:scenario) { "partitioned_disk.yaml" } + let(:scenario) { "disks.yaml" } let(:partitions0) { [root_partition, vda2, vda3] } let(:vda2) do - partition_config("/dev/vda2").tap { |c| c.delete = true } + partition_config(name: "/dev/vda2").tap { |c| c.delete = true } end let(:vda3) do - partition_config("/dev/vda3").tap { |c| c.delete = true } + partition_config(name: "/dev/vda3").tap { |c| c.delete = true } end before do @@ -497,12 +570,12 @@ def partition_config(name) end context "allowing to delete some partition" do - let(:scenario) { "partitioned_disk.yaml" } + let(:scenario) { "disks.yaml" } let(:partitions0) { [root_partition, vda3] } let(:vda3) do - partition_config("/dev/vda3").tap { |c| c.delete_if_needed = true } + partition_config(name: "/dev/vda3").tap { |c| c.delete_if_needed = true } end before do @@ -545,12 +618,12 @@ def partition_config(name) # Testing precedence. This configuration should not be possible. context "if the partition config indicates both force to delete and allow to delete" do - let(:scenario) { "partitioned_disk.yaml" } + let(:scenario) { "disks.yaml" } let(:partitions0) { [root_partition, vda3] } let(:vda3) do - partition_config("/dev/vda3").tap do |config| + partition_config(name: "/dev/vda3").tap do |config| config.delete = true config.delete_if_needed = true end @@ -570,5 +643,122 @@ def partition_config(name) expect(root.filesystem.mount_path).to eq("/") end end + + context "when reusing a partition" do + let(:scenario) { "disks.yaml" } + + let(:drives) { [drive] } + + let(:drive) do + drive_config.tap { |c| c.partitions = [partition] } + end + + let(:partition) { partition_config(name: name, filesystem: "ext3") } + + context "if trying to reuse the file system" do + before do + partition.filesystem.reuse = true + end + + context "and the partition is already formatted" do + let(:name) { "/dev/vda2" } + + it "reuses the file system" do + vda2 = Y2Storage::StorageManager.instance.probed.find_by_name("/dev/vda2") + fs_sid = vda2.filesystem.sid + + devicegraph = proposal.propose + + filesystem = devicegraph.find_by_name("/dev/vda2").filesystem + expect(filesystem.sid).to eq(fs_sid) + end + end + + context "and the partition is not formatted" do + let(:name) { "/dev/vda1" } + + it "creates the file system" do + devicegraph = proposal.propose + + filesystem = devicegraph.find_by_name("/dev/vda1").filesystem + expect(filesystem).to_not be_nil + expect(filesystem.type).to eq(Y2Storage::Filesystems::Type::EXT3) + end + end + end + + context "if not trying to reuse the file system" do + before do + partition.filesystem.reuse = false + end + + context "and the partition is already formatted" do + let(:name) { "/dev/vda2" } + + it "creates the file system" do + vda2 = Y2Storage::StorageManager.instance.probed.find_by_name("/dev/vda2") + fs_sid = vda2.filesystem.sid + + devicegraph = proposal.propose + + filesystem = devicegraph.find_by_name("/dev/vda2").filesystem + expect(filesystem.sid).to_not eq(fs_sid) + expect(filesystem.type).to eq(Y2Storage::Filesystems::Type::EXT3) + end + end + + context "and the partition is not formatted" do + let(:name) { "/dev/vda1" } + + it "creates the file system" do + devicegraph = proposal.propose + + filesystem = devicegraph.find_by_name("/dev/vda1").filesystem + expect(filesystem).to_not be_nil + expect(filesystem.type).to eq(Y2Storage::Filesystems::Type::EXT3) + end + end + end + end + + context "when creating a new partition" do + let(:scenario) { "disks.yaml" } + + let(:drives) { [drive] } + + let(:drive) do + drive_config.tap { |c| c.partitions = [partition] } + end + + let(:partition) { partition_config(filesystem: "ext3", size: Y2Storage::DiskSize.GiB(1)) } + + context "if trying to reuse the file system" do + before do + partition.filesystem.reuse = true + end + + it "creates the file system" do + devicegraph = proposal.propose + + filesystem = devicegraph.find_by_name("/dev/vda4").filesystem + expect(filesystem).to_not be_nil + expect(filesystem.type).to eq(Y2Storage::Filesystems::Type::EXT3) + end + end + + context "if not trying to reuse the file system" do + before do + partition.filesystem.reuse = false + end + + it "creates the file system" do + devicegraph = proposal.propose + + filesystem = devicegraph.find_by_name("/dev/vda4").filesystem + expect(filesystem).to_not be_nil + expect(filesystem.type).to eq(Y2Storage::Filesystems::Type::EXT3) + end + end + end end end From 547798942a83a94c6edbef2b057f5b89fd801dd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 5 Sep 2024 07:36:11 +0100 Subject: [PATCH 4/4] service: changelog --- service/package/rubygem-agama-yast.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index 417b097a01..ed8f65eeff 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Thu Sep 5 17:35:04 UTC 2024 - José Iván López González + +- 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