Skip to content

Commit

Permalink
fix: UI crashes with Multipath or RAID (#1212)
Browse files Browse the repository at this point in the history
## Problem

The storage UI crashes when the system has Multipath or BIOS RAID
devices.

Source of the problem:

* `Storage1.Multipath` and `Storage1.RAID` D-Bus interfaces export the
path of the wires and RAID devices respectively.
* Neither Multipath wires nor RAID devices are exported on D-Bus.
* The UI fails to build the Multipath wires and/or RAID devices because
they are not found in the list of devices.

## Solution

* Modify the D-Bus interfaces in order to export the name of the
Multipath wires and RAID devices instead of their D-Bus path.
* Adapt the UI storage client to use the name of the Multipath wires and
RAID devices instead of trying to build a complete device.

Note: At this moment, the D-Bus storage service only exports the devices
useful for the installation. The Multipath wires or RAID devices are not
exported because they cannot be used for installing the system. In the
future we can consider to export those devices too, adapting the UI to
avoid selecting that devices for the installation.

<details>
<summary>Toggle screenshots</summary>

![localhost_8080_
(3)](https://github.com/openSUSE/agama/assets/1112304/f3c1baae-cb57-4ba0-817d-fab95cf76821)

![localhost_8080_
(4)](https://github.com/openSUSE/agama/assets/1112304/1b3ffd08-3a98-4c2a-8638-1bc54dd0ccd8)

![localhost_8080_
(5)](https://github.com/openSUSE/agama/assets/1112304/93a4cfc5-3127-49f4-b0f7-94ece4b37f05)

</details>
  • Loading branch information
joseivanlopez authored May 15, 2024
2 parents 742d5eb + 4c1808a commit e3ca84c
Showing 9 changed files with 79 additions and 37 deletions.
4 changes: 2 additions & 2 deletions rust/agama-lib/src/storage/model.rs
Original file line number Diff line number Diff line change
@@ -611,7 +611,7 @@ pub struct Md {
#[derive(Debug, Clone, Serialize, Deserialize, utoipa::ToSchema)]
#[serde(rename_all = "camelCase")]
pub struct Multipath {
pub wires: Vec<DeviceSid>,
pub wires: Vec<String>,
}

#[derive(Debug, Clone, Serialize, Deserialize, utoipa::ToSchema)]
@@ -653,5 +653,5 @@ impl TryFrom<zbus::zvariant::Value<'_>> for UnusedSlot {
#[derive(Debug, Clone, Serialize, Deserialize, utoipa::ToSchema)]
#[serde(rename_all = "camelCase")]
pub struct Raid {
pub devices: Vec<DeviceSid>,
pub devices: Vec<String>,
}
8 changes: 5 additions & 3 deletions service/lib/agama/dbus/storage/interfaces/device/multipath.rb
Original file line number Diff line number Diff line change
@@ -44,17 +44,19 @@ def self.apply?(storage_device)
MULTIPATH_INTERFACE = "org.opensuse.Agama.Storage1.Multipath"
private_constant :MULTIPATH_INTERFACE

# Paths of the D-Bus objects representing the multipath wires.
# Name of the multipath wires.
#
# @note: The multipath wires are not exported in D-Bus yet.
#
# @return [Array<String>]
def multipath_wires
storage_device.parents.map { |p| tree.path_for(p) }
storage_device.parents.map(&:name)
end

def self.included(base)
base.class_eval do
dbus_interface MULTIPATH_INTERFACE do
dbus_reader :multipath_wires, "ao", dbus_name: "Wires"
dbus_reader :multipath_wires, "as", dbus_name: "Wires"
end
end
end
8 changes: 5 additions & 3 deletions service/lib/agama/dbus/storage/interfaces/device/raid.rb
Original file line number Diff line number Diff line change
@@ -44,17 +44,19 @@ def self.apply?(storage_device)
RAID_INTERFACE = "org.opensuse.Agama.Storage1.RAID"
private_constant :RAID_INTERFACE

# Paths of the D-Bus objects representing the devices used by the DM RAID.
# Name of the devices used by the DM RAID.
#
# @note: The devices used by a DM RAID are not exported in D-Bus yet.
#
# @return [Array<String>]
def raid_devices
storage_device.parents.map { |p| tree.path_for(p) }
storage_device.parents.map(&:name)
end

def self.included(base)
base.class_eval do
dbus_interface RAID_INTERFACE do
dbus_reader :raid_devices, "ao", dbus_name: "Devices"
dbus_reader :raid_devices, "as", dbus_name: "Devices"
end
end
end
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 @@
-------------------------------------------------------------------
Wed May 15 12:52:42 UTC 2024 - José Iván López González <jlopez@suse.com>

- Export the device name of the Multipath wires and RAID devices
instead of their D-Bus path (gh#openSUSE/agama#1212).

-------------------------------------------------------------------
Mon May 6 05:13:11 UTC 2024 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>

Original file line number Diff line number Diff line change
@@ -28,12 +28,9 @@
let(:device) { devicegraph.multipaths.first }

describe "#multipath_wires" do
it "returns the D-Bus path of the Multipath wires" do
sda = devicegraph.find_by_name("/dev/sda")
sdb = devicegraph.find_by_name("/dev/sdb")

it "returns the name of the Multipath wires" do
expect(subject.multipath_wires)
.to contain_exactly(tree.path_for(sda), tree.path_for(sdb))
.to contain_exactly("/dev/sda", "/dev/sdb")
end
end
end
Original file line number Diff line number Diff line change
@@ -28,12 +28,9 @@
let(:device) { devicegraph.dm_raids.first }

describe "#raid_devices" do
it "returns the D-Bus path of the RAID devices" do
sdb = devicegraph.find_by_name("/dev/sdb")
sdc = devicegraph.find_by_name("/dev/sdc")

it "returns the name of the RAID devices" do
expect(subject.raid_devices)
.to contain_exactly(tree.path_for(sdb), tree.path_for(sdc))
.to contain_exactly("/dev/sdb", "/dev/sdc")
end
end
end
3 changes: 1 addition & 2 deletions service/test/agama/software/manager_test.rb
Original file line number Diff line number Diff line change
@@ -230,10 +230,9 @@
describe "#products" do
it "returns the list of known products" do
products = subject.products
expect(products.size).to eq(4)
expect(products.size).to eq(3)
expect(products).to all(be_a(Agama::Software::Product))
expect(products).to contain_exactly(
an_object_having_attributes(id: "ALP-Dolomite"),
an_object_having_attributes(id: "Tumbleweed"),
an_object_having_attributes(id: "MicroOS"),
an_object_having_attributes(id: "MicroOS-Desktop")
33 changes: 21 additions & 12 deletions web/src/client/storage.js
Original file line number Diff line number Diff line change
@@ -228,6 +228,22 @@ class DevicesManager {
*/
async getDevices() {
const buildDevice = (jsonDevice, jsonDevices) => {
/** @type {() => StorageDevice} */
const buildDefaultDevice = () => {
return {
sid: 0,
name: "",
description: "",
isDrive: false,
type: ""
};
};

/** @type {(names: string[]) => StorageDevice[]} */
const buildCollectionFromNames = (names) => {
return names.map(name => ({ ...buildDefaultDevice(), name }));
};

/** @type {(sids: String[], jsonDevices: object[]) => StorageDevice[]} */
const buildCollection = (sids, jsonDevices) => {
if (sids === null || sids === undefined) return [];
@@ -251,20 +267,20 @@ class DevicesManager {

/** @type {(device: StorageDevice, info: object) => void} */
const addRaidInfo = (device, info) => {
device.devices = buildCollection(info.devices, jsonDevices);
device.devices = buildCollectionFromNames(info.devices);
};

/** @type {(device: StorageDevice, info: object) => void} */
const addMultipathInfo = (device, info) => {
device.wires = buildCollection(info.wires, jsonDevices);
device.wires = buildCollectionFromNames(info.wires);
};

/** @type {(device: StorageDevice, info: object) => void} */
const addMDInfo = (device, info) => {
device.type = "md";
device.level = info.level;
device.uuid = info.uuid;
addRaidInfo(device, info);
device.devices = buildCollection(info.devices, jsonDevices);
};

/** @type {(device: StorageDevice, info: object) => void} */
@@ -282,7 +298,7 @@ class DevicesManager {
};

/** @type {(device: StorageDevice, info: object) => void} */
const addLvInfo = (device, info) => {
const addLvInfo = (device, _info) => {
device.type = "lvmLv";
};

@@ -317,14 +333,7 @@ class DevicesManager {
};
};

/** @type {StorageDevice} */
const device = {
sid: 0,
name: "",
description: "",
isDrive: false,
type: ""
};
const device = buildDefaultDevice();

/** @type {(jsonProperty: String, info: function) => void} */
const process = (jsonProperty, method) => {
40 changes: 35 additions & 5 deletions web/src/client/storage.test.js
Original file line number Diff line number Diff line change
@@ -421,9 +421,39 @@ sdf1.component = {

md0.devices = [sda1, sda2];

raid.devices = [sdb, sdc];

multipath.wires = [sdd, sde];
raid.devices = [
{
sid: 0,
name: "/dev/sdb",
description: "",
isDrive: false,
type: ""
},
{
sid: 0,
name: "/dev/sdc",
description: "",
isDrive: false,
type: ""
}
];

multipath.wires = [
{
sid: 0,
name: "/dev/sdd",
description: "",
isDrive: false,
type: ""
},
{
sid: 0,
name: "/dev/sde",
description: "",
isDrive: false,
type: ""
}
];

lvmVg.logicalVolumes = [lvmLv1];
lvmVg.physicalVolumes = [sdf1];
@@ -905,7 +935,7 @@ const contexts = {
}
},
raid: {
devices: [62, 63]
devices: ["/dev/sdb", "/dev/sdc"]
}
},
{
@@ -938,7 +968,7 @@ const contexts = {
}
},
multipath: {
wires: [64, 65]
wires: ["/dev/sdd", "/dev/sde"]
}
},
{

0 comments on commit e3ca84c

Please sign in to comment.