onpanic: add support for multipathed zfcp-attached SCSI disks #108

steffen-maier · 2023-10-16T11:14:43Z

Depends on https://build.opensuse.org/package/show/Base:System/s390-tools commit ("mkdump: add support for multipathed zfcp-attached SCSI disks") [patch 7 in SUSE bug 1216257].

Problem

Users should use multipathing for all zfcp-attached SCSI disks. Since a long time, zipl can write the partition-based zfcpdump standalone dumper boot record to a multipath device. This avoids users having to flush multipath maps or maintain a multipath exception list just for zfcp dump volumes. Always having multipathing for everything also provides redundant access to the dump volume on importing the dump from the volume into a filesystem after the standalone dumper had written the dump.

Solution

An updated mkdump.pl from SUSE s390-tools understands and lists such multipath devices. Make "yast onpanic" understand /dev/mapper/... multipath devices as better alternative to single path SCSI disks /dev/sd[a-z]+.

Fixes SUSE bug 1020336.

Testing

Tested manually

Screenshots

@jiribohac @hramrach @hreinecke @mwilck @teclator

steffen-maier · 2023-12-06T14:17:15Z

Friendly ping.

Depends on https://build.opensuse.org/package/show/Base:System/s390-tools commit ("mkdump: add support for multipathed zfcp-attached SCSI disks") [patch 7 in SUSE bug 1216257].

I think @ngueorguiev applied the dependencies and they are waiting for acceptance.

Fixes SUSE bug 1020336.

@jiribohac @hramrach @hreinecke @mwilck @teclator

Looks like I need "at least 1 approving review is required by reviewers with write access" as merge pre-req.
Not sure who possible reviewers with such access would be, so I try to mention some who did merge/commit lately:

@joseivanlopez @jreidinger @lslezak @shundhammer @dgdavid

and from https://github.com/yast/yast-s390/blob/master/MAINTAINER:

@aschnell

and from https://github.com/yast/yast.github.io/wiki/The-YaST-Team:

@jsrain @wfeldt

Apologies for the mentioning list.

Seems I'm not allowed to request a PR review from someone specific (because I'm not owner and do not have write access).

joseivanlopez

Thanks for this PR. Please, add a changelog entry and bump the version.

teclator · 2023-12-12T13:29:56Z

@steffen-maier just fixing it for SLE-15-SP6 is good enough?

steffen-maier · 2023-12-12T15:11:39Z

Thanks for this PR. Please, add a changelog entry and bump the version.

I looked at prior art in git log history and https://yastgithubio.readthedocs.io/en/latest/versioning/#new-schema-version-numbers-are-related-to-suse-versions and https://yastgithubio.readthedocs.io/en/latest/contributing/#code-changes. This is all new to me; hopefully I got it right.

just fixing it for SLE-15-SP6 is good enough?

Primarily it was meant for a new major release such as SLE-16, but backporting to SLE-15-SP6 would also be OK (if stated s390-tools requirements are fulfilled there) and would be sufficient.

Depends on https://build.opensuse.org/package/show/Base:System/s390-tools commit ("mkdump: add support for multipathed zfcp-attached SCSI disks") [patch 7 in SUSE bug 1216257].

joseivanlopez

LGTM

coveralls · 2023-12-13T11:25:58Z

coverage: 21.975%. remained the same
when pulling cbf00c6 on steffen-maier:zdev
into e44c38c on yast:master.

dgdavid · 2023-12-13T11:26:23Z

Thanks for this PR. Please, add a changelog entry and bump the version.

I looked at prior art in git log history and https://yastgithubio.readthedocs.io/en/latest/versioning/#new-schema-version-numbers-are-related-to-suse-versions and https://yastgithubio.readthedocs.io/en/latest/contributing/#code-changes. This is all new to me; hopefully I got it right.

Thanks a lot! 💯

steffen-maier · 2023-12-13T13:24:06Z

I suppose the Rubocop test fail is not due to my code change?:

Run rake check:rubocop
/usr/bin/rubocop.ruby3.2-0.71.0
/usr/bin/rubocop.ruby3.2-0.71.0 -P 
wrong number of arguments (given 2, expected 1)
/usr/lib64/ruby/3.2.0/psych.rb:398:in `parse'
/usr/lib64/ruby/gems/3.2.0/gems/rubocop-0.71.0/lib/rubocop/yaml_duplication_checker.rb:8:in `check'
/usr/lib64/ruby/gems/3.2.0/gems/rubocop-0.71.0/lib/rubocop/config_loader.rb:211:in `check_duplication'
/usr/lib64/ruby/gems/3.2.0/gems/rubocop-0.71.0/lib/rubocop/config_loader.rb:197:in `load_yaml_configuration'

jreidinger · 2023-12-13T16:03:59Z

@steffen-maier nope, it is issue with yast2-s390 and adapting to the latest ruby version. BTW master is for TW only. Do you want change in SLE15 or master only? ( btw for more details it is caused by API incompatible change in yaml processing and need specific wrapper to support multiple ruby versions )

steffen-maier · 2023-12-13T16:22:31Z

David changed base branch to SLE-15-SP6, which is fine with me, so I'll rebase to that and force push to allow a clean merge against that new target.

steffen-maier · 2023-12-13T16:28:48Z

Are you going to somehow bring the change into master as well?

steffen-maier · 2023-12-13T16:37:00Z

BTW master is for TW only.

What does "TW" mean?

Do you want change in SLE15 or master only?

My original thought was master (hoping this means SLE-16)
and optionally also in SLE-15-SP6

dgdavid · 2023-12-13T16:42:06Z

@steffen-maier

TW means Tumbleweed. Now master branch is for openSUSE Tumbleweed while SLE-15-SP6 branch is for SUSE Linux Enterprise 15 Service Pack 6. A bit messy, sorry.

As you can see, I've changed the target branch to SP6 one based on your comment above (#108 (comment))

Now I'm solving the conflicts in the changes file. I'll let you know when it's ready. Sorry for the inconvenience on getting this merge.

Depends on https://build.opensuse.org/package/show/Base:System/s390-tools commit ("mkdump: add support for multipathed zfcp-attached SCSI disks") from SUSE bug 1216257. Users should use multipathing for all zfcp-attached SCSI disks. Since a long time, zipl can write the partition-based zfcpdump standalone dumper boot record to a multipath device. This avoids users having to flush multipath maps or maintain a multipath exception list just for zfcp dump volumes. Always having multipathing for everything also provides redundant access to the dump volume on importing the dump from the volume into a filesystem after the standalone dumper had written the dump. An updated mkdump.pl from SUSE s390-tools understands and lists such multipath devices. Make "yast onpanic" understand /dev/mapper/... multipath devices as better alternative to single path SCSI disks /dev/sd[a-z]+. Fixes SUSE bug 1020336. Signed-off-by: Steffen Maier <[email protected]>

Signed-off-by: Steffen Maier <[email protected]>

dgdavid · 2023-12-13T17:32:51Z

I really sorry for making a push --force here, but I had no other option 😅

Now I think everything is in the place. Please, be aware that I had to send your changes on top of the SLE-15-SP6 branch, for which I did a git format-patch of your commits and then a git am.

That means that, unless I'm wrong, there are few commits from master that are no part of the SLE-15-SP6, namely

If you don't mind, please check that your changes still working as you expected.

Please, bear in mind that usually is not as difficult to send a contribution to YaST codebase 😭 You followed the right steps for a normal contribution. The thing is that with SP6 things has changed and, as said above, it does not follow the master branch anymore.

@jreidinger and @lslezak can explain it better.

dgdavid · 2023-12-13T17:39:25Z

@steffen-maier also excuse me because I was working on it and didn't realize that you did a force-push as soon as I changed the target branch. That was so quick!

jreidinger · 2023-12-13T18:26:48Z

Are you going to somehow bring the change into master as well?

I guess we can backport it if needed.

It is not backport, but it is called merge and it follows are code conventions -> apply to the oldest applicable code stream and then merge fix to all newer ones

jreidinger

just please use version 4.6.5 to avoid collision of previous TW releases as can be seen in https://github.com/yast/yast-s390/blob/master/package/yast2-s390.changes

To avoid collisions with changes already send to master branch as 4.6.0, 4.6.1, 4.6.2, 4.6.3, and 4.6.4.

dgdavid · 2023-12-13T18:49:27Z

After a short talk with @jreidinger in the IRC channel, I've sent a commit updating version to 4.6.5 to avoid collision with master branch 🤷‍♂️ 🤷‍♂️

Hope @jreidinger don't mind, quoting his explanation below

<jreidinger_> dgdavid: well, version should not be 4.6.1 for sure as it will collide. It should be 4.6.5, but I worry that we are missing there also some backports from master. Looks like I overlook that s390 module when doing backports.
...
<jreidinger_> dgdavid: or use 4.6.5 and merge it and I will do another backport and bump tomorrow. Just remind me :)
<jreidinger_> dgdavid: ok, I double check what is backported and it is correct as changes done by jilopez is agama only and HuHa fix is already there just under different version. So just use version 4.6.5 and everything is correct

jreidinger · 2023-12-13T21:03:33Z

thanks, so now it is ready for merge

dgdavid · 2023-12-14T11:09:21Z

I'm going to merge it as soon as @steffen-maier can confirm everything is in place after my push force.

steffen-maier · 2023-12-14T11:25:15Z

I really sorry for making a push --force here, but I had no other option 😅
Now I think everything is in the place. Please, be aware that I had to send your changes on top of the SLE-15-SP6 branch, for which I did a git format-patch of your commits and then a git am.

That means that, unless I'm wrong, there are few commits from master that are no part of the SLE-15-SP6, namely

Prevent Crash if Closing the DASD Context Menu without Action [SLE-15-SP5] #103

zFCP refactoring needed for Agama #105

Fix zFCP runnning mode detection #107

Cannot comment on those as I don't know anything about their origin.

If you don't mind, please check that your changes still working as you expected.

f7f7813 still looks like my original and thus correct. The other two patches for changelog and version bump also look good to me. I think this can be merged.

yast-bot · 2023-12-14T11:45:54Z

✔️ Internal Jenkins job #3 successfully finished
✔️ Created IBS submit request #315712

joseivanlopez reviewed Dec 12, 2023

View reviewed changes

steffen-maier force-pushed the zdev branch from dbdd9d5 to cbf00c6 Compare December 12, 2023 15:10

joseivanlopez approved these changes Dec 13, 2023

View reviewed changes

dgdavid changed the base branch from master to SLE-15-SP6 December 13, 2023 16:11

steffen-maier force-pushed the zdev branch from cbf00c6 to 683fd7f Compare December 13, 2023 16:31

steffen-maier added 2 commits December 13, 2023 17:01

Version and changelog

e9f264b

Signed-off-by: Steffen Maier <[email protected]>

dgdavid force-pushed the zdev branch 2 times, most recently from cbf00c6 to e9f264b Compare December 13, 2023 17:18

This comment was marked as outdated.

Sign in to view

jreidinger approved these changes Dec 13, 2023

View reviewed changes

jreidinger requested changes Dec 13, 2023

View reviewed changes

jreidinger approved these changes Dec 13, 2023

View reviewed changes

Fix version bumping it to 4.6.5

68c51e2

To avoid collisions with changes already send to master branch as 4.6.0, 4.6.1, 4.6.2, 4.6.3, and 4.6.4.

dgdavid merged commit 1a20b8f into yast:SLE-15-SP6 Dec 14, 2023
5 checks passed

jreidinger mentioned this pull request Mar 6, 2024

Mkinitrd removal master #111

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

onpanic: add support for multipathed zfcp-attached SCSI disks #108

onpanic: add support for multipathed zfcp-attached SCSI disks #108

steffen-maier commented Oct 16, 2023

steffen-maier commented Dec 6, 2023

joseivanlopez left a comment

teclator commented Dec 12, 2023

steffen-maier commented Dec 12, 2023

joseivanlopez left a comment

coveralls commented Dec 13, 2023

dgdavid commented Dec 13, 2023

steffen-maier commented Dec 13, 2023

jreidinger commented Dec 13, 2023

steffen-maier commented Dec 13, 2023

steffen-maier commented Dec 13, 2023

steffen-maier commented Dec 13, 2023

dgdavid commented Dec 13, 2023 •

edited

Loading

dgdavid commented Dec 13, 2023 •

edited

Loading

dgdavid commented Dec 13, 2023

This comment was marked as outdated.

jreidinger commented Dec 13, 2023

jreidinger left a comment

dgdavid commented Dec 13, 2023 •

edited

Loading

jreidinger commented Dec 13, 2023

dgdavid commented Dec 14, 2023

steffen-maier commented Dec 14, 2023

yast-bot commented Dec 14, 2023

onpanic: add support for multipathed zfcp-attached SCSI disks #108

onpanic: add support for multipathed zfcp-attached SCSI disks #108

Conversation

steffen-maier commented Oct 16, 2023

Problem

Solution

Testing

Screenshots

steffen-maier commented Dec 6, 2023

joseivanlopez left a comment

Choose a reason for hiding this comment

teclator commented Dec 12, 2023

steffen-maier commented Dec 12, 2023

joseivanlopez left a comment

Choose a reason for hiding this comment

coveralls commented Dec 13, 2023

dgdavid commented Dec 13, 2023

steffen-maier commented Dec 13, 2023

jreidinger commented Dec 13, 2023

steffen-maier commented Dec 13, 2023

steffen-maier commented Dec 13, 2023

steffen-maier commented Dec 13, 2023

dgdavid commented Dec 13, 2023 • edited Loading

dgdavid commented Dec 13, 2023 • edited Loading

dgdavid commented Dec 13, 2023

This comment was marked as outdated.

jreidinger commented Dec 13, 2023

jreidinger left a comment

Choose a reason for hiding this comment

dgdavid commented Dec 13, 2023 • edited Loading

jreidinger commented Dec 13, 2023

dgdavid commented Dec 14, 2023

steffen-maier commented Dec 14, 2023

yast-bot commented Dec 14, 2023

dgdavid commented Dec 13, 2023 •

edited

Loading

dgdavid commented Dec 13, 2023 •

edited

Loading

dgdavid commented Dec 13, 2023 •

edited

Loading