Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-08 Thread ykaplan 
Yeela Kaplan has posted comments on this change.

Change subject: image: Calculation of chain to remove is inaccurate
..


Patch Set 1: Verified+1

(2 comments)


Commit Message
Line 3: AuthorDate: 2013-10-03 18:25:47 +0200
Line 4: Commit: Yeela Kaplan ykap...@redhat.com
Line 5: CommitDate: 2013-10-03 18:25:47 +0200
Line 6: 
Line 7: image: Calculation of chain to remove is inaccurate
Yes
Line 8: 
Line 9: The if condition will always return false,
Line 10: therefore list 'rmChain' will always be composed of only one volume.
Line 11: 


Line 6: 
Line 7: image: Calculation of chain to remove is inaccurate
Line 8: 
Line 9: The if condition will always return false,
Line 10: therefore list 'rmChain' will always be composed of only one volume.
You can see in the code, that the chain to remove is calculated the wrong way, 
Which means we won't delete the volumes we need to.
Line 11: 
Line 12: Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9


-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-08 Thread danken 
Dan Kenigsberg has submitted this change and it was merged.

Change subject: image: Calculation of chain to remove is inaccurate
..


image: Calculation of chain to remove is inaccurate

The if condition will always return false,
therefore list 'rmChain' will always be composed of only one volume.

Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Signed-off-by: Yeela Kaplan ykap...@redhat.com
Reviewed-on: http://gerrit.ovirt.org/19852
Reviewed-by: Dan Kenigsberg dan...@redhat.com
Reviewed-by: Ayal Baron aba...@redhat.com
Reviewed-by: Daniel Erez de...@redhat.com
---
M vdsm/storage/image.py
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Ayal Baron: Looks good to me, approved
  Yeela Kaplan: Verified
  Daniel Erez: Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, but someone else must approve



-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-07 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: image: Calculation of chain to remove is inaccurate
..


Patch Set 1:

Is this fix related to https://bugzilla.redhat.com/show_bug.cgi?id=1015071 ?

-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-07 Thread ykaplan 
Yeela Kaplan has posted comments on this change.

Change subject: image: Calculation of chain to remove is inaccurate
..


Patch Set 1:

No. Just fixed it along the way.

-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-06 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: image: Calculation of chain to remove is inaccurate
..


Patch Set 1:

(1 comment)

Another improvement for another patch.


File vdsm/storage/image.py
Line 1011: #   src
Line 1012: for ch in chList:
Line 1013: ch.prepare(rw=True, chainrw=True, setrw=True, force=True)
Line 1014: backingVolPath = os.path.join('..', 
srcVolParams['imgUUID'],
Line 1015:   srcVolParams['volUUID'])
Additionally, there is no need to create the backingVolPath on each iteration - 
it should be done once out of the loop.
Line 1016: try:
Line 1017: ch.rebase(srcVolParams['volUUID'], backingVolPath,
Line 1018:   volParams['volFormat'], unsafe=True, 
rollback=True)
Line 1019: finally:


-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-05 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: image: Calculation of chain to remove is inaccurate
..


Patch Set 1:

(3 comments)

Commit message is not clear, note about unsafe try-finally usage in patched 
code.


Commit Message
Line 3: AuthorDate: 2013-10-03 18:25:47 +0200
Line 4: Commit: Yeela Kaplan ykap...@redhat.com
Line 5: CommitDate: 2013-10-03 18:25:47 +0200
Line 6: 
Line 7: image: Calculation of chain to remove is inaccurate
Did you mean Fix inaccurate calculation of chain to remove?
Line 8: 
Line 9: The if condition will always return false,
Line 10: therefore list 'rmChain' will always be composed of only one volume.
Line 11: 


Line 6: 
Line 7: image: Calculation of chain to remove is inaccurate
Line 8: 
Line 9: The if condition will always return false,
Line 10: therefore list 'rmChain' will always be composed of only one volume.
Not clear. Can you explain what is the issue cause by the inaccurate 
calculation?
Line 11: 
Line 12: Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9



File vdsm/storage/image.py
Line 1011: #   src
Line 1012: for ch in chList:
Line 1013: ch.prepare(rw=True, chainrw=True, setrw=True, force=True)
Line 1014: backingVolPath = os.path.join('..', 
srcVolParams['imgUUID'],
Line 1015:   srcVolParams['volUUID'])
If this line raises, ch.teardown will be skipped. When using try-finally, the 
statement that need the finally *must* be the last statement before the try.

Not related to this patch - just so we don't forget to fix this.
Line 1016: try:
Line 1017: ch.rebase(srcVolParams['volUUID'], backingVolPath,
Line 1018:   volParams['volFormat'], unsafe=True, 
rollback=True)
Line 1019: finally:


-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-05 Thread derez 
Daniel Erez has posted comments on this change.

Change subject: image: Calculation of chain to remove is inaccurate
..


Patch Set 1: Code-Review+1

-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-04 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: image: Calculation of chain to remove is inaccurate
..


Patch Set 1: Code-Review+1

-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-04 Thread abaron 
Ayal Baron has posted comments on this change.

Change subject: image: Calculation of chain to remove is inaccurate
..


Patch Set 1: Code-Review+2

I believe engine only ever passes a single volume so possibly it would be 
better to just remove the support for removal of subchain and significantly 
simplify the code.
This does not detract from the correctness of this patch though so +2.

-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-03 Thread ykaplan 
Yeela Kaplan has uploaded a new change for review.

Change subject: image: Calculation of chain to remove is inaccurate
..

image: Calculation of chain to remove is inaccurate

The if condition will always return false,
therefore list 'rmChain' will always be composed of only one volume.

Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Signed-off-by: Yeela Kaplan ykap...@redhat.com
---
M vdsm/storage/image.py
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/52/19852/1

diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index da3e42d..2b9b448 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -1022,7 +1022,7 @@
 
 # Prepare chain for future erase
 rmChain = [vol.volUUID for
-   vol in chain if srcVol.volUUID != srcVolParams['volUUID']]
+   vol in chain if vol.volUUID != srcVolParams['volUUID']]
 rmChain.append(tmpUUID)
 
 return rmChain


-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: image: Calculation of chain to remove is inaccurate

2013-10-03 Thread oVirt Jenkins CI Server 
oVirt Jenkins CI Server has posted comments on this change.

Change subject: image: Calculation of chain to remove is inaccurate
..


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4683/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4759/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3874/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/19852
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches