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

[sshXtermIPMI] Overrides reset function for ipmi console #1021

Merged
merged 1 commit into from Nov 12, 2018
Merged

[sshXtermIPMI] Overrides reset function for ipmi console #1021

merged 1 commit into from Nov 12, 2018

Conversation

XGWang0
Copy link
Contributor

@XGWang0 XGWang0 commented Sep 6, 2018

Make sure sol connection is deactivated after reset (rest_consoles) operation, which guarantees it is successful for sol connecting again.

Related ticket:
poo#40544

Verification run:
test on super-micro machine : http://10.67.19.191/tests/209
test on dell machine : http://10.67.19.191/tests/201
test for xen on dell machine: http://10.67.132.86/tests/170

@codecov
Copy link

codecov bot commented Sep 6, 2018

Codecov Report

Merging #1021 into master will decrease coverage by 0.41%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1021      +/-   ##
==========================================
- Coverage   40.19%   39.77%   -0.42%     
==========================================
  Files          34       34              
  Lines        4526     4526              
  Branches      771      771              
==========================================
- Hits         1819     1800      -19     
- Misses       2389     2408      +19     
  Partials      318      318
Impacted Files Coverage Δ
commands.pm 42.16% <0%> (-4.87%) ⬇️
testapi.pm 5.2% <0%> (-1.16%) ⬇️
backend/baseclass.pm 55.72% <0%> (-0.68%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff83b57...33a361d. Read the comment docs.

@mitiao
Copy link
Contributor

mitiao commented Sep 10, 2018

LGTM

@XGWang0 XGWang0 changed the title [sshXtermIPMI] refactor reset function for ipmi console [WIP][sshXtermIPMI] refactor reset function for ipmi console Sep 10, 2018
@XGWang0
Copy link
Contributor Author

XGWang0 commented Sep 10, 2018

Hi @foursixnine @mitiao, please do not merge now, we are trying to fix half-open issue in the roots, maybe need to change this PR.

@okurz
Copy link
Member

okurz commented Sep 21, 2018

I don't think your change is a "refactoring", especially as you are adding a new method. I suggest to rephrase the commit message and PR subject

@XGWang0
Copy link
Contributor Author

XGWang0 commented Sep 26, 2018

@okurz Yes, i will change the RP subject

@XGWang0 XGWang0 changed the title [WIP][sshXtermIPMI] refactor reset function for ipmi console [WIP][sshXtermIPMI] Overrides reset function for ipmi console Sep 26, 2018
@XGWang0
Copy link
Contributor Author

XGWang0 commented Nov 12, 2018

Hi Reviewers: This is a single/whole fix for supporing dell/super-micro machine, please re-open review process. @foursixnine @mitiao

@mitiao mitiao removed the not-ready label Nov 12, 2018
@XGWang0 XGWang0 changed the title [WIP][sshXtermIPMI] Overrides reset function for ipmi console [sshXtermIPMI] Overrides reset function for ipmi console Nov 12, 2018
@foursixnine
Copy link
Member

@XGWang0 Do you have any proof runs?

@foursixnine
Copy link
Member

@alvarocarvajald Mind testing this commit in your os-autoinst? (@coolo wdyt could be related?)

Copy link
Contributor

@coolo coolo 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

@coolo coolo merged commit 2e3ef3a into os-autoinst:master Nov 12, 2018
@alvarocarvajald
Copy link

alvarocarvajald commented Nov 12, 2018

@alvarocarvajald Mind testing this commit in your os-autoinst? (@coolo wdyt could be related?)

OK, this seems to do the trick in my development system. Test is able to pass boot/qa_net_boot_from_hdd, which in my development system was crashing with SIGSEGV, and the PXE Boot menu is visible which is the problem I'm currently experiencing in openqa.suse.de.

@okurz
Copy link
Member

okurz commented Nov 12, 2018

@SergioAtSUSE might be interesting for you

@XGWang0
Copy link
Contributor Author

XGWang0 commented Nov 13, 2018

@foursixnine No extra test run except verification run in comment i posted.

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

6 participants