From 43726235aca0b153339b971d6cea6035316e5b71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Mar 2024 14:56:20 +0000 Subject: [PATCH 1/8] service: Export device sid in the proposal actions --- service/lib/agama/dbus/storage/proposal.rb | 1 + service/test/agama/dbus/storage/proposal_test.rb | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/service/lib/agama/dbus/storage/proposal.rb b/service/lib/agama/dbus/storage/proposal.rb index 2d55331f35..591d142451 100644 --- a/service/lib/agama/dbus/storage/proposal.rb +++ b/service/lib/agama/dbus/storage/proposal.rb @@ -155,6 +155,7 @@ def dbus_settings # @return [Hash] def to_dbus_action(action) { + "Device" => action.target_device.sid, "Text" => action.sentence, "Subvol" => action.device_is?(:btrfs_subvolume), "Delete" => action.delete? diff --git a/service/test/agama/dbus/storage/proposal_test.rb b/service/test/agama/dbus/storage/proposal_test.rb index 790014eeaa..fc61302129 100644 --- a/service/test/agama/dbus/storage/proposal_test.rb +++ b/service/test/agama/dbus/storage/proposal_test.rb @@ -257,14 +257,18 @@ let(:action1) do instance_double(Y2Storage::CompoundAction, - sentence: "test1", device_is?: false, delete?: false) + sentence: "test1", target_device: device1, device_is?: false, delete?: false) end let(:action2) do instance_double(Y2Storage::CompoundAction, - sentence: "test2", device_is?: true, delete?: true) + sentence: "test2", target_device: device2, device_is?: true, delete?: true) end + let(:device1) { instance_double(Y2Storage::Device, sid: 1) } + + let(:device2) { instance_double(Y2Storage::Device, sid: 2) } + it "returns a list with a hash for each action" do expect(subject.actions.size).to eq(2) expect(subject.actions).to all(be_a(Hash)) @@ -272,12 +276,14 @@ action1, action2 = subject.actions expect(action1).to eq({ + "Device" => 1, "Text" => "test1", "Subvol" => false, "Delete" => false }) expect(action2).to eq({ + "Device" => 2, "Text" => "test2", "Subvol" => true, "Delete" => true From ca3497862e26e275581ef8d7238728afac119b8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Mar 2024 14:58:08 +0000 Subject: [PATCH 2/8] service: Export unused slots --- .../interfaces/device/partition_table.rb | 11 +++++++ .../device/partition_table_examples.rb | 32 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/service/lib/agama/dbus/storage/interfaces/device/partition_table.rb b/service/lib/agama/dbus/storage/interfaces/device/partition_table.rb index 546ddfd934..97dcb90e25 100644 --- a/service/lib/agama/dbus/storage/interfaces/device/partition_table.rb +++ b/service/lib/agama/dbus/storage/interfaces/device/partition_table.rb @@ -60,11 +60,22 @@ def partition_table_partitions storage_device.partition_table.partitions.map { |p| tree.path_for(p) } end + # Available slots within a partition table, that is, the spaces that can be used to + # create a new partition. + # + # @return [Array] The first block and the size of each slot. + def partition_table_unused_slots + storage_device.partition_table.unused_partition_slots.map do |slot| + [slot.region.start, slot.region.size.to_i] + end + end + def self.included(base) base.class_eval do dbus_interface PARTITION_TABLE_INTERFACE do dbus_reader :partition_table_type, "s", dbus_name: "Type" dbus_reader :partition_table_partitions, "ao", dbus_name: "Partitions" + dbus_reader :partition_table_unused_slots, "a(tt)", dbus_name: "UnusedSlots" end end end diff --git a/service/test/agama/dbus/storage/interfaces/device/partition_table_examples.rb b/service/test/agama/dbus/storage/interfaces/device/partition_table_examples.rb index 1af30111ab..22fb9efae1 100644 --- a/service/test/agama/dbus/storage/interfaces/device/partition_table_examples.rb +++ b/service/test/agama/dbus/storage/interfaces/device/partition_table_examples.rb @@ -20,6 +20,9 @@ # find current contact information at www.suse.com. require_relative "../../../../../test_helper" +require "y2storage/disk_size" +require "y2storage/partition_tables/partition_slot" +require "y2storage/region" shared_examples "PartitionTable interface" do describe "PartitionTable D-Bus interface" do @@ -39,5 +42,34 @@ expect(subject.partition_table_partitions).to contain_exactly(tree.path_for(md0p1)) end end + + describe "#partition_table_unused_slots" do + before do + allow(device).to receive(:partition_table).and_return(partition_table) + allow(partition_table).to receive(:unused_partition_slots).and_return(unused_slots) + end + + let(:partition_table) { device.partition_table } + + let(:unused_slots) do + [ + instance_double(Y2Storage::PartitionTables::PartitionSlot, region: region1), + instance_double(Y2Storage::PartitionTables::PartitionSlot, region: region2) + ] + end + + let(:region1) do + instance_double(Y2Storage::Region, start: 234, size: Y2Storage::DiskSize.new(1024)) + end + + let(:region2) do + instance_double(Y2Storage::Region, start: 987, size: Y2Storage::DiskSize.new(2048)) + end + + it "returns the information about the unused slots" do + md0p1 = devicegraph.find_by_name("/dev/md0p1") + expect(subject.partition_table_unused_slots).to contain_exactly([234, 1024], [987, 2048]) + end + end end end From 02a93e9761e5876a77d0d3653f8527163e9e644f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Mar 2024 14:58:57 +0000 Subject: [PATCH 3/8] service: Export more info about block devices --- .../dbus/storage/interfaces/device/block.rb | 16 ++++++++++++++++ .../storage/interfaces/device/block_examples.rb | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/service/lib/agama/dbus/storage/interfaces/device/block.rb b/service/lib/agama/dbus/storage/interfaces/device/block.rb index b5c0e00017..fa7c3e301b 100644 --- a/service/lib/agama/dbus/storage/interfaces/device/block.rb +++ b/service/lib/agama/dbus/storage/interfaces/device/block.rb @@ -51,6 +51,13 @@ def block_name storage_device.name end + # Position of the first block of the region. + # + # @return [Integer] + def block_start + storage_device.start + end + # Whether the block device is currently active # # @return [Boolean] @@ -58,6 +65,13 @@ def block_active storage_device.active? end + # Whether the block device is encrypted. + # + # @return [Boolean] + def block_encrypted + storage_device.encrypted? + end + # Name of the udev by-id links # # @return [Array] @@ -100,7 +114,9 @@ def self.included(base) base.class_eval do dbus_interface BLOCK_INTERFACE do dbus_reader :block_name, "s", dbus_name: "Name" + dbus_reader :block_start, "t", dbus_name: "Start" dbus_reader :block_active, "b", dbus_name: "Active" + dbus_reader :block_encrypted, "b", dbus_name: "Encrypted" dbus_reader :block_udev_ids, "as", dbus_name: "UdevIds" dbus_reader :block_udev_paths, "as", dbus_name: "UdevPaths" dbus_reader :block_size, "t", dbus_name: "Size" diff --git a/service/test/agama/dbus/storage/interfaces/device/block_examples.rb b/service/test/agama/dbus/storage/interfaces/device/block_examples.rb index 11590a3390..6211a2d913 100644 --- a/service/test/agama/dbus/storage/interfaces/device/block_examples.rb +++ b/service/test/agama/dbus/storage/interfaces/device/block_examples.rb @@ -33,6 +33,16 @@ end end + describe "#block_start" do + before do + allow(device).to receive(:start).and_return(345) + end + + it "returns the first block of the region" do + expect(subject.block_start).to eq(345) + end + end + describe "#block_active" do before do allow(device).to receive(:active?).and_return(true) @@ -43,6 +53,12 @@ end end + describe "#block_encrypted" do + it "returns whether the device is encrypted" do + expect(subject.block_encrypted).to eq(false) + end + end + describe "#block_udev_ids" do before do allow(device).to receive(:udev_ids).and_return(udev_ids) From 26fb99da1cf56aae7b3f44a9cf782b59f5d00884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Mar 2024 15:02:14 +0000 Subject: [PATCH 4/8] service: Export LVM devices --- .../lib/agama/dbus/storage/devices_tree.rb | 9 +- .../agama/dbus/storage/interfaces/device.rb | 1 + .../storage/interfaces/device/component.rb | 2 + .../dbus/storage/interfaces/device/lvm_vg.rb | 90 +++++++++++++++++++ .../test/agama/dbus/storage/device_test.rb | 19 +++- .../interfaces/device/lvm_vg_examples.rb | 64 +++++++++++++ service/test/fixtures/trivial_lvm.yml | 24 +++++ 7 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 service/lib/agama/dbus/storage/interfaces/device/lvm_vg.rb create mode 100644 service/test/agama/dbus/storage/interfaces/device/lvm_vg_examples.rb create mode 100644 service/test/fixtures/trivial_lvm.yml diff --git a/service/lib/agama/dbus/storage/devices_tree.rb b/service/lib/agama/dbus/storage/devices_tree.rb index c06501bd1e..a930c7840b 100644 --- a/service/lib/agama/dbus/storage/devices_tree.rb +++ b/service/lib/agama/dbus/storage/devices_tree.rb @@ -70,13 +70,16 @@ def dbus_object?(dbus_object, device) # Right now, only the required information for calculating a proposal is exported, that is: # * Potential candidate devices (i.e., disk devices, MDs). # * Partitions of the candidate devices in order to indicate how to find free space. - # - # TODO: export LVM VGs and file systems of directly formatted devices. + # * LVM volume groups and logical volumes. # # @param devicegraph [Y2Storage::Devicegraph] # @return [Array] def devices(devicegraph) - devices = devicegraph.disk_devices + devicegraph.software_raids + devices = devicegraph.disk_devices + + devicegraph.software_raids + + devicegraph.lvm_vgs + + devicegraph.lvm_lvs + devices + partitions_from(devices) end diff --git a/service/lib/agama/dbus/storage/interfaces/device.rb b/service/lib/agama/dbus/storage/interfaces/device.rb index 98f09690c2..f721da72ef 100644 --- a/service/lib/agama/dbus/storage/interfaces/device.rb +++ b/service/lib/agama/dbus/storage/interfaces/device.rb @@ -35,6 +35,7 @@ module Device require "agama/dbus/storage/interfaces/device/component" require "agama/dbus/storage/interfaces/device/drive" require "agama/dbus/storage/interfaces/device/filesystem" +require "agama/dbus/storage/interfaces/device/lvm_vg" require "agama/dbus/storage/interfaces/device/md" require "agama/dbus/storage/interfaces/device/multipath" require "agama/dbus/storage/interfaces/device/partition_table" diff --git a/service/lib/agama/dbus/storage/interfaces/device/component.rb b/service/lib/agama/dbus/storage/interfaces/device/component.rb index c66422b2b4..a4de9ce3e3 100644 --- a/service/lib/agama/dbus/storage/interfaces/device/component.rb +++ b/service/lib/agama/dbus/storage/interfaces/device/component.rb @@ -84,6 +84,8 @@ def self.included(base) base.class_eval do dbus_interface COMPONENT_INTERFACE do dbus_reader :component_type, "s", dbus_name: "Type" + # The names are provided just in case the device is component of a device that + # is not exported yet (e.g., Bcache devices). dbus_reader :component_device_names, "as", dbus_name: "DeviceNames" dbus_reader :component_devices, "ao", dbus_name: "Devices" end diff --git a/service/lib/agama/dbus/storage/interfaces/device/lvm_vg.rb b/service/lib/agama/dbus/storage/interfaces/device/lvm_vg.rb new file mode 100644 index 0000000000..b69b84a5f5 --- /dev/null +++ b/service/lib/agama/dbus/storage/interfaces/device/lvm_vg.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "dbus" + +module Agama + module DBus + module Storage + module Interfaces + module Device + # Interface for a LVM Volume Group. + # + # @note This interface is intended to be included by {Agama::DBus::Storage::Device} if + # needed. + module LvmVg + # Whether this interface should be implemented for the given device. + # + # @note LVM Volume Groups implement this interface. + # + # @param storage_device [Y2Storage::Device] + # @return [Boolean] + def self.apply?(storage_device) + storage_device.is?(:lvm_vg) + end + + VOLUME_GROUP_INTERFACE = "org.opensuse.Agama.Storage1.LVM.VolumeGroup" + private_constant :VOLUME_GROUP_INTERFACE + + # Name of the volume group + # + # @return [String] e.g., "/dev/mapper/vg0" + def lvm_vg_name + storage_device.name + end + + # Size of the volume group in bytes + # + # @return [Integer] + def lvm_vg_size + storage_device.size.to_i + end + + # D-Bus paths of the objects representing the physical volumes. + # + # @return [Array] + def lvm_vg_pvs + storage_device.lvm_pvs.map { |p| tree.path_for(p.plain_blk_device) } + end + + # D-Bus paths of the objects representing the logical volumes. + # + # @return [Array] + def lvm_vg_lvs + storage_device.lvm_lvs.map { |l| tree.path_for(l) } + end + + def self.included(base) + base.class_eval do + dbus_interface VOLUME_GROUP_INTERFACE do + dbus_reader :lvm_vg_name, "s", dbus_name: "Name" + dbus_reader :lvm_vg_size, "t", dbus_name: "Size" + dbus_reader :lvm_vg_pvs, "ao", dbus_name: "PhysicalVolumes" + dbus_reader :lvm_vg_lvs, "ao", dbus_name: "LogicalVolumes" + end + end + end + end + end + end + end + end +end diff --git a/service/test/agama/dbus/storage/device_test.rb b/service/test/agama/dbus/storage/device_test.rb index 649131a83f..457951fadb 100644 --- a/service/test/agama/dbus/storage/device_test.rb +++ b/service/test/agama/dbus/storage/device_test.rb @@ -19,19 +19,20 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. -require "agama/dbus/storage/device" -require "agama/dbus/storage/devices_tree" -require "dbus" require_relative "../../../test_helper" require_relative "../../storage/storage_helpers" require_relative "./interfaces/device/block_examples" require_relative "./interfaces/device/component_examples" require_relative "./interfaces/device/drive_examples" require_relative "./interfaces/device/filesystem_examples" +require_relative "./interfaces/device/lvm_vg_examples" require_relative "./interfaces/device/md_examples" require_relative "./interfaces/device/multipath_examples" require_relative "./interfaces/device/partition_table_examples" require_relative "./interfaces/device/raid_examples" +require "agama/dbus/storage/device" +require "agama/dbus/storage/devices_tree" +require "dbus" describe Agama::DBus::Storage::Device do include Agama::RSpec::StorageHelpers @@ -110,6 +111,16 @@ end end + context "when the given device is a LVM volume group" do + let(:scenario) { "trivial_lvm.yml" } + + let(:device) { devicegraph.find_by_name("/dev/vg0") } + + it "defines the LVM.VolumeGroup interface" do + expect(subject).to include_dbus_interface("org.opensuse.Agama.Storage1.LVM.VolumeGroup") + end + end + context "when the given device has a partition table" do let(:scenario) { "partitioned_md.yml" } @@ -142,6 +153,8 @@ include_examples "Block interface" + include_examples "LVM.VolumeGroup interface" + include_examples "PartitionTable interface" include_examples "Filesystem interface" diff --git a/service/test/agama/dbus/storage/interfaces/device/lvm_vg_examples.rb b/service/test/agama/dbus/storage/interfaces/device/lvm_vg_examples.rb new file mode 100644 index 0000000000..ad646ca618 --- /dev/null +++ b/service/test/agama/dbus/storage/interfaces/device/lvm_vg_examples.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require_relative "../../../../../test_helper" + +shared_examples "LVM.VolumeGroup interface" do + describe "LVM.VolumeGroup D-Bus interface" do + let(:scenario) { "trivial_lvm.yml" } + + let(:device) { devicegraph.find_by_name("/dev/vg0") } + + describe "#lvm_vg_name" do + it "returns the name of the volume group" do + expect(subject.lvm_vg_name).to eq("/dev/vg0") + end + end + + describe "#lvm_vg_size" do + before do + allow(device).to receive(:size).and_return(size) + end + + let(:size) { Y2Storage::DiskSize.new(1024) } + + it "returns the size in bytes" do + expect(subject.lvm_vg_size).to eq(1024) + end + end + + describe "#lvm_vg_pvs" do + it "returns the D-Bus path of the physical volumes" do + sda1 = devicegraph.find_by_name("/dev/sda1") + + expect(subject.lvm_vg_pvs).to contain_exactly(tree.path_for(sda1)) + end + end + + describe "#lvm_vg_lvs" do + it "returns the D-Bus path of the logical volumes" do + lv1 = devicegraph.find_by_name("/dev/vg0/lv1") + + expect(subject.lvm_vg_lvs).to contain_exactly(tree.path_for(lv1)) + end + end + end +end diff --git a/service/test/fixtures/trivial_lvm.yml b/service/test/fixtures/trivial_lvm.yml new file mode 100644 index 0000000000..86f3fc904e --- /dev/null +++ b/service/test/fixtures/trivial_lvm.yml @@ -0,0 +1,24 @@ +--- +- disk: + name: /dev/sda + size: 200 GiB + partition_table: gpt + partitions: + + - partition: + size: unlimited + name: /dev/sda1 + id: lvm + +- lvm_vg: + vg_name: vg0 + lvm_pvs: + - lvm_pv: + blk_device: /dev/sda1 + + lvm_lvs: + - lvm_lv: + size: unlimited + lv_name: lv1 + file_system: btrfs + mount_point: / From 36ff75ae49f225721d45d8dce3909892d3719ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Mar 2024 15:06:21 +0000 Subject: [PATCH 5/8] service: Avoid updating D-Bus nodes of devices tree --- service/lib/agama/dbus/base_tree.rb | 9 ++++++-- service/lib/agama/dbus/storage/device.rb | 20 ++++++++++++++---- .../lib/agama/dbus/storage/devices_tree.rb | 21 +++++++++++++------ .../test/agama/dbus/storage/device_test.rb | 6 ------ 4 files changed, 38 insertions(+), 18 deletions(-) diff --git a/service/lib/agama/dbus/base_tree.rb b/service/lib/agama/dbus/base_tree.rb index db28aa4877..a699f03bf3 100644 --- a/service/lib/agama/dbus/base_tree.rb +++ b/service/lib/agama/dbus/base_tree.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -47,11 +47,16 @@ def initialize(service, root_path, logger: nil) # # @param objects [Array] def objects=(objects) - try_add_objects(objects) try_update_objects(objects) + try_add_objects(objects) try_delete_objects(objects) end + # Unexports the current D-Bus objects of this tree. + def clean + dbus_objects.each { |o| service.unexport(o) } + end + private # @return [::DBus::ObjectServer] diff --git a/service/lib/agama/dbus/storage/device.rb b/service/lib/agama/dbus/storage/device.rb index 4e1b07a206..17ecbe167e 100644 --- a/service/lib/agama/dbus/storage/device.rb +++ b/service/lib/agama/dbus/storage/device.rb @@ -30,8 +30,15 @@ module Storage # # The D-Bus object includes the required interfaces for the storage object that it represents. class Device < BaseObject - # @return [Y2Storage::Device] - attr_reader :storage_device + # sid of the Y2Storage device. + # + # @note A Y2Storage device is a wrapper over a libstorage-ng object. If the source + # devicegraph does not exist anymore (e.g., after reprobing), then the Y2Storage device + # object cannot be used (memory error). The device sid is stored to avoid accessing to + # the old Y2Storage device when updating the represented device, see {#storage_device=}. + # + # @return [Integer] + attr_reader :sid # Constructor # @@ -43,6 +50,7 @@ def initialize(storage_device, path, tree, logger: nil) super(path, logger: logger) @storage_device = storage_device + @sid = storage_device.sid @tree = tree add_interfaces end @@ -54,12 +62,13 @@ def initialize(storage_device, path, tree, logger: nil) # # @param value [Y2Storage::Device] def storage_device=(value) - if value.sid != storage_device.sid + if value.sid != sid raise "Cannot update the D-Bus object because the given device has a different sid: " \ - "#{value} instead of #{storage_device.sid}" + "#{value} instead of #{sid}" end @storage_device = value + @sid = value.sid interfaces_and_properties.each do |interface, properties| dbus_properties_changed(interface, properties, []) @@ -71,6 +80,9 @@ def storage_device=(value) # @return [DevicesTree] attr_reader :tree + # @return [Y2Storage::Device] + attr_reader :storage_device + # Adds the required interfaces according to the storage object. def add_interfaces interfaces = Interfaces::Device.constants diff --git a/service/lib/agama/dbus/storage/devices_tree.rb b/service/lib/agama/dbus/storage/devices_tree.rb index a930c7840b..c49a0d3d73 100644 --- a/service/lib/agama/dbus/storage/devices_tree.rb +++ b/service/lib/agama/dbus/storage/devices_tree.rb @@ -36,10 +36,19 @@ def path_for(device) ::DBus::ObjectPath.new(File.join(root_path, device.sid.to_s)) end - # Updates the D-Bus tree according to the given devicegraph + # Updates the D-Bus tree according to the given devicegraph. + # + # @note In the devices tree it is important to avoid updating D-Bus nodes. Note that an + # already exported D-Bus object could require to add or remove interfaces (e.g., an + # existing partition needs to add the Filesystem interface after formatting the + # partition). Dynamically adding or removing intefaces is not possible with ruby-dbus + # once the object is exported on D-Bus. + # + # Updating the currently exported D-Bus objects is avoided by calling to {#clean} first. # # @param devicegraph [Y2Storage::Devicegraph] def update(devicegraph) + clean self.objects = devices(devicegraph) end @@ -52,17 +61,17 @@ def create_dbus_object(device) end # @see BaseTree - # @param dbus_object [Device] - # @param device [Y2Storage::Device] - def update_dbus_object(dbus_object, device) - dbus_object.storage_device = device + # + # @note D-Bus objects representing devices cannot be safely updated, see {#update}. + def update_dbus_object(_dbus_object, _device) + nil end # @see BaseTree # @param dbus_object [Device] # @param device [Y2Storage::Device] def dbus_object?(dbus_object, device) - dbus_object.storage_device.sid == device.sid + dbus_object.sid == device.sid end # Devices to be exported. diff --git a/service/test/agama/dbus/storage/device_test.rb b/service/test/agama/dbus/storage/device_test.rb index 457951fadb..f67ecaaa8a 100644 --- a/service/test/agama/dbus/storage/device_test.rb +++ b/service/test/agama/dbus/storage/device_test.rb @@ -181,12 +181,6 @@ context "if the given device has the same sid" do let(:new_device) { devicegraph.find_by_name("/dev/sda") } - it "sets the new device" do - subject.storage_device = new_device - - expect(subject.storage_device).to equal(new_device) - end - it "emits a properties changed signal for each interface" do subject.interfaces_and_properties.each_key do |interface| expect(subject).to receive(:dbus_properties_changed).with(interface, anything, anything) From fa1ace321f1b2f14f70a119cb96fcf24da3a4d51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Mar 2024 15:07:11 +0000 Subject: [PATCH 6/8] service: Export staging devices --- service/lib/agama/dbus/storage/manager.rb | 12 ++- .../agama/dbus/storage/devices_tree_test.rb | 82 +++++++++---------- 2 files changed, 48 insertions(+), 46 deletions(-) diff --git a/service/lib/agama/dbus/storage/manager.rb b/service/lib/agama/dbus/storage/manager.rb index 188062579c..3d9380569d 100644 --- a/service/lib/agama/dbus/storage/manager.rb +++ b/service/lib/agama/dbus/storage/manager.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022-2023] SUSE LLC +# Copyright (c) [2022-2024] SUSE LLC # # All Rights Reserved. # @@ -289,6 +289,7 @@ def register_proposal_callbacks proposal.on_calculate do export_proposal proposal_properties_changed + refresh_staging_devices end end @@ -332,6 +333,11 @@ def refresh_system_devices system_devices_tree.update(devicegraph) end + def refresh_staging_devices + devicegraph = Y2Storage::StorageManager.instance.staging + staging_devices_tree.update(devicegraph) + end + def refresh_iscsi_nodes nodes = backend.iscsi.nodes iscsi_nodes_tree.update(nodes) @@ -348,6 +354,10 @@ def system_devices_tree @system_devices_tree ||= DevicesTree.new(@service, tree_path("system"), logger: logger) end + def staging_devices_tree + @staging_devices_tree ||= DevicesTree.new(@service, tree_path("staging"), logger: logger) + end + def tree_path(tree_root) File.join(PATH, tree_root) end diff --git a/service/test/agama/dbus/storage/devices_tree_test.rb b/service/test/agama/dbus/storage/devices_tree_test.rb index 42e0024a33..a876c2f77d 100644 --- a/service/test/agama/dbus/storage/devices_tree_test.rb +++ b/service/test/agama/dbus/storage/devices_tree_test.rb @@ -22,8 +22,8 @@ require_relative "../../../test_helper" require_relative "../../storage/storage_helpers" require "agama/dbus/storage/devices_tree" -require "y2storage" require "dbus" +require "y2storage" describe Agama::DBus::Storage::DevicesTree do include Agama::RSpec::StorageHelpers @@ -87,7 +87,8 @@ mock_storage(devicegraph: scenario) allow(service).to receive(:get_node).with(root_path, anything).and_return(root_node) - allow(root_node).to receive(:descendant_objects).and_return(dbus_objects) + # Returning an empty list for the second call to mock the effect of calling to #clear. + allow(root_node).to receive(:descendant_objects).and_return(dbus_objects, []) allow(service).to receive(:export) allow(service).to receive(:unexport) @@ -102,57 +103,48 @@ let(:devicegraph) { Y2Storage::StorageManager.instance.probed } - context "if a device is not exported yet" do - let(:dbus_objects) { [] } + let(:dbus_objects) { [dbus_object1, dbus_object2] } + let(:dbus_object1) { Agama::DBus::Storage::Device.new(sda, subject.path_for(sda), subject) } + let(:dbus_object2) { Agama::DBus::Storage::Device.new(sdb, subject.path_for(sdb), subject) } + let(:sda) { devicegraph.find_by_name("/dev/sda") } + let(:sdb) { devicegraph.find_by_name("/dev/sdb") } - it "exports a D-Bus object" do - sda = devicegraph.find_by_name("/dev/sda") - sdb = devicegraph.find_by_name("/dev/sdb") - md0 = devicegraph.find_by_name("/dev/md0") - sda1 = devicegraph.find_by_name("/dev/sda1") - sda2 = devicegraph.find_by_name("/dev/sda2") - md0p1 = devicegraph.find_by_name("/dev/md0p1") - - expect(service).to export_object("#{root_path}/#{sda.sid}") - expect(service).to export_object("#{root_path}/#{sdb.sid}") - expect(service).to export_object("#{root_path}/#{md0.sid}") - expect(service).to export_object("#{root_path}/#{sda1.sid}") - expect(service).to export_object("#{root_path}/#{sda2.sid}") - expect(service).to export_object("#{root_path}/#{md0p1.sid}") - expect(service).to_not receive(:export) + it "unexports the current D-Bus objects" do + expect(service).to unexport_object("#{root_path}/#{sda.sid}") + expect(service).to unexport_object("#{root_path}/#{sdb.sid}") - subject.update(devicegraph) - end + subject.update(devicegraph) end - context "if a device is already exported" do - let(:dbus_objects) { [dbus_object1] } - let(:dbus_object1) { Agama::DBus::Storage::Device.new(sda, subject.path_for(sda), subject) } - let(:sda) { devicegraph.find_by_name("/dev/sda") } - - it "does not export a D-Bus object" do - expect(service).to_not export_object("#{root_path}/#{sda.sid}") - - subject.update(devicegraph) - end - - it "updates the D-Bus object" do - expect(dbus_object1.storage_device).to equal(sda) + it "exports disk devices and partitions" do + md0 = devicegraph.find_by_name("/dev/md0") + sda1 = devicegraph.find_by_name("/dev/sda1") + sda2 = devicegraph.find_by_name("/dev/sda2") + md0p1 = devicegraph.find_by_name("/dev/md0p1") + + expect(service).to export_object("#{root_path}/#{sda.sid}") + expect(service).to export_object("#{root_path}/#{sdb.sid}") + expect(service).to export_object("#{root_path}/#{md0.sid}") + expect(service).to export_object("#{root_path}/#{sda1.sid}") + expect(service).to export_object("#{root_path}/#{sda2.sid}") + expect(service).to export_object("#{root_path}/#{md0p1.sid}") + expect(service).to_not receive(:export) + + subject.update(devicegraph) + end - subject.update(devicegraph) + context "if there are LVM volume groups" do + let(:scenario) { "trivial_lvm.yml" } - expect(dbus_object1.storage_device).to_not equal(sda) - expect(dbus_object1.storage_device.sid).to equal(sda.sid) - end - end + let(:dbus_objects) { [] } - context "if an exported D-Bus object does not represent any of the current devices" do - let(:dbus_objects) { [dbus_object1] } - let(:dbus_object1) { Agama::DBus::Storage::Device.new(sdd, subject.path_for(sdd), subject) } - let(:sdd) { instance_double(Y2Storage::Disk, sid: 1, is?: false, filesystem: false) } + it "exports the LVM volume groups and the logical volumes" do + vg0 = devicegraph.find_by_name("/dev/vg0") + lv1 = devicegraph.find_by_name("/dev/vg0/lv1") - it "unexports the D-Bus object" do - expect(service).to unexport_object("#{root_path}/1") + expect(service).to receive(:export) + expect(service).to export_object("#{root_path}/#{vg0.sid}") + expect(service).to export_object("#{root_path}/#{lv1.sid}") subject.update(devicegraph) end From d43bb46715e8e033fd0f1f09e523615f14ae0b79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 4 Mar 2024 16:12:04 +0000 Subject: [PATCH 7/8] service: Fix rubocop config - agama.gemspec was renamed, see https://github.com/openSUSE/agama/pull/1056. - The file name was not updated in the rubocop config. --- service/.rubocop.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/.rubocop.yml b/service/.rubocop.yml index 137ea30537..ada6a9f15e 100644 --- a/service/.rubocop.yml +++ b/service/.rubocop.yml @@ -8,7 +8,7 @@ AllCops: Exclude: - vendor/**/* - lib/agama/dbus/y2dir/**/* - - agama.gemspec + - agama-yast.gemspec - package/*.spec # a D-Bus method definition may take up more line lenght than usual From 8128fd7381ba2bd7416940319d9344e163312cad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Mar 2024 16:11:46 +0000 Subject: [PATCH 8/8] service: Fix typo --- service/lib/agama/dbus/storage/devices_tree.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/lib/agama/dbus/storage/devices_tree.rb b/service/lib/agama/dbus/storage/devices_tree.rb index c49a0d3d73..000bc17255 100644 --- a/service/lib/agama/dbus/storage/devices_tree.rb +++ b/service/lib/agama/dbus/storage/devices_tree.rb @@ -41,7 +41,7 @@ def path_for(device) # @note In the devices tree it is important to avoid updating D-Bus nodes. Note that an # already exported D-Bus object could require to add or remove interfaces (e.g., an # existing partition needs to add the Filesystem interface after formatting the - # partition). Dynamically adding or removing intefaces is not possible with ruby-dbus + # partition). Dynamically adding or removing interfaces is not possible with ruby-dbus # once the object is exported on D-Bus. # # Updating the currently exported D-Bus objects is avoided by calling to {#clean} first.