Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-05 Thread Alex Ough 

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

Ship it!


Ship It!

- Alex Ough


On Dec. 5, 2013, 3:28 p.m., Alex Ough wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15763/
> ---
> 
> (Updated Dec. 5, 2013, 3:28 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> When a host is switched to the 'maintenance mode', the vms in that host are 
> migrated to another available hosts. Currently, the vm migrations are 
> executed sequentially, it can take a lot of time when there are quite number 
> of vms. So I changed 'MigrateCommand' to get the value of 'executeInSequence' 
> property from the global setting value of 
> 'execute.in.sequence.hypervisor.commands' so that users can execute the 
> migrations in parallel to save time.
> 
> 
> Diffs
> -
> 
>   core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c 
>   engine/api/src/com/cloud/vm/VirtualMachineManager.java fbbed21 
>   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 
> 5136130 
>   setup/db/db/schema-430to440.sql 8098ad8 
> 
> Diff: https://reviews.apache.org/r/15763/diff/
> 
> 
> Testing
> ---
> 
> Tested in the simulator mode and Xen servers.
> 
> 
> Thanks,
> 
> Alex Ough
> 
>

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-05 Thread Koushik Das 


> On Dec. 6, 2013, 4:19 a.m., Koushik Das wrote:
> > Ship It!

master -> 88f64b2b93a3898e7868e9e55c1feca57b2ed48e


- Koushik


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


On Dec. 5, 2013, 3:28 p.m., Alex Ough wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15763/
> ---
> 
> (Updated Dec. 5, 2013, 3:28 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> When a host is switched to the 'maintenance mode', the vms in that host are 
> migrated to another available hosts. Currently, the vm migrations are 
> executed sequentially, it can take a lot of time when there are quite number 
> of vms. So I changed 'MigrateCommand' to get the value of 'executeInSequence' 
> property from the global setting value of 
> 'execute.in.sequence.hypervisor.commands' so that users can execute the 
> migrations in parallel to save time.
> 
> 
> Diffs
> -
> 
>   core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c 
>   engine/api/src/com/cloud/vm/VirtualMachineManager.java fbbed21 
>   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 
> 5136130 
>   setup/db/db/schema-430to440.sql 8098ad8 
> 
> Diff: https://reviews.apache.org/r/15763/diff/
> 
> 
> Testing
> ---
> 
> Tested in the simulator mode and Xen servers.
> 
> 
> Thanks,
> 
> Alex Ough
> 
>

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-05 Thread Koushik Das 

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

Ship it!


Ship It!

- Koushik Das


On Dec. 5, 2013, 3:28 p.m., Alex Ough wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15763/
> ---
> 
> (Updated Dec. 5, 2013, 3:28 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> When a host is switched to the 'maintenance mode', the vms in that host are 
> migrated to another available hosts. Currently, the vm migrations are 
> executed sequentially, it can take a lot of time when there are quite number 
> of vms. So I changed 'MigrateCommand' to get the value of 'executeInSequence' 
> property from the global setting value of 
> 'execute.in.sequence.hypervisor.commands' so that users can execute the 
> migrations in parallel to save time.
> 
> 
> Diffs
> -
> 
>   core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c 
>   engine/api/src/com/cloud/vm/VirtualMachineManager.java fbbed21 
>   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 
> 5136130 
>   setup/db/db/schema-430to440.sql 8098ad8 
> 
> Diff: https://reviews.apache.org/r/15763/diff/
> 
> 
> Testing
> ---
> 
> Tested in the simulator mode and Xen servers.
> 
> 
> Thanks,
> 
> Alex Ough
> 
>

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-05 Thread Alex Ough 
Hi Koushik,

I followed your instructions and this is the merged commit.

https://reviews.apache.org/r/15763/diff/

Let me know if there is anything missing/incorrect.
Thanks
Alex Ough



On Wed, Dec 4, 2013 at 11:44 PM, Koushik Das  wrote:

>  Alex,
>
>  This is what you need to do.
>
>  1. Create a single commit for all the changes you have made. Currently I
> see that you have made 2 commits - your original set of changes is one
> commit and then there is the review comment related changes which is the
> other commit. You need to create a single commit out of them.
> 2. Create a patch from that commit
> 3. Update the review request with that patch.
>
>  -Koushik
>
>   On 04-Dec-2013, at 8:52 PM, Alex Ough  wrote:
>
>  Hi Koushik,
>
>  This time I reloaded only the 2 changes.
> To make it clean, I tried to remove the previous incorrect one, but I
> couldn't find how.
> So let me know if it is better to discard this review and create a new one.
>
>  Thanks
> Alex Ough
>
>
> On Wed, Dec 4, 2013 at 12:04 AM, Koushik Das wrote:
>
>>
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>>  https://reviews.apache.org/r/15763/#review29716
>> ---
>>
>>
>> Looks like the latest patch has some issues, the diff is not showing up.
>> Also there are 2 commits in the patch file. Can you resubmit the patch with
>> only a single commit?
>>
>> - Koushik Das
>>
>>
>> On Dec. 3, 2013, 5:07 p.m., Alex Ough wrote:
>> >
>> > ---
>> > This is an automatically generated e-mail. To reply, visit:
>> > https://reviews.apache.org/r/15763/
>>  > ---
>> >
>> > (Updated Dec. 3, 2013, 5:07 p.m.)
>> >
>> >
>> > Review request for cloudstack.
>> >
>> >
>> > Repository: cloudstack-git
>> >
>> >
>> > Description
>> > ---
>> >
>>  > When a host is switched to the 'maintenance mode', the vms in that
>> host are migrated to another available hosts. Currently, the vm migrations
>> are executed sequentially, it can take a lot of time when there are quite
>> number of vms. So I changed 'MigrateCommand' to get the value of
>> 'executeInSequence' property from the global setting value of
>> 'execute.in.sequence.hypervisor.commands' so that users can execute the
>> migrations in parallel to save time.
>> >
>> >
>>  > Diffs
>> > -
>> >
>> >   core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c
>> >   engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f
>> >   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
>> 3a3de70
>> >   setup/db/db/schema-410to420.sql 522ccc4
>> >   setup/db/db/schema-410to420.sql 522ccc4
>> >   setup/db/db/schema-430to440.sql 9bb9602
>> >
>> > Diff: https://reviews.apache.org/r/15763/diff/
>> >
>> >
>> > Testing
>> > ---
>> >
>>  > Tested in the simulator mode and Xen servers.
>> >
>> >
>>  > Thanks,
>> >
>> > Alex Ough
>> >
>> >
>>
>>
>
>

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-05 Thread Alex Ough 

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

(Updated Dec. 5, 2013, 3:28 p.m.)


Review request for cloudstack.


Changes
---

merged two commits into one


Repository: cloudstack-git


Description
---

When a host is switched to the 'maintenance mode', the vms in that host are 
migrated to another available hosts. Currently, the vm migrations are executed 
sequentially, it can take a lot of time when there are quite number of vms. So 
I changed 'MigrateCommand' to get the value of 'executeInSequence' property 
from the global setting value of 'execute.in.sequence.hypervisor.commands' so 
that users can execute the migrations in parallel to save time.


Diffs (updated)
-

  core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c 
  engine/api/src/com/cloud/vm/VirtualMachineManager.java fbbed21 
  engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 5136130 
  setup/db/db/schema-430to440.sql 8098ad8 

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


Testing
---

Tested in the simulator mode and Xen servers.


Thanks,

Alex Ough

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-04 Thread Koushik Das 
Alex,

This is what you need to do.

1. Create a single commit for all the changes you have made. Currently I see 
that you have made 2 commits - your original set of changes is one commit and 
then there is the review comment related changes which is the other commit. You 
need to create a single commit out of them.
2. Create a patch from that commit
3. Update the review request with that patch.

-Koushik

On 04-Dec-2013, at 8:52 PM, Alex Ough 
mailto:alex.o...@sungard.com>> wrote:

Hi Koushik,

This time I reloaded only the 2 changes.
To make it clean, I tried to remove the previous incorrect one, but I couldn't 
find how.
So let me know if it is better to discard this review and create a new one.

Thanks
Alex Ough


On Wed, Dec 4, 2013 at 12:04 AM, Koushik Das 
mailto:koushik@citrix.com>> wrote:

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


Looks like the latest patch has some issues, the diff is not showing up. Also 
there are 2 commits in the patch file. Can you resubmit the patch with only a 
single commit?

- Koushik Das


On Dec. 3, 2013, 5:07 p.m., Alex Ough wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15763/
> ---
>
> (Updated Dec. 3, 2013, 5:07 p.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> ---
>
> When a host is switched to the 'maintenance mode', the vms in that host are 
> migrated to another available hosts. Currently, the vm migrations are 
> executed sequentially, it can take a lot of time when there are quite number 
> of vms. So I changed 'MigrateCommand' to get the value of 'executeInSequence' 
> property from the global setting value of 
> 'execute.in.sequence.hypervisor.commands' so that users can execute the 
> migrations in parallel to save time.
>
>
> Diffs
> -
>
>   core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c
>   engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f
>   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70
>   setup/db/db/schema-410to420.sql 522ccc4
>   setup/db/db/schema-410to420.sql 522ccc4
>   setup/db/db/schema-430to440.sql 9bb9602
>
> Diff: https://reviews.apache.org/r/15763/diff/
>
>
> Testing
> ---
>
> Tested in the simulator mode and Xen servers.
>
>
> Thanks,
>
> Alex Ough
>
>

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-04 Thread Alex Ough 
Hi Koushik,

This time I reloaded only the 2 changes.
To make it clean, I tried to remove the previous incorrect one, but I
couldn't find how.
So let me know if it is better to discard this review and create a new one.

Thanks
Alex Ough


On Wed, Dec 4, 2013 at 12:04 AM, Koushik Das  wrote:

>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15763/#review29716
> ---
>
>
> Looks like the latest patch has some issues, the diff is not showing up.
> Also there are 2 commits in the patch file. Can you resubmit the patch with
> only a single commit?
>
> - Koushik Das
>
>
> On Dec. 3, 2013, 5:07 p.m., Alex Ough wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/15763/
> > ---
> >
> > (Updated Dec. 3, 2013, 5:07 p.m.)
> >
> >
> > Review request for cloudstack.
> >
> >
> > Repository: cloudstack-git
> >
> >
> > Description
> > ---
> >
> > When a host is switched to the 'maintenance mode', the vms in that host
> are migrated to another available hosts. Currently, the vm migrations are
> executed sequentially, it can take a lot of time when there are quite
> number of vms. So I changed 'MigrateCommand' to get the value of
> 'executeInSequence' property from the global setting value of
> 'execute.in.sequence.hypervisor.commands' so that users can execute the
> migrations in parallel to save time.
> >
> >
> > Diffs
> > -
> >
> >   core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c
> >   engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f
> >   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
> 3a3de70
> >   setup/db/db/schema-410to420.sql 522ccc4
> >   setup/db/db/schema-410to420.sql 522ccc4
> >   setup/db/db/schema-430to440.sql 9bb9602
> >
> > Diff: https://reviews.apache.org/r/15763/diff/
> >
> >
> > Testing
> > ---
> >
> > Tested in the simulator mode and Xen servers.
> >
> >
> > Thanks,
> >
> > Alex Ough
> >
> >
>
>

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-04 Thread Alex Ough 

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

(Updated Dec. 4, 2013, 3:16 p.m.)


Review request for cloudstack.


Changes
---

reloading only with the latest changes


Repository: cloudstack-git


Description
---

When a host is switched to the 'maintenance mode', the vms in that host are 
migrated to another available hosts. Currently, the vm migrations are executed 
sequentially, it can take a lot of time when there are quite number of vms. So 
I changed 'MigrateCommand' to get the value of 'executeInSequence' property 
from the global setting value of 'execute.in.sequence.hypervisor.commands' so 
that users can execute the migrations in parallel to save time.


Diffs (updated)
-

  engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f 
  setup/db/db/schema-410to420.sql 522ccc4 

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


Testing
---

Tested in the simulator mode and Xen servers.


Thanks,

Alex Ough

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-03 Thread Koushik Das 

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


Looks like the latest patch has some issues, the diff is not showing up. Also 
there are 2 commits in the patch file. Can you resubmit the patch with only a 
single commit?

- Koushik Das


On Dec. 3, 2013, 5:07 p.m., Alex Ough wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15763/
> ---
> 
> (Updated Dec. 3, 2013, 5:07 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> When a host is switched to the 'maintenance mode', the vms in that host are 
> migrated to another available hosts. Currently, the vm migrations are 
> executed sequentially, it can take a lot of time when there are quite number 
> of vms. So I changed 'MigrateCommand' to get the value of 'executeInSequence' 
> property from the global setting value of 
> 'execute.in.sequence.hypervisor.commands' so that users can execute the 
> migrations in parallel to save time.
> 
> 
> Diffs
> -
> 
>   core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c 
>   engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f 
>   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 
> 3a3de70 
>   setup/db/db/schema-410to420.sql 522ccc4 
>   setup/db/db/schema-410to420.sql 522ccc4 
>   setup/db/db/schema-430to440.sql 9bb9602 
> 
> Diff: https://reviews.apache.org/r/15763/diff/
> 
> 
> Testing
> ---
> 
> Tested in the simulator mode and Xen servers.
> 
> 
> Thanks,
> 
> Alex Ough
> 
>

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-03 Thread Alex Ough 

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

(Updated Dec. 3, 2013, 5:07 p.m.)


Review request for cloudstack.


Changes
---

To fix what Koushik Das pointed out.


Repository: cloudstack-git


Description
---

When a host is switched to the 'maintenance mode', the vms in that host are 
migrated to another available hosts. Currently, the vm migrations are executed 
sequentially, it can take a lot of time when there are quite number of vms. So 
I changed 'MigrateCommand' to get the value of 'executeInSequence' property 
from the global setting value of 'execute.in.sequence.hypervisor.commands' so 
that users can execute the migrations in parallel to save time.


Diffs (updated)
-

  core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c 
  engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f 
  engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 
  setup/db/db/schema-410to420.sql 522ccc4 
  setup/db/db/schema-410to420.sql 522ccc4 
  setup/db/db/schema-430to440.sql 9bb9602 

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


Testing
---

Tested in the simulator mode and Xen servers.


Thanks,

Alex Ough

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-03 Thread Koushik Das 


> On Dec. 3, 2013, 8:44 a.m., Koushik Das wrote:
> > core/src/com/cloud/agent/api/MigrateCommand.java, line 32
> > 
> >
> > I see that you have tested for XS. In the mail thread people have 
> > mentioned that it should work on Vmware and KVM. Will it be possible to do 
> > a quick verification?
> > 
> > Otherwise (except for a few minor comments) the change looks good to me.

Now that HyperV is also there can there be any issues with that?


- Koushik


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


On Nov. 21, 2013, 6:39 p.m., Alex Ough wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15763/
> ---
> 
> (Updated Nov. 21, 2013, 6:39 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> When a host is switched to the 'maintenance mode', the vms in that host are 
> migrated to another available hosts. Currently, the vm migrations are 
> executed sequentially, it can take a lot of time when there are quite number 
> of vms. So I changed 'MigrateCommand' to get the value of 'executeInSequence' 
> property from the global setting value of 
> 'execute.in.sequence.hypervisor.commands' so that users can execute the 
> migrations in parallel to save time.
> 
> 
> Diffs
> -
> 
>   core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c 
>   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 
> 3a3de70 
>   setup/db/db/schema-410to420.sql 522ccc4 
>   setup/db/db/schema-430to440.sql 9bb9602 
> 
> Diff: https://reviews.apache.org/r/15763/diff/
> 
> 
> Testing
> ---
> 
> Tested in the simulator mode and Xen servers.
> 
> 
> Thanks,
> 
> Alex Ough
> 
>

Re: Review Request 15763: Changed 'MigrateCommand' to use the global setting value of 'execute.in.sequence.hypervisor.commands'

2013-12-03 Thread Koushik Das 

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



core/src/com/cloud/agent/api/MigrateCommand.java


I see that you have tested for XS. In the mail thread people have mentioned 
that it should work on Vmware and KVM. Will it be possible to do a quick 
verification?

Otherwise (except for a few minor comments) the change looks good to me.



setup/db/db/schema-410to420.sql


Since the change is for master, this is not required. 430-440 upgrade 
should suffice.



setup/db/db/schema-430to440.sql


Also the description needs to be updated in VirtualMachineManager.java 
where ExecuteInSequence is defined.


- Koushik Das


On Nov. 21, 2013, 6:39 p.m., Alex Ough wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15763/
> ---
> 
> (Updated Nov. 21, 2013, 6:39 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> ---
> 
> When a host is switched to the 'maintenance mode', the vms in that host are 
> migrated to another available hosts. Currently, the vm migrations are 
> executed sequentially, it can take a lot of time when there are quite number 
> of vms. So I changed 'MigrateCommand' to get the value of 'executeInSequence' 
> property from the global setting value of 
> 'execute.in.sequence.hypervisor.commands' so that users can execute the 
> migrations in parallel to save time.
> 
> 
> Diffs
> -
> 
>   core/src/com/cloud/agent/api/MigrateCommand.java 0d8f70c 
>   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 
> 3a3de70 
>   setup/db/db/schema-410to420.sql 522ccc4 
>   setup/db/db/schema-430to440.sql 9bb9602 
> 
> Diff: https://reviews.apache.org/r/15763/diff/
> 
> 
> Testing
> ---
> 
> Tested in the simulator mode and Xen servers.
> 
> 
> Thanks,
> 
> Alex Ough
> 
>