Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/909
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/885
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/909#issuecomment-145436050
I will merge this once travis is done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your proje
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/909#issuecomment-145434199
commited, squashed and force-pushed the change requested by Rajani
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/909#issuecomment-145423925
one minor comment. otherwise, LGTM (only did code review)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user karuturi commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/909#discussion_r41109279
--- Diff:
engine/orchestration/src/com/cloud/vm/VirtualMachinePowerStateSyncImpl.java ---
@@ -111,7 +111,19 @@ private void processReport(long hostId, Ma
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/909#issuecomment-145382427
@karuturi @koushik-das You reviewed PR #885, @dahn has added one commit and
made this PR ontop of the previous one. Would you be so kind to review it? Need
one m
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/909#issuecomment-145381656
Tested everything I tested in PR #885 again in this PR. @dahn already run
the tests, I verified the functionality manually by destoying VMs out-of-band
and see h
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/909#issuecomment-145369924
```bash
nosetests --with-marvin
--marvin-config=/data/shared/marvin/mct-zone1-kvm1.cfg -s -a \
tags=advanced,required_hardware=false smoke/test_routers.p
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/909#issuecomment-145324612
one test skipped, that requires two hosts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If yo
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/909#issuecomment-145324409
Test router internal advanced zone ... === TestName:
test_02_router_internal_adv | Status : SUCCESS ===
ok
Test restart network ... === TestName: test_03
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/909#issuecomment-145154565
Hi @DaanHoogland can you please run the same tests as we did on PR #885 and
post output? Let me know if you need help.
---
If your project is set up for it, you
Let's merge this as-is for 4.6 and solve a blocker. The improvement can be done
later in a separate PR.
Sent from my iPhone
> On 02 Oct 2015, at 10:29, DaanHoogland wrote:
>
> Github user DaanHoogland commented on the pull request:
>
>https://github.com/apache/cloudstack/pull/885#issueco
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-144958879
Had a look, the NPE won't occur within the scope of this fix. I would
suggest some changes anyway to prevent future issues as the method
isPowerStateUpToDate()
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-144936216
Talked to @DaanHoogland he will have a look soon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-144935223
@resmo can you respond to the comment about the possible NP please?
@dahn can you close PR #829 as we'll be using PR #885 (or am I mistaken?)
Please conf
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-16693
The functionality of this PR also seems to work:
```
WARN [c.c.v.VirtualMachinePowerStateSyncImpl]
(AgentManager-Handler-5:null) Detected missing VM
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-144430354
@resmo I tested a series of BVT tests (not all of them) and the result is
fine.
First test run:
```
marvinCfg=/data/shared/marvin/mct-zone1-kvm1.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-144377723
@resmo Just a heads-up that I am testing this as we speak. Running the BVT
tests against this branch to verify it all works. Once everything is done I'll
post re
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-144361517
ok, no further comment (please see the possible null pointer one)
---
If your project is set up for it, you can reply to this email and have your
reply appear o
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-144346880
well I need it in 4.5.3. I would suggest we take this fix for now for 4.6
as well and make a proper refactor for 4.7/5.0.
---
If your project is set up for it, you ca
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-144334476
I think this will work but it is a fix on a broken state machine. The
statemachine expects a power report every so and so time-interval and when it
doesn't come
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/885#discussion_r40772428
--- Diff: engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java ---
@@ -805,6 +805,12 @@ public Boolean doInTransaction(TransactionStatus
status)
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-144030748
@resmo The changes LGTM.
About testing on XS, the following should work:
- Deploy a VM
- Destroy the VM outside of CS (for e.g. from XenCenter)
- I
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143685748
@resmo Thanks! I'll run some tests today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143639563
LGTM on the basis of code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does n
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143591694
@remibergsma done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this fe
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143589352
@resmo Thanks for the fix! Could you rebase against current master please?
Then we'll be able to run some tests against this branch.
---
If your project is set
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143549250
@karuturi @anshul1886
So, I 'll checked the handling again and what I see is, it will not work
with this one line change only:
~~~
Date vmStateUpdateTime
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143544132
@karuturi I understand what @anshul1886 trying to point out: the powerState
should already be updated to the new state `powerReportMissing`.
And because it ch
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143543503
@anshul1886 without the new method, how do you ensure that
PowerStateUpdateTime is up to date for the vm? It may be a very old value and
can be a false alarm(same i
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143542706
code LGTM. I am facing some issues with my devcloud4 setup and hence
couldnt run tests.
---
If your project is set up for it, you can reply to this email and have
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143442228
We don't need new method isPowerStateUpToDate(long instanceId) as we are
already performing the same operations in
_instanceDao.updatePowerState(entry.getKey(), h
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-143227931
I successfully tested the patch on 4.5.2:
~~~
2015-09-25 15:38:11,953 WARN [c.c.v.VirtualMachinePowerStateSyncImpl]
(DirectAgentCronJob-7:ctx-65344d8a) Detecte
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-142950081
It is actually not easy to test because you must hit a race condition.
What I do is on vCenter make a few migrations of a VR to another hosts and
look on the
@resmo I will find some time tomorrow to test this. Can you add steps to
test please? Or are they already available on the other pr?
On Thu, Sep 24, 2015 at 19:39 PM, resmo wrote:
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-14
GitHub user resmo opened a pull request:
https://github.com/apache/cloudstack/pull/885
CLOUDSTACK-8848: ensure power state is up to date when handling missiâ¦
â¦ng VMs in powerReport
There 2 things which has been changed.
* We look on power_state_update_time inst
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/885#issuecomment-142941464
/cc @DaanHoogland @koushik-das @anshul1886
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your proj
38 matches
Mail list logo
