Change in vdsm[master]: BZ#808116 - Fix post-zeroing process during delition of prea...

[Igor Lvovsky]

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(a)redhat.com&gt;

Change in vdsm[master]: BZ#808116 - Fix post-zeroing process during deletion of prea...

[Dan Kenigsberg]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;

Change in vdsm[master]: BZ#808116 - Fix post-zeroing process during deletion of prea...

[Dan Kenigsberg]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;

Change in vdsm[master]: BZ#808116 - Fix post-zeroing process during deletion of prea...

[Igor Lvovsky]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;

Change in vdsm[master]: BZ#808116 - Fix post-zeroing process during deletion of prea...

[Igor Lvovsky]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;

Change in vdsm[master]: BZ#808116 - Fix post-zeroing process during deletion of prea...

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;

[Igor Lvovsky]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;

[Igor Lvovsky]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;

[Dan Kenigsberg]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;