Expose partitions on D-Bus (#1016)

## Problem

The Agama storage service currently exports a tree of devices on D-Bus.
Such a tree only contains objects for the candidate devices for the
installation. But the information about their partitions is missing.
Partitions are need for serveral reasons, for example, to indicate how
to find space in the selected devices.

## Solution

Make the tree of devices to include objects for the partitions. The
partition object implements the *org.opensuse.Agama.Storage1.Block*
interface.

~~~
# busctl --address unix:path=/run/agama/bus tree
org.opensuse.Agama.Storage1
└─/org
  └─/org/opensuse
    └─/org/opensuse/Agama
      └─/org/opensuse/Agama/Storage1
        ├─/org/opensuse/Agama/Storage1/Proposal
        └─/org/opensuse/Agama/Storage1/system
          ├─/org/opensuse/Agama/Storage1/system/68
          ├─/org/opensuse/Agama/Storage1/system/69
          ├─/org/opensuse/Agama/Storage1/system/70
          ├─/org/opensuse/Agama/Storage1/system/71
          ├─/org/opensuse/Agama/Storage1/system/77
          ├─/org/opensuse/Agama/Storage1/system/79
          ├─/org/opensuse/Agama/Storage1/system/80
          ├─/org/opensuse/Agama/Storage1/system/82
          ├─/org/opensuse/Agama/Storage1/system/83
          └─/org/opensuse/Agama/Storage1/system/85

# busctl --address unix:path=/run/agama/bus introspect
org.opensuse.Agama.Storage1 /org/opensuse/Agama/Storage1/system/68
org.opensuse.Agama.Storage1.PartitionTable
NAME TYPE SIGNATURE RESULT/VALUE FLAGS
.Partitions property ao 1 "/org/opensuse/Agama/Storage1/system/…
emits-change
.Type property s "gpt" emits-change

# busctl --address unix:path=/run/agama/bus get-property
org.opensuse.Agama.Storage1 /org/opensuse/Agama/Storage1/system/68
org.opensuse.Agama.Storage1.PartitionTable Partitions
ao 1 "/org/opensuse/Agama/Storage1/system/77"

# busctl --address unix:path=/run/agama/bus introspect
org.opensuse.Agama.Storage1 /org/opensuse/Agama/Storage1/system/77
NAME TYPE SIGNATURE RESULT/VALUE FLAGS
org.freedesktop.DBus.Introspectable interface - - -
.Introspect method - s -
org.freedesktop.DBus.Properties interface - - -
.Get method ss v -
.GetAll method s a{sv} -
.Set method ssv - -
.PropertiesChanged signal sa{sv}as - -
org.opensuse.Agama.Storage1.Block interface - - -
.Active property b true emits-change
.Name property s "/dev/vdb1" emits-change
.RecoverableSize property t 2145386496 emits-change
.Size property t 2147483648 emits-change
.Systems property as 0 emits-change
.UdevIds property as 0 emits-change
.UdevPaths property as 1 "pci-0000:08:00.0-part1" emits-change
~~~

## Testing

* Added new unit tests
* Tested manually

service/lib/agama/dbus/storage/device.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -35,22 +35,46 @@ 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
+
         # Constructor
         #
         # @param storage_device [Y2Storage::Device] Storage device
         # @param path [::DBus::ObjectPath] Path for the D-Bus object
+        # @param tree [DevicesTree] D-Bus tree in which the device is exported
         # @param logger [Logger, nil]
-        def initialize(storage_device, path, logger: nil)
+        def initialize(storage_device, path, tree, logger: nil)
           super(path, logger: logger)
 
           @storage_device = storage_device
+          @tree = tree
           add_interfaces
         end
 
+        # Sets the represented storage device.
+        #
+        # @note A properties changed signal is emitted for each interface.
+        # @raise [RuntimeError] If the given device has a different sid.
+        #
+        # @param value [Y2Storage::Device]
+        def storage_device=(value)
+          if value.sid != storage_device.sid
+            raise "Cannot update the D-Bus object because the given device has a different sid: " \
+                  "#{value} instead of #{storage_device.sid}"
+          end
+
+          @storage_device = value
+
+          interfaces_and_properties.each do |interface, properties|
+            dbus_properties_changed(interface, properties, [])
+          end
+        end
+
       private
 
-        # @return [Y2Storage::Device]
-        attr_reader :storage_device
+        # @return [DevicesTree]
+        attr_reader :tree
 
         # Adds the required interfaces according to the storage object
         def add_interfaces # rubocop:disable Metrics/CyclomaticComplexity
@@ -82,7 +106,9 @@ def drive?
         #
         # @return [Boolean]
         def partition_table?
-          storage_device.is?(:blk_device) && storage_device.partition_table?
+          storage_device.is?(:blk_device) &&
+            storage_device.respond_to?(:partition_table?) &&
+            storage_device.partition_table?
         end
       end
     end

service/lib/agama/dbus/storage/devices_tree.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -19,25 +19,15 @@
 # To contact SUSE LLC about this file by physical or electronic mail, you may
 # find current contact information at www.suse.com.
 
-require "dbus/object_path"
+require "agama/dbus/base_tree"
 require "agama/dbus/storage/device"
+require "dbus/object_path"
 
 module Agama
   module DBus
     module Storage
       # Class representing a storage devices tree exported on D-Bus
-      class DevicesTree
-        # Constructor
-        #
-        # @param service [::DBus::ObjectServer]
-        # @param root_path [::DBus::ObjectPath]
-        # @param logger [Logger, nil]
-        def initialize(service, root_path, logger: nil)
-          @service = service
-          @root_path = root_path
-          @logger = logger
-        end
-
+      class DevicesTree < BaseTree
         # Object path for the D-Bus object representing the given device
         #
         # @param device [Y2Storage::Device]
@@ -48,58 +38,55 @@ def path_for(device)
 
         # Updates the D-Bus tree according to the given devicegraph
         #
-        # The current D-Bus nodes are all unexported.
-        #
         # @param devicegraph [Y2Storage::Devicegraph]
         def update(devicegraph)
-          unexport_devices
-          export_devices(devicegraph)
+          self.objects = devices(devicegraph)
         end
 
       private
 
-        # @return [::DBus::ObjectServer]
-        attr_reader :service
-
-        # @return [::DBus::ObjectPath]
-        attr_reader :root_path
-
-        # @return [Logger]
-        attr_reader :logger
-
-        # Exports a D-Bus object for each storage device
-        #
-        # @param devicegraph [Y2Storage::Devicegraph]
-        def export_devices(devicegraph)
-          # TODO: Right now, the goal of exporting the storage devices on D-Bus is to provide the
-          #   required information of the available devices for calculating a proposal. For that
-          #   reason, only the potential candidate diks are exported (i.e., disk devices and MDs).
-          #   Note that partitons, LVM, etc are not exported yet.
-          devices = devicegraph.disk_devices + devicegraph.software_raids
-          devices.each { |d| export_device(d) }
+        # @see BaseTree
+        # @param device [Y2Storage::Device]
+        def create_dbus_object(device)
+          Device.new(device, path_for(device), self, logger: logger)
         end
 
-        # Exports a D-Bus object for the given device
-        #
+        # @see BaseTree
+        # @param dbus_object [Device]
         # @param device [Y2Storage::Device]
-        def export_device(device)
-          dbus_node = Device.new(device, path_for(device), logger: logger)
-          service.export(dbus_node)
+        def update_dbus_object(dbus_object, device)
+          dbus_object.storage_device = device
         end
 
-        # Unexports the currently exported D-Bus objects
-        def unexport_devices
-          dbus_objects.each { |n| service.unexport(n) }
+        # @see BaseTree
+        # @param dbus_object [Device]
+        # @param device [Y2Storage::Device]
+        def dbus_object?(dbus_object, device)
+          dbus_object.storage_device.sid == device.sid
         end
 
-        # All exported D-Bus objects
+        # Devices to be exported.
+        #
+        # 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.
         #
-        # @return [Array<Device>]
-        def dbus_objects
-          root = service.get_node(root_path, create: false)
-          return [] unless root
+        # TODO: export LVM VGs and file systems of directly formatted devices.
+        #
+        # @param devicegraph [Y2Storage::Devicegraph]
+        # @return [Array<Y2Storage::Device>]
+        def devices(devicegraph)
+          devices = devicegraph.disk_devices + devicegraph.software_raids
+          devices + partitions_from(devices)
+        end
 
-          root.descendant_objects
+        # All partitions of the given devices.
+        #
+        # @param devices [Array<Y2Storage::Device>]
+        # @return [Array<Y2Storage::Partition>]
+        def partitions_from(devices)
+          devices.select { |d| d.is?(:blk_device) && d.respond_to?(:partitions) }
+            .flat_map(&:partitions)
         end
       end
     end

service/lib/agama/dbus/storage/interfaces/block.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -67,6 +67,13 @@ def block_size
             storage_device.size.to_i
           end
 
+          # Size of the space that could be theoretically reclaimed by shrinking the device.
+          #
+          # @return [Integer]
+          def block_recoverable_size
+            storage_device.recoverable_size.to_i
+          end
+
           # Name of the currently installed systems
           #
           # @return [Array<String>]
@@ -85,6 +92,7 @@ def self.included(base)
                 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"
+                dbus_reader :block_recoverable_size, "t", dbus_name: "RecoverableSize"
                 dbus_reader :block_systems, "as", dbus_name: "Systems"
               end
             end

service/lib/agama/dbus/storage/interfaces/partition_table.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -39,20 +39,18 @@ def partition_table_type
             storage_device.partition_table.type.to_s
           end
 
-          # Name of the partitions
+          # Paths of the D-Bus objects representing the partitions.
           #
-          # TODO: return the path of the partition objects once the partitions are exported.
-          #
-          # @return [Array<String>]
+          # @return [Array<::DBus::ObjectPath>]
           def partition_table_partitions
-            storage_device.partition_table.partitions.map(&:name)
+            storage_device.partition_table.partitions.map { |p| tree.path_for(p) }
           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, "as", dbus_name: "Partitions"
+                dbus_reader :partition_table_partitions, "ao", dbus_name: "Partitions"
               end
             end
           end

service/package/rubygem-agama.changes

@@ -1,3 +1,8 @@
+-------------------------------------------------------------------
+Mon Jan 29 13:51:30 UTC 2024 - José Iván López González <[email protected]>
+
+- Export partitions on D-Bus (gh#openSUSE/agama#1016).
+
 -------------------------------------------------------------------
 Thu Jan 18 14:55:36 UTC 2024 - José Iván López González <[email protected]>
 

service/test/agama/dbus/storage/device_test.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -28,6 +28,7 @@
 require_relative "./interfaces/md_examples"
 require_relative "./interfaces/partition_table_examples"
 require "agama/dbus/storage/device"
+require "agama/dbus/storage/devices_tree"
 require "dbus"
 
 describe Agama::DBus::Storage::Device do
@@ -44,7 +45,11 @@
     end
   end
 
-  subject { described_class.new(device, "/test") }
+  subject { described_class.new(device, "/test", tree) }
+
+  let(:tree) { Agama::DBus::Storage::DevicesTree.new(service, "/agama/devices") }
+
+  let(:service) { instance_double(::DBus::ObjectServer) }
 
   before do
     mock_storage(devicegraph: scenario)
@@ -136,4 +141,40 @@
   include_examples "Block interface"
 
   include_examples "PartitionTable interface"
+
+  describe "#storage_device=" do
+    before do
+      allow(subject).to receive(:dbus_properties_changed)
+    end
+
+    let(:scenario) { "partitioned_md.yml" }
+    let(:device) { devicegraph.find_by_name("/dev/sda") }
+
+    context "if the given device has a different sid" do
+      let(:new_device) { devicegraph.find_by_name("/dev/sdb") }
+
+      it "raises an error" do
+        expect { subject.storage_device = new_device }
+          .to raise_error(RuntimeError, /Cannot update the D-Bus object/)
+      end
+    end
+
+    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)
+        end
+
+        subject.storage_device = new_device
+      end
+    end
+  end
 end