Open steffen-maier opened 10 months ago
Please submit the first two commits as a separate PR (or PRs). They make sense of their own and are not tied to the rest.
Some of the commits [see their description] depend on certain commits from ibm-s390-linux/s390-tools#158.
was merged, so now turning this into a regular pull request
rebased onto latest master and force pushed to resolve the merge conflicts, which meanwhile appeared due to upstream commit de8ac6300d11 ("fix(github): update format of labeler")
Not a matter for this PR, but note that NetworkManager has dropped support for the ifcfg
format, therefore the cms
module will have to be ported off it eventually.
Not a matter for this PR, but note that NetworkManager has dropped support for the
ifcfg
format, therefore thecms
module will have to be ported off it eventually.
I'm struggling to understand the ifcfg vs. nm-connection in the current existing code of module 80cms. Even though I tested it with the code of this PR here, I did not rd.debug it to understand which code path it actually took. It "just worked" for me. https://github.com/dracutdevs/dracut/blob/4980bad34775da715a2639b736cba5e65a8a2604/modules.d/80cms/cmsifup.sh#L26-L43 It does look like there is a conditional branch to already use nm-connections, if nm-initrd-generator is available, and to me it looks like it delegates higher-level network-interface config to dracut cmdline option ip= (and nameserver=) so core dracut could handle this via nm-connections instead of ifcfg?
Not a matter for this PR, but note that NetworkManager has dropped support for the
ifcfg
format, therefore thecms
module will have to be ported off it eventually.I'm struggling to understand the ifcfg vs. nm-connection in the current existing code of module 80cms. Even though I tested it with the code of this PR here, I did not rd.debug it to understand which code path it actually took. It "just worked" for me.
It does look like there is a conditional branch to already use nm-connections, if nm-initrd-generator is available, and to me it looks like it delegates higher-level network-interface config to dracut cmdline option ip= (and nameserver=) so core dracut could handle this via nm-connections instead of ifcfg?
Eh. It seems my memory has failed me: I'd have sworn that cms
wrote ifcfg
files to configure the network. I should have checked...
Let's do it this way as I went over the patches in my review
OK bf9229cb refactor(cms): use zdev to simplify handling CMSDASD=... boot option
$(rpm -ql s390utils-base)
from inst_multiple
after refactoring??OK b91b45e9 refactor(cms): use consolidated zfcp config with zdev from s390-tools
OK 5a397f95 refactor(cms): use consolidated dasd config with zdev from s390-tools
OK c5cdbfec refactor(cms): use consolidated network config with zdev from s390-tools
OK 4cee5e38 feat(zfcp_rules): remove zfcp handling consolidated in s390-tools
OK 6e91a4bb refactor(zfcp): minimize zfcp handling consolidated in s390-tools
OK 1c628c82 feat(dasd_rules): remove dasd handling consolidated in s390-tools
OK 49398eb8 feat(dasd_mod): remove dasd handling consolidated in s390-tools
OK f5146389 refactor(dasd): minimize dasd handling consolidated in s390-tools
OK 33d7929c docs(dracut.cmdline): generalize description of rd.znet
?? ae3e6923 fix(znet): append to udev rules so each rd.znet_ifname is effective
ccw_ifname_end
label when multiple rules are copied into the file??OK 6bb660f0 feat(znet): use zdev for consolidated device configuration
OK 36e2cc6d feat(qeth_rules): remove qeth handling consolidated in 95znet
OK feb09a29 refactor(ifcfg): delete code duplication using iface_get_subchannels()
OK 255094b7 feat(ifcfg): minimize s390-specific network configuration aspects
OK cacaf073 feat(zfcp): finally remove zfcp handling consolidated in s390-tools OK c91ac8b6 feat(dasd): finally remove dasd handling consolidated in s390-tools
There is no single "upgrade point" in Fedora (or ELN), so the migration needs to be done via a regular package update. And this will need some coordination and setting versioned dependencies between the involved packages (dracut, s390utils, ...)
(I do have a git range-diff invocation, if the following text is too hard to read.)
During testing I realized that I had introduced a bug in 95znet/parse-ccw.sh.
Probably when I fixed all shellcheck reports and added missing double quotes after my function tests.
The "$*"
needs to be "$@"
, because otherwise:
Fixed up the commit messages (direct commit URL, and missing context) of:
OK bf9229c refactor(cms): use zdev to simplify handling CMSDASD=... boot option
- remove
$(rpm -ql s390utils-base)
frominst_multiple
after refactoring??
Inserted new commit 5: ("refactor(cms): remove now unnecessary inclusion of full s390utils-base")
Tested that dasd, zfcp, and znet still work.
OK 33d7929 docs(dracut.cmdline): generalize description of rd.znet
- perhaps replace "activates interface on s390" with "... interface on s390x platform"?
I felt more comfortable with appending "architecture" than "platform".
Additionally, I fixed up the commit description: The pre-req for the doc update is not a change in s390-tools, but the (meanwhile) preceding commit ("feat(znet): use zdev for consolidated device configuration") here in the dracut PR. For this, I moved previous commit 12 to position 11. So the order now is: 11: feat(znet): use zdev for consolidated device configuration 12: docs(dracut.cmdline): generalize description of rd.znet
?? ae3e692 fix(znet): append to udev rules so each rd.znet_ifname is effective
- hmm, wouldn't there be a conflict in the
ccw_ifname_end
label when multiple rules are copied into the file??
Indeed, thanks for catching this bug! This was the only commit, where I was sloppy in developing and did not really function test it.
Fixed by sanitizing the user input for ifname
(replacing non-word characters by underscore) and then using that as LABEL string to make the labels unique.
Tested successfully with two rd.znet and one rd.znet_ifname for each.
I moved this commit from previous position 11 to now 14, which is moving the bugfix to after the sequence of feature commits consolidating znet (11..12) and removing qeth_rules (13).
OK cacaf07 feat(zfcp): finally remove zfcp handling consolidated in s390-tools OK c91ac8b feat(dasd): finally remove dasd handling consolidated in s390-tools
- but those 2 must be a separate PR to be merged after we implement the migration via s390utils I think.
There is no single "upgrade point" in Fedora (or ELN), so the migration needs to be done via a regular package update. And this will need some coordination and setting versioned dependencies between the involved packages (dracut, s390utils, ...)
Understood.
Originally, 95dasd_mod/parse-dasd-mod.sh, 95zfcp/parse-zfcp.sh, and 95znet/parse-ccw.sh perform free from cio_ignore during dracut cmdline hook (and independently of any existing corresponding cmdline option on the kernel/dracut cmdline). This triggers actual device configuration by processing any /etc/dasd.conf and /etc/zfcp.conf which were copied into initrd.
The following commits had broken above behavior. I changed them to retain the cio_ignore handling so any /etc/dasd.conf or /etc/zfcp.conf copied from the root-fs into initrd still has the same effect (and only that; the actual dracut cmdline option parsing is already part of s390-tools since https://github.com/ibm-s390-linux/s390-tools/pull/158):
And I removed the last 2 commits you mentioned from this PR here, to move them to a new PR, which will intentionally be in draft mode to defer any merging until you give the go from a Fedora packaging point of view.
BTW: Strictly speaking, Fedora has been following a rule that anything in the (device) dependency tree to prepare for mounting the root-fs inside initrd needs to have rd.dasd, rd.zfcp, or rd.znet on the kernel cmdline, if there are corresponding s390 devices among the leaf nodes of the dependency tree. The anaconda installer (with the help of python-blivet) has been taking care of that since a long time for the initial Linux installation by means of dracut_setup_args(): rd.dasd, rd.zfcp, or rd.znet (some history). Hence, one could challenge any functionality depending on copying of /etc/dasd.conf or /etc/zfcp.conf into initrd and those copies having an effect during early initrd. Additionally, initrd should only activate required devices: https://github.com/dracutdevs/dracut#readme
"The initramfs has (basically) one purpose in life -- getting the rootfs mounted so that we can transition to the real rootfs. ... This helps to keep the time required in the initramfs as little as possible so that things like a 5 second boot aren't made impossible as a result of the very existence of an initramfs."
IMO, everything else should be activated after switch root during another udev cold plug. It's easier to debug those at that point in time when we already have the full root-fs with all debug tooling.
LaszloGombos requested a review from lnykryn 2023-10-30
Hello @lnykryn, I'm wondering how I can help getting the required two (green) approving reviews. With you being one of the required approvers, do @sharkcz's review comments with his "grey" approval and my corresponding subsequent code updates help? [1, 2] Can I do anything else?
minor clarifications on rd.zfcp
and rd.dasd
in man/dracut.cmdline.7.asc
Put temporary files under the subdir ${DRACUT_TMPDIR} in https://github.com/dracutdevs/dracut/pull/2534/commits/5fc66498de7072a10358f1873b23eb95ff9b3acd ("feat(znet): use zdev for consolidated device configuration"). Idea from a kdump dracut module (which additionally has its own subdir under $DRACUT_TMPDIR). Also used in the related https://github.com/ibm-s390-linux/s390-tools/commit/849aa5b1058399447d478b0e129f23219e9dc7f1 ("zdev/dracut: put temporary files under the subdir ${DRACUT_TMPDIR}").
@johannbg moving discussion on s390 dracut modules here:
Anyways back to the s390-tools PR's what's the current status of that and in which direction are the upstream 390 taking with regards to init systems, is it being generic or moving towards systemd only?
In case you refer to the merged https://github.com/ibm-s390-linux/s390-tools/pull/158 and the related pending https://github.com/dracutdevs/dracut/pull/2534 here, it is distro-independent and only uses udev. AFAIK, udev has always been the base for dracut so we should be good? I'm not aware of any dependencies on systemd (or any other init system) for low-level configuration of s390-specific devices.
That said I'm not particularly fond of "partial" upstream, with that I mean we should not support some aspects of 390 here upstream while other parts of it is being maintained in 390 upstream repo which will double the load on everyone involved and quadruple it for anykind of support issues.
Do you mean the fact, that currently handling of DASD and ZFCP is moving to s390-tools, but ZNET remains in dracut? [as in the "Changes" section of the description of #2534 here] Originally, I thought I would move ZNET also into s390-tools, but refrained from doing so because other dracut modules have dependencies on modules/95znet
, such as (these are the ones I'm aware of and were included in my function tests):
I suppose we could theoretically move znet into s390-tools upstream as well, if all affected downstream package maintainers are OK with adapting runtime package requirements on (a particular minimum level of a new future version of) s390-tools, so the znet dracut module exists at initrd build time when dracut module cms or the kdump module depend on znet. Would that be worth the effort? In no particular order: @dtardon ? @pvalena ? (dracut packaging), @coiby (kdump-utils being factored out of downstream kexec-tools), @rvykydal (just FYI because anaconda uses znet and 80cms) @sharkcz (s390utils downsteam packaging of s390-tools), @aafeijoo-suse @tblume (dracut packaging), @ngueorguiev (s390-tools packaging), @joseivanlopez (distro installer might depend on znet), @hoeppnerj @steffen-eiden @oberpar @vneethv (s390-tools upstream and the zdev dracut module in there).
Any other alternative ideas?
rebased to resolve merge conflict with just merged #2630
I suppose we could theoretically move znet into s390-tools upstream as well, if all affected downstream package maintainers are OK with adapting runtime package requirements on (a particular minimum level of a new future version of) s390-tools,
Doesn't this PR already require that?
so the znet dracut module exists at initrd build time when dracut module cms or the kdump module depend on znet. Would that be worth the effort?
Yes, it would.
Any other alternative ideas?
This would have to be tested, but I think it it'd not be a problem if both projects provided the module; they would just have to use a different leading number. (Because only the module with the lower number would be loaded, if I read the code correctly). Then, after some transitional period, the dracut znet
module could be dropped.
I suppose we could theoretically move znet into s390-tools upstream as well, if all affected downstream package maintainers are OK with adapting runtime package requirements on (a particular minimum level of a new future version of) s390-tools,
Doesn't this PR already require that?
so the znet dracut module exists at initrd build time when dracut module cms or the kdump module depend on znet. Would that be worth the effort?
Yes, it would.
Any other alternative ideas?
This would have to be tested, but I think it it'd not be a problem if both projects provided the module; they would just have to use a different leading number. (Because only the module with the lower number would be loaded, if I read the code correctly). Then, after some transitional period, the dracut
znet
module could be dropped.
I support that approach. We are already using chzdev from the s390-tools for creating the udev rules for network device activation instead of the dracut znet module in our latest versions @SUSE.
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 completes distribution independent s390 device configuration in initrds built by
dracut
. This also fixes a few known and newly discovered bugs in addition to the changes described below. Please see the commit descriptions for more bug fix information. Some of the commits [see their description] depend on certain commits from https://github.com/ibm-s390-linux/s390-tools/pull/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, SCSI devices, or network interfaces. 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), ZFCP (scsi), and ZNET representing channel-attached network (QETH incl. OSA and HiperSockets, LCS, CTC). 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.
@danimo @dtardon @haraldh @hramrach @hreinecke @jstodola @mwilck @sharkcz @smitterl @teclator
Changes
Dracut had 3 different plugin module sets to configure s390 devices. Those module sets partly competed with each other and partly contradicted each other such as activating too many devices for a memory-limited kdump environment.
dasd
,dasd_mod
,znet
,zfcp
.dasd_rules
,qeth_rules
,zfcp_rules
.Along with zdev as configuration backend, IBM introduced https://github.com/ibm-s390-linux/s390-tools/tree/master/zdev/dracut
zdev
,zdev-kdump
.This change consolidates the 3 dracut module sets into one single module set. The user syntax with rd.dasd, rd.znet, and rd.zfcp remains the same; only the backend implementation changes to zdev. Move
dasd
andzfcp
intozdev
of s390-tools (modules could alternatively exist in dracut but they are so tightly coupled with zdev that s390-tools seemed natural). It supports dracut features: hostonly, hostonly-cmdline, print-cmdline. Updateznet
andcms
(cmsdasd & cmsconffile feature used by anaconda installer) and keep them in dracut because other code depends on these modules.Checklist