On Sep 4, 2014, at 4:36 PM, Nitin Mehta <nitin.me...@citrix.com> wrote:
> > >> 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? >> >> Nitin Mehta wrote: >> 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. >> >> Rohit Yadav wrote: >> This was for 4.3.1, I guess this is a special case and not a blocker. The >> issue was reproducible when say an admin removed a template where a user may >> be trying to create a VM and was in a wizard. I'm closing as we're not >> putting this for 4.3.1 release. > > Please punt it for 4.5. We should fix it. > Is this the same bug but with a fix ? https://reviews.apache.org/r/25348/ > > - 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 >> >> >
