RE: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-17 Thread Musayev, Ilya 
Would this fix qualify for 4.1?


> -Original Message-
> From: Sheng Yang [mailto:nore...@reviews.apache.org] On Behalf Of Sheng
> Yang
> Sent: Wednesday, July 17, 2013 8:01 PM
> To: Sateesh Chodapuneedi; edison su; Chip Childers; Kelven Yang; Sheng
> Yang
> Cc: Venkata Siva Vijayendra Bhamidipati; cloudstack
> Subject: Re: Review Request 12612: Fix NPE when attaching vmware-tools iso
> to guest VM on ESX
> 
> 
> 
> > On July 17, 2013, 11:57 p.m., Sheng Yang wrote:
> > > Ship It!
> 
> Pushed to MASTER and 4.2
> 
> Thanks!
> 
> -Sheng
> 
> 
> - Sheng
> 
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12612/#review23347
> ---
> 
> 
> On July 17, 2013, 11:54 p.m., Venkata Siva Vijayendra Bhamidipati wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/12612/
> > ---
> >
> > (Updated July 17, 2013, 11:54 p.m.)
> >
> >
> > Review request for cloudstack, Chip Childers, edison su, Kelven Yang,
> Sateesh Chodapuneedi, and Sheng Yang.
> >
> >
> > Bugs: CLOUDSTACK-3554
> >
> >
> > Repository: cloudstack-git
> >
> >
> > Description
> > ---
> >
> > Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on
> ESX.
> >
> >
> > Diffs
> > -
> >
> >
> plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStor
> ageProcessor.java 4113803
> >
> > Diff: https://reviews.apache.org/r/12612/diff/
> >
> >
> > Testing
> > ---
> >
> > With the fix in place, vmware-tools.iso attach to a vmware guest VM works
> as expected.
> >
> >
> > Thanks,
> >
> > Venkata Siva Vijayendra Bhamidipati
> >
> >

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-17 Thread Sheng Yang 


> On July 17, 2013, 11:57 p.m., Sheng Yang wrote:
> > Ship It!

Pushed to MASTER and 4.2

Thanks!

-Sheng


- Sheng


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


On July 17, 2013, 11:54 p.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12612/
> ---
> 
> (Updated July 17, 2013, 11:54 p.m.)
> 
> 
> Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh 
> Chodapuneedi, and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-3554
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX.
> 
> 
> Diffs
> -
> 
>   
> plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
>  4113803 
> 
> Diff: https://reviews.apache.org/r/12612/diff/
> 
> 
> Testing
> ---
> 
> With the fix in place, vmware-tools.iso attach to a vmware guest VM works as 
> expected.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-17 Thread Sheng Yang 

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

Ship it!


Ship It!

- Sheng Yang


On July 17, 2013, 11:54 p.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12612/
> ---
> 
> (Updated July 17, 2013, 11:54 p.m.)
> 
> 
> Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh 
> Chodapuneedi, and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-3554
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX.
> 
> 
> Diffs
> -
> 
>   
> plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
>  4113803 
> 
> Diff: https://reviews.apache.org/r/12612/diff/
> 
> 
> Testing
> ---
> 
> With the fix in place, vmware-tools.iso attach to a vmware guest VM works as 
> expected.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-17 Thread Venkata Siva Vijayendra Bhamidipati 

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

(Updated July 17, 2013, 11:54 p.m.)


Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh 
Chodapuneedi, and Sheng Yang.


Changes
---

Incorporating Min's/Sheng's comments.


Bugs: CLOUDSTACK-3554


Repository: cloudstack-git


Description
---

Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX.


Diffs (updated)
-

  
plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
 4113803 

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


Testing
---

With the fix in place, vmware-tools.iso attach to a vmware guest VM works as 
expected.


Thanks,

Venkata Siva Vijayendra Bhamidipati

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-17 Thread Venkata Siva Vijayendra Bhamidipati 


> On July 17, 2013, 12:24 a.m., Sheng Yang wrote:
> > server/src/com/cloud/template/TemplateManagerImpl.java, line 999
> > 
> >
> > How can this happen?? Add Min Chen in reviewer.
> 
> Venkata Siva Vijayendra Bhamidipati wrote:
> This is because vmware tools or xenserver tools ISOs aren't actual ISOs. 
> They're internally provisioned by vmware/xenserver, so the URL will be null. 
> Since it isn't really resident anywhere, the image store will also be absent 
> for it, i.e., null.
> 
> Min Chen wrote:
> Vijay, I don't quite understand this. This check is to check if VM is at 
> running state. Original logic is that if VM that we are going to attach ISO 
> to is not at running, we just directly return true and update DB to put iso 
> id for this VM. Why did you change to return false here?
> 
> Venkata Siva Vijayendra Bhamidipati wrote:
> Hi Min, VMWare does not allow attaching the vmware tools iso if the VM is 
> not running. Here is the exception response we get from vCenter if true is 
> returned when the VM is not running, and we attempt to carry on with the 
> mount operation:
> 
> 2013-07-16 09:44:26,158 ERROR [storage.resource.VmwareStorageProcessor] 
> (DirectAgent-7:10.223.74.131) AttachIsoCommand(attach) failed due to 
> Exception: com.vmware.vim25.InvalidState
> message: []
> 
> com.vmware.vim25.InvalidStateFaultMsg: The operation is not allowed in 
> the current state.
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at java.lang.reflect.Constructor.newInstance(Constructor.java:532)
> at 
> com.sun.xml.internal.ws.fault.SOAPFaultBuilder.createException(SOAPFaultBuilder.java:130)
> at 
> com.sun.xml.internal.ws.client.sei.SyncMethodHandler.invoke(SyncMethodHandler.java:108)
> at 
> com.sun.xml.internal.ws.client.sei.SyncMethodHandler.invoke(SyncMethodHandler.java:78)
> at com.sun.xml.internal.ws.client.sei.SEIStub.invoke(SEIStub.java:107)
> at sun.proxy.$Proxy95.mountToolsInstaller(Unknown Source)
> at 
> com.cloud.hypervisor.vmware.mo.VirtualMachineMO.mountToolsInstaller(VirtualMachineMO.java:2152)
> at 
> com.cloud.storage.resource.VmwareStorageProcessor.attachIso(VmwareStorageProcessor.java:883)
> at 
> com.cloud.storage.resource.VmwareStorageProcessor.attachIso(VmwareStorageProcessor.java:768)
> at 
> com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.execute(StorageSubsystemCommandHandlerBase.java:125)
> at 
> com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.handleStorageCommands(StorageSubsystemCommandHandlerBase.java:55)
> at 
> com.cloud.hypervisor.vmware.resource.VmwareResource.executeRequest(VmwareResource.java:565)
> at 
> com.cloud.agent.manager.DirectAgentAttache$Task.run(DirectAgentAttache.java:186)
> at 
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
> at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
> at java.util.concurrent.FutureTask.run(FutureTask.java:166)
> at 
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$101(ScheduledThreadPoolExecutor.java:165)
> at 
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:266)
> at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146)
> at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> at java.lang.Thread.run(Thread.java:679)
> 
>
> 
> Sheng Yang wrote:
> In such case, it's VMware specific thing, you shouldn't change the 
> behavior of other hypervisors.
> 
> And according to Min, the attachment is done after VM boot up, here we 
> just modify DB. Why VMware behavior differently?

Turns out it's only a set of VMs in my setup that ran into the above issue - I 
looked at the logs but couldn't find out why ESX was disallowing iso attaches 
to them. I'll remove the return false; change because VMware does allow 
attaching an ISO to a VM in the stopped state. Will submit a new patch. Thanks 
for your inputs on this!


- Venkata Siva Vijayendra


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


On July 17, 2013, 12:39 a.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https:

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-17 Thread Sheng Yang 


> On July 17, 2013, 12:24 a.m., Sheng Yang wrote:
> > server/src/com/cloud/template/TemplateManagerImpl.java, line 999
> > 
> >
> > How can this happen?? Add Min Chen in reviewer.
> 
> Venkata Siva Vijayendra Bhamidipati wrote:
> This is because vmware tools or xenserver tools ISOs aren't actual ISOs. 
> They're internally provisioned by vmware/xenserver, so the URL will be null. 
> Since it isn't really resident anywhere, the image store will also be absent 
> for it, i.e., null.
> 
> Min Chen wrote:
> Vijay, I don't quite understand this. This check is to check if VM is at 
> running state. Original logic is that if VM that we are going to attach ISO 
> to is not at running, we just directly return true and update DB to put iso 
> id for this VM. Why did you change to return false here?
> 
> Venkata Siva Vijayendra Bhamidipati wrote:
> Hi Min, VMWare does not allow attaching the vmware tools iso if the VM is 
> not running. Here is the exception response we get from vCenter if true is 
> returned when the VM is not running, and we attempt to carry on with the 
> mount operation:
> 
> 2013-07-16 09:44:26,158 ERROR [storage.resource.VmwareStorageProcessor] 
> (DirectAgent-7:10.223.74.131) AttachIsoCommand(attach) failed due to 
> Exception: com.vmware.vim25.InvalidState
> message: []
> 
> com.vmware.vim25.InvalidStateFaultMsg: The operation is not allowed in 
> the current state.
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at java.lang.reflect.Constructor.newInstance(Constructor.java:532)
> at 
> com.sun.xml.internal.ws.fault.SOAPFaultBuilder.createException(SOAPFaultBuilder.java:130)
> at 
> com.sun.xml.internal.ws.client.sei.SyncMethodHandler.invoke(SyncMethodHandler.java:108)
> at 
> com.sun.xml.internal.ws.client.sei.SyncMethodHandler.invoke(SyncMethodHandler.java:78)
> at com.sun.xml.internal.ws.client.sei.SEIStub.invoke(SEIStub.java:107)
> at sun.proxy.$Proxy95.mountToolsInstaller(Unknown Source)
> at 
> com.cloud.hypervisor.vmware.mo.VirtualMachineMO.mountToolsInstaller(VirtualMachineMO.java:2152)
> at 
> com.cloud.storage.resource.VmwareStorageProcessor.attachIso(VmwareStorageProcessor.java:883)
> at 
> com.cloud.storage.resource.VmwareStorageProcessor.attachIso(VmwareStorageProcessor.java:768)
> at 
> com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.execute(StorageSubsystemCommandHandlerBase.java:125)
> at 
> com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.handleStorageCommands(StorageSubsystemCommandHandlerBase.java:55)
> at 
> com.cloud.hypervisor.vmware.resource.VmwareResource.executeRequest(VmwareResource.java:565)
> at 
> com.cloud.agent.manager.DirectAgentAttache$Task.run(DirectAgentAttache.java:186)
> at 
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
> at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
> at java.util.concurrent.FutureTask.run(FutureTask.java:166)
> at 
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$101(ScheduledThreadPoolExecutor.java:165)
> at 
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:266)
> at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146)
> at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> at java.lang.Thread.run(Thread.java:679)
> 
>

In such case, it's VMware specific thing, you shouldn't change the behavior of 
other hypervisors.

And according to Min, the attachment is done after VM boot up, here we just 
modify DB. Why VMware behavior differently?


- Sheng


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


On July 17, 2013, 12:39 a.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12612/
> ---
> 
> (Updated July 17, 2013, 12:39 a.m.)
> 
> 
> Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh 
> Chodapuneedi, and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-3554
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> Fixing NPE that occurs when attaching vmware-too

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-17 Thread Venkata Siva Vijayendra Bhamidipati 


> On July 17, 2013, 12:24 a.m., Sheng Yang wrote:
> > server/src/com/cloud/template/TemplateManagerImpl.java, line 999
> > 
> >
> > How can this happen?? Add Min Chen in reviewer.
> 
> Venkata Siva Vijayendra Bhamidipati wrote:
> This is because vmware tools or xenserver tools ISOs aren't actual ISOs. 
> They're internally provisioned by vmware/xenserver, so the URL will be null. 
> Since it isn't really resident anywhere, the image store will also be absent 
> for it, i.e., null.
> 
> Min Chen wrote:
> Vijay, I don't quite understand this. This check is to check if VM is at 
> running state. Original logic is that if VM that we are going to attach ISO 
> to is not at running, we just directly return true and update DB to put iso 
> id for this VM. Why did you change to return false here?

Hi Min, VMWare does not allow attaching the vmware tools iso if the VM is not 
running. Here is the exception response we get from vCenter if true is returned 
when the VM is not running, and we attempt to carry on with the mount operation:

2013-07-16 09:44:26,158 ERROR [storage.resource.VmwareStorageProcessor] 
(DirectAgent-7:10.223.74.131) AttachIsoCommand(attach) failed due to Exception: 
com.vmware.vim25.InvalidState
message: []

com.vmware.vim25.InvalidStateFaultMsg: The operation is not allowed in the 
current state.
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:532)
at 
com.sun.xml.internal.ws.fault.SOAPFaultBuilder.createException(SOAPFaultBuilder.java:130)
at 
com.sun.xml.internal.ws.client.sei.SyncMethodHandler.invoke(SyncMethodHandler.java:108)
at 
com.sun.xml.internal.ws.client.sei.SyncMethodHandler.invoke(SyncMethodHandler.java:78)
at com.sun.xml.internal.ws.client.sei.SEIStub.invoke(SEIStub.java:107)
at sun.proxy.$Proxy95.mountToolsInstaller(Unknown Source)
at 
com.cloud.hypervisor.vmware.mo.VirtualMachineMO.mountToolsInstaller(VirtualMachineMO.java:2152)
at 
com.cloud.storage.resource.VmwareStorageProcessor.attachIso(VmwareStorageProcessor.java:883)
at 
com.cloud.storage.resource.VmwareStorageProcessor.attachIso(VmwareStorageProcessor.java:768)
at 
com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.execute(StorageSubsystemCommandHandlerBase.java:125)
at 
com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.handleStorageCommands(StorageSubsystemCommandHandlerBase.java:55)
at 
com.cloud.hypervisor.vmware.resource.VmwareResource.executeRequest(VmwareResource.java:565)
at 
com.cloud.agent.manager.DirectAgentAttache$Task.run(DirectAgentAttache.java:186)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
at java.util.concurrent.FutureTask.run(FutureTask.java:166)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$101(ScheduledThreadPoolExecutor.java:165)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:266)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
at java.lang.Thread.run(Thread.java:679)


- Venkata Siva Vijayendra


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


On July 17, 2013, 12:39 a.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12612/
> ---
> 
> (Updated July 17, 2013, 12:39 a.m.)
> 
> 
> Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh 
> Chodapuneedi, and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-3554
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX.
> 
> 
> Diffs
> -
> 
>   
> plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
>  4113803 
>   server/src/com/cloud/template/TemplateManagerImpl.java c7cc818 
> 
> Diff: https://reviews.apache.org/r/12612/diff/
> 
> 
> Testing
> ---
> 
> With the fix in place, vmware-tools.iso attach to a vmware guest VM works as 
> expected.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-17 Thread Min Chen 


> On July 17, 2013, 12:24 a.m., Sheng Yang wrote:
> > server/src/com/cloud/template/TemplateManagerImpl.java, line 999
> > 
> >
> > How can this happen?? Add Min Chen in reviewer.
> 
> Venkata Siva Vijayendra Bhamidipati wrote:
> This is because vmware tools or xenserver tools ISOs aren't actual ISOs. 
> They're internally provisioned by vmware/xenserver, so the URL will be null. 
> Since it isn't really resident anywhere, the image store will also be absent 
> for it, i.e., null.

Vijay, I don't quite understand this. This check is to check if VM is at 
running state. Original logic is that if VM that we are going to attach ISO to 
is not at running, we just directly return true and update DB to put iso id for 
this VM. Why did you change to return false here?


- Min


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


On July 17, 2013, 12:39 a.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12612/
> ---
> 
> (Updated July 17, 2013, 12:39 a.m.)
> 
> 
> Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh 
> Chodapuneedi, and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-3554
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX.
> 
> 
> Diffs
> -
> 
>   
> plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
>  4113803 
>   server/src/com/cloud/template/TemplateManagerImpl.java c7cc818 
> 
> Diff: https://reviews.apache.org/r/12612/diff/
> 
> 
> Testing
> ---
> 
> With the fix in place, vmware-tools.iso attach to a vmware guest VM works as 
> expected.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-16 Thread Venkata Siva Vijayendra Bhamidipati 

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

(Updated July 17, 2013, 12:39 a.m.)


Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh 
Chodapuneedi, and Sheng Yang.


Changes
---

Fixing indent.


Bugs: CLOUDSTACK-3554


Repository: cloudstack-git


Description
---

Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX.


Diffs (updated)
-

  
plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
 4113803 
  server/src/com/cloud/template/TemplateManagerImpl.java c7cc818 

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


Testing
---

With the fix in place, vmware-tools.iso attach to a vmware guest VM works as 
expected.


Thanks,

Venkata Siva Vijayendra Bhamidipati

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-16 Thread Venkata Siva Vijayendra Bhamidipati 


> On July 17, 2013, 12:24 a.m., Sheng Yang wrote:
> > plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java,
> >  line 875
> > 
> >
> > need proper indent

Hmm, thanks for catching that. I cleaned some code in vim, must've caused the 
different indentation. Passed git diff --check though. Will upload a new diff. 


> On July 17, 2013, 12:24 a.m., Sheng Yang wrote:
> > server/src/com/cloud/template/TemplateManagerImpl.java, line 999
> > 
> >
> > How can this happen?? Add Min Chen in reviewer.

This is because vmware tools or xenserver tools ISOs aren't actual ISOs. 
They're internally provisioned by vmware/xenserver, so the URL will be null. 
Since it isn't really resident anywhere, the image store will also be absent 
for it, i.e., null.


- Venkata Siva Vijayendra


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


On July 17, 2013, 12:18 a.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12612/
> ---
> 
> (Updated July 17, 2013, 12:18 a.m.)
> 
> 
> Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh 
> Chodapuneedi, and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-3554
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX.
> 
> 
> Diffs
> -
> 
>   
> plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
>  4113803 
>   server/src/com/cloud/template/TemplateManagerImpl.java c7cc818 
> 
> Diff: https://reviews.apache.org/r/12612/diff/
> 
> 
> Testing
> ---
> 
> With the fix in place, vmware-tools.iso attach to a vmware guest VM works as 
> expected.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Re: Review Request 12612: Fix NPE when attaching vmware-tools iso to guest VM on ESX

2013-07-16 Thread Sheng Yang 

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



plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java


need proper indent



server/src/com/cloud/template/TemplateManagerImpl.java


How can this happen?? Add Min Chen in reviewer.


- Sheng Yang


On July 17, 2013, 12:18 a.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12612/
> ---
> 
> (Updated July 17, 2013, 12:18 a.m.)
> 
> 
> Review request for cloudstack, Chip Childers, edison su, Kelven Yang, Sateesh 
> Chodapuneedi, and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-3554
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> Fixing NPE that occurs when attaching vmware-tools.iso to a guest VM on ESX.
> 
> 
> Diffs
> -
> 
>   
> plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
>  4113803 
>   server/src/com/cloud/template/TemplateManagerImpl.java c7cc818 
> 
> Diff: https://reviews.apache.org/r/12612/diff/
> 
> 
> Testing
> ---
> 
> With the fix in place, vmware-tools.iso attach to a vmware guest VM works as 
> expected.
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>