You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Harikrishna Patnala <ha...@citrix.com> on 2013/02/01 15:20:44 UTC

Re: Review Request: CLOUDSTACK-667: VM's base image updation facility

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

(Updated Feb. 1, 2013, 2:20 p.m.)


Review request for cloudstack, Abhinandan Prateek and Koushik Das.


Changes
-------

updated the diff


Description
-------

CLOUDSTACK-667: VM's base image updation facility
disconnect the root disk >> destroy root disk >> create a new disk from new template >> attach new disk as root disk to the vm 


This addresses bug CLOUDSTACK-667.


Diffs (updated)
-----

  api/src/org/apache/cloudstack/api/command/user/vm/RestoreVMCmd.java d906c7f 
  server/src/com/cloud/vm/UserVmManagerImpl.java a53e132 
  server/test/com/cloud/vm/UserVmManagerTest.java PRE-CREATION 

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


Testing
-------

Did the manual testing locally


Thanks,

Harikrishna Patnala


Re: Review Request: CLOUDSTACK-667: VM's base image updation facility

Posted by Harikrishna Patnala <ha...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9158/
-----------------------------------------------------------

(Updated Feb. 21, 2013, 9:21 a.m.)


Review request for cloudstack, Abhinandan Prateek and Koushik Das.


Changes
-------

fixes tabs/spaces. can ship now


Description
-------

CLOUDSTACK-667: VM's base image updation facility
disconnect the root disk >> destroy root disk >> create a new disk from new template >> attach new disk as root disk to the vm 


This addresses bug CLOUDSTACK-667.


Diffs (updated)
-----

  api/src/org/apache/cloudstack/api/command/user/vm/RestoreVMCmd.java e98c2f2 
  server/src/com/cloud/vm/UserVmManagerImpl.java df97609 
  server/test/com/cloud/vm/UserVmManagerTest.java PRE-CREATION 

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


Testing
-------

Did the manual testing locally


Thanks,

Harikrishna Patnala


Re: Review Request: CLOUDSTACK-667: VM's base image updation facility

Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9158/#review16836
-----------------------------------------------------------

Ship it!


Ship It!

- Koushik Das


On Feb. 18, 2013, 6:44 p.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9158/
> -----------------------------------------------------------
> 
> (Updated Feb. 18, 2013, 6:44 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
> 
> 
> Description
> -------
> 
> CLOUDSTACK-667: VM's base image updation facility
> disconnect the root disk >> destroy root disk >> create a new disk from new template >> attach new disk as root disk to the vm 
> 
> 
> This addresses bug CLOUDSTACK-667.
> 
> 
> Diffs
> -----
> 
>   api/src/org/apache/cloudstack/api/command/user/vm/RestoreVMCmd.java e98c2f2 
>   server/src/com/cloud/vm/UserVmManagerImpl.java df97609 
>   server/test/com/cloud/vm/UserVmManagerTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9158/diff/
> 
> 
> Testing
> -------
> 
> Did the manual testing locally
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request: CLOUDSTACK-667: VM's base image updation facility

Posted by Harikrishna Patnala <ha...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9158/
-----------------------------------------------------------

(Updated Feb. 18, 2013, 6:44 p.m.)


Review request for cloudstack, Abhinandan Prateek and Koushik Das.


Changes
-------

updated new diff with fixes


Description
-------

CLOUDSTACK-667: VM's base image updation facility
disconnect the root disk >> destroy root disk >> create a new disk from new template >> attach new disk as root disk to the vm 


This addresses bug CLOUDSTACK-667.


Diffs (updated)
-----

  api/src/org/apache/cloudstack/api/command/user/vm/RestoreVMCmd.java e98c2f2 
  server/src/com/cloud/vm/UserVmManagerImpl.java df97609 
  server/test/com/cloud/vm/UserVmManagerTest.java PRE-CREATION 

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


Testing
-------

Did the manual testing locally


Thanks,

Harikrishna Patnala


Re: Review Request: CLOUDSTACK-667: VM's base image updation facility

Posted by Harikrishna Patnala <ha...@citrix.com>.

> On Feb. 15, 2013, 8:44 a.m., Koushik Das wrote:
> > server/src/com/cloud/vm/UserVmManagerImpl.java, line 4799
> > <https://reviews.apache.org/r/9158/diff/4/?file=256587#file256587line4799>
> >
> >     What if template is null? The null check that's happening in the 'else' case should be made for both existing as well as new template

As the newtemplate UUID is passed as parameter, this is taken care in the UUID to ID conversion while finding the objectVO and checking for its internal Id.


- Harikrishna


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


On Feb. 7, 2013, 11:32 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9158/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2013, 11:32 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
> 
> 
> Description
> -------
> 
> CLOUDSTACK-667: VM's base image updation facility
> disconnect the root disk >> destroy root disk >> create a new disk from new template >> attach new disk as root disk to the vm 
> 
> 
> This addresses bug CLOUDSTACK-667.
> 
> 
> Diffs
> -----
> 
>   api/src/org/apache/cloudstack/api/command/user/vm/RestoreVMCmd.java e98c2f2 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 390a2a2 
>   server/test/com/cloud/vm/UserVmManagerTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9158/diff/
> 
> 
> Testing
> -------
> 
> Did the manual testing locally
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request: CLOUDSTACK-667: VM's base image updation facility

Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9158/#review16635
-----------------------------------------------------------



server/src/com/cloud/vm/UserVmManagerImpl.java
<https://reviews.apache.org/r/9158/#comment35305>

    What if template is null? The null check that's happening in the 'else' case should be made for both existing as well as new template


- Koushik Das


On Feb. 7, 2013, 11:32 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9158/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2013, 11:32 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
> 
> 
> Description
> -------
> 
> CLOUDSTACK-667: VM's base image updation facility
> disconnect the root disk >> destroy root disk >> create a new disk from new template >> attach new disk as root disk to the vm 
> 
> 
> This addresses bug CLOUDSTACK-667.
> 
> 
> Diffs
> -----
> 
>   api/src/org/apache/cloudstack/api/command/user/vm/RestoreVMCmd.java e98c2f2 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 390a2a2 
>   server/test/com/cloud/vm/UserVmManagerTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9158/diff/
> 
> 
> Testing
> -------
> 
> Did the manual testing locally
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request: CLOUDSTACK-667: VM's base image updation facility

Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9158/#review16638
-----------------------------------------------------------



server/src/com/cloud/vm/UserVmManagerImpl.java
<https://reviews.apache.org/r/9158/#comment35312>

    In case of restoreVM can newTemplateId be null even if the user passes UUID for new template? If possible do you want to continue with restoreVm with original template or fail the operation


- Koushik Das


On Feb. 7, 2013, 11:32 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9158/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2013, 11:32 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
> 
> 
> Description
> -------
> 
> CLOUDSTACK-667: VM's base image updation facility
> disconnect the root disk >> destroy root disk >> create a new disk from new template >> attach new disk as root disk to the vm 
> 
> 
> This addresses bug CLOUDSTACK-667.
> 
> 
> Diffs
> -----
> 
>   api/src/org/apache/cloudstack/api/command/user/vm/RestoreVMCmd.java e98c2f2 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 390a2a2 
>   server/test/com/cloud/vm/UserVmManagerTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9158/diff/
> 
> 
> Testing
> -------
> 
> Did the manual testing locally
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request: CLOUDSTACK-667: VM's base image updation facility

Posted by Harikrishna Patnala <ha...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9158/
-----------------------------------------------------------

(Updated Feb. 7, 2013, 11:32 a.m.)


Review request for cloudstack, Abhinandan Prateek and Koushik Das.


Changes
-------

new diff with relevant fixes and added unit test


Description
-------

CLOUDSTACK-667: VM's base image updation facility
disconnect the root disk >> destroy root disk >> create a new disk from new template >> attach new disk as root disk to the vm 


This addresses bug CLOUDSTACK-667.


Diffs (updated)
-----

  api/src/org/apache/cloudstack/api/command/user/vm/RestoreVMCmd.java e98c2f2 
  server/src/com/cloud/vm/UserVmManagerImpl.java 390a2a2 
  server/test/com/cloud/vm/UserVmManagerTest.java PRE-CREATION 

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


Testing
-------

Did the manual testing locally


Thanks,

Harikrishna Patnala


Re: Review Request: CLOUDSTACK-667: VM's base image updation facility

Posted by Nitin Mehta <ni...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9158/#review16261
-----------------------------------------------------------


Your test file is giving an error on the review board. Can you please check ?

- Nitin Mehta


On Feb. 1, 2013, 2:20 p.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9158/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2013, 2:20 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
> 
> 
> Description
> -------
> 
> CLOUDSTACK-667: VM's base image updation facility
> disconnect the root disk >> destroy root disk >> create a new disk from new template >> attach new disk as root disk to the vm 
> 
> 
> This addresses bug CLOUDSTACK-667.
> 
> 
> Diffs
> -----
> 
>   api/src/org/apache/cloudstack/api/command/user/vm/RestoreVMCmd.java d906c7f 
>   server/src/com/cloud/vm/UserVmManagerImpl.java a53e132 
>   server/test/com/cloud/vm/UserVmManagerTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9158/diff/
> 
> 
> Testing
> -------
> 
> Did the manual testing locally
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>