from:"marcaurele"

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-21 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1768
  
@serg38 Not sure to understand what you mean with:
> If I get it right with your proposed changes, upgrade scripts become 
obsolete since all the changes can be done in upgrade scripts.

You meant: *If I get it right with your proposed changes, upgrade 
**cleanup** scripts become obsolete since all the changes can be done in 
upgrade scripts.* ?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-21 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1768
  
I'll try to make my point clearer with a better use case. Let say you were 
running version ACS 4.4.2 and wish to upgrade to 4.7.1. After installing the 
4.7.1, when ACS starts for the first time you will execute SQL scripts in that 
order (case A):
```
schema-442to450.sql   -> schema-442to450-cleanup.sql
  |  | |
  v  | v
schema-450to451.sql  |   schema-450to451-cleanup.sql
  |  | |
  v  | v
schema-451to452.sql  |   schema-451to452-cleanup.sql
  |  | |
  v  | v
schema-452to460.sql  |   schema-452to460-cleanup.sql
  |  | |
  v  | v
schema-460to461.sql  |   schema-460to461-cleanup.sql
  |  | |
  v  | v
schema-461to470.sql  |   schema-461to470-cleanup.sql
  |  | |
  v  | v
schema-470to471.sql >schema-470to471-cleanup.sql
```

But if you would have updated to each versions, one after the other, you 
would have run those scripts in that order (case B):
```
schema-442to450.sql -> schema-442to450-cleanup.sql
 |
   --
  |
  v
schema-450to451.sql -> schema-450to451-cleanup.sql
 |
   --
  |
  v
schema-451to452.sql -> schema-451to452-cleanup.sql
 |
   --
  |
  v
schema-452to460.sql -> schema-452to460-cleanup.sql
 |
   --
  |
  v
schema-460to461.sql -> schema-460to461-cleanup.sql
 |
   --
  |
  v
schema-461to470.sql -> schema-461to470-cleanup.sql
 |
   --
  |
  v
schema-470to471.sql -> schema-470to471-cleanup.sql
```

Since **case B** is that most developer would expect when fixing bugs and 
doing changes, but **case A** is the most common case of production upgrade, I 
wanted to correct the algorithm so that everyone will follow the same route 
(case B).

Most `-cleanup.sql` scripts are either empty or only updating the 
`configuration` table, so it's safe. There is only one possible problematic 
script: 
https://github.com/apache/cloudstack/blob/master/setup/db/db/schema-481to490-cleanup.sql
 today. This one does change views, which IMO was a mistake to put in the 
cleanup script file, it should have gone into `schema-481to490.sql` (@rhtyd ?). 
Leaving the mechanism as it is today would leave people with a possible bug 
while upgrading from any version prior to 4.9.0 *if* any future SQL script was 
to change the views modified inside `schema-481to490-cleanup.sql` because of 
scenario case A. Did I lost people there?

Any comment @remibergsma @DaanHoogland @syed @nvazquez ?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1768
  
As a reminder for `-cleanup` SQL scripts:
> We rarely need cleanup scripts, only when some field/value is required 
during the data migration and has to be dropped later on.


https://cwiki.apache.org/confluence/display/CLOUDSTACK/DB+Upgrade+in+CloudStack


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1768
  
@syed I would prefer to move what's inside `schema-481to490-cleanup.sql` to 
the end of the file `schema-481to490.sql` as it would have been the way of 
processing during an update from 481 to 490.
If I get the go about the move, I'll do it too in this PR.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-23 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1768
  
I looked at the code inside `Upgrade481to490.java` and I cannot understand 
why the table alterations have been made inside the Java class instead of the 
SQL file. Because now we cannot move the code from the `-cleanup` without 
changing that class too, other the column `role_id` won't be present when 
creating the `account_view`.
To make the change transparent, I'll have to change those too. I'll push 
another commit for that.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-23 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1768
  
Forget my last comment, the files can remains as they are.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1944: CLOUDSTACK-9783: Improve metrics view perform...

2017-02-26 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1944#discussion_r103143033
  
--- Diff: plugins/metrics/pom.xml ---
@@ -0,0 +1,55 @@
+
+http://maven.apache.org/POM/4.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
+  http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+  4.0.0
+  cloud-plugin-metrics
+  Apache CloudStack Plugin - Metrics
+  
+org.apache.cloudstack
+cloudstack-plugins
+4.9.3.0-SNAPSHOT
+../pom.xml
+  
+  
+
+  org.apache.cloudstack
+  cloud-api
+  ${project.version}
+
+
+  org.apache.cloudstack
+  cloud-utils
+  ${project.version}
+
+  
+  
+
--- End diff --

Would be nice to follow maven standards on new modules. Since you cannot 
inherit from `cloud-maven-standard` you have to put all those lines:
```

${basedir}/src/main/java

${basedir}/src/main/scripts
${basedir}/src/test/java
${basedir}/target/classes

${basedir}/target/test-classes

  
${basedir}/src/main/resources
  


  
${basedir}/src/test/resources
  

...

```


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1768
  
@rhtyd I don't see how I could write such a test case. My point is exactly 
what you're saying, the upgrade would be different today for anyone who comes 
from an older version, which I don't think you're currently testing. I want to 
avoid such situation, therefore I'm pushing this PR to stop having different 
upgrade paths.
Can you be more explicit in what you think can become a problem compared to 
the current situation we're in 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 project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1768: CLOUDSTACK 9601: Upgrade: change logic for up...

2017-02-27 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1768#discussion_r103394277
  
--- Diff: engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java 
---
@@ -424,27 +421,9 @@ protected void upgrade(CloudStackVersion dbVersion, 
CloudStackVersion currentVer
 }
 
 upgrade.performDataMigration(conn);
-boolean upgradeVersion = true;
-
-if (upgrade.getUpgradedVersion().equals("2.1.8")) {
--- End diff --

I thought that anything older than 4.x should be removed. The simulator and 
DB schema create a structure based on 4.0.0 or 4.1.0 if I recall correctly.
I initially added another commit (removed now) which was only leaving 
upgrade paths from version 4.0.0. That can be done later, and I think it would 
be a good cleanup.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1832: CLOUDSTACK-9652 Job framework - Cancelling async job...

2017-02-28 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1832
  
@karuturi Ok


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #2027: Activate NioTest following changes in CLOUDST...

2017-04-05 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/2027

Activate NioTest following changes in CLOUDSTACK-9348 PR #1549

The first PR #1493 re-enabled the NioTest but not the new PR #1549.

@rhtyd the test fails locally on my laptop. Is there any special 
configuration requirements?

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack niotest

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/2027.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2027


commit 226e79c8ce0686ba3d5690ed90134934e26b635d
Author: Marc-AurÃ¨le Brothier 
Date:   2017-04-05T10:25:17Z

Activate NioTest following changes in CLOUDSTACK-9348 PR #1549




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #2027: Activate NioTest following changes in CLOUDSTACK-934...

2017-04-05 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/2027
  
@rhtyd the `NioTest` result is not consistent on my laptop and fails from 
time to time.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #2027: Activate NioTest following changes in CLOUDSTACK-934...

2017-04-06 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/2027
  
@rhtyd I found one issue with the test and `NioConnection` class. This kind 
of intermittent problem are always hard to search for a root cause, but after 
lots of logging I finally found why. I updated the PR with the change.

If the main thread running the test is stopped there 
https://github.com/apache/cloudstack/blob/master/utils/src/main/java/com/cloud/utils/nio/NioConnection.java#L102
 due to context switching, the flag `_isRunning` isn't switched to True by the 
time the NioServer connection handler start it's call loop, and it exits on the 
`while(_isRunning)`

https://github.com/apache/cloudstack/blob/master/utils/src/main/java/com/cloud/utils/nio/NioConnection.java#L125
 directly. Therefore the server isn't listening at all and the connection 
cannot be made. The flag `_isRunning` must be turned `true` before submitting 
the task/thread.

I still digging into Nio thread handler as we are experiencing some problem 
in production when quite a few agents try to connect at the same time to a 
management server. None of them can connect.



---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #2027: Activate NioTest following changes in CLOUDSTACK-934...

2017-04-10 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/2027
  
@rhtyd I moved the PR against 4.9 and rebased the changes


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-10 Thread marcaurele

Github user marcaurele commented on the pull request:

https://github.com/apache/cloudstack/pull/1532#issuecomment-218370950
  
@swill did a force push by changing the last commit description


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-11 Thread marcaurele

Github user marcaurele closed the pull request at:

https://github.com/apache/cloudstack/pull/1532


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-11 Thread marcaurele

GitHub user marcaurele reopened a pull request:

https://github.com/apache/cloudstack/pull/1532

DAO: Hit the cache for entity flagged as removed too

I came along this part of the code and I don't see any reason why the cache 
should not be used when fetching with the "removed" ones. It will help decrease 
the number of DB queries.

*It can be merged in many CS versions*

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack fix/db-cache-miss

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1532.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1532


commit 696440a6754a60888fce7a82e8022ca7278b9be5
Author: Marc-AurÃ¨le Brothier 
Date:   2016-05-04T12:28:44Z

dao: Hit the cache for entity flagged as removed too since they are put
in cache afterwards.

commit 56fe0ec53dbc1331f1f4875f6f9e9bea62760c38
Author: Marc-AurÃ¨le Brothier 
Date:   2016-05-09T09:39:04Z

Rewrite the change for method findByIdIncludingRemoved(ID id) and also 
change the sibling method findById(final ID id)




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-12 Thread marcaurele

Github user marcaurele commented on the pull request:

https://github.com/apache/cloudstack/pull/1532#issuecomment-218731447
  
I don't get what's the relation between my change and the jenkins build 
failling. Should I trigger another one?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-13 Thread marcaurele

Github user marcaurele commented on the pull request:

https://github.com/apache/cloudstack/pull/1532#issuecomment-219010730
  
checks are finally ok...


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: API: improve resource limits comprenhensi...

2016-05-20 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1554

API: improve resource limits comprenhension

Add resource type name in request and response.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack feature/resourcelimits

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1554.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1554


commit 95d91148001484884e437038763a8aaf20220d93
Author: Marc-AurÃ¨le Brothier 
Date:   2016-05-20T08:56:04Z

API: improve resource limits comprenhension

Add resource type name in request and response.




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9238: Increase URL fields to 2...

2016-05-26 Thread marcaurele

Github user marcaurele commented on the pull request:

https://github.com/apache/cloudstack/pull/1341#issuecomment-222067277
  
Hi! Talking with @footplus on the mailing list, I realized that this patch 
is not correct. It's missing modifications on the VO objects to let the String 
holds more than 255 characters because in the `GenericDaoBase`, `String` fields 
are by default of 255 chars, see:

https://github.com/apache/cloudstack/blob/master/framework/db/src/com/cloud/utils/db/GenericDaoBase.java#L1510

This PR should be updated, or should there be a new one to add those 
changes?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9238: Fix URL length to 2048 f...

2016-05-26 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1567

CLOUDSTACK-9238: Fix URL length to 2048 for all url fields in VO

I will update the PR to add max field length in the API commands too

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack CLOUDSTACK-9238

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1567.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1567


commit a59ee03fd79806dcb3a8842c318e2cd06895183f
Author: Marc-AurÃ¨le Brothier 
Date:   2016-05-27T06:16:05Z

Fix URL length to 2048 for all url fields in VO




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9238: Increase URL fields to 2...

2016-05-26 Thread marcaurele

Github user marcaurele commented on the pull request:

https://github.com/apache/cloudstack/pull/1341#issuecomment-222071694
  
I guess a PR is easier, so here you go 
https://github.com/apache/cloudstack/pull/1567


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9238: Fix URL length to 2048 f...

2016-05-27 Thread marcaurele

Github user marcaurele commented on the pull request:

https://github.com/apache/cloudstack/pull/1567#issuecomment-222094671
  
Because not all varchar columns have been updated to a length of 2048 in 
the DB, so there will be a mismatche between API, VO and the database.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1638: CLOUDSTACK-9456: Use Spring 4 and Java 8

2016-08-12 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1638#discussion_r74574588
  
--- Diff: pom.xml ---
@@ -92,22 +85,22 @@
 1.5.1
 1.2.8
 2.0.4
-2.5
+3.1.0
 1.2
 1.2.1
 
1.0-20081010.060147
 6.0
-
3.2.16.RELEASE
+
4.2.7.RELEASE
--- End diff --

Any particular reason to not use the version 4.3.2 ?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1638: CLOUDSTACK-9456: Use Spring 4 and Java 8

2016-08-12 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1638#discussion_r74575182
  
--- Diff: pom.xml ---
@@ -92,22 +85,22 @@
 1.5.1
 1.2.8
 2.0.4
-2.5
+3.1.0
 1.2
 1.2.1
 
1.0-20081010.060147
 6.0
-
3.2.16.RELEASE
+
4.2.7.RELEASE
--- End diff --

Ok


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1638: CLOUDSTACK-9456: Use Spring 4 and Java 8

2016-08-12 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1638#discussion_r74588212
  
--- Diff: pom.xml ---
@@ -92,22 +85,22 @@
 1.5.1
 1.2.8
 2.0.4
-2.5
+3.1.0
 1.2
 1.2.1
 
1.0-20081010.060147
 6.0
-
3.2.16.RELEASE
+
4.2.7.RELEASE
--- End diff --

I'll have a look


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1638: CLOUDSTACK-9456: Use Spring 4 and Java 8

2016-08-15 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1638#discussion_r74739489
  
--- Diff: pom.xml ---
@@ -92,22 +85,22 @@
 1.5.1
 1.2.8
 2.0.4
-2.5
+3.1.0
 1.2
 1.2.1
 
1.0-20081010.060147
 6.0
-
3.2.16.RELEASE
+
4.2.7.RELEASE
--- End diff --

Pushed a PR on your branch with the fixes


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stop...

2016-08-15 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1640

CLOUDSTACK-9458: Fix HA bug when VMs are stopped on agent disconnect

VM are being restarted even if they don't have HA enabled. If the agent
gets back online before the command expires some VMs are being stopped
and not even restarted.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack bug/CLOUDSTACK-9458

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1640.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1640


commit c8f57541006536c4719044405b7691fad244fbf7
Author: Marc-AurÃ¨le Brothier 
Date:   2016-08-16T06:10:51Z

CLOUDSTACK-9458: Fix HA bug when VMs are stopped on agent disconnect

VM are being restarted even if they don't have HA enabled. If the agent
gets back online before the command expires some VMs are being stopped
and not even restarted.




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stop...

2016-08-17 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1640#discussion_r75091738
  
--- Diff: server/src/com/cloud/ha/HighAvailabilityManagerImpl.java ---
@@ -282,7 +282,9 @@ public void scheduleRestartForVmsOnHost(final HostVO 
host, boolean investigate)
 + hostId + " VM HA is done");
 continue;
 }
-scheduleRestart(vm, investigate);
+if (vm.isHaEnabled()) {
--- End diff --

Sure, I'll update


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stopped on ...

2016-08-17 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1640
  
I'll do the long explanation regarding the bug in production we had.

Twice we had the VPN going down between 2 of our zones which resulted in 
the lost of the communication between the management server and all the agents 
of the other zone. After a few minutes the network went back and as the agents 
were reconnecting we started to see VMs being shutdown, even though we don't 
have HA enabled. We have 2 management servers in front of haproxy to balance 
agents & requests. When the network went back, the agent reconnected to the 
other management server 01. The server 02 had already started to issue some 
commands to shutdown the VM.

The important lines are here, I kept the line showing one VM being shutdown 
`111750`:

```
srv02 2016-08-08 11:56:03,854 DEBUG [cloud.ha.HighAvailabilityManagerImpl] 
(AgentTaskPool-16:ctx-8b5b6956) PingInvestigator was able to determine host 
44692 is in Disconnected
srv02 2016-08-08 11:56:03,855 WARN  [agent.manager.AgentManagerImpl] 
(AgentTaskPool-16:ctx-8b5b6956) Agent is disconnected but the host is still up: 
44692-virt-hv041
srv02 2016-08-08 11:56:03,857 WARN  [apache.cloudstack.alerts] 
(AgentTaskPool-16:ctx-8b5b6956)  alertType:: 7 // dataCenterId:: 2 // podId:: 2 
// clusterId:: null // message:: Host disconnected, name: virt-hv041 
(id:44692), availability zone: ch-dk-2, pod: DK2-AZ1-POD01
srv02 2016-08-08 11:56:03,884 INFO  [agent.manager.AgentManagerImpl] 
(AgentTaskPool-16:ctx-8b5b6956) Host 44692 is disconnecting with event 
AgentDisconnected
srv02 2016-08-08 11:56:03,895 DEBUG [agent.manager.AgentManagerImpl] 
(AgentTaskPool-16:ctx-8b5b6956) The next status of agent 44692is Alert, current 
status is Up
srv02 2016-08-08 11:56:03,896 DEBUG [agent.manager.AgentManagerImpl] 
(AgentTaskPool-16:ctx-8b5b6956) Deregistering link for 44692 with state Alert
srv02 2016-08-08 11:56:03,896 DEBUG [agent.manager.AgentManagerImpl] 
(AgentTaskPool-16:ctx-8b5b6956) Remove Agent : 44692
srv02 2016-08-08 11:56:03,897 DEBUG [agent.manager.AgentAttache] 
(AgentTaskPool-16:ctx-8b5b6956) Seq 44692-4932286016900831481: Sending 
disconnect to class com.cloud.network.security.SecurityGroupListener
srv02 2016-08-08 11:56:03,897 DEBUG [agent.manager.AgentAttache] 
(AgentTaskPool-16:ctx-8b5b6956) Seq 44692-4932286016900834802: Sending 
disconnect to class com.cloud.network.security.SecurityGroupListener
srv02 2016-08-08 11:56:03,897 DEBUG [agent.manager.AgentAttache] 
(AgentTaskPool-16:ctx-8b5b6956) Seq 44692-4932286016900886805: Sending 
disconnect to class com.cloud.network.security.SecurityGroupListener
...
srv02 2016-08-08 11:56:03,979 DEBUG [cloud.network.NetworkUsageManagerImpl] 
(AgentTaskPool-16:ctx-8b5b6956) Disconnected called on 44692 with status Alert
srv02 2016-08-08 11:56:03,987 DEBUG [cloud.host.Status] 
(AgentTaskPool-16:ctx-8b5b6956) Transition:[Resource state = Enabled, Agent 
event = AgentDisconnected, Host id = 44692, name = virt-hv041]
srv02 2016-08-08 11:56:03,998 DEBUG [cloud.cluster.ClusterManagerImpl] 
(AgentTaskPool-16:ctx-8b5b6956) Forwarding 
[{"com.cloud.agent.api.ChangeAgentCommand":{"agentId":44692,"event":"AgentDisconnected","contextMap":{},"wait":0}}]
 to 345049010805
srv02 2016-08-08 11:56:03,998 DEBUG [cloud.cluster.ClusterManagerImpl] 
(Cluster-Worker-2:ctx-e087e71f) Cluster PDU 90520739220960 -> 345049010805. 
agent: 44692, pdu seq: 15, pdu ack seq: 0, json: 
[{"com.cloud.agent.api.ChangeAgentCommand":{"agentId":44692,"event":"AgentDisconnected","contextMap":{},"wait":0}}]
srv01 2016-08-08 11:56:04,002 DEBUG 
[agent.manager.ClusteredAgentManagerImpl] (Cluster-Worker-9:ctx-9101a6d4) 
Dispatch ->44692, json: 
[{"com.cloud.agent.api.ChangeAgentCommand":{"agentId":44692,"event":"AgentDisconnected","contextMap":{},"wait":0}}]
srv01 2016-08-08 11:56:04,002 DEBUG 
[agent.manager.ClusteredAgentManagerImpl] (Cluster-Worker-9:ctx-9101a6d4) 
Intercepting command for agent change: agent 44692 event: AgentDisconnected
srv01 2016-08-08 11:56:04,002 DEBUG 
[agent.manager.ClusteredAgentManagerImpl] (Cluster-Worker-9:ctx-9101a6d4) 
Received agent disconnect event for host 44692
srv02 2016-08-08 11:56:04,002 DEBUG [cloud.cluster.ClusterManagerImpl] 
(Cluster-Worker-2:ctx-e087e71f) Cluster PDU 90520739220960 -> 345049010805 
completed. time: 3ms. agent: 44692, pdu seq: 15, pdu ack seq: 0, json: 
[{"com.cloud.agent.api.ChangeAgentCommand":{"agentId":44692,"event":"AgentDisconnected","contextMap":{},"wait":0}}]
srv01 2016-08-08 11:56:04,004 DEBUG 
[agent.manager.ClusteredAgentManagerImpl] (Cluster-Work

[GitHub] cloudstack pull request #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stop...

2016-08-17 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1640#discussion_r75252604
  
--- Diff: server/src/com/cloud/ha/HighAvailabilityManagerImpl.java ---
@@ -497,7 +498,7 @@ protected Long restart(final HaWorkVO work) {
 
 boolean fenced = false;
 if (alive == null) {
-s_logger.debug("Fencing off VM that we don't know the 
state of");
+s_logger.debug("Fencing off VM " + vm + " that we 
don't know the state of");
--- End diff --

Turning all of those log debug lines without a `if` statement to lambda 
expressions when we require Java8 would be easier if there're not wrapped 
within a `if`:
```
s_logger.debug(() -> "Fencing off VM " + vm + " that we don't know the 
state of");
```
with
```
default void debug(Supplier stringSupplier) {
if (isDebugEnabled()) {
debug(stringSupplier.get());
}
}
```
The concatenation will be only done when debug is enabled.

You're right, I turn it to `warn`.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stopped on ...

2016-08-17 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1640
  
> Would you mind adding these notes to the bug ticket?

@jburwell All PR comments are going automatically into the jira ticket 
comment thanks to the ID matching (I think).


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stopped on ...

2016-08-17 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1640
  
@koushik-das We are running a fork based on 4.4.2 with lots of 
cherry-pickings.

But even if the host is down, why would you want to schedule a restart if 
the VM are not HA enabled?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stopped on ...

2016-08-18 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1640
  
I understand your point of the release, but we're not in an ideal world 
where everyone runs the latest version. I try to do my best to look at the 
current code in CS to find possible fixes of any bug/problem we encounter or 
changes we want to do in our version. I want us to get back to the master 
version but that's not the topic here, neither going to happen in the next 
weeks.

The point 2 does not make sense to me. If the management server cannot 
determine the state of the VM, it could mark them as stopped (*even though I 
don't think it should*). But it should not create a StopVM job, because that 
might trigger a proper stop of the VM if the agent is reconnecting while the 
job is picked by async job workers.
If the VM is really down because the host has crashed, then the command is 
pointless, and in a customer point of view it would not make a difference. If 
the host is still up and fine, but we have a network glitch, then requesting a 
stop of the VM is really bad in a customer point of view. By not doing 
anything, not requesting a stop, we would end up in a better situation.

Going back to which state should be set on the VM when the management 
server cannot determine it, taking the assumption that the VM is stopped 
because the management server cannot reach the agent is as much incorrect as 
leaving it as it is (running, migrating, creating...). I'd rather create a new 
state `UNKNOWN` for such special case, when the management server does really 
not know. In a management point of view it will be also easier to know there 
are *ghost* VMs somewhere for which the management server cannot determine the 
exact state and proper investigation (*manual*) should be done if the state 
stays like this, regarding the billing part too.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stopped on ...

2016-08-23 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1640
  
@koushik-das 
> If the MS is not able to determine the state of the VM, it tries fencing 
off the VM (using the various fencers available). If VM cannot be fenced off 
successfully, the state of the VM is left unchanged. 

Apparently I found a way where the VMs are successfully fenced off even 
though they should not.

What is the reason to try fencing off VMs when the MS is not able to 
determine its state? I cannot see a good reason so far but you seem to think 
there is at least one. Can you explain it?

@jburwell It does not cover my case exactly as it's a timing issue. I'll 
keep a note to find a way to create a scenario.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stopped on ...

2016-08-24 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1640
  
@koushik-das which is IMHO wrong.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stopped on ...

2016-09-13 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1640
  
@jburwell I changed the commit and PR to point to 4.9


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-04 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1532

DAO: Hit the cache for entity flagged as removed too

I came along this part of the code and I don't see any reason why the cache 
should not be used when fetching with the "removed" ones. It will help decrease 
the number of DB queries.

*It can be merged in many CS versions*

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack fix/db-cache-miss

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1532.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1532


commit 696440a6754a60888fce7a82e8022ca7278b9be5
Author: Marc-AurÃ¨le Brothier 
Date:   2016-05-04T12:28:44Z

dao: Hit the cache for entity flagged as removed too since they are put
in cache afterwards.




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-08 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1532#discussion_r62453258
  
--- Diff: framework/db/src/com/cloud/utils/db/GenericDaoBase.java ---
@@ -969,7 +969,12 @@ public T findByUuidIncludingRemoved(final String uuid) 
{
 @Override
 @DB()
 public T findByIdIncludingRemoved(ID id) {
-return findById(id, true, null);
+if (_cache != null) {
+final Element element = _cache.get(id);
+return element == null ? findById(id, true, null) : 
(T)element.getObjectValue();
+} else {
--- End diff --

The test for the existence of the cache is made on other lines in this 
class, so I kept it. My change is to use the cached value if there is one, as 
it wasn't the case before.
The function looks pretty much the same as the `findById` (line 944).

@DaanHoogland in your last code sample, on the first line you go first to 
the DB to retrieve the object. Then if the cache isn't null, you fetch the 
value from the cache and discard the one you just got. IMO it's a downgrade in 
terms of performance as you never use directly the value in cache but always do 
a query against the DB.

@jburwell I could change the code as you're suggesting, but then the 
exception should be raised at other places too to keep the code consistent when 
the cache isn't configured, which wasn't my initial goal.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-09 Thread marcaurele

Github user marcaurele commented on the pull request:

https://github.com/apache/cloudstack/pull/1532#issuecomment-217820309
  
@sateesh-chodapuneedi not quite, you're missing a case. If the cache is 
null (not configured) you will always return `null`, which is not the goal.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-09 Thread marcaurele

Github user marcaurele commented on the pull request:

https://github.com/apache/cloudstack/pull/1532#issuecomment-217820629
  
I pushed a change to write the method following the good practive you're 
mentioning. I also updated the sibling method findById to follow the same 
practice.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1709: CLOUDSTACK-7982 - KVM live migration

2016-10-16 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1709

CLOUDSTACK-7982 - KVM live migration

This is the forward port of our KVM live migration implementation in 4.4.2 
to the master branch. 
Feedback is welcome. I still have to write the documentation to explain 
some configuration for libvirt and some config value in CS.

Things left:
- [ ] Write documentation in cloustack-docs-install & open PR with a link 
to this one
- [ ] Add to the upgrade SQL file:
```SQL
UPDATE  `cloud`.`hypervisor_capabilities`
SET  `storage_motion_supported` =  1
WHERE  `hypervisor_capabilities`.`hypervisor_type` =  'KVM';
```
- [ ] Update the PR with a final description

Jira: https://issues.apache.org/jira/browse/CLOUDSTACK-7982

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack acs/CLOUDSTACK-7982

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1709.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1709


commit fb78b20cc3fd93a7a7f2ebd55f6141595f5a2abe
Author: Marc-AurÃ¨le Brothier 
Date:   2016-04-05T12:00:03Z

Merge pull request #12 from 
exoscale/exoscale/feature/kvm-vm-move-with-storage

KVM live migration with non shared storage

commit 886ec5aeeeb226550b80366aead3dd783f8873bf
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-15T13:21:38Z

live migration: cancel migration job on timeout exception

Currently if a migration job takes longer than the max time for a CS job,
then the disk at the remote is deleted but the job keeps running. This
leads to a situation were the VM could be moved on the destination
hypervisors but without a visible disk on the filesystem.

This fix attempt to cancel the migration job on the source host before
requesting the disk removal at the destination.

commit 26ac7e270eff601a94e8d109f5f9deb286818bb2
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-16T05:47:35Z

live migration: delete volume only when migration is cancelled successfully

commit cadeea790ab28463c98ca76f55d0785001bf0fe7
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-16T15:23:50Z

live migration: incorrect check on return value

commit afc49f5a6b224a08e1e7ebc6b7b5050bc940ca65
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-17T07:59:39Z

live migration: getting correct TO object

commit 8f33408251768566bc7cec9db08d2e2b4d974ec3
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-17T08:41:35Z

live migration: fix another ClassCast

commit 18c3e4fda864dd1f4fe480dd77bd92474e4c2744
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-17T10:03:55Z

live migration: propagating command result

commit 23d5217ed8db035d1bc52de53f48823f7e2f3b9d
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-20T11:46:40Z

live migration: more debug information

commit 680ab3f5ecadba4cbc60f3a5239316f6e83a4a6d
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-21T07:56:08Z

more debug

commit a8cc823db321c3de1a697b3044981dcfc041ea42
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-21T09:56:32Z

force command as failed

commit 2783515778c55846f9aebe0fef531a5d6c45a809
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-21T14:46:44Z

live migration: adjustment on the response

commit 2f310e5f8d2f8ef20d90578dcc26deb97f11dca6
Author: Marc-AurÃ¨le Brothier 
Date:   2016-06-22T09:05:55Z

remove debug code

commit 625b46903645aa8235e7bf859ba97908bb986b10
Author: Marc-AurÃ¨le Brothier 
Date:   2016-07-05T09:26:02Z

Live migration fixes (#20)

Multiple bugs fixes:

* Incorrect disk size: when provisioning the volume at the destination,
  the size was taken from the template and not from the original volume
  itself.

* Incorrect details when returning an error: now if an exception is
  raised by Libvirt at the agent level, the error message is forwarded
  correctly all the way into the job response text.

* Incorrect volume state: when a migration failed, the volume state was
  inconsistent and stayed in "Migrating", those interfering with future
  commands on the VM. As a result, the event message wasn't sent
  either.

commit 684dc923af8d853bb2c73ba37ddf347397965ab4
Author: Marc-AurÃ¨le Brothier 
Date:   2016-07-11T13:42:06Z

live migration: add custom config timeout for MigrateWithStorageCommand 
(#22)




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1709: CLOUDSTACK-7982 - KVM live migration

2016-10-17 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1709
  
Is there a way to get the management server log of the travis job?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1709: CLOUDSTACK-7982 - KVM live migration

2016-10-21 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1709
  
@rhtyd I found the log but it doesn't give the mgmt server log output, only 
the job's output which got a `Command failed due to Internal Server Error` in 
an API response. I'll rebase & squash.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1723: Fix GroupBy (+ having) condition and tests

2016-10-21 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1723

Fix GroupBy (+ having) condition and tests

The GroupBy + having isn't used currently in the code but was not clean. It 
removes unused arguments and variables and adds a test based on a DAO to show a 
full example of how to use it.

Signed-off-by: Marc-AurÃ¨le Brothier 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack fix/group-by

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1723.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1723


commit 2ee97e8fa5c179bb48fb121ce6422df6d0794154
Author: Marc-AurÃ¨le Brothier 
Date:   2016-10-21T12:43:11Z

Fix GroupBy (+ having) condition and tests

The GroupBy + having isn't used currently in the code but was not clean.
It removes unused arguments and variables and adds a test based on a DAO
to show a full example of how to use it.

Signed-off-by: Marc-AurÃ¨le Brothier 




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1709: CLOUDSTACK-7982 - KVM live migration

2016-10-24 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1709#discussion_r84632272
  
--- Diff: 
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
 ---
@@ -943,6 +962,11 @@ public boolean configure(final String name, final 
Map params) th
 _mountPoint = "/mnt";
 }
 
+_libvirtConnectionProtocol = (String) 
params.get("libvirt.connection.protocol");
+if (_libvirtConnectionProtocol == null) {
+_libvirtConnectionProtocol = "qemu://";
--- End diff --

It's something I have to document in the installation doc. If you want to 
run the migration over TLS, you have to specify here `qemu+tls://`


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1730: VMTemplateZone needs some love

2016-10-25 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1730

VMTemplateZone needs some love

It's incorrect to use the findIncludingRemovedBy and
listIncludingRemovedBy for the common list and find operation. Moreover
the DAO is trying to update the `removed` attribute to null but will
failed (GenericDBaseDao.generateValue() will overwrite the null value
and set a new date.

Signed-off-by: Marc-AurÃ¨le Brothier 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack fix/VMTemplateZoneDao

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1730.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1730


commit 3519bfb8d8aa7b90e97a57c0e79a1d520e210592
Author: Marc-AurÃ¨le Brothier 
Date:   2016-10-25T13:18:59Z

VMTemplateZone needs some love

It's incorrect to use the findIncludingRemovedBy and
listIncludingRemovedBy for the common list and find operation. Moreover
the DAO is trying to update the `removed` attribute to null but will
failed (GenericDBaseDao.generateValue() will overwrite the null value
and set a new date.

Signed-off-by: Marc-AurÃ¨le Brothier 




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1730: VMTemplateZone needs some love

2016-11-02 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1730#discussion_r86242383
  
--- Diff: 
engine/schema/src/com/cloud/storage/dao/VMTemplateZoneDaoImpl.java ---
@@ -84,14 +84,11 @@ public VMTemplateZoneVO findByZoneTemplate(long zoneId, 
long templateId) {
 }
 
 @Override
+@DB
 public void deletePrimaryRecordsForTemplate(long templateId) {
 SearchCriteria sc = TemplateSearch.create();
 sc.setParameters("template_id", templateId);
-TransactionLegacy txn = TransactionLegacy.currentTxn();
--- End diff --

It's done through the `@DB` annotation


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1730: VMTemplateZone needs some love

2016-11-03 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1730
  
@rhtyd I triggered another jenkins build by rebasing my branch on master 
this morning. I hope it will fix the failing test.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1730: VMTemplateZone needs some love

2016-11-04 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1730
  
No, there isn't any bug related to this yet. I discovered this when I 
implemented something new in our CS (allow a template to be re-downloaded from 
its url via the `CopyTemplate` command). This DAO isn't following the logic of 
other DAOs currently so I thought it was enough to push a PR.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1730: VMTemplateZone needs some love

2016-11-09 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1730#discussion_r87170352
  
--- Diff: 
engine/schema/src/com/cloud/storage/dao/VMTemplateZoneDaoImpl.java ---
@@ -84,14 +84,11 @@ public VMTemplateZoneVO findByZoneTemplate(long zoneId, 
long templateId) {
 }
 
 @Override
+@DB
 public void deletePrimaryRecordsForTemplate(long templateId) {
 SearchCriteria sc = TemplateSearch.create();
 sc.setParameters("template_id", templateId);
-TransactionLegacy txn = TransactionLegacy.currentTxn();
--- End diff --

I looked a bit more into that, and `@DB` use the same class as 
`TransactionLegacy`, which is different than `Transaction`. The change 
f62e28c1ec125263613d32856b11ef50b815e573 added a new transaction API.

Having the annotation or the line of code to create the transaction is 
similar. IMO the annotation is cleaner.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1760: CLOUDSTACK-9593: userdata: enforce data is a ...

2016-11-11 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1760

CLOUDSTACK-9593: userdata: enforce data is a multiple of 4 characters

Python base64 requires that the string is a multiple of 4 characters but
the Apache codec does not. RFC says is not mandatory but the data should
not fail the VR script (vmdata.py).

Signed-off-by: Marc-AurÃ¨le Brothier 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack fix/CLOUDSTACK-9593

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1760.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1760


commit 9575195c01ddd8079f3fdacf5712071db14d5207
Author: Marc-AurÃ¨le Brothier 
Date:   2016-11-11T07:53:41Z

userdata: enforce data is a multiple of 4 characters

Python base64 requires that the string is a multiple of 4 characters but
the Apache codec does not. RFC says is not mandatory but the data should
not fail the VR script (vmdata.py).

Signed-off-by: Marc-AurÃ¨le Brothier 




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1760: CLOUDSTACK-9593: userdata: enforce data is a multipl...

2016-11-15 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1760
  
@jburwell  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 feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1764: Should not fetch resource count for removed e...

2016-11-15 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1764

Should not fetch resource count for removed entity

Fetch the number of resourceCount by domain and account excluding the 
removed ones.

Signed-off-by: Marc-AurÃ¨le Brothier 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack fix/CLOUDSTACK-9597

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1764.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1764


commit 267597174140009e6717de15030021c36cf27877
Author: Marc-AurÃ¨le Brothier 
Date:   2016-11-15T08:39:11Z

Should not fetch resource count for removed entity

Signed-off-by: Marc-AurÃ¨le Brothier 




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1764: CLOUDSTACK-9597: Should not fetch resource count for...

2016-11-15 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1764
  
Will fix the checks in `ConfigurationServerImpl`:
- 
https://github.com/apache/cloudstack/blob/master/server/src/com/cloud/server/ConfigurationServerImpl.java#L1426
- 
https://github.com/apache/cloudstack/blob/master/server/src/com/cloud/server/ConfigurationServerImpl.java#L1455


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1760: CLOUDSTACK-9593: userdata: enforce data is a multipl...

2016-11-17 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1760
  
I changed the code to make it resilient to incorrect data that are already 
in the database. Moreover, instead of throwing an error when the padding is not 
present, it appends it. I think it's a better solution. I'll squash it when 
it's an accepted.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1768: CLOUDSTACK 9601: Upgrade: change logic for up...

2016-11-18 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1768

CLOUDSTACK 9601: Upgrade: change logic for update path for files

For going from version A to version D, it uses to run the SQL files in
that order: A -> B -> C -> D -> A-cleanup -> B-cleanup -> C-cleanup ->
D-cleanup. If you had upgraded each version separatively you would have
run A -> A-cleanup -> B -> B-cleanup -> C -> C-cleanup -> D ->
D-cleanup.
This change the logic to follow the same path if you are jumping over
versions.

Signed-off-by: Marc-AurÃ¨le Brothier 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack fix/CLOUDSTACK-9601

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1768.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1768


commit 66a839af4cf10ac65bd66cf4c7875c6fc76d6317
Author: Marc-AurÃ¨le Brothier 
Date:   2016-11-18T14:40:13Z

Upgrade: change logic for update path for files

For going from version A to version D, it uses to run the SQL files in
that order: A -> B -> C -> D -> A-cleanup -> B-cleanup -> C-cleanup ->
D-cleanup. If you had upgraded each version separatively you would have
run A -> A-cleanup -> B -> B-cleanup -> C -> C-cleanup -> D ->
D-cleanup.
This change the logic to follow the same path if you are jumping over
versions.

Signed-off-by: Marc-AurÃ¨le Brothier 

commit 0dba4be398e0966eb506ef76d2b15bf88c8c0100
Author: Marc-AurÃ¨le Brothier 
Date:   2016-11-18T14:46:30Z

Cleanup upgrade path from too old version

Today, the schema is created as a version 4.0.0

Signed-off-by: Marc-AurÃ¨le Brothier 




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2016-11-18 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1768
  
The second commit can be removed if the cleanup is not wanted.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1640: CLOUDSTACK-9458: Fix HA bug when VMs are stopped on ...

2016-11-21 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1640
  
To get back to your previous comment @koushik-das on the broken scenario: 
what happen if the host is not reachable and the VMs are using a remote 
storage. With the fencing operation marking the VM as stopped, does it mean 
that the same remote disk volume is used if the VM is spawned on another host 
(while the other one still running on the first host)?

@abhinandanprateek if the reason to fence off the VM is to clean up 
resources, IMO this should be the job of the VM sync, on the ping 
command/startup command. In case a host is lost, the capacity of the cluster 
should reflect the lose of that host and the stat capacity should calculate its 
value based on the hosts that are Up only. When a host comes back (possibly 
with some VMs still running), the startup command should sync the VM states and 
the capacity of the cluster/zone should be updated. 
In short, cleaning up resources that are not "reachable" anymore should not 
be needed and should not be taken into account when calculating the actual 
capacity of the cluster/zone.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1760: CLOUDSTACK-9593: userdata: enforce data is a multipl...

2016-11-21 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1760
  
@rhtyd the SQL function I'm using to fix current user data in the database 
is not present in MySQL 5.5, but only in 5.6 (TO_BASE64, FROM_BASE64). I have 
to find a workaround, either in SQL or in Java to fix previous data.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1554: CLOUDSTACK-9602: API: improve resource limits compre...

2016-11-22 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1554
  
I added the JIRA ticket. Isn't it simpler if you do the squash when you'll 
merge the PR on master?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1760: CLOUDSTACK-9593: userdata: enforce data is a multipl...

2016-11-24 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1760
  
@rhtyd The DB update class for the DatabaseUpgradeChecker doesn't work on 
Travis. Is there something I'm doing wrong, or missed something?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1760: CLOUDSTACK-9593: userdata: enforce data is a multipl...

2016-11-25 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1760
  
right, I forgot about the match required with the version in the pom. I 
thought I should prepare the stuff for the next 4.9.x release but that cannot 
work unless the pom files are updated too. I will just move the code inside 
Upgrade490to4910.java to Upgrade4910to4920.java when it's available.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1798: API: affinitygroupids or affinitygroupnames m...

2016-11-29 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1798

API: affinitygroupids or affinitygroupnames must be given

Return an exception if both parameter are missing.

Signed-off-by: Marc-AurÃ¨le Brothier 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack fix/CLOUDSTACK-9631

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1798.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1798


commit 907c46ae73bd5f5f4a7182c38665ccc64b62a3c4
Author: Marc-AurÃ¨le Brothier 
Date:   2016-11-29T12:54:28Z

API: affinitygroupids or affinitygroupnames must be given

Return an exception if both parameter are missing.

Signed-off-by: Marc-AurÃ¨le Brothier 




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1832: CLOUDSTACK-9652 Job framework - Cancelling async job...

2016-12-15 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1832
  
@karuturi It's a good feature but I don't see where the job gets actually 
cancelled. Reading the changes I can only see that the response of the job will 
become a cancelled operation, but the actual job does not get kill/cancel on 
the hypervisor for example. Did I miss something or is it not fully implemented 
yet?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1832: CLOUDSTACK-9652 Job framework - Cancelling async job...

2016-12-15 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1832
  
@karuturi Ok thanks for the clarifications, and it's the scenario I thought 
about too. That being said, I'm currently thinking of a new approach for the 
command sequencer because having implemented the live migration, the 
non-parallel commands isn't optimal at all when you have long running 
sequential commands on a hypervisor. And I tend to think that's the reason 
behind your PR, isn't it? The way it's currently done is too simple (if a job 
cannot be run in parallel on the HV, it will put in a queue any other coming 
job that needs to run on this same HV). IMO this sequencing should take into 
account what kind of job is coming and for which type of resources. For example 
a security group update, a VM start and a migration for different VMs should be 
able to run in parallel because they are unrelated. With today design, it isn't 
possible.

So don't you think we're better of rewriting the sequencer to let more 
commands being executed in parallel to avoid this bottleneck on the 
AgentAttache? It would normally make the cancellation not needed in the way you 
implemented it since less jobs will be queued.

If we wish to be able to cancel a job, IMHO it should cancel the job down 
on the hypervisor too, thus clearing normally the resources involved as if the 
execution didn't go well.

Otherwise, the way you implemented it. I would not let a job being cancel 
if it has been sent to the hypervisor to clearly return to the user that it 
wasn't cancelable anymore (you're too late! -> seq number isn't in `_requests` 
anymore so it has been sent to the HV). I'm putting more comment in the code.

What do you think?


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling as...

2016-12-15 Thread marcaurele

Github user marcaurele commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1832#discussion_r92676861
  
--- Diff: 
engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java ---
@@ -399,10 +414,22 @@ public void send(final Request req, final Listener 
listener) throws AgentUnavail
 try {
 for (int i = 0; i < 2; i++) {
 Answer[] answers = null;
+job = _agentMgr._asyncJobDao.findById(jobId);
+if (job != null && job.getStatus() == 
JobInfo.Status.CANCELLED) {
+throw new 
OperationCancelledException(req.getCommands(), _id, seq, wait, false);
+}
 try {
 answers = sl.waitFor(wait);
+job = _agentMgr._asyncJobDao.findById(jobId);
+if (job != null && job.getStatus() == 
JobInfo.Status.CANCELLED) {
+throw new 
OperationCancelledException(req.getCommands(), _id, seq, wait, false);
--- End diff --

Why do you want to throw an `OperationCancelledException` if the we have 
the job answer. It's better to let the normal response come back to the user.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1934: [CLOUDSTACK-9772] Template: perform a HEAD re...

2017-02-06 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1934

[CLOUDSTACK-9772] Template: perform a HEAD request to check file size from 
a URL

Signed-off-by: Marc-AurÃ¨le Brothier 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack fix/CLOUDSTACK-9772

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1934.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1934


commit 46fb454f4deb6c36f52a6e943b1f23c45508ab04
Author: Marc-AurÃ¨le Brothier 
Date:   2017-02-06T12:51:14Z

Template: perform a HEAD request to check file size from a URL

Signed-off-by: Marc-AurÃ¨le Brothier 




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1936: [CLOUDSTACK-9773] API: don't break API output...

2017-02-06 Thread marcaurele

GitHub user marcaurele opened a pull request:

https://github.com/apache/cloudstack/pull/1936

[CLOUDSTACK-9773] API: don't break API output with non-printable characters

Signed-off-by: Marc-AurÃ¨le Brothier 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/exoscale/cloudstack CLOUDSTACK-9773

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1936.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1936


commit 215d8f6752219933e6b93f4f0c8579086f6ddcf9
Author: Marc-AurÃ¨le Brothier 
Date:   2017-02-06T15:18:23Z

API: don't break API output with non-printable characters

Signed-off-by: Marc-AurÃ¨le Brothier 




---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1936: [CLOUDSTACK-9773] API: don't break API output with n...

2017-02-12 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1936
  
Forced push to trigger another build. Looks like the failed test is not 
stable.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1934: [CLOUDSTACK-9772] Template: perform a HEAD request t...

2017-02-12 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1934
  
@borisstoyanov Dunno what's wrong with the failed tests. Its' failing at 
https://github.com/apache/cloudstack/blob/master/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java#L83.
 The log says that the ssvm endpoint was not ready.
Could you try to launch another bueorangutan test suite?

We're running that fix in production since a week and haven't hit a problem 
yet.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1934: [CLOUDSTACK-9772] Template: perform a HEAD request t...

2017-02-14 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1934
  
@remibergsma Good point, I was aware of that difference, which I think 
doesn't help to make systems reliable.
Another improvement would be to remove this function and refactor the code 
to read sizes from the template objects in the DB or on the GET requests when 
downloading them.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-21 Thread marcaurele

Github user marcaurele commented on the issue:

https://github.com/apache/cloudstack/pull/1768
  
@rhtyd Changing the sequence is only idempotent if you have been upgrading 
to each new versions step by step. If you jumped other versions, then the path 
is different for each original version. This fix wants to avoid such a 
difference.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

74 matches

Mail list logo