Igor Lvovsky has posted comments on this change.
Change subject: [WIP] New repository system
......................................................................
Patch Set 6: (3 inline comments)
....................................................
File vdsm/storage/storageRepository/engines/localFS.py
Line 3: import pickle
Line 4: import uuid
Line 5: from contextlib import contextmanager
Line 6: import glob
Line 7: from storageRepository.engines.common import DCAP_SPARSE_SLABS
what is storageRepository.engines.common?
where is it?
Line 8:
Line 9: def safeWrite(path, data):
Line 10: tmpPath = path + ".tmp"
Line 11: with open(tmpPath, "wb") as f:
Line 178:
Line 179: yield os.path.basename(path)
Line 180:
Line 181: def create(self, path):
Line 182: # There is no bootstrapping for this kind of repository
why? what is bootstrapping here?
Who is know (except youself) what is a purposes of all these functions?
Can you please explain somewhere (docstrings, comments, reference to some docs, ....).
Line 183: return
Line 184:
Line 185: ENGINE_INFO = ("localfs", create, LocalFsRepo)
Line 186
....................................................
File vdsm/storage/storageRepository/factory.py
Line -3
Line -2
Line -1
Line 0
Line 1: from urllib2 import urlparse
I think 'urlparse' placed in module 'urlparse' and not urllib2 (at least in 2.7)
http://docs.python.org/library/urlparse.html?highlight=urlparse#module-urlp…
Line 2: from glob import iglob
Line 3: import os
Line 4: import logging
Line 5:
--
To view, visit http://gerrit.usersys.redhat.com/1080
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I21fc36b33720da9529df40ddf6297fcb4fd4f3bc
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Igor Lvovsky has posted comments on this change.
Change subject: [WIP]Extracted image related functinality to another class
......................................................................
Patch Set 8: (1 inline comment)
....................................................
File vdsm/storage/sp.py
Line 198: self.spmRole = SPM_FREE
Line 199: self._imageManipulator = LegacyImageManipulator(os.path.join(self.storage_repository, self.spUUID))
Line 200:
Line 201: # I know it exposes everything as unsecured.
Line 202: # I DON'T CARE. Will be handeled later.
where? when?
Line 203: @unsecured
Line 204: def getImageManipulator(self):
Line 205: return self._imageManipulator
Line 206:
--
To view, visit http://gerrit.usersys.redhat.com/1074
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic29dc8d964f0c2e56329e7044a3b2f48a9f2e193
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Igor Lvovsky has posted comments on this change.
Change subject: Fixed device mapper failing on partition check for non dm devices
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm/storage/devicemapper.py
Line 109: if os.path.exists(possiblePartPath):
Line 110: return True
Line 111:
Line 112: if not isVirtualDevice(devName) or not isDmDevice(devName):
Line 113: return False
Is it a right check? I am a little bit confusing...
can you explain please, why?
Line 114:
Line 115:
Line 116: mpathName = getDevName(dmName)
Line 117: for holder in getHolders(dmName):
--
To view, visit http://gerrit.usersys.redhat.com/1078
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b4713d7e0ee04722e82887378f448243241c631
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Igor Lvovsky has posted comments on this change.
Change subject: StatsThread does not need to inherit from threading.Thread
......................................................................
Patch Set 4: I would prefer that you didn't submit this
Again, why we need it?
--
To view, visit http://gerrit.usersys.redhat.com/1077
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b1f9ffb0ad1442ef08d565fa10d1a2fd8bc259
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Igor Lvovsky has posted comments on this change.
Change subject: AdvancedStatsThread does not need to inherit from threading.Thread
......................................................................
Patch Set 3: I would prefer that you didn't submit this
why we need it? can you explain please?
--
To view, visit http://gerrit.usersys.redhat.com/1076
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I79899b694c57fba3e81d8eef2b226082d3c99f70
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Igor Lvovsky has posted comments on this change.
Change subject: dir is a builtin, overriding it is not nice
......................................................................
Patch Set 3: Looks good to me, approved
--
To view, visit http://gerrit.usersys.redhat.com/1087
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I292cd9d5b47bb1051b765c02b55f351807316ea7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Dan Kenigsberg has submitted this change and it was merged.
Change subject: fix typo in trying to copy iscsid.conf template
......................................................................
fix typo in trying to copy iscsid.conf template
Change-Id: I211a97dbd4c945e3ff30d4b42d072fd7d92761c2
---
M vdsm/storage/iscsi.py
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Dan Kenigsberg: Verified; Looks good to me, approved
--
To view, visit http://gerrit.usersys.redhat.com/1088
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I211a97dbd4c945e3ff30d4b42d072fd7d92761c2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: fix typo in trying to copy iscsid.conf template
......................................................................
Patch Set 1: Looks good to me, approved
--
To view, visit http://gerrit.usersys.redhat.com/1088
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I211a97dbd4c945e3ff30d4b42d072fd7d92761c2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: fix typo in trying to copy iscsid.conf template
......................................................................
Patch Set 1: Verified
tried on Dave's box.
--
To view, visit http://gerrit.usersys.redhat.com/1088
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I211a97dbd4c945e3ff30d4b42d072fd7d92761c2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>