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

fix: UI crashes with Multipath or RAID #1212

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
4 changes: 2 additions & 2 deletions rust/agama-lib/src/storage/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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
Expand Up @@ -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
Expand Down
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
Expand Up @@ -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
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 @@
-------------------------------------------------------------------
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>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions service/test/agama/software/manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
33 changes: 21 additions & 12 deletions web/src/client/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
Expand All @@ -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} */
Expand All @@ -282,7 +298,7 @@ class DevicesManager {
};

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

Expand Down Expand Up @@ -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) => {
Expand Down
40 changes: 35 additions & 5 deletions web/src/client/storage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -905,7 +935,7 @@ const contexts = {
}
},
raid: {
devices: [62, 63]
devices: ["/dev/sdb", "/dev/sdc"]
}
},
{
Expand Down Expand Up @@ -938,7 +968,7 @@ const contexts = {
}
},
multipath: {
wires: [64, 65]
wires: ["/dev/sdd", "/dev/sde"]
}
},
{
Expand Down
Loading