Change in vdsm[master]: image: Calculation of chain to remove is inaccurate
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 --- 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 ___ 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
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 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
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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo 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
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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo 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
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 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Nir Soffer 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
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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Nir Soffer 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
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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Nir Soffer 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
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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Nir Soffer 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
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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yeela Kaplan 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
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 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yeela Kaplan 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
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 Reviewed-on: http://gerrit.ovirt.org/19852 Reviewed-by: Dan Kenigsberg Reviewed-by: Ayal Baron Reviewed-by: Daniel Erez --- 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches