4:20 p.m.
Eduardo has uploaded a new change for review.
Change subject: [WIP] Check move image conditions from SD data.
......................................................................
[WIP] Check move image conditions from SD data.
Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Signed-off-by: Eduardo <ewarszaw(a)redhat.com>
---
M vdsm/storage/hsm.py
M vdsm/storage/image.py
M vdsm/storage/sp.py
3 files changed, 42 insertions(+), 46 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/3466/1
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
10:04 a.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Ayal Baron has posted comments on this change.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
Patch Set 7: I would prefer that you didn't submit this
(12 inline comments)
....................................................
Commit Message
Line 7: BZ#788640 - Check move image conditions from SD data.
what does that mean? why?
....................................................
File vdsm/storage/hsm.py
Line 1300: destination SD.
s/*/
Moving an image based on a template to a data domain is only allowed if the template
exists on the target domain.
Moving a template from a data domain is only allowed if there are no images based on it in
the source data domain.
Line 1307: if imgUUID in v.imgs)
isn't this filter repeated in several places? seeing as it is not trivially clear what
it does, maybe have a function with a clear name and proper comment that does this?
Line 1308: if any(len(imgs) > 1 for imgs in srcVolsImgs.itervalues()):
imageFromVolume = any(len...)
if imageFromVolume:
get rid of comment
Line 1312: if len(imgs) > 1:
instead of comment above and if len(imgs):
isTemplate = len(imgs) > 1
if isTemplate:
Line 1313: # This is the template. Should be only one.
comment is redundant
Line 1316: # Template self image is the 1st entry
To make it clear you can change the following to:
movingTemplate = (imgUUID == tImgs[0])
templateInDestination = tname in dstAllVols.keys()
if not movingTemplate and not templateInDestination:
Line 1317: if imgUUID != tImgs[0] and tName not in dstAllVols.keys():
looking at the above:
if any...:
for...
the if any is redundant, just do the for list and instead of "break" continue if
len(imgs) <= 1
Line 1321: elif imgUUID == tImgs[0] and not srcDom.isBackup():
assuming above change:
elif movingTemplate and not ...
Line 1341: self.isImageMoveLegal(srcDom, dstDom, imgUUID)
you're not checking the return value hence the name of the function should be:
validateImageMove ('is' indicates a boolean which needs to be checked)
Line 1356: # Therefore there is no special semantics for other SDs types as backup.
comment is unclear and redundant.
Line 1374: # Validate the conditions for each move
comment is redundant
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
4:42 p.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Igor Lvovsky has posted comments on this change.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
Patch Set 11: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
4:19 p.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Haim Ateya has posted comments on this change.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
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/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
4:09 p.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
BZ#788640 - Check move image conditions from SD data.
getAllChildrenList() should be removed since it is intrinsically
race prone.
Removing uses of getAllChilfrenList and validating conditions
near the input instead of waste process.
Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Signed-off-by: Eduardo <ewarszaw(a)redhat.com>
---
M vdsm/storage/hsm.py
M vdsm/storage/image.py
M vdsm/storage/sp.py
3 files changed, 38 insertions(+), 46 deletions(-)
Approvals:
Ayal Baron: Looks good to me, approved
Dan Kenigsberg: Verified
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
4:09 p.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
Patch Set 14: Verified
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
2:42 p.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Ayal Baron has posted comments on this change.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
Patch Set 14: Looks good to me, approved
(1 inline comment)
Please get rid of the redundant comment
....................................................
File vdsm/storage/hsm.py
Line 1307: # Filter volumes related to this image
please get rid of this comment
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
10:15 p.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
Patch Set 12: I would prefer that you didn't submit this
(2 inline comments)
-0.2, actually.
....................................................
File vdsm/storage/hsm.py
Line 1307: # Filter volumes related to this image
if you need such a comment, the name of sd.getVolsOfImage() is not clear enough.
how about sd.filterAllVolsByImage() ?
Line 1315: if imgUUID != tImgs[0] and tName not in dstAllVols.keys():
keys() is a needless copy
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
2:48 p.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Ayal Baron has posted comments on this change.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
Patch Set 8: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
10:34 a.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Igor Lvovsky has posted comments on this change.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
Patch Set 8: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
10:20 a.m.
New subject: Change in vdsm[master]: BZ#788640 - Check move image conditions from SD data.
Igor Lvovsky has posted comments on this change.
Change subject: BZ#788640 - Check move image conditions from SD data.
......................................................................
Patch Set 7: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
9:33 a.m.
New subject: Change in vdsm[master]: [WIP] BZ#788640 - Check move image conditions from SD data.
Igor Lvovsky has posted comments on this change.
Change subject: [WIP] BZ#788640 - Check move image conditions from SD data.
......................................................................
Patch Set 6: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
11:07 a.m.
Igor Lvovsky has posted comments on this change.
Change subject: [WIP] Check move image conditions from SD data.
......................................................................
Patch Set 5: I would prefer that you didn't submit this
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
11:07 a.m.
Igor Lvovsky has posted comments on this change.
Change subject: [WIP] Check move image conditions from SD data.
......................................................................
Patch Set 5: (6 inline comments)
....................................................
File vdsm/storage/hsm.py
Line 1290: Determines if the image move is feasible.
legal. not feasible
Line 1294: Move regular image not template based is always allowed
what about regular image template based ?
Line 1300: srcVolsImgs = dict((k, v) for k, v in srcAllVols.iteritems() if imgUUID
in v[0])
you promissed me to use normal variables names instead of 'k' and 'v'
Line 1314: return True
Hmm, i have a feeling that this is only partial.
But maybe I am wrong. Edu let's talk about this
Line 1366: images = {}
'images' should be out of the 'for'
Line 1372: images[imgUUID.strip()] = misc.parseBool(pZero)
you don't need it anymore
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
7:38 p.m.
Eduardo has posted comments on this change.
Change subject: [WIP] Check move image conditions from SD data.
......................................................................
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/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
6:58 p.m.
Eduardo has posted comments on this change.
Change subject: [WIP] Check move image conditions from SD data.
......................................................................
Patch Set 1: (1 inline comment)
....................................................
File vdsm/storage/hsm.py
Line 1288: def isImageMoveLegal(self, srcDom, dstDom, imgUUID):
At the end this a parameters validation and should be done when the input is received.
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
11:30 a.m.
Igor Lvovsky has posted comments on this change.
Change subject: [WIP] Check move image conditions from SD data.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/storage/hsm.py
Line 1288: def isImageMoveLegal(self, srcDom, dstDom, imgUUID):
hmmm, I am not sure about having this function in hsm.py
Line 1311: # 3) Regular image (no template based image) is always allowed
3) Move regular image ....
Anyway, I think these comments should be a part of validation function
Line 1343: # DEPRECATED
We have @deprecated
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
5:42 p.m.
Igor Lvovsky has posted comments on this change.
Change subject: [WIP] Check move image conditions from SD data.
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm/storage/hsm.py
Line 1288: def isImageMoveLegal(self, srcDom, dstDom, imgUUID):
If you decided to keep this function in hsm, I think it should be in the class but not
global (maybe classmethod).
Anyway, don't put the global function in the middle of the class functions.
--
To view, visit http://gerrit.ovirt.org/3466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2422217717fe0f3cb668b4dc5f775e4a971c8d9b
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
4363
days inactive
4404
days old
vdsm-patches@lists.fedorahosted.org
17 comments
5 participants
participants (5)
-
abaron＠redhat.com -
Dan Kenigsberg -
ewarszaw＠redhat.com -
Haim Ateya -
Igor Lvovsky