You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by John Dilley <jo...@citrix.com> on 2014/08/14 12:42:20 UTC
Review Request 24696: Only attempt to shrink volume in test if CLVM
is in use
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24696/
-----------------------------------------------------------
Review request for cloudstack and Santhosh Edukulla.
Bugs: CLOUDSTACK-7228
https://issues.apache.org/jira/browse/CLOUDSTACK-7228
Repository: cloudstack-git
Description
-------
Currently only CLVM supports shrinking volumes. This patch will only attempt to shrink a volume if CLVM is in use.
Diffs
-----
test/integration/smoke/test_volumes.py e2e0d76
Diff: https://reviews.apache.org/r/24696/diff/
Testing
-------
Tested on XenServer to check that the test now passes.
Thanks,
John Dilley
Re: Review Request 24696: Only attempt to shrink volume in test if
CLVM is in use
Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24696/#review52138
-----------------------------------------------------------
Ship it!
Ship It!
- SrikanteswaraRao Talluri
On Sept. 2, 2014, 1:38 p.m., John Dilley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24696/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2014, 1:38 p.m.)
>
>
> Review request for cloudstack, Santhosh Edukulla and SrikanteswaraRao Talluri.
>
>
> Bugs: CLOUDSTACK-7228
> https://issues.apache.org/jira/browse/CLOUDSTACK-7228
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently only CLVM supports shrinking volumes. This patch will only attempt to shrink a volume if CLVM is in use.
>
>
> Diffs
> -----
>
> test/integration/smoke/test_volumes.py e2e0d76
>
> Diff: https://reviews.apache.org/r/24696/diff/
>
>
> Testing
> -------
>
> Tested on XenServer to check that the test now passes.
>
>
> Thanks,
>
> John Dilley
>
>
Re: Review Request 24696: Only attempt to shrink volume in test if
CLVM is in use
Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24696/#review52141
-----------------------------------------------------------
Pushed to master : 9b783d19ad7e784d3751f4eaa2a672401d122a01
- SrikanteswaraRao Talluri
On Sept. 2, 2014, 1:38 p.m., John Dilley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24696/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2014, 1:38 p.m.)
>
>
> Review request for cloudstack, Santhosh Edukulla and SrikanteswaraRao Talluri.
>
>
> Bugs: CLOUDSTACK-7228
> https://issues.apache.org/jira/browse/CLOUDSTACK-7228
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently only CLVM supports shrinking volumes. This patch will only attempt to shrink a volume if CLVM is in use.
>
>
> Diffs
> -----
>
> test/integration/smoke/test_volumes.py e2e0d76
>
> Diff: https://reviews.apache.org/r/24696/diff/
>
>
> Testing
> -------
>
> Tested on XenServer to check that the test now passes.
>
>
> Thanks,
>
> John Dilley
>
>
Re: Review Request 24696: Only attempt to shrink volume in test if
CLVM is in use
Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24696/#review52136
-----------------------------------------------------------
test/integration/smoke/test_volumes.py
<https://reviews.apache.org/r/24696/#comment90882>
yes, this should go in a separate patch.
test/integration/smoke/test_volumes.py
<https://reviews.apache.org/r/24696/#comment90886>
As far as this patch is concerned , It looks fine.
- SrikanteswaraRao Talluri
On Sept. 2, 2014, 1:38 p.m., John Dilley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24696/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2014, 1:38 p.m.)
>
>
> Review request for cloudstack, Santhosh Edukulla and SrikanteswaraRao Talluri.
>
>
> Bugs: CLOUDSTACK-7228
> https://issues.apache.org/jira/browse/CLOUDSTACK-7228
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently only CLVM supports shrinking volumes. This patch will only attempt to shrink a volume if CLVM is in use.
>
>
> Diffs
> -----
>
> test/integration/smoke/test_volumes.py e2e0d76
>
> Diff: https://reviews.apache.org/r/24696/diff/
>
>
> Testing
> -------
>
> Tested on XenServer to check that the test now passes.
>
>
> Thanks,
>
> John Dilley
>
>
Re: Review Request 24696: Only attempt to shrink volume in test if
CLVM is in use
Posted by John Dilley <jo...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24696/
-----------------------------------------------------------
(Updated Sept. 2, 2014, 1:38 p.m.)
Review request for cloudstack, Santhosh Edukulla and SrikanteswaraRao Talluri.
Bugs: CLOUDSTACK-7228
https://issues.apache.org/jira/browse/CLOUDSTACK-7228
Repository: cloudstack-git
Description
-------
Currently only CLVM supports shrinking volumes. This patch will only attempt to shrink a volume if CLVM is in use.
Diffs
-----
test/integration/smoke/test_volumes.py e2e0d76
Diff: https://reviews.apache.org/r/24696/diff/
Testing
-------
Tested on XenServer to check that the test now passes.
Thanks,
John Dilley
Re: Review Request 24696: Only attempt to shrink volume in test if
CLVM is in use
Posted by John Dilley <jo...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24696/#review50571
-----------------------------------------------------------
test/integration/smoke/test_volumes.py
<https://reviews.apache.org/r/24696/#comment88434>
There's a bit at the top where we stop the VM if it's XenServer. This is to check that after the appropraite resize operations, we can actually start the VM again.
I've not indented it with everything else. The logic goes something like this
if xenserver:
stop vm
grow volume
if possible to shrink:
shrink volume
if xenserver:
start vm again to check it can still run
- John Dilley
On Aug. 14, 2014, 10:42 a.m., John Dilley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24696/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2014, 10:42 a.m.)
>
>
> Review request for cloudstack and Santhosh Edukulla.
>
>
> Bugs: CLOUDSTACK-7228
> https://issues.apache.org/jira/browse/CLOUDSTACK-7228
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently only CLVM supports shrinking volumes. This patch will only attempt to shrink a volume if CLVM is in use.
>
>
> Diffs
> -----
>
> test/integration/smoke/test_volumes.py e2e0d76
>
> Diff: https://reviews.apache.org/r/24696/diff/
>
>
> Testing
> -------
>
> Tested on XenServer to check that the test now passes.
>
>
> Thanks,
>
> John Dilley
>
>
Re: Review Request 24696: Only attempt to shrink volume in test if
CLVM is in use
Posted by John Dilley <jo...@citrix.com>.
> On Aug. 14, 2014, 11:16 a.m., Santhosh Edukulla wrote:
> > test/integration/smoke/test_volumes.py, line 640
> > <https://reviews.apache.org/r/24696/diff/1/?file=660476#file660476line640>
> >
> > Just an additional refactor, we can do the check at the start before attach volume and skip, this reduces some time if these are not applicable for vmware and hyperv
This has nothing to do with the bug I'm fixing, so should be addressed in a different patch.
> On Aug. 14, 2014, 11:16 a.m., Santhosh Edukulla wrote:
> > test/integration/smoke/test_volumes.py, line 712
> > <https://reviews.apache.org/r/24696/diff/1/?file=660476#file660476line712>
> >
> > This code was redundant in the same test case twice for both resize commands.May be we can refactor here as well, as such we have touched the test case.
This has nothing to do with the bug I'm fixing, so should be addressed in a different patch.
> On Aug. 14, 2014, 11:16 a.m., Santhosh Edukulla wrote:
> > test/integration/smoke/test_volumes.py, line 734
> > <https://reviews.apache.org/r/24696/diff/1/?file=660476#file660476line734>
> >
> > My point was, what is the significance of this logic "if possible to shrink", start vm can still be possible with out previous if, so what check additionally we do if or if not for previous if?
I don't follow? If the hypervisor is XenServer, we want to attempt to restart the VM at the end of the test, regardless of whether or not we attempted to shrink the volume. That's why this code hasn't changed, and isn't in the new "if can_shrink" block.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24696/#review50572
-----------------------------------------------------------
On Aug. 14, 2014, 10:42 a.m., John Dilley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24696/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2014, 10:42 a.m.)
>
>
> Review request for cloudstack and Santhosh Edukulla.
>
>
> Bugs: CLOUDSTACK-7228
> https://issues.apache.org/jira/browse/CLOUDSTACK-7228
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently only CLVM supports shrinking volumes. This patch will only attempt to shrink a volume if CLVM is in use.
>
>
> Diffs
> -----
>
> test/integration/smoke/test_volumes.py e2e0d76
>
> Diff: https://reviews.apache.org/r/24696/diff/
>
>
> Testing
> -------
>
> Tested on XenServer to check that the test now passes.
>
>
> Thanks,
>
> John Dilley
>
>
Re: Review Request 24696: Only attempt to shrink volume in test if
CLVM is in use
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24696/#review50572
-----------------------------------------------------------
test/integration/smoke/test_volumes.py
<https://reviews.apache.org/r/24696/#comment88435>
Just an additional refactor, we can do the check at the start before attach volume and skip, this reduces some time if these are not applicable for vmware and hyperv
test/integration/smoke/test_volumes.py
<https://reviews.apache.org/r/24696/#comment88436>
This code was redundant in the same test case twice for both resize commands.May be we can refactor here as well, as such we have touched the test case.
test/integration/smoke/test_volumes.py
<https://reviews.apache.org/r/24696/#comment88437>
My point was, what is the significance of this logic "if possible to shrink", start vm can still be possible with out previous if, so what check additionally we do if or if not for previous if?
- Santhosh Edukulla
On Aug. 14, 2014, 10:42 a.m., John Dilley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24696/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2014, 10:42 a.m.)
>
>
> Review request for cloudstack and Santhosh Edukulla.
>
>
> Bugs: CLOUDSTACK-7228
> https://issues.apache.org/jira/browse/CLOUDSTACK-7228
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently only CLVM supports shrinking volumes. This patch will only attempt to shrink a volume if CLVM is in use.
>
>
> Diffs
> -----
>
> test/integration/smoke/test_volumes.py e2e0d76
>
> Diff: https://reviews.apache.org/r/24696/diff/
>
>
> Testing
> -------
>
> Tested on XenServer to check that the test now passes.
>
>
> Thanks,
>
> John Dilley
>
>
Re: Review Request 24696: Only attempt to shrink volume in test if
CLVM is in use
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24696/#review50570
-----------------------------------------------------------
test/integration/smoke/test_volumes.py
<https://reviews.apache.org/r/24696/#comment88433>
Test case pass\fail depends upon the previous if, then it will pass even with out going to the previous if? As well, the test case in itself is not clear as what we are trying to achieve?
- Santhosh Edukulla
On Aug. 14, 2014, 10:42 a.m., John Dilley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24696/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2014, 10:42 a.m.)
>
>
> Review request for cloudstack and Santhosh Edukulla.
>
>
> Bugs: CLOUDSTACK-7228
> https://issues.apache.org/jira/browse/CLOUDSTACK-7228
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently only CLVM supports shrinking volumes. This patch will only attempt to shrink a volume if CLVM is in use.
>
>
> Diffs
> -----
>
> test/integration/smoke/test_volumes.py e2e0d76
>
> Diff: https://reviews.apache.org/r/24696/diff/
>
>
> Testing
> -------
>
> Tested on XenServer to check that the test now passes.
>
>
> Thanks,
>
> John Dilley
>
>