Igor Lvovsky has uploaded a new change for review.
Change subject: BZ#808116 - Fix post-zeroing process during delition of preallocated volumes ......................................................................
BZ#808116 - Fix post-zeroing process during delition of preallocated volumes
Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 --- M vdsm/storage/blockVolume.py M vdsm/storage/hsm.py 2 files changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/3207/1 -- To view, visit http://gerrit.ovirt.org/3207 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#808116 - Fix post-zeroing process during delition of preallocated volumes ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(2 inline comments)
.................................................... File vdsm/storage/blockVolume.py Line 696: lvNames = [vol.volUUID for vol in volumes] is this intentional?
.................................................... File vdsm/storage/hsm.py Line 1274: img = image.Image(os.path.join(self.storage_repository, spUUID)) could you explain the bug and its solution? I'm surprised to see a prepare() without a matching teardown()
-- To view, visit http://gerrit.ovirt.org/3207 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: BZ#808116 - Fix post-zeroing process during delition of preallocated volumes ......................................................................
Patch Set 1: (2 inline comments)
.................................................... File vdsm/storage/blockVolume.py Line 696: lvNames = [vol.volUUID for vol in volumes] Yes. This is a first part of the bug because in tuple case we get iterator instead of UUIDs list
.................................................... File vdsm/storage/hsm.py Line 1274: img = image.Image(os.path.join(self.storage_repository, spUUID)) OK. The bug is very simple. We just lost a capabilities to post-zeroing preallocated images :) To allow it again we need to activate these LVs. We will remove LV right after zeroing so we nor really need to teardown it
-- To view, visit http://gerrit.ovirt.org/3207 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: BZ#808116 - Fix post-zeroing process during delition of preallocated volumes ......................................................................
Patch Set 1: Verified
Verified by Dafna
-- To view, visit http://gerrit.ovirt.org/3207 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: BZ#808116 - Fix post-zeroing process during deletion of preallocated volumes ......................................................................
Patch Set 4: Verified
-- To view, visit http://gerrit.ovirt.org/3207 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Eduardo has posted comments on this change.
Change subject: BZ#808116 - Fix post-zeroing process during deletion of preallocated volumes ......................................................................
Patch Set 4: Looks good to me, but someone else must approve
(2 inline comments)
.................................................... File vdsm/storage/blockVolume.py Line 706: if lv.name in lvNames: TODO: Should warn if len(lvm.getLV) < len(lvNames), meaning that required volumes were not zeroed. Use sets.
.................................................... File vdsm/storage/hsm.py Line 1260: vars.task.getExclusiveLock(STORAGE, imgUUID) TODO: This lock should be taken in the other hsm image related functions.
-- To view, visit http://gerrit.ovirt.org/3207 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: BZ#808116 - Fix post-zeroing process during deletion of preallocated volumes ......................................................................
Patch Set 4: (2 inline comments)
.................................................... File vdsm/storage/blockVolume.py Line 706: if lv.name in lvNames: Almost right. Should warn if lvNames has volumes that not in lvm.getLv. But it is issue for different patch
.................................................... File vdsm/storage/hsm.py Line 1260: vars.task.getExclusiveLock(STORAGE, imgUUID) Right. But it's a very dangerous patch that should be verified very carefully
-- To view, visit http://gerrit.ovirt.org/3207 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: BZ#808116 - Fix post-zeroing process during deletion of preallocated volumes ......................................................................
BZ#808116 - Fix post-zeroing process during deletion of preallocated volumes
Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 --- M vdsm/storage/blockVolume.py M vdsm/storage/hsm.py M vdsm/storage/sp.py 3 files changed, 16 insertions(+), 16 deletions(-)
Approvals: Eduardo: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Igor Lvovsky: Verified
-- To view, visit http://gerrit.ovirt.org/3207 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#808116 - Fix post-zeroing process during deletion of preallocated volumes ......................................................................
Patch Set 4: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/3207 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ia594ae7bea92754f25a5568e1c20a6891ff0eca5 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
vdsm-patches@lists.fedorahosted.org