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

Use wwn attr instead of removed wwid. (#1565693) #1503

Merged
merged 1 commit into from
Aug 13, 2018

Conversation

dwlehman
Copy link
Contributor

@dwlehman dwlehman commented Jun 8, 2018

The 'wwid' attribute was removed in blivet-3.1.0. Now all disk-like
devices have a 'wwn' attribute that gets its value from udev's
ID_WWN_WITH_EXTENSION property.

This supercedes #1499 and also fixes the display formatting issue contained therein.

The 'wwid' attribute was removed in blivet-3.1.0. Now all disk-like
devices have a 'wwn' attribute that gets its value from udev's
ID_WWN_WITH_EXTENSION property.
Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@jkonecny12
Copy link
Member

jenkins, test this please

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me too.

@jkonecny12
Copy link
Member

It looks like it needs a little bit more polishing.
iscsi-multipath-bug

@jkonecny12
Copy link
Member

Also I'm not sure if this is happening because of my settings but when I'm trying to attach 1 disk from 2 different ip addresses there is no wwn value showed. Do you know why it is not there?

Also I think we need to change tab name to WWN when you look on that screenshot there is WWID.

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

We need to solve that crash above.

@jkonecny12 jkonecny12 added master Please, use the `f39` label instead. manual testing required This issue can't be merged without manual testing labels Jun 11, 2018
@dwlehman dwlehman dismissed jkonecny12’s stale review June 29, 2018 17:07

Crash should be resolved.

@jkonecny12 jkonecny12 self-assigned this Jul 2, 2018
@jkonecny12
Copy link
Member

This looks correct, however it looks that there is a bug in blivet. Based on my tests the wwn attribute is empty for multipath device.

New wwn:
screen_bad_wwn

Old wwid:
screen_good_wwn

I was looking into that in debugger and the disk.wwn is really empty. This is causing crash when I select mpath device and clicking on back button.

crash

Another question is if the WWID column title in the table shouldn't change to WWN too?

@jkonecny12
Copy link
Member

Ohh I see you already have PR for that. Then please leave here a note when new blivet with this fix will be released.

@dwlehman
Copy link
Contributor Author

I'm planning to build blivet-3.1.0.b2 (storaged-project/blivet@cd698c04) on Tuesday (tomorrow) for rawhide. I don't have firm plans for any other builds as of now.

@jkonecny12
Copy link
Member

OK thanks, I will test and merge this tomorrow.

@rvykydal
Copy link
Contributor

rvykydal commented Aug 8, 2018

I've tested the PR with latest rawhide Fedora-Rawhide-20180802.n.0 and it seem to be good.
Once the patches mentioned above for blivet (storaged-project/blivet#701) are built for rhel we can merge the PR.
wwn

@rvykydal rvykydal merged commit 36dcf7f into rhinstaller:modularization-devel Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
manual testing required This issue can't be merged without manual testing master Please, use the `f39` label instead.
Development

Successfully merging this pull request may close these issues.

4 participants