You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Prachi Damle <pr...@citrix.com> on 2013/05/22 20:13:27 UTC

Review Request: CLOUDSTACK-2568 ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11330/
-----------------------------------------------------------

Review request for cloudstack and Chip Childers.


Description
-------

Patch created for 4.1, changes done:
- In VolumeReservationVO, the getter method of a column had a typo, causing us to create a wrong searchbuilder. It was searching over the 'id' column instead of 'vm_reservation_id' causing
- This bug was causing the vm deployment to choose a wrong pool during deployment since the search was choosing incorrectly
- This bug in the GenericSearchBuilder is also fixed - if the getter method does not use the standard 'get' or 'is' prefix, one should annotate that method using
 @Column(name = "<column_name>") and indicate which column this method refers to. This will cause the GenericSearchBuilder to identify the field correctly.
- Also, let planner search for pools instead of selecting the one reserved - because there is no way currently to pass multiple pool information to the planner and this may cause issues when a VM has multiple disks.


This addresses bug CLOUDSTACK-2568.


Diffs
-----

  engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java 6bfe953 
  engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/VolumeReservationVO.java cbad9ab 
  engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/dao/VolumeReservationDaoImpl.java 4b1b1e6 
  server/src/com/cloud/deploy/FirstFitPlanner.java c49d25e 
  utils/src/com/cloud/utils/db/GenericSearchBuilder.java 8bbe07c 

Diff: https://reviews.apache.org/r/11330/diff/


Testing
-------

Manual testing done, also the fix has been already verified on a test environment by the bug reporter.


Thanks,

Prachi Damle


Re: Review Request: CLOUDSTACK-2568 ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)

Posted by Chip Childers <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11330/#review20905
-----------------------------------------------------------


This does not apply:

Applying: CLOUDSTACK-2568: ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)
/home/sg-user/incubator-cloudstack/.git/rebase-apply/patch:167: space before tab in indent.
    			    String colName = ann.name();
/home/sg-user/incubator-cloudstack/.git/rebase-apply/patch:168: space before tab in indent.
    			    for (Map.Entry<String, Attribute> attr : _attrs.entrySet()) {
/home/sg-user/incubator-cloudstack/.git/rebase-apply/patch:169: space before tab in indent.
        			    if (colName.equals(attr.getValue().columnName)) {
/home/sg-user/incubator-cloudstack/.git/rebase-apply/patch:170: space before tab in indent.
            			    set(attr.getKey());
/home/sg-user/incubator-cloudstack/.git/rebase-apply/patch:171: space before tab in indent.
            			    return null;
error: patch failed: engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/VolumeReservationVO.java:16
error: engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/VolumeReservationVO.java: patch does not apply
error: patch failed: engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/dao/VolumeReservationDaoImpl.java:49
error: engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/dao/VolumeReservationDaoImpl.java: patch does not apply
Patch failed at 0001 CLOUDSTACK-2568: ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

- Chip Childers


On May 22, 2013, 6:13 p.m., Prachi Damle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11330/
> -----------------------------------------------------------
> 
> (Updated May 22, 2013, 6:13 p.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> Patch created for 4.1, changes done:
> - In VolumeReservationVO, the getter method of a column had a typo, causing us to create a wrong searchbuilder. It was searching over the 'id' column instead of 'vm_reservation_id' causing
> - This bug was causing the vm deployment to choose a wrong pool during deployment since the search was choosing incorrectly
> - This bug in the GenericSearchBuilder is also fixed - if the getter method does not use the standard 'get' or 'is' prefix, one should annotate that method using
>  @Column(name = "<column_name>") and indicate which column this method refers to. This will cause the GenericSearchBuilder to identify the field correctly.
> - Also, let planner search for pools instead of selecting the one reserved - because there is no way currently to pass multiple pool information to the planner and this may cause issues when a VM has multiple disks.
> 
> 
> This addresses bug CLOUDSTACK-2568.
> 
> 
> Diffs
> -----
> 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java 6bfe953 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/VolumeReservationVO.java cbad9ab 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/dao/VolumeReservationDaoImpl.java 4b1b1e6 
>   server/src/com/cloud/deploy/FirstFitPlanner.java c49d25e 
>   utils/src/com/cloud/utils/db/GenericSearchBuilder.java 8bbe07c 
> 
> Diff: https://reviews.apache.org/r/11330/diff/
> 
> 
> Testing
> -------
> 
> Manual testing done, also the fix has been already verified on a test environment by the bug reporter.
> 
> 
> Thanks,
> 
> Prachi Damle
> 
>


Re: Review Request: CLOUDSTACK-2568 ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)

Posted by Chip Childers <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11330/#review20909
-----------------------------------------------------------

Ship it!


Committed to 4.1:

commit 78186c3b0201ecf55779b3f4bb6a3105fec1288d
Author: Prachi Damle <pr...@citrix.com>
Date:   Wed May 22 20:14:32 2013 +0100

    CLOUDSTACK-2568: ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)
    
    Patch for 4.1, changes:
    - In VolumeReservationVO, the getter method of a column had a typo, causing us to create a wrong searchbuilder. It was searching over the 'id' column instead of 'vm_reservation_id' causing
    - This bug was causing the vm deployment to choose a wrong pool during deployment since the search was choosing incorrectly
    - This bug in the GenericSearchBuilder is also fixed - if the getter method does not use the standard 'get' or 'is' prefix, one should annotate that method using
     @Column(name = "<column_name>") and indicate which column this method refers to. This will cause the GenericSearchBuilder to identify the field correctly.
    - Also, let planner search for pools instead of selecting the one reserved - because there is no way currently to pass multiple pool information to the planner and this may cause issues when a VM has multiple disks.
    
    Signed-off-by: Chip Childers <ch...@gmail.com>

- Chip Childers


On May 22, 2013, 6:13 p.m., Prachi Damle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11330/
> -----------------------------------------------------------
> 
> (Updated May 22, 2013, 6:13 p.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> Patch created for 4.1, changes done:
> - In VolumeReservationVO, the getter method of a column had a typo, causing us to create a wrong searchbuilder. It was searching over the 'id' column instead of 'vm_reservation_id' causing
> - This bug was causing the vm deployment to choose a wrong pool during deployment since the search was choosing incorrectly
> - This bug in the GenericSearchBuilder is also fixed - if the getter method does not use the standard 'get' or 'is' prefix, one should annotate that method using
>  @Column(name = "<column_name>") and indicate which column this method refers to. This will cause the GenericSearchBuilder to identify the field correctly.
> - Also, let planner search for pools instead of selecting the one reserved - because there is no way currently to pass multiple pool information to the planner and this may cause issues when a VM has multiple disks.
> 
> 
> This addresses bug CLOUDSTACK-2568.
> 
> 
> Diffs
> -----
> 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java 6bfe953 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/VolumeReservationVO.java cbad9ab 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/dao/VolumeReservationDaoImpl.java 4b1b1e6 
>   server/src/com/cloud/deploy/FirstFitPlanner.java c49d25e 
>   utils/src/com/cloud/utils/db/GenericSearchBuilder.java 8bbe07c 
> 
> Diff: https://reviews.apache.org/r/11330/diff/
> 
> 
> Testing
> -------
> 
> Manual testing done, also the fix has been already verified on a test environment by the bug reporter.
> 
> 
> Thanks,
> 
> Prachi Damle
> 
>


Re: Review Request: CLOUDSTACK-2568 ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11330/#review20908
-----------------------------------------------------------


Commit 78186c3b0201ecf55779b3f4bb6a3105fec1288d in branch refs/heads/4.1 from Chip Childers <ch...@gmail.com>
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=78186c3 ]

CLOUDSTACK-2568: ACS41 regression in storage subsystem (seen with local storage and 2 or more hosts)

Patch for 4.1, changes:
- In VolumeReservationVO, the getter method of a column had a typo, causing us to create a wrong searchbuilder. It was searching over the 'id' column instead of 'vm_reservation_id' causing
- This bug was causing the vm deployment to choose a wrong pool during deployment since the search was choosing incorrectly
- This bug in the GenericSearchBuilder is also fixed - if the getter method does not use the standard 'get' or 'is' prefix, one should annotate that method using
 @Column(name = "<column_name>") and indicate which column this method refers to. This will cause the GenericSearchBuilder to identify the field correctly.
- Also, let planner search for pools instead of selecting the one reserved - because there is no way currently to pass multiple pool information to the planner and this may cause issues when a VM has multiple disks.

Signed-off-by: Chip Childers <ch...@gmail.com>


- ASF Subversion and Git Services


On May 22, 2013, 6:13 p.m., Prachi Damle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11330/
> -----------------------------------------------------------
> 
> (Updated May 22, 2013, 6:13 p.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> Patch created for 4.1, changes done:
> - In VolumeReservationVO, the getter method of a column had a typo, causing us to create a wrong searchbuilder. It was searching over the 'id' column instead of 'vm_reservation_id' causing
> - This bug was causing the vm deployment to choose a wrong pool during deployment since the search was choosing incorrectly
> - This bug in the GenericSearchBuilder is also fixed - if the getter method does not use the standard 'get' or 'is' prefix, one should annotate that method using
>  @Column(name = "<column_name>") and indicate which column this method refers to. This will cause the GenericSearchBuilder to identify the field correctly.
> - Also, let planner search for pools instead of selecting the one reserved - because there is no way currently to pass multiple pool information to the planner and this may cause issues when a VM has multiple disks.
> 
> 
> This addresses bug CLOUDSTACK-2568.
> 
> 
> Diffs
> -----
> 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java 6bfe953 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/VolumeReservationVO.java cbad9ab 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/db/dao/VolumeReservationDaoImpl.java 4b1b1e6 
>   server/src/com/cloud/deploy/FirstFitPlanner.java c49d25e 
>   utils/src/com/cloud/utils/db/GenericSearchBuilder.java 8bbe07c 
> 
> Diff: https://reviews.apache.org/r/11330/diff/
> 
> 
> Testing
> -------
> 
> Manual testing done, also the fix has been already verified on a test environment by the bug reporter.
> 
> 
> Thanks,
> 
> Prachi Damle
> 
>