diff --git a/service/lib/agama/storage/zfcp/manager.rb b/service/lib/agama/storage/zfcp/manager.rb
index 0fd8f8ff22..354247f682 100644
--- a/service/lib/agama/storage/zfcp/manager.rb
+++ b/service/lib/agama/storage/zfcp/manager.rb
@@ -21,6 +21,7 @@
require "yast"
require "y2s390/zfcp"
+require "y2storage/storage_manager"
require "agama/storage/zfcp/controller"
require "agama/storage/zfcp/disk"
@@ -54,14 +55,20 @@ def on_disks_change(&block)
# Probes zFCP
#
- # Callbacks are called at the end, see {#on_probe}.
+ # Callbacks {#on_probe} are called after probing, and callbacks {#on_disks_change} are
+ # called if there is any change in the disks.
def probe
logger.info "Probing zFCP"
+ previous_disks = disks
yast_zfcp.probe_controllers
yast_zfcp.probe_disks
@on_probe_callbacks&.each { |c| c.call(controllers, disks) }
+
+ return unless disks_changed?(previous_disks)
+
+ @on_disks_change_callbacks&.each { |c| c.call(disks) }
end
# zFCP controllers
@@ -91,13 +98,22 @@ def allow_lun_scan?
# Activates the controller with the given channel id
#
- # @note: If "allow_lun_scan" is active, then all LUNs are automatically activated.
+ # @note: If "allow_lun_scan" is active, then all its LUNs are automatically activated.
#
# @param channel [String]
# @return [Integer] Exit code of the chzdev command (0 on success)
def activate_controller(channel)
output = yast_zfcp.activate_controller(channel)
- update_disks if output["exit"] == 0
+ if output["exit"] == 0
+ # LUNs activation could delay after activating the controller. This usually happens when
+ # activating a controller for first time because some SCSI initialization. Probing the
+ # disks should be done after all disks are activated.
+ #
+ # FIXME: waiting 2 seconds should be enough, but there is no guarantee that all the
+ # disks are actually activated.
+ sleep(2)
+ probe
+ end
output["exit"]
end
@@ -110,7 +126,7 @@ def activate_controller(channel)
# @return [Integer] Exit code of the chzdev command (0 on success)
def activate_disk(channel, wwpn, lun)
output = yast_zfcp.activate_disk(channel, wwpn, lun)
- update_disks if output["exit"] == 0
+ probe if output["exit"] == 0
output["exit"]
end
@@ -125,7 +141,7 @@ def activate_disk(channel, wwpn, lun)
# @return [Integer] Exit code of the chzdev command (0 on success)
def deactivate_disk(channel, wwpn, lun)
output = yast_zfcp.deactivate_disk(channel, wwpn, lun)
- update_disks if output["exit"] == 0
+ probe if output["exit"] == 0
output["exit"]
end
@@ -154,15 +170,28 @@ def find_luns(channel, wwpn)
# @return [Logger]
attr_reader :logger
- # Probes and runs the on_disk_change callbacsk if disks have changed
- def update_disks
- disk_records = yast_zfcp.disks.sort_by { |d| d["dev_name"] }
- probe
- new_disk_records = yast_zfcp.disks.sort_by { |d| d["dev_name"] }
+ # Whether threre is any change in the disks
+ #
+ # Checks whether any of the removed disks is still probed or whether any of the current
+ # disks is not probed yet.
+ #
+ # @param previous_disks [Array] Previously activated disks (before zFCP (re)probing)
+ # @return [Booelan]
+ def disks_changed?(previous_disks)
+ removed_disks = previous_disks - disks
+
+ return true if removed_disks.any? { |d| exist_disk?(d) }
+ return true if disks.any? { |d| !exist_disk?(d) }
- return if disk_records == new_disk_records
+ false
+ end
- @on_disks_change_callbacks&.each { |c| c.call(disks) }
+ # Whether the given disk is probed
+ #
+ # @param disk [Disk]
+ # @return [Booelan]
+ def exist_disk?(disk)
+ !Y2Storage::StorageManager.instance.probed.find_by_name(disk.name).nil?
end
# Creates a zFCP controller from YaST data
diff --git a/service/package/rubygem-agama.changes b/service/package/rubygem-agama.changes
index 1269fd80a1..fa17a0775c 100644
--- a/service/package/rubygem-agama.changes
+++ b/service/package/rubygem-agama.changes
@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Wed Jul 5 14:02:23 UTC 2023 - José Iván López González
+
+- Delay zFCP probing after activating a controller and ensure the
+ system is marked as deprecated if needed after probing zFCP
+ (gh#openSUSE/agama#650).
+
-------------------------------------------------------------------
Wed Jun 14 15:11:56 UTC 2023 - José Iván López González
diff --git a/service/test/agama/storage/zfcp/manager_test.rb b/service/test/agama/storage/zfcp/manager_test.rb
index 7ca8d6866a..fefae6a640 100644
--- a/service/test/agama/storage/zfcp/manager_test.rb
+++ b/service/test/agama/storage/zfcp/manager_test.rb
@@ -23,6 +23,9 @@
require "agama/storage/zfcp/manager"
require "agama/storage/zfcp/controller"
require "agama/storage/zfcp/disk"
+require "y2storage/storage_manager"
+require "y2storage/devicegraph"
+require "y2storage/disk"
# yast2-s390 is not available for all architectures. Defining Y2S390::ZFCP constant here in order to
# make possible to mock that class independently on the architecture. Note that Y2S390 classes are
@@ -38,6 +41,9 @@ module Y2S390
before do
allow(Y2S390::ZFCP).to receive(:new).and_return(yast_zfcp)
+ allow(Y2Storage::StorageManager).to receive(:instance).and_return(storage_manager)
+ allow(storage_manager).to receive(:probed).and_return(devicegraph)
+ allow(devicegraph).to receive(:find_by_name).and_return(storage_disk)
allow(yast_zfcp).to receive(:probe_controllers)
allow(yast_zfcp).to receive(:probe_disks)
allow(yast_zfcp).to receive(:controllers).and_return(*controller_records)
@@ -46,6 +52,12 @@ module Y2S390
let(:yast_zfcp) { double(Y2S390::ZFCP) }
+ let(:storage_manager) { instance_double(Y2Storage::StorageManager) }
+
+ let(:devicegraph) { instance_double(Y2Storage::Devicegraph) }
+
+ let(:storage_disk) { instance_double(Y2Storage::Disk) }
+
let(:controller_records) { [[]] }
let(:disk_records) { [[]] }
@@ -80,15 +92,55 @@ module Y2S390
subject.probe
end
- let(:callback) { proc { |_, _| } }
-
it "runs the on_probe callbacks" do
+ callback = proc { |_, _| }
subject.on_probe(&callback)
expect(callback).to receive(:call).with([], [])
subject.probe
end
+
+ context "if any zFCP disk was deactivated" do
+ let(:disk_records) { [[sda_record, sdb_record], [sda_record]] }
+
+ it "runs the on_disks_change callbacks" do
+ callback = proc { |_| }
+ subject.on_disks_change(&callback)
+
+ expect(callback).to receive(:call)
+
+ subject.probe
+ end
+ end
+
+ context "if any new zFCP disk was activated" do
+ let(:disk_records) { [[sda_record], [sda_record, sdb_record]] }
+
+ let(:storage_disk) { nil }
+
+ it "runs the on_disks_change callbacks" do
+ callback = proc { |_| }
+ subject.on_disks_change(&callback)
+
+ expect(callback).to receive(:call)
+
+ subject.probe
+ end
+ end
+
+ context "if the zFCP disks have not changed" do
+ let(:disk_records) { [[sda_record, sdb_record], [sda_record, sdb_record]] }
+
+ it "does not run the on_disks_change callbacks" do
+ callback = proc { |_| }
+ subject.on_disks_change(&callback)
+
+ expect(callback).to_not receive(:call)
+
+ subject.probe
+ end
+ end
end
describe "#controllers" do
@@ -179,14 +231,11 @@ module Y2S390
describe "#activate_controller" do
before do
allow(yast_zfcp).to receive(:activate_controller).and_return(output)
-
- subject.on_disks_change(&callback)
+ allow(subject).to receive(:sleep)
end
let(:output) { { "exit" => 3 } }
- let(:callback) { proc { |_| } }
-
it "tries to activate the controller with the given channel id" do
expect(yast_zfcp).to receive(:activate_controller).with("0.0.fa00")
@@ -207,26 +256,6 @@ module Y2S390
subject.activate_controller("0.0.fa00")
end
-
- context "and the zFCP disks have changed" do
- let(:disk_records) { [[sda_record], [sdb_record]] }
-
- it "runs the on_disks_change callbacks" do
- expect(callback).to receive(:call)
-
- subject.activate_controller("0.0.fa00")
- end
- end
-
- context "and the zFCP disks have not changed" do
- let(:disk_records) { [[sda_record], [sda_record]] }
-
- it "does not run the on_disks_change callbacks" do
- expect(callback).to_not receive(:call)
-
- subject.activate_controller("0.0.fa00")
- end
- end
end
context "if the controller was not activated" do
@@ -237,26 +266,16 @@ module Y2S390
subject.activate_controller("0.0.fa00")
end
-
- it "does not run the on_disks_change callbacks" do
- expect(callback).to_not receive(:call)
-
- subject.activate_controller("0.0.fa00")
- end
end
end
describe "#activate_disk" do
before do
allow(yast_zfcp).to receive(:activate_disk).and_return(output)
-
- subject.on_disks_change(&callback)
end
let(:output) { { "exit" => 3 } }
- let(:callback) { proc { |_| } }
-
it "tries to activate the zFCP diks with the given channel id, WWPN and LUN" do
expect(yast_zfcp)
.to receive(:activate_disk).with("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
@@ -278,26 +297,6 @@ module Y2S390
subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
end
-
- context "and the zFCP disks have changed" do
- let(:disk_records) { [[sda_record], [sdb_record]] }
-
- it "runs the on_disks_change callbacks" do
- expect(callback).to receive(:call)
-
- subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
- end
- end
-
- context "and the zFCP disks have not changed" do
- let(:disk_records) { [[sda_record], [sda_record]] }
-
- it "does not run the on_disks_change callbacks" do
- expect(callback).to_not receive(:call)
-
- subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
- end
- end
end
context "if the zFCP disk was not activated" do
@@ -308,26 +307,16 @@ module Y2S390
subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
end
-
- it "does not run the on_disks_change callbacks" do
- expect(callback).to_not receive(:call)
-
- subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
- end
end
end
describe "#deactivate_disk" do
before do
allow(yast_zfcp).to receive(:deactivate_disk).and_return(output)
-
- subject.on_disks_change(&callback)
end
let(:output) { { "exit" => 3 } }
- let(:callback) { proc { |_| } }
-
it "tries to deactivate the zFCP diks with the given channel id, WWPN and LUN" do
expect(yast_zfcp)
.to receive(:deactivate_disk).with("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
@@ -349,26 +338,6 @@ module Y2S390
subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
end
-
- context "and the zFCP disks have changed" do
- let(:disk_records) { [[sda_record], [sdb_record]] }
-
- it "runs the on_disks_change callbacks" do
- expect(callback).to receive(:call)
-
- subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
- end
- end
-
- context "and the zFCP disks have not changed" do
- let(:disk_records) { [[sda_record], [sda_record]] }
-
- it "does not run the on_disks_change callbacks" do
- expect(callback).to_not receive(:call)
-
- subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
- end
- end
end
context "if the zFCP disk was not deactivated" do
@@ -379,12 +348,6 @@ module Y2S390
subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
end
-
- it "does not run the on_disks_change callbacks" do
- expect(callback).to_not receive(:call)
-
- subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000")
- end
end
end
diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes
index ef18ecdcb8..75f7c248e6 100644
--- a/web/package/cockpit-agama.changes
+++ b/web/package/cockpit-agama.changes
@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Wed Jul 5 13:59:58 UTC 2023 - José Iván López González
+
+- Add info about deactivated zFCP auto_lun_scan and sort members in
+ device selector (gh#openSUSE/agama#650).
+
-------------------------------------------------------------------
Mon Jul 3 10:18:32 UTC 2023 - José Iván López González
diff --git a/web/src/components/storage/DeviceSelector.jsx b/web/src/components/storage/DeviceSelector.jsx
index 9c3ea1834c..8826405b50 100644
--- a/web/src/components/storage/DeviceSelector.jsx
+++ b/web/src/components/storage/DeviceSelector.jsx
@@ -122,7 +122,7 @@ const ItemContent = ({ device }) => {
const members = device.members.map(m => m.split("/").at(-1));
- return Members: {members.join(", ")}
;
+ return Members: {members.sort().join(", ")}
;
};
const RAIDInfo = () => {
@@ -130,7 +130,7 @@ const ItemContent = ({ device }) => {
const devices = device.devices.map(m => m.split("/").at(-1));
- return Devices: {devices.join(", ")}
;
+ return Devices: {devices.sort().join(", ")}
;
};
const MultipathInfo = () => {
@@ -138,7 +138,7 @@ const ItemContent = ({ device }) => {
const wires = device.wires.map(m => m.split("/").at(-1));
- return Wires: {wires.join(", ")}
;
+ return Wires: {wires.sort().join(", ")}
;
};
return (
diff --git a/web/src/components/storage/ZFCPPage.jsx b/web/src/components/storage/ZFCPPage.jsx
index c8cdf59075..179243fd99 100644
--- a/web/src/components/storage/ZFCPPage.jsx
+++ b/web/src/components/storage/ZFCPPage.jsx
@@ -422,26 +422,31 @@ const ControllersSection = ({ client, manager, load = noop, isLoading = false })
};
const Content = () => {
- const AllowLUNScan = () => {
+ const LUNScanInfo = () => {
return (
-
- The system is configured to perform automatic LUN scan. If a controller is running in
- NPIV mode, then all its LUNs are automatically activated.
-
+
+ Automatic LUN scan is enabled. Activating a controller which is running in NPIV
+ mode will automatically configures all its LUNs.
+
+ }
+ else={
+
+ Automatic LUN scan is disabled. LUNs have to be manually configured after
+ activating a controller.
+
+ }
+ />
);
};
return (
- }
- else={
- <>
- } />
-
- >
- }
- />
+ <>
+
+
+ >
);
};
diff --git a/web/src/components/storage/ZFCPPage.test.jsx b/web/src/components/storage/ZFCPPage.test.jsx
index c96a701a5b..b42274aa52 100644
--- a/web/src/components/storage/ZFCPPage.test.jsx
+++ b/web/src/components/storage/ZFCPPage.test.jsx
@@ -98,7 +98,7 @@ describe("if allow-lun-scan is activated", () => {
it("renders an explanation about allow-lun-scan", async () => {
installerRender();
- await screen.findByText(/is configured to perform automatic LUN scan/);
+ await screen.findByText(/automatically configures all its LUNs/);
});
});
@@ -107,10 +107,10 @@ describe("if allow-lun-scan is not activated", () => {
client.getAllowLUNScan = jest.fn().mockResolvedValue(false);
});
- it("does not render an explanation about allow-lun-scan", async () => {
+ it("renders an explanation about not using allow-lun-scan", async () => {
installerRender();
- await waitFor(() => expect(screen.queryByText(/is configured to perform automatic LUN scan/)).toBeNull());
+ await screen.findByText(/LUNs have to be manually configured/);
});
});