consoleproxy: Enable console for vms in Stopping/Migrating state#3640
Conversation
6d3c7da to
27f5dcb
Compare
27f5dcb to
9f8ef74
Compare
|
@andrijapanicsb this fixes your issue #3631 |
|
@ustcweizhou nice enhancement! |
|
I need to improve my Outlook filter (to see some of those almost 10.000 emails) Thanks @ustcweizhou |
|
@andrijapanicsb a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-321 |
|
Tested, looks fine, thx @ustcweizhou LGTM |
|
@weizhouapache sounds intresting. not tested but i hope it will go into the master. we encounter the same problem. |
|
@rhtyd @andrijapanicsb can you kick off a test again ? |
|
@blueorangutan test |
|
@andrijapanicsb a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-557)
|
|
need one more LGTM from code POV, otherwise tested manually fine and tests are passing. |
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
I am fine with the code 👍 and with the functionality, it no longer requires you to be admin to open the console in some instances, which is fine by me.
the jenkins/travis tests have failed, giving it a quick look.
| if (vm != null && vm.getState() != State.Starting && vm.getState() != State.Running | ||
| && vm.getState() != State.Stopping && vm.getState() != State.Migrating) { |
There was a problem hiding this comment.
:) a lot of getState()s there
There was a problem hiding this comment.
@DaanHoogland not very much , only 4 :-)
will pay attention in the future.
There was a problem hiding this comment.
NP, code can be such a joy.
| if (vm.getState() == State.Migrating && vm.getLastHostId() != null) { | ||
| answer = (GetVncPortAnswer)_agentMgr.easySend(vm.getLastHostId(), new GetVncPortCommand(vm.getId(), vm.getInstanceName())); | ||
| } else { |
There was a problem hiding this comment.
So we are making the decision we are opening a console to the old VM. looks good but may have some problems. I suspect you tested @ustcweizhou .
problems I see are:
- this is a second migrate and the old host is no longer valid.
- the migration is already successful but not administered yet.
There was a problem hiding this comment.
@DaanHoogland indeed there are some problems with vm console, for example
(1) vm is Starting state, but is not running on any server (eg applying dhcp/dns entries in virtual routers)
(2) vm is Stopping state, but is stopped on server
(3) vm is Migrating state, when we open the console of vm (on source host), and migration is done, the console is invalid.
However, I think all these problems are not real issues. When vm state is updated, refresh the web page and a new console will work.
There was a problem hiding this comment.
I agree, @weizhouapache these are not big issues. I just wanted to have mentioned them and am not 👎ing this!
|
travis job 5 ran out of log space. this has happened more often recently. no issue concerning this PR afaiac |
* 4.13: consoleproxy: Enable console for vms in Stopping/Migrating state (#3640)
7 participants
Description
Currently vm console is not enabled if vm is in stopping or migrating.
Enabling vm console will help us
Fixes: #3631
Types of changes
Screenshots (if appropriate):
How Has This Been Tested?