cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nitin Mehta" <nitin.me...@citrix.com>
Subject Re: Review Request 25248: Fix NPE in case VM is started and its template does not exist anymore
Date Tue, 02 Sep 2014 20:45:10 GMT


> On Sept. 2, 2014, 7:01 p.m., Nitin Mehta wrote:
> > engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java, line 814
> > <https://reviews.apache.org/r/25248/diff/1/?file=673914#file673914line814>
> >
> >     Checking for template==null masks the whole problem. 
> >     1. Such validations should have happenned in the deployvm api layer if it comes
from that api.
> >     2. If its coming from a startvm api its perfectly fine to have the template
removed since the volume already exists. 
> >     3. If you see how template is used below...if it has to 'create' a new volume
the template shouldnt be removed but again the validations should be in api layer.
> 
> Rohit Yadav wrote:
>     So, I can read code too, upper layers are not passing the template so what do you
propose? How may I fix this then?

Firstly, check whether the issue is reproducible. Just realized that from 4.3 onwards templates
have 'Inactive' state to mark it removed. Removed attribute should never be set so this exception
shouldnt be hit . Check when is the removed flag set as it should be a bug(check CLOUDSTACK-5997
and backporting it already fixes that). 
Secondly, even if this bug doesnt exist, do a sanity check and see this kind of check is in
the api which will ultimately call this method. Check if apis are missing them. But some apis
might not need it like StartVm, Rebootvm where new volume is not created. Do write a util
method which could be shared by all apis. I guess that should be good enough.


- Nitin


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


On Sept. 2, 2014, 1:53 p.m., Rohit Yadav wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25248/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2014, 1:53 p.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk, edison su, Darren Shepherd, Sebastien
Goasguen, and Hugo Trippaers.
> 
> 
> Bugs: CLOUDSTACK-6945
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6945
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fixes https://issues.apache.org/jira/browse/CLOUDSTACK-6945
> 
> 
> Diffs
> -----
> 
>   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 2fd7a52 
> 
> Diff: https://reviews.apache.org/r/25248/diff/
> 
> 
> Testing
> -------
> 
> Builds cleanly, will throw resource not available exception when template does not exist.
> 
> 
> Thanks,
> 
> Rohit Yadav
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message