Skip to content

zFCP D-Bus API#594

Merged
joseivanlopez merged 11 commits intoagama-project:masterfrom
joseivanlopez:dbus-zfcp
Jun 13, 2023
Merged

zFCP D-Bus API#594
joseivanlopez merged 11 commits intoagama-project:masterfrom
joseivanlopez:dbus-zfcp

Conversation

@joseivanlopez
Copy link
Copy Markdown
Contributor

@joseivanlopez joseivanlopez commented May 29, 2023

Problem

The storage service already provides a D-Bus API for managing iSCSI and DASD devices, but zFCP is not supported yet.

Solution

Add D-Bus API for managing zFCP devices (requires yast/yast-s390#105).

Show API
s390vsl111:~ # 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/zfcp_controllers
        │ ├─/org/opensuse/Agama/Storage1/zfcp_controllers/1
        │ └─/org/opensuse/Agama/Storage1/zfcp_controllers/2
        └─/org/opensuse/Agama/Storage1/zfcp_disks
          ├─/org/opensuse/Agama/Storage1/zfcp_disks/1
          └─/org/opensuse/Agama/Storage1/zfcp_disks/2

s390vsl111:~ # busctl --address unix:path=/run/agama/bus introspect org.opensuse.Agama.Storage1 /org/opensuse/Agama/Storage1 org.opensuse.Agama.Storage1.ZFCP.Manager
NAME                                     TYPE      SIGNATURE RESULT/VALUE FLAGS
.Probe                                   method    -         -            -

s390vsl111:~ # busctl --address unix:path=/run/agama/bus introspect org.opensuse.Agama.Storage1 /org/opensuse/Agama/Storage1/zfcp_controllers/1 org.opensuse.Agama.Storage1.ZFCP.Controller
NAME                                        TYPE      SIGNATURE RESULT/VALUE FLAGS
.Activate                                   method    -         u            -
.ActivateDisk                               method    ss        u            -
.DeactivateDisk                             method    ss        u            -
.GetLUNs                                    method    s         as           -
.GetWWPNs                                   method    -         as           -
.Active                                     property  b         true         emits-change
.Channel                                    property  s         "0.0.fa00"   emits-change

s390vsl111:~ # busctl --address unix:path=/run/agama/bus introspect org.opensuse.Agama.Storage1 /org/opensuse/Agama/Storage1/zfcp_disks/1 org.opensuse.Agama.Storage1.ZFCP.Disk
NAME                                  TYPE      SIGNATURE RESULT/VALUE         FLAGS
.Channel                              property  s         "0.0.fa00"           emits-change
.LUN                                  property  s         "0x0005000000000000" emits-change
.Name                                 property  s         "/dev/sdb"           emits-change
.WWPN                                 property  s         "0x500507630708d3b3" emits-change

Follow-up:

zFCP controller deactivation

Directly deactivating a zFCP controller (e.g. chzdev -d 0.0.fa00) does not deactive its LUNs immediately. It takes some time (minutes) until its LUNs are deactivated (independently on the auto-lun-scan option).

That behaviour has some impact in YaST and Agama. For example:

  • If a storage probing is done before the zFCP disks are deactivated, then the zFCP disks will be available for the installation.
  • If zFCP probing is done before the zFCP disks are deactivated, then the zFCP disks will be visible in the D-Bus tree.

The YaST module for managing zFCP devices does not offer an option for deactivating a zFCP controller. For now, the Agama D-Bus API will not offer it neither. Lacking of a deativate option has some implications if auto-lun-scan is active. With auto-lun-scan, the zFCP disks cannot be deactivated. So, after activating a controller, there is no way of undoing the disks activation.

Testing

  • Added new unit tests
  • Tested manually

@joseivanlopez joseivanlopez force-pushed the dbus-zfcp branch 2 times, most recently from fa373b7 to b7d4937 Compare May 29, 2023 13:14
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 2, 2023

Pull Request Test Coverage Report for Build 5245607799

  • 276 of 297 (92.93%) changed or added relevant lines in 12 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.6%) to 76.826%

Changes Missing Coverage Covered Lines Changed/Added Lines %
service/lib/agama/dbus/storage/interfaces/dasd_manager.rb 3 4 75.0%
service/lib/agama/dbus/storage/zfcp_controller.rb 42 44 95.45%
service/lib/agama/dbus/base_tree.rb 39 42 92.86%
service/lib/agama/dbus/storage/interfaces/zfcp_manager.rb 21 27 77.78%
service/lib/agama/storage/zfcp.rb 0 9 0.0%
Files with Coverage Reduction New Missed Lines %
service/lib/agama/dbus/storage/manager.rb 1 81.94%
Totals Coverage Status
Change from base Build 5244994145: 0.6%
Covered Lines: 5626
Relevant Lines: 7080

💛 - Coveralls

Copy link
Copy Markdown
Contributor

@mvidner mvidner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just nitpicks so far

@mvidner

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@mvidner mvidner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is about the specific info that I miss in the API docs.

BTW, writing it in the ruby code where I commented it will unfortunately not make it visible at https://opensuse.github.io/agama/dbus/ , maybe I should add some dbus_annotation DSL to ruby-dbus (or even YARDoc comment conversion?)

@joseivanlopez joseivanlopez force-pushed the dbus-zfcp branch 2 times, most recently from d658a37 to 717906f Compare June 12, 2023 15:21
@joseivanlopez
Copy link
Copy Markdown
Contributor Author

This part is about the specific info that I miss in the API docs.

BTW, writing it in the ruby code where I commented it will unfortunately not make it visible at https://opensuse.github.io/agama/dbus/ , maybe I should add some dbus_annotation DSL to ruby-dbus (or even YARDoc comment conversion?)

Yes, ideally I would like to write the documentation directly in the ruby code. Right now there is some duplication because I write some doc in ruby code and also in *.doc.xml. And sometimes I am not sure what to write in one place and the other.

Copy link
Copy Markdown
Contributor

@mvidner mvidner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, LGTM!

@joseivanlopez joseivanlopez merged commit 46b1873 into agama-project:master Jun 13, 2023
This was referenced Jun 20, 2023
@joseivanlopez joseivanlopez mentioned this pull request Jul 3, 2023
@imobachgs imobachgs mentioned this pull request Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants