Change in vdsm[master]: [WIP] Add sd.getTemplateSelfImg(). Template relink refactore...

Eduardo has uploaded a new change for review.

Change subject: [WIP] Add sd.getTemplateSelfImg(). Template relink refactored.
......................................................................

[WIP] Add sd.getTemplateSelfImg(). Template relink refactored.

Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Signed-off-by: Eduardo 
---
M vdsm/storage/image.py
M vdsm/storage/sd.py
2 files changed, 35 insertions(+), 14 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/3467/1
--
To view, visit http://gerrit.ovirt.org/3467
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo

Eduardo has posted comments on this change.

Change subject: [WIP] Add sd.getTemplateSelfImg(). Template relink refactored.
......................................................................

Patch Set 4: I would prefer that you didn't submit this

This is a clean up commit.
PLEASE IGNORE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: [WIP] Template relink refactored.

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: [WIP] Template relink refactored.
......................................................................

Patch Set 5: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/image.py
Line 392:                            "volume. Found  in images: %s ", volUUID, tImgs)
hmmm, I know that you hate produces, but I am not sure about change single produce with very heavy getAllVolumes.
Maybe it's pretty simple in block domains, but don't forget in NFS it performs the glob

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: [WIP] BZ#788640 - Template relink refactored.

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: [WIP] BZ#788640 - Template relink refactored.
......................................................................

Patch Set 6: I would prefer that you didn't submit this

(1 inline comment)

only minor 'oop' issue

....................................................
File vdsm/storage/image.py
Line 401:             os.link(os.path.join(self.repoPath, destDom.sdUUID, sd.DOMAIN_IMAGES, tSelfImage, volUUID), tLink)
oop

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 7: Looks good to me, but someone else must approve

(1 inline comment)

....................................................
File vdsm/storage/image.py
Line 392:                            tImgs, allVols)
I know that it's good to have all data in the log, but is allVols too big for logging?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

Eduardo has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 7: (1 inline comment)

....................................................
File vdsm/storage/image.py
Line 392:                            tImgs, allVols)
May be.
But this log is for an impossible case.
Should be such rare that we can live with it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

Ayal Baron has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 7: I would prefer that you didn't submit this

(5 inline comments)

....................................................
Commit Message
Line 7: BZ#788640 - Template relink refactored.
why?

....................................................
File vdsm/storage/image.py
Line 380:         Relink all hardlinks of the template 'volUUID' in all VMs based on it
This function assumes that dom is backup dom
and that template image is used by other volumes

Line 389:         if len(tImgs) < 2:
test is redundant

Line 394:         tSelfImage = tImgs[0]
s/tSelfImage/templateImage/

Line 401:             oop.getProcessPool(destDom.sdUUID).os.link(os.path.join(self.repoPath, destDom.sdUUID, sd.DOMAIN_IMAGES, tSelfImage, volUUID), tLink)
why not reverse and do this atomically?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 8: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

[Haim Ateya]

Haim Ateya has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 11: Verified

tested extensively on both block & file domains running most of the core scenarios: copy, move, delete, import, export volumes\images.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 11: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 12: I would prefer that you didn't submit this

(5 inline comments)

mainly minor murmurs.

....................................................
File vdsm/storage/image.py
Line 382:         This function assumes that dom is backup dom and that template image is
s/dom is/destDom/

Line 398:         relinkImgs = tuple(img for img in tImgs if img != templateImage)
isn't this a complex way to write

relinkImgs = tuple(tImage[1:]) ?

Line 400:             # Assumed that the new replace template was already created
I cannot understand the English here.

Line 401:             # All the relevant images and template namespaces should be locked
"this function assumes that all relevant images and template namespaces are locked" is clearer.

Line 404:             oop.getProcessPool(destDom.sdUUID).os.link(os.path.join(self.repoPath, destDom.sdUUID, sd.DOMAIN_IMAGES, templateImage, volUUID), tLink)
ELONGLINE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

Ayal Baron has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 14: I would prefer that you didn't submit this

(2 inline comments)

....................................................
File vdsm/storage/image.py
Line 400:             # Assumed that the new replace template was already created.
Please get rid of this comment.

Line 405:             oop.getProcessPool(destDom.sdUUID).os.link(os.path.join(
link(tempName)
mv(newName)
to make it atomic?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

Ayal Baron has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 15: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

Eduardo has posted comments on this change.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

Patch Set 15: Verified

Previously verified by Hateya

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya 
Gerrit-Reviewer: Igor Lvovsky

Change in vdsm[master]: BZ#788640 - Template relink refactored.

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: BZ#788640 - Template relink refactored.
......................................................................

BZ#788640 - Template relink refactored.

getAllChildrenList() should be removed since it is intrinsically
race prone.

Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Signed-off-by: Eduardo 
---
M vdsm/storage/image.py
1 file changed, 21 insertions(+), 16 deletions(-)

Approvals:
  Ayal Baron: Looks good to me, approved
  Eduardo: Verified
  Dan Kenigsberg:

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I087fc777e202088615c82be1a88fcde15b846628
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya 
Gerrit-Reviewer: Igor Lvovsky