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

consolidated s390 device configuration #1162

Closed
wants to merge 7 commits into from

Conversation

steffen-maier
Copy link
Contributor

@steffen-maier steffen-maier commented Oct 13, 2023

Consolidate the persistent and dynamic configuration of s390-specific devices in Linux distributions by delegating the configuration to the existing framework zdev from s390-tools.

This pull request prepares consolidated s390 device configuration in anaconda.
The top commit [see its description] depends on a certain commit from ibm-s390-linux/s390-tools#158 and thus https://github.com/ibm-s390-linux/s390-tools/releases/tag/v2.31.0.

Zdev's job is to perform low-level configuration after which the user gets architecture-independent objects such as block devices, or SCSI devices. Those can and should in turn be configured with existing common code mechanisms. So there's a clear separated layering for configuration duties.

In particular, the s390-specific devices currently are: DASD (traditional disk), and ZFCP (scsi). Zdev has a stable command line user interface and abstracts from sysfs and from a persistent configuration representation. Zdev encapsulates configuration details. Systems management code can simply delegate configuration to zdev and thus reduce architecture-specific code.

This improves user experience, serviceability, maintainability, and reduces test effort.

@jstodola @poncovka @sharkcz

Even though this is a draft pull request, I would appreciate review comments.
It's only a draft until we sorted out the dependencies and merge order of pull requests for different related projects.

The old existing test preceding the removed code was only designed for the
old zfcp before it got automatic LUN scan. Hence, the test is incomplete.
With zfcp auto LUN scan, zfcp can just have SCSI devices without any
zfcp unit representation in sysfs.
Do not bother cleaning up an unused FCP device and just remove the code.

Note: Do not confuse zfcp auto port scan with zfcp auto LUN scan.

Signed-off-by: Steffen Maier <[email protected]>
Gone since 2008 Linux kernel v2.6.27 commit 235f7f25f492 ("[SCSI] zfcp:
Remove sysfs attribute port_add").

Signed-off-by: Steffen Maier <[email protected]>
@StorageGhoul
Copy link

Can one of the admins verify this patch?

@vojtechtrefny
Copy link
Member

Jenkins, ok to test.

@steffen-maier
Copy link
Contributor Author

This reminds me of a similar chicken and egg problem like rhinstaller/anaconda#5249 (comment).
How to untangle the dependencies?:

2181a7e#diff-1aea9ad79758b521c7bab07267b2e2893723c4eddd2ea591e912742ec25e3be5L531
removes DASDDevice.opts here in blivet
but the anaconda unit tests (which I can hardly update here in a blivet PR ?) running here in the blivet git workflow now complain because they still expect old blivet code:

=================================== FAILURES ===================================
_________________ DASDInterfaceTestCase.test_find_formattable __________________
unit_tests/pyanaconda_tests/modules/storage/test_module_dasd.py:82:
E TypeError: DiskDevice.init() got an unexpected keyword argument 'opts'

____________ DeviceTreeInterfaceTestCase.test_get_dasd_device_data _____________
unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py:168:
E TypeError: DiskDevice.init() got an unexpected keyword argument 'opts'

Copy link
Member

@vojtechtrefny vojtechtrefny left a comment

Choose a reason for hiding this comment

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

I am not an s390 expert, but this looks good to me in general, thank you.

@vojtechtrefny
Copy link
Member

You can ignore the ci/x86_64/f_37 CI job failure, that's a problem in our CI configuration.

@vojtechtrefny
Copy link
Member

This reminds me of a similar chicken and egg problem like rhinstaller/anaconda#5249 (comment). How to untangle the dependencies?:

I also answered in the Anaconda PR, but short version is that we will merge the Blivet change even with the Anaconda unit tests failing here in our CI and then make sure the Anaconda PR works with the latest Blivet build and also later make sure to release Blivet and Anaconda together.

steffen-maier added a commit to steffen-maier/anaconda that referenced this pull request Nov 15, 2023
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
steffen-maier added a commit to steffen-maier/s390-tools that referenced this pull request Nov 28, 2023
Converts zdev configuration into the syntax of the kernel module parameter
dasd_mod.dasd. Only DASD options with non-default values are emitted. The
result string occurs on stdout. It represents one device-specification for
the given DASD device bus-ID.

Example:
/lib/s390-tools/zdev-to-dasd_mod.dasd persistent 0.0.da5d
0.0.da5d(erplog)

User:
storaged-project/blivet#1162
to generate dracut cmdline entries such as rd.dasd=0.0.da5d(erplog)

Signed-off-by: Steffen Maier <[email protected]>
hoeppnerj pushed a commit to ibm-s390-linux/s390-tools that referenced this pull request Dec 14, 2023
Converts zdev configuration into the syntax of the kernel module parameter
dasd_mod.dasd. Only DASD options with non-default values are emitted. The
result string occurs on stdout. It represents one device-specification for
the given DASD device bus-ID.

Example:
/lib/s390-tools/zdev-to-dasd_mod.dasd persistent 0.0.da5d
0.0.da5d(erplog)

User:
storaged-project/blivet#1162
to generate dracut cmdline entries such as rd.dasd=0.0.da5d(erplog)

Github-ID: #158
Acked-by: Vineeth Vijayan <[email protected]>
Acked-by: Peter Oberparleiter <[email protected]>
Signed-off-by: Steffen Maier <[email protected]>
Signed-off-by: Jan Höppner <[email protected]>
@steffen-maier
Copy link
Contributor Author

Just updated the commit descriptions with the actual upstream commits since ibm-s390-linux/s390-tools#158 got merged.
Added explicit package dependency in spec file, to model the dependencies (similar to rhinstaller/anaconda#5250 (review)).
No code changes.

steffen-maier added a commit to steffen-maier/anaconda that referenced this pull request Jan 11, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
steffen-maier added a commit to steffen-maier/anaconda that referenced this pull request Jan 11, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
steffen-maier added a commit to steffen-maier/anaconda that referenced this pull request Feb 29, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
…(#1802482,#1937049)

Implements the zfcp part of referenced bugs.

https://github.com/ibm-s390-linux/s390-tools/tree/master/zdev/
handles everything as of
ibm-s390-linux/s390-tools@06a30ae
("zdev/dracut: add rd.zfcp cmdline option handling").

It is no longer necessary to perform individual pre-req steps, such as
setting an FCP device online, when we want to attach a LUN. Just call
chzdev to configure zfcp LUNs and let it do what is necessary, including
cio_ignore handling and udev settle.

The spec file update reflects the new dependency on `chzdev` from the
s390 architecture specific sub-package s390utils-core. Actually, this
commit here only depends on `chzdev` in older versions already packaged
and shipped, so no version comparison necessary here.

Since chzdev now implicitly sets the FCP device online
and there is no more preceding explicit FCP device online,
move the path over-specification warning after the call to chzdev.
Otherwise, the FCP device could still be offline and its
port_type unknown, so has_auto_lun_scan() would get wrong information
regarding the port_type being NPIV.

Anaconda handles the persistent config of all s390 device types as of
commit ("write persistent config of any (dasd,zfcp,znet) s390 devices to
sysroot"), so drop the special handling in zfcp.write().

Signed-off-by: Steffen Maier <[email protected]>
…2,#1937049)

Implements the zfcp part of referenced bugs.

Since
rhinstaller/anaconda@87ab1ab
("Support cio_ignore functionality for zFCP devices (#533492)"),
/etc/zfcp.conf replaced /tmp/fcpconfig.

Since
rhinstaller/anaconda@011ea0a
("Remove linuxrc.s390"), /etc/zfcp.conf only exists if the user specified
dracut cmdline parameter rd.zfcp=.

https://github.com/ibm-s390-linux/s390-tools/tree/master/zdev/
handles parsing of rd.zfcp= without /etc/zfcp.conf as of
ibm-s390-linux/s390-tools@06a30ae
("zdev/dracut: add rd.zfcp cmdline option handling").

https://src.fedoraproject.org/rpms/s390utils.git
no longer writes /etc/zfcp.conf during deprecated parsing of rd.zfcp=
as of commit
("zfcp: migrate to consolidated persistent device config with zdev")

Hence, nothing populates /etc/zfcp.conf during installer boot anymore.

Anaconda imports configuration for all s390 device types as of
commit ("write persistent config of any (dasd,zfcp,znet) s390 devices to
sysroot"). The only remaining import source is from dracut boot parameters.

Signed-off-by: Steffen Maier <[email protected]>
Complements RHBZ#1937030.

Signed-off-by: Steffen Maier <[email protected]>
…2,#1937049)

Implements the dasd part of referenced bugs.

Depends on
ibm-s390-linux/s390-tools@689b894
("zdev: add helper to convert from zdev config to dasd_mod.dasd").
The spec file update reflects the new dependency on `zdev-to-dasd_mod.dasd`
in the new v2.31.0 of the s390 architecture specific sub-package
s390utils-core.

Delegate the generation of rd.dasd statements to a helper tool from
s390-tools, which gets its low-level config information from the
consolidated persistent configuration mechanism using chzdev.

Signed-off-by: Steffen Maier <[email protected]>
@steffen-maier
Copy link
Contributor Author

Just updated the commit descriptions with the actual upstream commits since ibm-s390-linux/s390-tools#158 got merged. Added explicit package dependency in spec file, to model the dependencies (similar to rhinstaller/anaconda#5250 (review)). No code changes.

Now the s390-tools upstream version v2.31.0 is known and I updated the commit touching the spec file ("DASDDevice: dracut_setup_args() without deprecated dasd.conf (#1802482,#1937049)").
While at it, replaced all references to commits on other packages with full URLs instead of just package name and short git commit ID (idea from dracutdevs/dracut#2534 (comment)).

rvykydal pushed a commit to rvykydal/anaconda that referenced this pull request May 27, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
rvykydal pushed a commit to rvykydal/anaconda that referenced this pull request Jul 16, 2024
Resolves: RHEL-24057

storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
rvykydal pushed a commit to rvykydal/anaconda that referenced this pull request Jul 16, 2024
Resolves: RHEL-24057

storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
@vojtechtrefny
Copy link
Member

Replaced by #1259

steffen-maier added a commit to steffen-maier/anaconda that referenced this pull request Sep 25, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
rvykydal pushed a commit to rvykydal/anaconda that referenced this pull request Oct 9, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
steffen-maier added a commit to steffen-maier/anaconda that referenced this pull request Oct 10, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Signed-off-by: Steffen Maier <[email protected]>
KKoukiou pushed a commit to KKoukiou/anaconda that referenced this pull request Oct 25, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Cherry-picked from master commit: 9e3cb46

Signed-off-by: Steffen Maier <[email protected]>
KKoukiou pushed a commit to KKoukiou/anaconda that referenced this pull request Oct 25, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Cherry-picked from master commit: 9e3cb46

Signed-off-by: Steffen Maier <[email protected]>
KKoukiou pushed a commit to KKoukiou/anaconda that referenced this pull request Oct 25, 2024
storaged-project/blivet#1162 (comment)
removes DASDDevice.opts. Anticipating that blivet change, update
the anaconda unit tests making use of DASDDevice.

Cherry-picked from master commit: 9e3cb46

Signed-off-by: Steffen Maier <[email protected]>
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