RE: Review Request 12510: CLOUDSTACK 3476 : deleteDomain api should fail when release dedicated resource to that domain fails:

2013-07-18 Thread Saksham Srivastava
The fix should qualify for 4.1.1

Thanks,
Saksham

-Original Message-
From: Musayev, Ilya [mailto:imusa...@webmd.net] 
Sent: Thursday, July 18, 2013 6:41 AM
To: dev@cloudstack.apache.org; Alena Prokharchyk; Devdeep Singh
Cc: Saksham Srivastava; cloudstack
Subject: RE: Review Request 12510: CLOUDSTACK 3476 : deleteDomain api should 
fail when release dedicated resource to that domain fails:

Does this fix qualify for 4.1.1?

 -Original Message-
 From: Alena Prokharchyk [mailto:nore...@reviews.apache.org] On Behalf 
 Of Alena Prokharchyk
 Sent: Wednesday, July 17, 2013 2:26 PM
 To: Devdeep Singh; Alena Prokharchyk
 Cc: Saksham Srivastava; cloudstack
 Subject: Re: Review Request 12510: CLOUDSTACK 3476 : deleteDomain api 
 should fail when release dedicated resource to that domain fails:
 
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/12510/#review23314
 ---
 
 Ship it!
 
 
 Ship It!
 
 - Alena Prokharchyk
 
 
 On July 13, 2013, 6 a.m., Saksham Srivastava wrote:
 
  ---
  This is an automatically generated e-mail. To reply, visit:
  https://reviews.apache.org/r/12510/
  ---
 
  (Updated July 13, 2013, 6 a.m.)
 
 
  Review request for cloudstack, Alena Prokharchyk and Devdeep Singh.
 
 
  Bugs: 3476
 
 
  Repository: cloudstack-git
 
 
  Description
  ---
 
  In case the release dedicate resource fails, deletion of domain 
  should not
 happen.
  Whenever deleting a domain all the resources dedicated to it must be
 released of dedication and moved to shared pool.
  Currently even if release API fails the deleteDomain API is executed
 successfully.
 
  Further if there are dedicated resources to a domiain and cleanup is 
  not
 true, resources should not be released.
  Added checks to prohibit this behaviour.
 
 
  Diffs
  -
 
server/src/com/cloud/user/DomainManagerImpl.java aad5787
 
  Diff: https://reviews.apache.org/r/12510/diff/
 
 
  Testing
  ---
 
  If domain has dedicated resources, cleanup=true will release 
  dedication
 and delete the domain.
  cleanup=false will not release dedication and will not delete the domain.
 
 
  Thanks,
 
  Saksham Srivastava
 
 



Re: Review Request 12510: CLOUDSTACK 3476 : deleteDomain api should fail when release dedicated resource to that domain fails:

2013-07-17 Thread Alena Prokharchyk

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

Ship it!


Ship It!

- Alena Prokharchyk


On July 13, 2013, 6 a.m., Saksham Srivastava wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/12510/
 ---
 
 (Updated July 13, 2013, 6 a.m.)
 
 
 Review request for cloudstack, Alena Prokharchyk and Devdeep Singh.
 
 
 Bugs: 3476
 
 
 Repository: cloudstack-git
 
 
 Description
 ---
 
 In case the release dedicate resource fails, deletion of domain should not 
 happen. 
 Whenever deleting a domain all the resources dedicated to it must be released 
 of dedication and moved to shared pool. 
 Currently even if release API fails the deleteDomain API is executed 
 successfully. 
 
 Further if there are dedicated resources to a domiain and cleanup is not 
 true, resources should not be released.
 Added checks to prohibit this behaviour.
 
 
 Diffs
 -
 
   server/src/com/cloud/user/DomainManagerImpl.java aad5787 
 
 Diff: https://reviews.apache.org/r/12510/diff/
 
 
 Testing
 ---
 
 If domain has dedicated resources, cleanup=true will release dedication and 
 delete the domain.
 cleanup=false will not release dedication and will not delete the domain.
 
 
 Thanks,
 
 Saksham Srivastava
 




RE: Review Request 12510: CLOUDSTACK 3476 : deleteDomain api should fail when release dedicated resource to that domain fails:

2013-07-17 Thread Musayev, Ilya
Does this fix qualify for 4.1.1?

 -Original Message-
 From: Alena Prokharchyk [mailto:nore...@reviews.apache.org] On Behalf Of
 Alena Prokharchyk
 Sent: Wednesday, July 17, 2013 2:26 PM
 To: Devdeep Singh; Alena Prokharchyk
 Cc: Saksham Srivastava; cloudstack
 Subject: Re: Review Request 12510: CLOUDSTACK 3476 : deleteDomain api
 should fail when release dedicated resource to that domain fails:
 
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/12510/#review23314
 ---
 
 Ship it!
 
 
 Ship It!
 
 - Alena Prokharchyk
 
 
 On July 13, 2013, 6 a.m., Saksham Srivastava wrote:
 
  ---
  This is an automatically generated e-mail. To reply, visit:
  https://reviews.apache.org/r/12510/
  ---
 
  (Updated July 13, 2013, 6 a.m.)
 
 
  Review request for cloudstack, Alena Prokharchyk and Devdeep Singh.
 
 
  Bugs: 3476
 
 
  Repository: cloudstack-git
 
 
  Description
  ---
 
  In case the release dedicate resource fails, deletion of domain should not
 happen.
  Whenever deleting a domain all the resources dedicated to it must be
 released of dedication and moved to shared pool.
  Currently even if release API fails the deleteDomain API is executed
 successfully.
 
  Further if there are dedicated resources to a domiain and cleanup is not
 true, resources should not be released.
  Added checks to prohibit this behaviour.
 
 
  Diffs
  -
 
server/src/com/cloud/user/DomainManagerImpl.java aad5787
 
  Diff: https://reviews.apache.org/r/12510/diff/
 
 
  Testing
  ---
 
  If domain has dedicated resources, cleanup=true will release dedication
 and delete the domain.
  cleanup=false will not release dedication and will not delete the domain.
 
 
  Thanks,
 
  Saksham Srivastava
 
 



Re: Review Request 12510: CLOUDSTACK 3476 : deleteDomain api should fail when release dedicated resource to that domain fails:

2013-07-12 Thread Alena Prokharchyk

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


1) Replace

hasDedicatedResources == false

with 

!hasDedicatedResources


and else if (hasDedicatedResources == true) {

with 

else if (hasDedicatedResources) {


2) Add rollbackState=true to this statement:

for (DedicatedResourceVO dr : dedicatedResources){
if (!_dedicatedDao.remove(dr.getId())) {
s_logger.warn(Fail to release dedicated 
resources for domain  + domain.getId());
insert here
return false;
}
}

- Alena Prokharchyk


On July 12, 2013, 2:01 p.m., Saksham Srivastava wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/12510/
 ---
 
 (Updated July 12, 2013, 2:01 p.m.)
 
 
 Review request for cloudstack, Alena Prokharchyk and Devdeep Singh.
 
 
 Bugs: 3476
 
 
 Repository: cloudstack-git
 
 
 Description
 ---
 
 In case the release dedicate resource fails, deletion of domain should not 
 happen. 
 Whenever deleting a domain all the resources dedicated to it must be released 
 of dedication and moved to shared pool. 
 Currently even if release API fails the deleteDomain API is executed 
 successfully. 
 
 Further if there are dedicated resources to a domiain and cleanup is not 
 true, resources should not be released.
 Added checks to prohibit this behaviour.
 
 
 Diffs
 -
 
   server/src/com/cloud/user/DomainManagerImpl.java aad5787 
 
 Diff: https://reviews.apache.org/r/12510/diff/
 
 
 Testing
 ---
 
 If domain has dedicated resources, cleanup=true will release dedication and 
 delete the domain.
 cleanup=false will not release dedication and will not delete the domain.
 
 
 Thanks,
 
 Saksham Srivastava
 




Re: Review Request 12510: CLOUDSTACK 3476 : deleteDomain api should fail when release dedicated resource to that domain fails:

2013-07-12 Thread Saksham Srivastava


 On July 12, 2013, 5:08 p.m., Alena Prokharchyk wrote:
  1) Replace
  
  hasDedicatedResources == false
  
  with 
  
  !hasDedicatedResources
  
  
  and else if (hasDedicatedResources == true) {
  
  with 
  
  else if (hasDedicatedResources) {
  
  
  2) Add rollbackState=true to this statement:
  
  for (DedicatedResourceVO dr : dedicatedResources){
  if (!_dedicatedDao.remove(dr.getId())) {
  s_logger.warn(Fail to release 
  dedicated resources for domain  + domain.getId());
  insert here
  return false;
  }
  }

Alena for comment #2 :
rollBackState is not a local variable to this method(cleanupDomain)
Also this should not be required because if cleanupDomain fails (returns false) 
, rollBackState is always set to true in deleteDomain.


- Saksham


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


On July 12, 2013, 2:01 p.m., Saksham Srivastava wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/12510/
 ---
 
 (Updated July 12, 2013, 2:01 p.m.)
 
 
 Review request for cloudstack, Alena Prokharchyk and Devdeep Singh.
 
 
 Bugs: 3476
 
 
 Repository: cloudstack-git
 
 
 Description
 ---
 
 In case the release dedicate resource fails, deletion of domain should not 
 happen. 
 Whenever deleting a domain all the resources dedicated to it must be released 
 of dedication and moved to shared pool. 
 Currently even if release API fails the deleteDomain API is executed 
 successfully. 
 
 Further if there are dedicated resources to a domiain and cleanup is not 
 true, resources should not be released.
 Added checks to prohibit this behaviour.
 
 
 Diffs
 -
 
   server/src/com/cloud/user/DomainManagerImpl.java aad5787 
 
 Diff: https://reviews.apache.org/r/12510/diff/
 
 
 Testing
 ---
 
 If domain has dedicated resources, cleanup=true will release dedication and 
 delete the domain.
 cleanup=false will not release dedication and will not delete the domain.
 
 
 Thanks,
 
 Saksham Srivastava