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

network-ng: Added ctc connection config #916

Merged
merged 2 commits into from Aug 13, 2019

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Aug 5, 2019

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 64.845% when pulling 9d720bd on network-ng-add_ctc_connection into 625202b on network-ng.

@teclator teclator marked this pull request as ready for review August 11, 2019 09:32
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

In general, it looks good to me. My only concern is whether it makes sense to have a reader/writer which do not implement update_file or update_connection_config.

attr_accessor :write_channel
# @return [Integer] connection protocol (0, 1, 3, or 4)
# 0 Compatibility with peers other than OS/390®.
# 1 Enhanced package checking for Linux peers.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curiosity: where is the number "2"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably does not make sense in Linux. To be honest, No idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -23,6 +23,39 @@ module Y2Network
module ConnectionConfig
# Configuration for qeth connections
class Qeth < Base
# For CCW devices and CCW group devices, this device ID is the device
Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear, the "read channel" is the "device ID" you are talking about, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the device ID is for both, the read channel and write channel. That is for example "0.0.8000" is the read channel device id and "0.0.8001" is the write channel device id.

https://www.ibm.com/support/knowledgecenter/linuxonibm/com.ibm.linux.z.lgdd/lxctcrot.jpg
https://github.com/SUSE/s390-tools/blob/master/ctc_configure#L10

@teclator
Copy link
Contributor Author

In general, it looks good to me. My only concern is whether it makes sense to have a reader/writer which do not implement update_file or update_connection_config.

The thing is that each connection looks for a reader or writer of its class, when all of them does not have a particular reader or writer. That is, will use mainly the same base options.

@teclator
Copy link
Contributor Author

I will merge it as it is. But will create a macro with ccw group device information in next PR to add to s390 connnection config types.

@teclator teclator merged commit f85c53f into network-ng Aug 13, 2019
@teclator teclator deleted the network-ng-add_ctc_connection branch August 13, 2019 09:42
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.

None yet

5 participants