Change in vdsm[master]: xmlrpc: Support HTTP 1.1
Ayal Baron has posted comments on this change. Change subject: xmlrpc: Support HTTP 1.1 .. Patch Set 6: (1 comment) http://gerrit.ovirt.org/#/c/24294/6//COMMIT_MSG Commit Message: Line 16: methods are used only on Python 2.6. Python 2.7 already include these Line 17: changes. Then the protocol_version is set to HTTP/1.1, enabling Line 18: automatic keep-alive. Line 19: Line 20: A new configuration option xmlrpc_http11 can be used to disbale this Using HTTP/1.1, the server threads are long living, and each thread handle The default value should be the one we want to go with (i.e. 1.1). However, I agree with Nir that in the field, having the ability to disable something allows us to deal with emergencies easily (change to 1.0, fix issue in background, relatively at our own leisure, then upgrade and change back). Since this changes the transport protocol, we cannot predict where we will hit issues (what flows). So imo it makes sense. Note that this does not mean that we support the 1.0 configuration or that users should be changing it. Line 21: feature and use HTTP/1.0 as used before. Line 22: Line 23: We can see in the logs that only few threads are created now for service Line 24: XMLRPC requests, and most requests are handled by the same thread. -- To view, visit http://gerrit.ovirt.org/24294 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie033d5c53c81c8db99d5c26697a1727be030e0b4 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Barak Azulay bazu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Roy Golan rgo...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [WIP] Towards a more (block) secure HSM.
Ayal Baron has posted comments on this change. Change subject: [WIP] Towards a more (block) secure HSM. .. Patch Set 7: (3 comments) http://gerrit.ovirt.org/#/c/2218/7/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 310: Line 311: def validateSPM(self, spUUID): Line 312: pool = self.getPool(spUUID) Line 313: if pool.spmRole != sp.SPM_ACQUIRED: Line 314: lvm.setLvmROMD() redundant Line 315: raise se.SpmStatusError(spUUID) Line 316: Line 317: def validateNotSPM(self, spUUID): Line 318: pool = self.getPool(spUUID) Line 601: :raises: :exc:`storage_exception.TaskInProgress` Line 602: if there are tasks running for this pool. Line 603: Line 604: Line 605: lvm.setLvmROMD() should be in stopSpm inside sp Line 606: vars.task.setDefaultException(se.SpmStopError(spUUID)) Line 607: vars.task.getExclusiveLock(STORAGE, spUUID) Line 608: Line 609: pool = self.getPool(spUUID) http://gerrit.ovirt.org/#/c/2218/7/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 267: Line 268: try: Line 269: self.lver = int(oldlver) + 1 Line 270: Line 271: blockSD.lvm.setLvmRWMD() # Do it later 1. do it later where? why? Line 272: Line 273: self._backend.setSpmStatus(self.lver, self.id, Line 274:__securityOverride=True) Line 275: self._maxHostID = maxHostID -- To view, visit http://gerrit.ovirt.org/2218 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30df4ee5cdb6b44cf14d8cb155436aac7442a07d Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewars...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Haim Ateya haim.at...@gmail.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [WIP] lvm: Add an option to replace locking type 4
Ayal Baron has posted comments on this change.
Change subject: [WIP] lvm: Add an option to replace locking type 4
..
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/23645/2/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:
Line 291: self._pvs = {}
Line 292: self._vgs = {}
Line 293: self._lvs = {}
Line 294:
Line 295: def cmd(self, cmd, devices=tuple(), safe=True):
In my point of view,
safe indeed sounds more like 'readonly' than 'safe to run in cluster mode'.
But since it is obviously not immediately clear then name should be rwaccess or
something that is unambiguous
Line 296:
Line 297: Use safe as False only for lvm cluster safe commands.
Line 298: These are cmds that don't change metadata of an existing VG.
Line 299:
--
To view, visit http://gerrit.ovirt.org/23645
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a67a7fa20145763d8ab5cdbf293a9c3eb070067
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [WIP] Create storage pool using command type 1
Ayal Baron has posted comments on this change.
Change subject: [WIP] Create storage pool using command type 1
..
Patch Set 2:
(5 comments)
http://gerrit.ovirt.org/#/c/23647/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 529: # to belong to the metadata volume. Since the metadata is at
Line 530: # least SDMETADATA/METASIZE units, we know we can use the
first
Line 531: # SDMETADATA bytes of the metadata volume for the SD metadata.
Line 532: # pass metadata's dev to ensure it is the first mapping
Line 533: #mapping = cls.getMetaDataMapping(vgName)
in final version comment needs to move with code and this needs to disappear
Line 534:
Line 535: # Create the rest of the BlockSD internal volumes
Line 536: lvm.createLV(vgName, sd.LEASES, sd.LEASES_SIZE, safe=False)
Line 537: lvm.createLV(vgName, sd.IDS, sd.IDS_SIZE, safe=False)
Line 572: # no one reads it from here anyway
Line 573: initialMetadata = {
Line 574: sd.DMDK_VERSION: version,
Line 575: sd.DMDK_SDUUID: sdUUID,
Line 576: sd.DMDK_TYPE: sd.storageType(storageType),
this looks like a change in semantics that shouldn't be here or in the least
deserves an explanation?
Line 577: sd.DMDK_CLASS: sd.class2name(domClass),
Line 578: sd.DMDK_DESCRIPTION: domainName,
Line 579: sd.DMDK_ROLE: sd.REGULAR_DOMAIN,
Line 580: sd.DMDK_POOLS: '',
Line 573: initialMetadata = {
Line 574: sd.DMDK_VERSION: version,
Line 575: sd.DMDK_SDUUID: sdUUID,
Line 576: sd.DMDK_TYPE: sd.storageType(storageType),
Line 577: sd.DMDK_CLASS: sd.class2name(domClass),
same
Line 578: sd.DMDK_DESCRIPTION: domainName,
Line 579: sd.DMDK_ROLE: sd.REGULAR_DOMAIN,
Line 580: sd.DMDK_POOLS: '',
Line 581: sd.DMDK_LOCK_POLICY: '',
Line 576: sd.DMDK_TYPE: sd.storageType(storageType),
Line 577: sd.DMDK_CLASS: sd.class2name(domClass),
Line 578: sd.DMDK_DESCRIPTION: domainName,
Line 579: sd.DMDK_ROLE: sd.REGULAR_DOMAIN,
Line 580: sd.DMDK_POOLS: '',
?
Line 581: sd.DMDK_LOCK_POLICY: '',
Line 582: sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC:
sd.DEFAULT_LEASE_PARAMS[
Line 583: sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC],
Line 584: sd.DMDK_LEASE_TIME_SEC: sd.DEFAULT_LEASE_PARAMS[
Line 593: }
Line 594:
Line 595: initialMetadata.update(mapping)
Line 596: toAdd = encodeVgTags(initialMetadata)
Line 597: lvm.changeVGTags(vgName, delTags=(), addTags=toAdd,
safe=False)
if it is not safe/possible to call md.update then need to explain this
Line 598:
Line 599: # Mark VG with Storage Domain Tag
Line 600: try:
Line 601: lvm.replaceVGTag(vgName, STORAGE_UNREADY_DOMAIN_TAG,
--
To view, visit http://gerrit.ovirt.org/23647
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia64f6dd2df38d2968f03ce66094f3ba7b4343503
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: Remove invalid validation of stale meta data
Ayal Baron has posted comments on this change. Change subject: sp: Remove invalid validation of stale meta data .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/24568 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I27ab243a03f23b1155867f2eeec98b1481b5b72d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: stop all poolMonitoredDomains on disconnect
Ayal Baron has posted comments on this change. Change subject: sp: stop all poolMonitoredDomains on disconnect .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/24088 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibc77988b01491f687fd7afa6bb34512b5f2451af Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: Nest try-finally for temporary secure change
Ayal Baron has posted comments on this change. Change subject: sp: Nest try-finally for temporary secure change .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/23955 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9054063db0eeacd156a8e586681496515f80e2b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: discover volume path from xml definition
Ayal Baron has posted comments on this change. Change subject: vm: discover volume path from xml definition .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/24202 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I322f1f879fbd5b6415789f3b307e8741d846d694 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: api: domainDict is optional in connectStoragePool
Ayal Baron has posted comments on this change. Change subject: api: domainDict is optional in connectStoragePool .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/24089 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05b4f5073a33b2479404298ddafd4802e949e8f4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: stop all poolMonitoredDomains on disconnect
Ayal Baron has posted comments on this change. Change subject: sp: stop all poolMonitoredDomains on disconnect .. Patch Set 1: Code-Review-1 (1 comment) http://gerrit.ovirt.org/#/c/24088/1/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 628: return True Line 629: Line 630: @unsecured Line 631: def stopMonitoringDomains(self): Line 632: for sdUUID in self.domainMonitor.poolMonitoredDomains: we need to check whether we're actually managing the same data in 2 places (getDomains, poolMonitoredDomains) if so we need to unify the 2 (possibly simply by implementing getDomains using poolMonitoredDomains Line 633: self.domainMonitor.stopMonitoring(sdUUID) Line 634: return True Line 635: Line 636: @unsecured -- To view, visit http://gerrit.ovirt.org/24088 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibc77988b01491f687fd7afa6bb34512b5f2451af Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: stop all poolMonitoredDomains on disconnect
Ayal Baron has posted comments on this change. Change subject: sp: stop all poolMonitoredDomains on disconnect .. Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/24088/1/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 628: return True Line 629: Line 630: @unsecured Line 631: def stopMonitoringDomains(self): Line 632: for sdUUID in self.domainMonitor.poolMonitoredDomains: getDomains returns all the domains in the pool (also the attached ones) fro and reversing it? Line 633: self.domainMonitor.stopMonitoring(sdUUID) Line 634: return True Line 635: Line 636: @unsecured -- To view, visit http://gerrit.ovirt.org/24088 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibc77988b01491f687fd7afa6bb34512b5f2451af Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: clientIF: Remove unnecessary device is disk check in prepare...
Ayal Baron has posted comments on this change. Change subject: clientIF: Remove unnecessary device is disk check in prepareVolumePath .. Patch Set 2: Code-Review+2 This patch is fine, the discussion about yes or no removing more code elsewhere is besides the point. This patch is blocking http://gerrit.ovirt.org/#/c/21973 and is the sole reason why this patch exists. -- To view, visit http://gerrit.ovirt.org/22363 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I98317e805e6770df5dacd3237a383aaca78fde1e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: clientIF: Teardown volume path only for VDSM images
Ayal Baron has posted comments on this change. Change subject: clientIF: Teardown volume path only for VDSM images .. Patch Set 7: Code-Review+2 Symmetricity issue is solved by http://gerrit.ovirt.org/#/c/22363 -- To view, visit http://gerrit.ovirt.org/21973 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I041a306636c75a7aa37d4d7c0811366d80fe609c Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: improve pool creation error handling
Ayal Baron has posted comments on this change. Change subject: sp: improve pool creation error handling .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/22818/2/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 139: and not self.spmRole == SPM_ACQUIRED): Line 140: self.invalidateMetadata() Line 141: poolMeta = self._getPoolMD(self.masterDomain) Line 142: Line 143: return poolMeta[PMDK_LVER], poolMeta[PMDK_SPM_ID] why are you not returning the role? this changes the API Line 144: Line 145: def setSpmStatus(self, lVer=None, spmId=None): Line 146: self.invalidateMetadata() Line 147: metaParams = dict(filter(lambda (k, v): v is not None, -- To view, visit http://gerrit.ovirt.org/22818 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cce08e368dec09c081609d0663d7990ab10 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: fix pool membership check in attachSD
Ayal Baron has posted comments on this change.
Change subject: sp: fix pool membership check in attachSD
..
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.ovirt.org/#/c/23446/2/vdsm/storage/sp.py
File vdsm/storage/sp.py:
Line 869: sdCache.invalidateStorage()
Line 870: dom = sdCache.produce(sdUUID)
Line 871:
Line 872: try:
Line 873: self.validateAttachedDomain(dom)
the only thing I don't like is that validate expects it to be there so it gets
the domain md (possibly goes to disk) and if the pool is not there it
invalidates the md and checks again, but attach flow is pretty rare anyway and
this is not the long part of it so no big deal
Line 874: except (se.StorageDomainNotMemberOfPool,
se.StorageDomainNotInPool):
Line 875: pass # domain is not attached to this pool yet
Line 876: else:
Line 877: log.warning('domain %s is already attached to pool %s',
--
To view, visit http://gerrit.ovirt.org/23446
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4169ddcb4da29c8b806b5788080c16349b642197
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: fix pool membership check in attachSD
Ayal Baron has posted comments on this change. Change subject: sp: fix pool membership check in attachSD .. Patch Set 2: -Code-Review well, once you fix Dan's comment -- To view, visit http://gerrit.ovirt.org/23446 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4169ddcb4da29c8b806b5788080c16349b642197 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sd: add inquireClusterLock method to StorageDomain
Ayal Baron has posted comments on this change. Change subject: sd: add inquireClusterLock method to StorageDomain .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21426 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I10e64d74319ea6591a7edf8e17809d367a758386 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: receive domains map in connectStoragePool
Ayal Baron has posted comments on this change. Change subject: sp: receive domains map in connectStoragePool .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21427 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I393677f643a62e3711af2a3cfb8b4b9a5ce11c2d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: refactor out the metadata access from StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: refactor out the metadata access from StoragePool .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22132 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I75493d1db60e51cccd5231b516f963c970d24c99 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: use setDomainsMap in reconstructMaster
Ayal Baron has posted comments on this change. Change subject: sp: use setDomainsMap in reconstructMaster .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/23385 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0ce8ad258be09b40ee9ad06399cc841f374bf816 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: securable: refactor the securable implementation
Ayal Baron has posted comments on this change. Change subject: securable: refactor the securable implementation .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22115 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5a8be7536748481746795e27701fbecc7c3318c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: changing un-shared treatment in the createVolume function
Ayal Baron has posted comments on this change. Change subject: sp: changing un-shared treatment in the createVolume function .. Patch Set 9: Code-Review+2 The changes don't cover the general safety problem that existed before this patch but I don't see how it's related so I'm acking this patch. What needs to happen is to add a shared lock on the parent for the *entire* flow and propagate it to exclusive when setting it as shared. -- To view, visit http://gerrit.ovirt.org/23260 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9a4b84795b176b9696684045e9774d324a42eca Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Oved Ourfali oourf...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Oved Ourfali oourf...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: securable: refactor the securable implementation
Ayal Baron has posted comments on this change. Change subject: securable: refactor the securable implementation .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/22115 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5a8be7536748481746795e27701fbecc7c3318c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sd: add inquireClusterLock method to StorageDomain
Ayal Baron has posted comments on this change. Change subject: sd: add inquireClusterLock method to StorageDomain .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21426 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I10e64d74319ea6591a7edf8e17809d367a758386 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: refactor out the metadata access from StoragePool
Ayal Baron has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool
..
Patch Set 2:
(3 comments)
http://gerrit.ovirt.org/#/c/22132/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 600: @staticmethod
Line 601: def _getSpmStatusInfo(pool):
Line 602: return dict(
Line 603: zip(('spmStatus', 'spmLver', 'spmId'),
Line 604: (pool.spmRole,) + pool.getBackend().getSpmStatus()))
why is getSpmStatus not wrapped by the pool? (i.e. instead of calling
getBackend here, leave as is and expose getSpmStatus in pool object
Line 605:
Line 606: @public
Line 607: def getSpmStatus(self, spUUID, options=None):
Line 608: pool = self.getPool(spUUID)
Line 1035: self._updateStoragePool(pool, hostID, msdUUID,
masterVersion)
Line 1036: return True
Line 1037:
Line 1038: pool = sp.StoragePool(spUUID, self.domainMonitor,
self.taskMng)
Line 1039: pool.backend = spbackends.StoragePoolMetaBackend(pool)
why is this not set in __init__? (see same comment in next patch since once
domainsMap is passed then it is easy to pass that to pool object ctor and
determine backend type accordingly
Line 1040:
Line 1041: # Must register domain state change callbacks *before*
connecting
Line 1042: # the pool, which starts domain monitor threads.
Otherwise we will
Line 1043: # miss the first event from the monitor thread.
Line 1821: sd.validateSDStatus(status)
Line 1822: except:
Line 1823: domDict[d] = sd.validateSDDeprecatedStatus(status)
Line 1824:
Line 1825: return pool.getBackend().reconstructMaster(
again, expose reconstructMaster in pool object?
Line 1826: hostId, poolName, masterDom, domDict, masterVersion,
leaseParams)
Line 1827:
Line 1828: def _logResp_getDeviceList(self, response):
Line 1829: logableDevs = deepcopy(response)
--
To view, visit http://gerrit.ovirt.org/22132
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I75493d1db60e51cccd5231b516f963c970d24c99
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: receive domains map in connectStoragePool
Ayal Baron has posted comments on this change.
Change subject: sp: receive domains map in connectStoragePool
..
Patch Set 2:
(4 comments)
http://gerrit.ovirt.org/#/c/21427/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 1006:
Line 1007: if domainsMap is None:
Line 1008: if not isinstance(pool.getBackend(),
Line 1009: spbackends.StoragePoolMetaBackend):
Line 1010: raise se.StoragePoolConnected('Cannot downgrade pool
backend')
1. why is this hsm logic and not pool logic?
2. shouldn't the backend say whether it requires domainsMap or not?
e.g.
if domainsMap is None and pool.getBackend.requiresDomainsMap()
raise...
else if domainsMap and not pool.getBackend.requiresDomainsMap():
upgrade
pool.refresh()
Line 1011: else:
Line 1012: if isinstance(pool.getBackend(),
spbackends.StoragePoolNew):
Line 1013: pool.getBackend().setDomainsMap(domainsMap)
Line 1014: else:
Line 1054:
Line 1055: pool = sp.StoragePool(spUUID, self.domainMonitor,
self.taskMng)
Line 1056:
Line 1057: if domainsMap is None:
Line 1058:
pool.setBackend(spbackends.StoragePoolMetaBackend(pool))
why is this not in pool __init__?
Line 1059: else:
Line 1060: pool.setBackend(
Line 1061: spbackends.StoragePoolNew(pool, masterVersion,
domainsMap))
Line 1062:
http://gerrit.ovirt.org/#/c/21427/2/vdsm/storage/spbackends.py
File vdsm/storage/spbackends.py:
Line 326: futureMaster.releaseClusterLock()
Line 327:
Line 328:
Line 329: @secured
Line 330: class StoragePoolNew(object):
StoragePoolMemoryBackend ?
And rename StoragePoolMetaBackend to StoragePoolDiskBackend ?
Line 331:
Line 332: __slots__ = ('pool', 'masterVersion', 'domainsMap')
Line 333:
Line 334: log = logging.getLogger('Storage.StoragePoolNew')
Line 332: __slots__ = ('pool', 'masterVersion', 'domainsMap')
Line 333:
Line 334: log = logging.getLogger('Storage.StoragePoolNew')
Line 335:
Line 336: def __init__(self, pool, masterVersion, domainsMap):
what is the APi that a backend needs to implement?
e.g. is reconstructMaster required, and if so, why isn't it here?
Line 337: self.pool = weakref.proxy(pool)
Line 338: self.masterVersion = masterVersion
Line 339: self.setDomainsMap(domainsMap)
Line 340:
--
To view, visit http://gerrit.ovirt.org/21427
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I393677f643a62e3711af2a3cfb8b4b9a5ce11c2d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storageTests: Extract storage types and versions to an envir...
Ayal Baron has posted comments on this change. Change subject: storageTests: Extract storage types and versions to an environment variable .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/23259 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I84f1ee57e43d921ae38afaabddecca9f38dd2687 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: changing un-shared treatment in the volume share function
Ayal Baron has posted comments on this change. Change subject: changing un-shared treatment in the volume share function .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.ovirt.org/#/c/23260/3/vdsm/storage/volume.py File vdsm/storage/volume.py: Line 299: Line 300: self.log.debug(Share volume %s to %s, self.volUUID, dstImgPath) Line 301: Line 302: if not self.isShared(): Line 303: if self.getParent() == BLANK_UUID: Ayal do you want the RAW check as well? I can't think of any assumption we' I see no reason to block it here, we can just not use it from the engine atm to avoid risk of regressions until tested thoroughly Line 304: self.log.debug(Volume %s is not shared. Setting it as shared, Line 305:self.volUUID) Line 306: self.setShared() Line 307: else: -- To view, visit http://gerrit.ovirt.org/23260 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9a4b84795b176b9696684045e9774d324a42eca Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Oved Ourfali oourf...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Oved Ourfali oourf...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move validatePoolSD in the StoragePool class
Ayal Baron has posted comments on this change. Change subject: sp: move validatePoolSD in the StoragePool class .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21787 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I75f0245ec5449d57431df7055be401d73975312b Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: improve masterMigrate safety
Ayal Baron has posted comments on this change. Change subject: sp: improve masterMigrate safety .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22419 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I155b36d41df43230f99ba610699bba05bee3f0c0 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: downloadFromStream
Ayal Baron has posted comments on this change.
Change subject: downloadFromStream
..
Patch Set 4:
(7 comments)
Commit Message
Line 5: CommitDate: 2014-01-12 14:05:19 +0200
Line 6:
Line 7: downloadFromStream
Line 8:
Line 9: needed:
needed: commit message that specifies what the patch does
Line 10: *further error handling
Line 11: *return xml/json as response body
Line 12: *socket timeout inspection
Line 13: *tests?
File vdsm/BindingXMLRPC.py
Line 152: contentLength = self.headers.\
Line 153: getheader('content-length')
Line 154:
Line 155: if contentLength is None:
Line 156: self.send_response(411)
use a constant? LENGTH_REQUIRED_ERROR = 411
Line 157: self.finish()
Line 158: return
Line 159:
Line 160: methodArgs = {'method': 'stream',
File vdsm/storage/imageSharing.py
Line 78: buffer_size = streamGetBufferSize(methodArgs)
Line 79: stream = methodArgs.get('fileObj')
Line 80:
Line 81: cmd = [constants.EXT_DD, of=%s % dstImgPath, bs=%s %
Line 82:str(storageConstants.BLOCK_SIZE), count=%d % bytes_left]
using block_size writes is extremely inefficient and can overload a storage
array, writes should be aggregated to at least 1M
count needs to be bytes_left / bs.
currently you're writing block_size * bytes_left which is incorrect
Line 83: p = utils.execCmd(cmd, sudo=False, sync=False)
Line 84: log.debug(str(stream._getclosed()))
Line 85:
Line 86: while bytes_left 0:
Line 85:
Line 86: while bytes_left 0:
Line 87: i = buffer_size
Line 88: if not i:
Line 89: i = storageConstants.BLOCK_SIZE
why is the above inside the 'while' loop?
BLOCK_SIZE is not relevant here either
Line 90: if (bytes_left i):
Line 91: i = bytes_left
Line 92: data = stream.read(i)
Line 93: if not data:
Line 92: data = stream.read(i)
Line 93: if not data:
Line 94: break
Line 95: p.stdin.write(data)
Line 96: p.stdin.flush()
why flush after every write?
Line 97: bytes_left = bytes_left - i
Line 98:
Line 99: p.stdin.close()
Line 100: p.wait()
Line 112: except KeyError:
Line 113: raise RuntimeError(Sharing method not specified)
Line 114:
Line 115:
Line 116: def _getSharingMethodsImplementation(methodArgs):
1. the word 'Implementation' here is not adding any information. This looks
like java ;)
2. what is the purpose of this change?
Line 117: method = getSharingMethod(methodArgs)
Line 118:
Line 119: try:
Line 120: return _METHOD_IMPLEMENTATIONS[method]
File vdsm/storage/storageConstants.py
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20:
Line 21: STORAGE = Storage
Line 22: BLOCK_SIZE = 512
if the underlying block size of the domain is larger this will cause even more
problems.
--
To view, visit http://gerrit.ovirt.org/23131
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I85bc63365273adf306c73ef1de7fea578ec42662
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar lara...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Tal Nisan tni...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: improve masterMigrate safety
Ayal Baron has posted comments on this change.
Change subject: sp: improve masterMigrate safety
..
Patch Set 3:
(1 comment)
File vdsm/storage/sp.py
Line 866: os.path.join(newmsd.domaindir, sd.MASTER_FS_DIR),
Line 867: exclude=('./lost+found',))
Line 868:
Line 869: newmsd.changeRole(sd.MASTER_DOMAIN)
Line 870: self.switchMasterDomain(curmsd, newmsd, masterVersion)
Why did you not address this in latest patchset?
Line 871: except Exception:
Line 872: self.log.exception('migration to new master failed')
Line 873: newmsd.unmountMaster()
Line 874: newmsd.releaseClusterLock()
--
To view, visit http://gerrit.ovirt.org/22419
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I155b36d41df43230f99ba610699bba05bee3f0c0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move validatePoolSD in the StoragePool class
Ayal Baron has posted comments on this change. Change subject: sp: move validatePoolSD in the StoragePool class .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21787 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I75f0245ec5449d57431df7055be401d73975312b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: split metadata transaction in createMaster
Ayal Baron has posted comments on this change. Change subject: sp: split metadata transaction in createMaster .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22418 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b22af92b1f9a481be9af844b1acc47ae513d078 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: bootstrap: Return recovery error until hsm is ready
Ayal Baron has posted comments on this change.
Change subject: bootstrap: Return recovery error until hsm is ready
..
Patch Set 5:
(1 comment)
File vdsm/storage/dispatcher.py
Line 92: class Dispatcher:
Line 93: log = logging.getLogger('Storage.Dispatcher')
Line 94:
Line 95: def __init__(self, obj):
Line 96: self._obj = obj
why save _obj and not move the 'ready' snippet in here?
i.e.
self.ready = ...
Line 97: self.storage_repository = config.get('irs', 'repository')
Line 98: self._exposeFunctions(obj)
Line 99: self.log.info(Starting StorageDispatcher...)
Line 100:
--
To view, visit http://gerrit.ovirt.org/21530
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id74468917c5b7c05d4183854e2f1255de98325dc
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Daniel Erez de...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Vered Volansky vvola...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: Rescan multipath before loading lvm cache
Ayal Baron has posted comments on this change. Change subject: hsm: Rescan multipath before loading lvm cache .. Patch Set 3: Code-Review+2 Actually the race doesn't exist since sdCache.refreshStorage calls lvm.invalidateCache so calling: lvm.bootstrap lvm.invalidateCache by definition means that we'll have redundant lvm calls. The only question I have is whether we can just drop the bootstrap call (since effectively this has been the behaviour up until now). -- To view, visit http://gerrit.ovirt.org/21567 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6ec7beee1db193a47d9e8109929badfd5b322c02 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Daniel Erez de...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: improve masterMigrate safety
Ayal Baron has posted comments on this change.
Change subject: sp: improve masterMigrate safety
..
Patch Set 3:
(1 comment)
File vdsm/storage/sp.py
Line 869: newmsd.changeRole(sd.MASTER_DOMAIN)
Line 870: self.switchMasterDomain(curmsd, newmsd, masterVersion)
Line 871: except Exception:
Line 872: self.log.exception('migration to new master failed')
Line 873: newmsd.unmountMaster()
would unmountMaster fail or succeed if already unmounted?
if this throws an exception we will loose the original one.
maybe try..except..else ?
Line 874: newmsd.releaseClusterLock()
Line 875: raise
Line 876:
Line 877: # From this point on we have a new master and should not fail
--
To view, visit http://gerrit.ovirt.org/22419
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I155b36d41df43230f99ba610699bba05bee3f0c0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: remove master info from getStorageDomainInfo
Ayal Baron has posted comments on this change. Change subject: hsm: remove master info from getStorageDomainInfo .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22824 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8c046db3884b56b57cc1bf040f3e9cc136c6d877 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: misc: Fix exception re-raising in RollbackContext
Ayal Baron has posted comments on this change. Change subject: misc: Fix exception re-raising in RollbackContext .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/22860 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0052717e2307ad6fec2225b3ad5f438c5a60e1c6 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: utils: Moved RollbackContext from misc to utils
Ayal Baron has posted comments on this change. Change subject: utils: Moved RollbackContext from misc to utils .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/22861 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I985103650a5706d35d9cd519618d09c692feb0be Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: utils: cleanup - typos, grammar and comments refinement
Ayal Baron has posted comments on this change. Change subject: utils: cleanup - typos, grammar and comments refinement .. Patch Set 8: (1 comment) File lib/vdsm/utils.py Line 55: Line 56: from cpopen import CPopen Line 57: from . import constants Line 58: Line 59: BUFFSIZE = 1024 I actually prefer having insight into where such magic numbers come from because then you know if it is safe to change them or not Line 60: Line 61: SUDO_NON_INTERACTIVE_FLAG = -n Line 62: Line 63: _THP_STATE_PATH = '/sys/kernel/mm/transparent_hugepage/enabled' -- To view, visit http://gerrit.ovirt.org/22862 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I04a1c4d444f2604b66b44bb9deac7d780db04aaf Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: improve pool creation error handling
Ayal Baron has posted comments on this change. Change subject: sp: improve pool creation error handling .. Patch Set 1: (1 comment) File vdsm/storage/sp.py Line 620: # lock Line 621: # TBD: create will receive only master domain and further attaches Line 622: # should be done under SPM Line 623: try: Line 624: for sdUUID in domList: why not just delete this code? engine doesn't create a pool with multiple domains and never has. Line 625: # Master domain was already attached (in createMaster) Line 626: if sdUUID != msdUUID: Line 627: self.attachSD(sdUUID) Line 628: except Exception: -- To view, visit http://gerrit.ovirt.org/22818 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cce08e368dec09c081609d0663d7990ab10 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: refresh metadata on hsm when listing domains
Ayal Baron has posted comments on this change. Change subject: sp: refresh metadata on hsm when listing domains .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21786 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I095cd0760076fb4be97a776498af78a40ff84112 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: split metadata transaction in createMaster
Ayal Baron has posted comments on this change.
Change subject: sp: split metadata transaction in createMaster
..
Patch Set 4:
(1 comment)
See patch set 4
File vdsm/storage/sp.py
Line 695: self.stopMonitoringDomains()
Line 696: return True
Line 697:
Line 698: @unsecured
Line 699: def initMasterDomain(self, poolName, domain, masterVersion):
but it's not the master metadata, it's the pool metadata.
that is the differentiation between initMaster and this.
Since it is in the context of the pool then it could be just initMetadata
Line 700: self._getPoolMD(domain).update({
Line 701: PMDK_SPM_ID: SPM_ID_FREE,
Line 702: PMDK_LVER: LVER_INVALID,
Line 703: PMDK_MASTER_VER: masterVersion,
--
To view, visit http://gerrit.ovirt.org/22418
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b22af92b1f9a481be9af844b1acc47ae513d078
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Vered Volansky vvola...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: split metadata transaction in createMaster
Ayal Baron has posted comments on this change.
Change subject: sp: split metadata transaction in createMaster
..
Patch Set 4:
(1 comment)
File vdsm/storage/sp.py
Line 695: self.stopMonitoringDomains()
Line 696: return True
Line 697:
Line 698: @unsecured
Line 699: def initMasterDomain(self, poolName, domain, masterVersion):
isn't this initPoolMD or something? (name wise)
Line 700: self._getPoolMD(domain).update({
Line 701: PMDK_SPM_ID: SPM_ID_FREE,
Line 702: PMDK_LVER: LVER_INVALID,
Line 703: PMDK_MASTER_VER: masterVersion,
--
To view, visit http://gerrit.ovirt.org/22418
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b22af92b1f9a481be9af844b1acc47ae513d078
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Vered Volansky vvola...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: improve masterMigrate safety
Ayal Baron has posted comments on this change.
Change subject: sp: improve masterMigrate safety
..
Patch Set 2:
(2 comments)
File vdsm/storage/sp.py
Line 768: self.stopMonitoringDomains()
Line 769: else:
Line 770: futureMaster.releaseClusterLock()
Line 771:
Line 772: def switchMasterDomain(self, currentMasterDomain, newMasterDomain,
sorry for saying this now, but it would be a lot easier to review if you split
the param name changes into a separate patch
Line 773:newMasterVersion):
Line 774: prevPoolMD = self._getPoolMD(currentMasterDomain)
Line 775: domains = prevPoolMD[PMDK_DOMAINS]
Line 776: pool_descr = prevPoolMD[PMDK_POOL_DESCRIPTION]
Line 869: newmsd.unmountMaster()
Line 870: except: # logging only
Line 871: self.log.error('unable to unmount master filesystem '
Line 872:'for domain %s', newmsd.sdUUID)
Line 873: newmsd.releaseClusterLock()
why is it ok to release the lock if unmount failed? (obviously need to make
sure we're not mounted anymore if it matters)
Line 874: raise
Line 875:
Line 876: # From this point on we have a new master and should not fail
Line 877: try:
--
To view, visit http://gerrit.ovirt.org/22419
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I155b36d41df43230f99ba610699bba05bee3f0c0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: remove master info from getStorageDomainInfo
Ayal Baron has posted comments on this change. Change subject: hsm: remove master info from getStorageDomainInfo .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22824 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8c046db3884b56b57cc1bf040f3e9cc136c6d877 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: encapsulate spm status in StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: encapsulate spm status in StoragePool .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21527 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b71349e2a0dfc453b68cd2d9ca6c563b1bee90c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: split metadata transaction in createMaster
Ayal Baron has posted comments on this change.
Change subject: sp: split metadata transaction in createMaster
..
Patch Set 2:
(1 comment)
File vdsm/storage/sd.py
Line 771: pools = self.getPools()
Line 772:
Line 773: if len(pools) 0:
Line 774: self.log.info('initializing master domain %s detaching
pools %s',
Line 775: self.sdUUID, ', '.join((str(x) for x in
pools)))
reconstructMaster is never run on a domain from a different pool, only on a
detached domain or on a domain from the same pool...
Line 776:
Line 777: with self._metadata.transaction():
Line 778: self.changeLeaseParams(leaseParams)
Line 779: self.setMetaParam(DMDK_POOLS, [spUUID])
--
To view, visit http://gerrit.ovirt.org/22418
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b22af92b1f9a481be9af844b1acc47ae513d078
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Vered Volansky vvola...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Revert clientIF: rescan devices on failed hotplugDisk
Ayal Baron has posted comments on this change. Change subject: Revert clientIF: rescan devices on failed hotplugDisk .. Patch Set 4: Looks like there is a rebase issue with this patch, other than that looks fine. -- To view, visit http://gerrit.ovirt.org/21173 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib978fada8ee82e3c6da9de9b79a66edafba4eb70 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: Check all devices visibility after multipath rescan
Ayal Baron has posted comments on this change. Change subject: hsm: Check all devices visibility after multipath rescan .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21182 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I46ede1124f4c5f849b040b598b49ee784c540ecc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: Do not hide errors when checking device visibility
Ayal Baron has posted comments on this change.
Change subject: hsm: Do not hide errors when checking device visibility
..
Patch Set 4: Code-Review-1
(1 comment)
File vdsm/storage/hsm.py
Line 2005: try:
Line 2006: res = (os.stat('/dev/mapper/' + guid).st_mode
Line 2007:stat.S_IRUSR != 0)
Line 2008: except OSError as e:
Line 2009: if e.errno != errno.NOENT:
s/NOENT/ENOENT/
Line 2010: raise
Line 2011: res = False
Line 2012: return res
Line 2013:
--
To view, visit http://gerrit.ovirt.org/21211
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7411d7d2eaa9172f612fd1b1e04177bef270ab19
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: clientIF: Rescan devices if device not found after vmHotplug...
Ayal Baron has posted comments on this change. Change subject: clientIF: Rescan devices if device not found after vmHotplugDisk .. Patch Set 6: (1 comment) File vdsm/clientIF.py Line 266: res = self.irs.appropriateDevice(drive[GUID], vmId) Line 267: if res['status']['code']: Line 268: raise vm.VolumeError(drive) Line 269: Line 270: volPath = os.path.join(/dev/mapper, drive[GUID]) I still want another patch that makes clientIF agnostic to the implementation detail that we rely on /dev/mapper Line 271: Line 272: # UUID drive format Line 273: elif UUID in drive: Line 274: volPath = self._getUUIDSpecPath(drive[UUID]) -- To view, visit http://gerrit.ovirt.org/21181 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3d75ac53ca809b7a25d7fb237ec661dd865e31fc Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: clientIF: Rescan devices if device not found after vmHotplug...
Ayal Baron has posted comments on this change. Change subject: clientIF: Rescan devices if device not found after vmHotplugDisk .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21181 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3d75ac53ca809b7a25d7fb237ec661dd865e31fc Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Revert clientIF: rescan devices on failed hotplugDisk
Ayal Baron has posted comments on this change. Change subject: Revert clientIF: rescan devices on failed hotplugDisk .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21173 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib978fada8ee82e3c6da9de9b79a66edafba4eb70 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: Do not hide errors when checking device visibility
Ayal Baron has posted comments on this change. Change subject: hsm: Do not hide errors when checking device visibility .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21211 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7411d7d2eaa9172f612fd1b1e04177bef270ab19 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: clientIF: Rescan devices if device not found after vmHotplug...
Ayal Baron has posted comments on this change. Change subject: clientIF: Rescan devices if device not found after vmHotplugDisk .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21181 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3d75ac53ca809b7a25d7fb237ec661dd865e31fc Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: fileSD: Fix image deletion on gluster domain
Ayal Baron has posted comments on this change. Change subject: fileSD: Fix image deletion on gluster domain .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/22563 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8ee6cb963a428f18b6e43f0585d461d0975c377a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Hunt Xu mhun...@gmail.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: Check all devices visibility after multipath rescan
Ayal Baron has posted comments on this change. Change subject: hsm: Check all devices visibility after multipath rescan .. Patch Set 5: Code-Review+2 u -- To view, visit http://gerrit.ovirt.org/21182 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I46ede1124f4c5f849b040b598b49ee784c540ecc Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: supervdsmServer: Run sourceRouteThread in a thread
Ayal Baron has posted comments on this change. Change subject: supervdsmServer: Run sourceRouteThread in a thread .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22718 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5eb78b57b9ae6ada9948815020ac3d5ac005d20a Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Assaf Muller amul...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: split metadata transaction in createMaster
Ayal Baron has posted comments on this change.
Change subject: sp: split metadata transaction in createMaster
..
Patch Set 2:
(3 comments)
Commit Message
Line 21: The two side effects of this change are:
Line 22: - we always initialize the pool metadata even when the domain metadata
Line 23: transaction fails
Line 24: - the createMaster operation takes now two storage operations instead
Line 25: of one
are you sure it did not actually reduce the number of ops?
i.e. with futurePoolMD.transaction(): really prevents domain.attach and the
like to commit until transaction is finished?
(if it doesn't perform it in one operation today then the first side effect
would not be correct either)
Line 26:
Line 27: Change-Id: I3b22af92b1f9a481be9af844b1acc47ae513d078
File vdsm/storage/sd.py
Line 771: pools = self.getPools()
Line 772:
Line 773: if len(pools) 0:
Line 774: self.log.info('initializing master domain %s detaching
pools %s',
Line 775: self.sdUUID, ', '.join((str(x) for x in
pools)))
why is this ok?
i.e. why is the check here not:
numPools = len(pools)
if numPools 1 or (numPools == 1 and pools[0] != spUUID):
raise ...
Line 776:
Line 777: with self._metadata.transaction():
Line 778: self.changeLeaseParams(leaseParams)
Line 779: self.setMetaParam(DMDK_POOLS, [spUUID])
File vdsm/storage/sp.py
Line 722:
Line 723: if not misc.isAscii(poolName) and not
domain.supportsUnicode():
Line 724: raise se.UnicodeArgumentException()
Line 725:
Line 726: self.initMasterDomain(domain, poolName, masterVersion)
why not keep the same order of params?
Line 727: domain.initMaster(self.spUUID, leaseParams)
Line 728:
Line 729: @unsecured
Line 730: def reconstructMaster(self, hostId, poolName, msdUUID, domDict,
--
To view, visit http://gerrit.ovirt.org/22418
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b22af92b1f9a481be9af844b1acc47ae513d078
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Vered Volansky vvola...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: remove unused recoveryMode from spmStart
Ayal Baron has posted comments on this change. Change subject: sp: remove unused recoveryMode from spmStart .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21556 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4be0a2aad270e87cf6a83f8b1fd36d928312104e Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: remove unused scsiFencing parameter
Ayal Baron has posted comments on this change. Change subject: sp: remove unused scsiFencing parameter .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21555 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d9b62c08cf8925fbc1856655dc49dd41eacd2d9 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: specific method to validate the master version
Ayal Baron has posted comments on this change. Change subject: sp: specific method to validate the master version .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22130 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a055c9eb21cc5681d1f9afde5e7eab899aa65c0 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: refresh pool connection on connectStoragePool
Ayal Baron has posted comments on this change. Change subject: hsm: refresh pool connection on connectStoragePool .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22467 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I46d7c36b48f5d58edbc89947ccd2bcd60408a729 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: add setMasterDomain to StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: add setMasterDomain to StoragePool .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21659 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I99caffb6143d3a4fff16102c3b7c00616373e193 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: remove unused transaction in createMaster
Ayal Baron has posted comments on this change. Change subject: sp: remove unused transaction in createMaster .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22418 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b22af92b1f9a481be9af844b1acc47ae513d078 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: remove automatic storage pool reconnection
Ayal Baron has posted comments on this change. Change subject: sp: remove automatic storage pool reconnection .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: improve masterMigrate safety
Ayal Baron has posted comments on this change.
Change subject: sp: improve masterMigrate safety
..
Patch Set 1:
(6 comments)
File vdsm/storage/sp.py
Line 879: curmsd = sdCache.produce(sdUUID)
Line 880: newmsd = sdCache.produce(msdUUID)
Line 881: self._refreshDomainLinks(newmsd)
Line 882: curmsd.invalidateMetadata()
Line 883: self._convertDomain(newmsd, curmsd.getFormat())
is it ok to call convertDomain before acquiring the lease?
Line 884: self._copyLeaseParameters(curmsd, newmsd)
Line 885:
Line 886: # new 'master' should be in 'active' status
Line 887: domList = self.getDomains()
Line 880: newmsd = sdCache.produce(msdUUID)
Line 881: self._refreshDomainLinks(newmsd)
Line 882: curmsd.invalidateMetadata()
Line 883: self._convertDomain(newmsd, curmsd.getFormat())
Line 884: self._copyLeaseParameters(curmsd, newmsd)
doesn't switch master cover this?
Line 885:
Line 886: # new 'master' should be in 'active' status
Line 887: domList = self.getDomains()
Line 888: if msdUUID not in domList:
Line 893: raise se.StorageDomainNotActive(msdUUID)
Line 894: if newmsd.isISO():
Line 895: raise se.IsoCannotBeMasterDomain(msdUUID)
Line 896: if newmsd.isBackup():
Line 897: raise se.BackupCannotBeMasterDomain(msdUUID)
shouldn't we perform all of these checks *before* alling _convertDomain,
copyLeaseParameters, etc?
Line 898:
Line 899: # If the new master domain is using safelease (version 3)
then
Line 900: # we can speed up the cluster lock acquirement by resetting
the
Line 901: # SPM lease.
Line 927: src = os.path.join(curmsd.domaindir, sd.MASTER_FS_DIR)
Line 928: dst = os.path.join(newmsd.domaindir, sd.MASTER_FS_DIR)
Line 929: fileUtils.tarCopy(src, dst, exclude=('./lost+found',))
Line 930:
Line 931: if not newmsd.getMDPath():
shouldn't this check be performed before call to mountMaster, together with all
the above validations? (before any change actually)
Line 932: raise se.StorageDomainLayoutError(domain, msdUUID)
Line 933:
Line 934: newmsd.changeRole(sd.MASTER_DOMAIN)
Line 935: self.switchMasterDomain(curmsd, newmsd, masterVersion)
Line 932: raise se.StorageDomainLayoutError(domain, msdUUID)
Line 933:
Line 934: newmsd.changeRole(sd.MASTER_DOMAIN)
Line 935: self.switchMasterDomain(curmsd, newmsd, masterVersion)
Line 936: except Exception:
no finally with release?
Line 937: self.log.exception('migration to new master failed')
Line 938: try:
Line 939: newmsd.unmountMaster()
Line 940: except: # logging only
Line 944: raise
Line 945:
Line 946: # From this point on we have a new master and should not fail
Line 947: try:
Line 948: self.refresh(msdUUID, masterVersion)
move all code within try...except into a separate method? (same thing above)
Line 949: self._saveReconnectInformation(
Line 950: self.id, self.scsiKey, newmsd.sdUUID, masterVersion)
Line 951:
Line 952: # From this point on there is a new master domain in the
pool
--
To view, visit http://gerrit.ovirt.org/22419
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I155b36d41df43230f99ba610699bba05bee3f0c0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: consolidate getMasterVersion in one method
Ayal Baron has posted comments on this change. Change subject: sp: consolidate getMasterVersion in one method .. Patch Set 4: (1 comment) File vdsm/storage/sp.py Line 1503: self.log.error(Requested master domain %s does not belong to pool Line 1504: %s, msdUUID, self.spUUID) Line 1505: raise se.StoragePoolWrongMaster(self.spUUID, msdUUID) Line 1506: Line 1507: version = self.getMasterVersion(useMasterDomain=domain) optionally passing the domain in this call is really bugging me and it made me think whether this version check shouldn't actually be moved into domain.isMaster. e.g. if not domain.isMaster(expectedVersion = masterVersion): self.log.error(Requested domain %s is not marked as a master domain or its version is wrong, msdUUID). If you think about it, master domain is a combination of role and version so I think this makes sense. Line 1508: if version != int(masterVersion): Line 1509: self.log.error(Requested master domain %s does not have expected Line 1510:version %s it is version %s, Line 1511:msdUUID, masterVersion, version) -- To view, visit http://gerrit.ovirt.org/22130 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a055c9eb21cc5681d1f9afde5e7eab899aa65c0 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: Reboot capability for VM
Ayal Baron has posted comments on this change. Change subject: vdsm: Reboot capability for VM .. Patch Set 38: Dan, this is patchset number 37. The patch started out as seperate patches and that didn't make sense (there is no sense in the callback chain change if we're not adding reboot). reboot functionality is not a lot of code and doesn't make reviewing the code any more difficult (and it puts the callback chain in perspective)... -- To view, visit http://gerrit.ovirt.org/15829 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I12737e363a80679ffb1db55f14eaee158312d7da Gerrit-PatchSet: 38 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Betak mbe...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Better Saggi bettersa...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Giuseppe Vallarelli gvall...@redhat.com Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com Gerrit-Reviewer: Martin Betak mbe...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Martin Sivák msi...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Omer Frenkel ofren...@redhat.com Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 8: (2 comments) File vdsm/storage/hsm.py Line 1040: else: Line 1041: pool.update(hostID, scsiKey, msdUUID, masterVersion) Line 1042: return True Line 1043: Line 1044: pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) why is it ok to not add pool.update here? Line 1045: Line 1046: # Must register domain state change callbacks *before* connecting Line 1047: # the pool, which starts domain monitor threads. Otherwise we will Line 1048: # miss the first event from the monitor thread. File vdsm/storage/sp.py Line 677: f.writelines(pers) Line 678: Line 679: @unsecured Line 680: def getPoolParams(self, hostID, scsiKey, msdUUID, masterVersion): Line 681: if all((hostID, scsiKey, msdUUID, masterVersion)): I still hate this behaviour Line 682: return hostID, scsiKey, msdUUID, masterVersion Line 683: Line 684: self.log.info( Line 685: Using saved connection parameters: hostID=%s, scsiKey=%s, -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: encapsulate spm status in StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: encapsulate spm status in StoragePool .. Patch Set 7: (1 comment) File vdsm/storage/sp.py Line 167: @unsecured Line 168: def forceFreeSpm(self): Line 169: # DO NOT USE, STUPID, HERE ONLY FOR BC Line 170: # TODO: SCSI Fence the 'lastOwner' Line 171: self.setSpmStatus(LVER_INVALID, SPM_ID_FREE) this code can't work without securityOverride=True Line 172: self.spmRole = SPM_FREE Line 173: Line 174: def _upgradePoolDomain(self, sdUUID, isValid): Line 175: # This method is called everytime the onDomainStateChange -- To view, visit http://gerrit.ovirt.org/21527 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b71349e2a0dfc453b68cd2d9ca6c563b1bee90c Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: add setMasterDomain to StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: add setMasterDomain to StoragePool .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21659 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I99caffb6143d3a4fff16102c3b7c00616373e193 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: clientIF: Teardown volume path only for VDSM images
Ayal Baron has posted comments on this change.
Change subject: clientIF: Teardown volume path only for VDSM images
..
Patch Set 5: Code-Review+1
(1 comment)
File vdsm/clientIF.py
Line 326: def teardownVolumePath(self, drive):
Line 327: res = {'status': doneCode}
Line 328:
Line 329: if vm.isVdsmImage(drive):
Line 330: res = self.irs.teardownImage(drive['domainID'],
making this method symmetric to prepare is not what this patch is about.
it is about preventing improper logging that is causing a lot of confusion for
users.
So symmetry can be handled separately.
With regards to *how* it should be handled, it looks like the right thing to do
is not to add drive['device'] == 'disk' test here but rather remove it from
prepare.
In addition, prepare and teardown are not symmetrical in many other ways.
prepareVolumePath is preparing floppies, cdrom etc which teardown is not taking
care of.
the 'teardown' for floppy is _cleanupFloppy etc.
Line 331: drive['poolID'],
drive['imageID'])
Line 332: return res['status']['code']
Line 333:
Line 334: def getDiskAlignment(self, drive):
--
To view, visit http://gerrit.ovirt.org/21973
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I041a306636c75a7aa37d4d7c0811366d80fe609c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: Allon Mureinik amure...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: clientIF: Check device visibility on failed hotplugDisk
Ayal Baron has posted comments on this change. Change subject: clientIF: Check device visibility on failed hotplugDisk .. Patch Set 4: (1 comment) File vdsm/clientIF.py Line 289: res = self.irs.appropriateDevice(drive[GUID], vmId) Line 290: if res['status']['code']: Line 291: raise vm.VolumeError(drive) Line 292: Line 293: volPath = os.path.join(/dev/mapper, drive[GUID]) If already moving this, the knowledge that this device is a dev mapper device should not reside in clientIF at all (should return from getDevicesVisibility or appropriateDevice or something) Line 294: Line 295: # UUID drive format Line 296: elif UUID in drive: Line 297: volPath = self._getUUIDSpecPath(drive[UUID]) -- To view, visit http://gerrit.ovirt.org/21181 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3d75ac53ca809b7a25d7fb237ec661dd865e31fc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: consolidate getMasterVersion in one method
Ayal Baron has posted comments on this change. Change subject: sp: consolidate getMasterVersion in one method .. Patch Set 1: (1 comment) File vdsm/storage/sp.py Line 489: Line 490: @unsecured Line 491: def getMasterVersion(self, useMasterDomain=None): Line 492: domain = (self.masterDomain Line 493: if useMasterDomain is None else useMasterDomain) why not split this into 2 functions: _getMasterVersionFromDomain(self, domain): return self._getPoolMD(domain)[PMDK_MASTER_VER] getMasterVersion(self): return self._getMasterVersionFromDomain(self.masterDomain) ? Line 494: return self._getPoolMD(domain)[PMDK_MASTER_VER] Line 495: Line 496: def setDomainMasterRole(self, domain, role, masterVersion): Line 497: poolMeta = self._getPoolMD(domain) -- To view, visit http://gerrit.ovirt.org/22130 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a055c9eb21cc5681d1f9afde5e7eab899aa65c0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: securable: refactor the scurable implementation
Ayal Baron has posted comments on this change. Change subject: securable: refactor the scurable implementation .. Patch Set 2: (3 comments) Commit Message Line 3: AuthorDate: 2013-12-05 07:16:59 -0500 Line 4: Commit: Federico Simoncelli fsimo...@redhat.com Line 5: CommitDate: 2013-12-06 11:54:48 -0500 Line 6: Line 7: securable: refactor the scurable implementation refactor why? Line 8: Line 9: Change-Id: Id5a8be7536748481746795e27701fbecc7c3318c File vdsm/storage/securable.py Line 32: def secured(cls): Line 33: for fun, val in cls.__dict__.iteritems(): Line 34: if (callable(val) and getattr(val, SECURE_FIELD, True) Line 35: and not fun.startswith(__) # skipping builtins Line 36: and not fun == SECURE_METHOD): I actually think the previous style is more legible. Line 37: setattr(cls, fun, secure_method(val)) Line 38: cls.__securable__ = True Line 39: return cls Line 40: Line 43: setattr(f, SECURE_FIELD, False) Line 44: return f Line 45: Line 46: Line 47: def secure_method(method): did we change the function naming convention to using underscores already? Line 48: def wrapper(self, *args, **kwargs): Line 49: if not hasattr(self, __securable__): Line 50: raise RuntimeError(Secured object is not a securable) Line 51: -- To view, visit http://gerrit.ovirt.org/22115 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5a8be7536748481746795e27701fbecc7c3318c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: avoid masking uuid in StoragePool methods
Ayal Baron has posted comments on this change. Change subject: sp: avoid masking uuid in StoragePool methods .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21873 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d108e9daa42290c46faa1de3d43e8883487e83c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: domainMonitor: Separate change detection from lastCheck value
Ayal Baron has posted comments on this change. Change subject: domainMonitor: Separate change detection from lastCheck value .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21878 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e0df2ee054146b5d9429fd83a4f914ff751 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Daniel Erez de...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Liron Ar lara...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Vered Volansky vvola...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: threadPool: Do not keep reference to tasks
Ayal Baron has posted comments on this change. Change subject: threadPool: Do not keep reference to tasks .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22136 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6e024e0727e11a4c98f4e5ede20229d148fdc58 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: _refreshDomainLinks must not change the metadata
Ayal Baron has posted comments on this change. Change subject: sp: _refreshDomainLinks must not change the metadata .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22131 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ecf801d58b34c1c811e311e3779887a406af5f0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: threadPool: Do not keep reference to tasks
Ayal Baron has posted comments on this change. Change subject: threadPool: Do not keep reference to tasks .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/22136 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6e024e0727e11a4c98f4e5ede20229d148fdc58 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: domainMonitor: Rename confusing lastCheck variable
Ayal Baron has posted comments on this change. Change subject: domainMonitor: Rename confusing lastCheck variable .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21879 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56ce7c2903c33c25693601dde5a1af376b789e59 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Liron Ar lara...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: domainMonitor: Improve logging
Ayal Baron has posted comments on this change. Change subject: domainMonitor: Improve logging .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/21904 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I20feb40b097fa65ebebf851e33c051fc184ff029 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Ohad Basan oba...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: avoid masking uuid in StoragePool methods
Ayal Baron has posted comments on this change. Change subject: sp: avoid masking uuid in StoragePool methods .. Patch Set 1: (1 comment) File vdsm/storage/sp.py Line 1895: imageResourcesNamespace = sd.getNamespace(sdUUID, IMAGE_NAMESPACE) Line 1896: Line 1897: with rmanager.acquireResource(imageResourcesNamespace, imgUUID, Line 1898: rm.LockType.exclusive): Line 1899: volUUID = sdCache.produce(sdUUID).createVolume( now you're overriding the volUUID parameter passed into the function. Possibly this is ok, but I want to make sure (or avoid confusion by using a different name such as newVolUUID) Line 1900: imgUUID=imgUUID, size=size, volFormat=volFormat, Line 1901: preallocate=preallocate, diskType=diskType, volUUID=volUUID, Line 1902: desc=desc, srcImgUUID=srcImgUUID, srcVolUUID=srcVolUUID) Line 1903: return dict(uuid=volUUID) -- To view, visit http://gerrit.ovirt.org/21873 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d108e9daa42290c46faa1de3d43e8883487e83c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: remove the getFormat method
Ayal Baron has posted comments on this change. Change subject: sp: remove the getFormat method .. Patch Set 3: Are you saying that there's a logical difference between getVersion and getFormat? Or you're saying that we can get rid of getVersion and keep getFormat (handling the str/int differences). I don't understand why we need both, are they the same value just by accident? The original 'getVersion' was a misnomer as it is a format (store in LV or store in VG tags or any other way). One is not superior to the other, just different. I'm fine with getting rid of getVersion, but make sure there will be no BC issues there. -- To view, visit http://gerrit.ovirt.org/21785 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb292ef57f94e1fea67f3111e5f414cc41c346a9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: getStorageDomainInfo: SP keys when disconnected.
Ayal Baron has posted comments on this change. Change subject: getStorageDomainInfo: SP keys when disconnected. .. Patch Set 2: (1 comment) File vdsm/storage/hsm.py Line 2726: # getSharedLock(connectionsResource...) Line 2727: Line 2728: vars.task.getSharedLock(STORAGE, sdUUID) Line 2729: info = dom.getInfo() Line 2730: # This only occurred because someone As far as I can see engine never read this part of the info. As you wrote in the commit message, getStorageDomainInfo should not return pool info. I could not agree more. Pending a bit testing, I see no reason why not to just delete lines 2730 - 2767 here (everything regarding the pool). Line 2731: # thought it would be clever to return pool Line 2732: # information in the domain.getInfo() method Line 2733: # In a perfect world I would have just stopped Line 2734: # giving this information in the response. -- To view, visit http://gerrit.ovirt.org/19555 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b7323d4ccaaaec0f39d5590a06879faa7fc999e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewars...@redhat.com Gerrit-Reviewer: Aharon Canan aharo...@gmail.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Elad Ben Aharon eladba1...@gmail.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Gadi Ickowicz gicko...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: Fix getStorageDomainInfo() logic.
Ayal Baron has posted comments on this change. Change subject: Fix getStorageDomainInfo() logic. .. Patch Set 2: Verified-1 Code-Review-1 This patch will break 3.3 as it expects pool.getInfo to return the info without the 'info' key but that was only introduced in commit f9cf58b7bced4e10c6a7a9fc89e329c6eb58124c (Make getRepoStats() a hsm method) which was not backported into 3.3 -- To view, visit http://gerrit.ovirt.org/20209 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: encapsulate spm status in StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: encapsulate spm status in StoragePool .. Patch Set 6: (1 comment) File vdsm/storage/sp.py Line 145: Line 146: return poolMeta[PMDK_LVER], poolMeta[PMDK_SPM_ID] Line 147: Line 148: def setSpmStatus(self, lVer, spmId): Line 149: self.invalidateMetadata() why are you invalidating here? if this is secured then our md should not be stale and if overriding then it's caller's responsibility to invalidate? Line 150: # this method must be secured (as it changes the pool metadata), Line 151: # but since it is also used during the SPM status transition by Line 152: # default we override the security for setMetaParams. Line 153: # NOTE: this introduces a race when the method is used in the -- To view, visit http://gerrit.ovirt.org/21527 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b71349e2a0dfc453b68cd2d9ca6c563b1bee90c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: add setMasterDomain to StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: add setMasterDomain to StoragePool .. Patch Set 4: 1. I've never noticed this 2. You failed to explain that connectStoragePool returns true if you're already connected and if it is called after a reconstructMaster with the new params then without this patch although it would succeed the self.masterDomain value would be incorrect. So this patch is not just cosmetic, it fixes behaviour. +2 on the code, please update the commit message though. -- To view, visit http://gerrit.ovirt.org/21659 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I99caffb6143d3a4fff16102c3b7c00616373e193 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: remove unused scsiFencing parameter
Ayal Baron has posted comments on this change. Change subject: sp: remove unused scsiFencing parameter .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21555 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d9b62c08cf8925fbc1856655dc49dd41eacd2d9 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: remove unused recoveryMode from spmStart
Ayal Baron has posted comments on this change. Change subject: sp: remove unused recoveryMode from spmStart .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21556 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4be0a2aad270e87cf6a83f8b1fd36d928312104e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: remove the getFormat method
Ayal Baron has posted comments on this change. Change subject: sp: remove the getFormat method .. Patch Set 3: why? Until we get rid of the whole thing Format is actually the correct terminology (vs. the old 'version'). -- To view, visit http://gerrit.ovirt.org/21785 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb292ef57f94e1fea67f3111e5f414cc41c346a9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: refresh metadata on hsm when listing domains
Ayal Baron has posted comments on this change. Change subject: sp: refresh metadata on hsm when listing domains .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21786 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I095cd0760076fb4be97a776498af78a40ff84112 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move validatePoolSD in the StoragePool class
Ayal Baron has posted comments on this change. Change subject: sp: move validatePoolSD in the StoragePool class .. Patch Set 3: (2 comments) File vdsm/storage/hsm.py Line 1343: :type sdUUID: UUID Line 1344: :param options: ? Line 1345: Line 1346: pool = self.getPool(spUUID) Line 1347: if not sdUUID or sdUUID != sd.BLANK_UUID: s/!=/==/ Line 1348: sdUUID = pool.masterDomain.sdUUID Line 1349: vars.task.getSharedLock(STORAGE, sdUUID) Line 1350: vms = pool.getVmsList(sdUUID) Line 1351: return dict(vmlist=vms) File vdsm/storage/sp.py Line 1054: dom = sdCache.produce(sdUUID) Line 1055: Line 1056: # Avoid detach domains if not owned by pool Line 1057: self.validatePoolSD(sdUUID) Line 1058: self.validateAttachedDomain(dom) validateAttachedDomain starts with a call to validatePoolSD... ? Line 1059: Line 1060: if sdUUID == self.masterDomain.sdUUID: Line 1061: raise se.CannotDetachMasterStorageDomain(sdUUID) Line 1062: -- To view, visit http://gerrit.ovirt.org/21787 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I75f0245ec5449d57431df7055be401d73975312b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sd: add inquireClusterLock method to StorageDomain
Ayal Baron has posted comments on this change. Change subject: sd: add inquireClusterLock method to StorageDomain .. Patch Set 2: Code-Review+1 (1 comment) Commit Message Line 3: AuthorDate: 2013-11-19 10:25:16 -0500 Line 4: Commit: Federico Simoncelli fsimo...@redhat.com Line 5: CommitDate: 2013-11-28 12:44:10 -0500 Line 6: Line 7: sd: add inquireClusterLock method to StorageDomain why? Line 8: Line 9: Change-Id: I10e64d74319ea6591a7edf8e17809d367a758386 -- To view, visit http://gerrit.ovirt.org/21426 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I10e64d74319ea6591a7edf8e17809d367a758386 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: getStorageDomainInfo: SP keys when disconnected.
Ayal Baron has posted comments on this change.
Change subject: getStorageDomainInfo: SP keys when disconnected.
..
Patch Set 2: Code-Review-1
(6 comments)
1. No clear motivation for this patch.
2. It is breaking encapsulation
Commit Message
Line 12: directly the pool MD for any MSD.
Line 13: This changes the previous behaviour.
Line 14:
Line 15: TODO: pool related information should not be returned as part of
Line 16: this API response.
+1
Line 17:
Line 18: Change-Id: I8b7323d4ccaaaec0f39d5590a06879faa7fc999e
File vdsm/storage/hsm.py
Line 2735: # This, of-course breaks backward compatibility.
Line 2736: # These keys are not likely to change (also because of
Line 2737: # BC) so it's not that horrible. In any case please
Line 2738: # remove this when we can stop supporting this API.
Line 2739: if info['role'] != sd.MASTER_DOMAIN:
remove this if
Line 2740: info.update({'lver': -1, 'spm_id': -1, 'master_ver': 0})
Line 2741: else:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2737: # BC) so it's not that horrible. In any case please
Line 2738: # remove this when we can stop supporting this API.
Line 2739: if info['role'] != sd.MASTER_DOMAIN:
Line 2740: info.update({'lver': -1, 'spm_id': -1, 'master_ver': 0})
Line 2741: else:
change else to: if info['role'] == sd.MASTER_DOMAIN:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2744: info['master_ver'] = pmd[sp.PMDK_MASTER_VER]
Line 2745: info['spm_id'] = -1
Line 2738: # remove this when we can stop supporting this API.
Line 2739: if info['role'] != sd.MASTER_DOMAIN:
Line 2740: info.update({'lver': -1, 'spm_id': -1, 'master_ver': 0})
Line 2741: else:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
instead of accessing internals, you should be adding a method that returns the
info you need or something
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2744: info['master_ver'] = pmd[sp.PMDK_MASTER_VER]
Line 2745: info['spm_id'] = -1
Line 2746: try:
Line 2740: info.update({'lver': -1, 'spm_id': -1, 'master_ver': 0})
Line 2741: else:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2744: info['master_ver'] = pmd[sp.PMDK_MASTER_VER]
I see no reason to return this info if getPool fails?
i.e. move both lines down to join the spm_id line
Line 2745: info['spm_id'] = -1
Line 2746: try:
Line 2747: # Verify that the host is connected to the same pool
which
Line 2748: # the SD is attached to.
Line 2741: else:
Line 2742: pmd = sp.StoragePool._getPoolMD(dom)
Line 2743: info['lver'] = pmd[sp.PMDK_LVER]
Line 2744: info['master_ver'] = pmd[sp.PMDK_MASTER_VER]
Line 2745: info['spm_id'] = -1
remove this line (spm_id = -1) as with above changes it is redundant
Line 2746: try:
Line 2747: # Verify that the host is connected to the same pool
which
Line 2748: # the SD is attached to.
Line 2749: pool = self.getPool(info['pool'][0])
--
To view, visit http://gerrit.ovirt.org/19555
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b7323d4ccaaaec0f39d5590a06879faa7fc999e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Ayal Baron aba...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Eduardo ewars...@redhat.com
Gerrit-Reviewer: Elad Ben Aharon eladba1...@gmail.com
Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com
Gerrit-Reviewer: Gadi Ickowicz gicko...@redhat.com
Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com
Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: Fix getStorageDomainInfo() logic.
Ayal Baron has posted comments on this change. Change subject: Fix getStorageDomainInfo() logic. .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/20209 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: move reconnection info check to StoragePool
Ayal Baron has posted comments on this change. Change subject: sp: move reconnection info check to StoragePool .. Patch Set 3: Code-Review-1 (5 comments) File vdsm/storage/hsm.py Line 1027: except se.StoragePoolUnknown: Line 1028: pass # pool not connected yet Line 1029: else: Line 1030: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.shared): Line 1031: pool.update(hostID, scsiKey, msdUUID, masterVersion) after acquiring the lock we can't even be sure we're connected to the same pool (chances are slim, but still)... Line 1032: return True Line 1033: Line 1034: with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive): Line 1035: try: Line 1036: pool = self.getPool(spUUID) Line 1037: except se.StoragePoolUnknown: Line 1038: pass # pool not connected yet Line 1039: else: Line 1040: pool.update(hostID, scsiKey, msdUUID, masterVersion) same Line 1041: return True Line 1042: Line 1043: pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) Line 1044: res = pool.connect(hostID, scsiKey, msdUUID, masterVersion) File vdsm/storage/sp.py Line 688: msg = Pools temp data dir: %s does not exist % ( Line 689: self._poolsTmpDir) Line 690: raise se.StoragePoolConnectionError(msg) Line 691: Line 692: self._saveReconnectInformation(hostID, scsiKey, msdUUID, masterVersion) I wonder if the mistake is not reverse than the purpose of this patch. i.e. not that reading the reconnection info belong inside connect but rather that persisting it belongs outside. this method has 2 different ways of getting its parameters and if someone called it and did not pass them then we fail back to the other method. Instead, maybe we should have a separate reconnect method and whenever connect here is called, the parameters have to be passed. The only place we actually call connectStoragePool without params is in the init of hsm. So we could just retrieve the info there and persist it also at hsm level in connect. Alternatively, I'm not even clear on whether this still works and engine automatically reconnects and we also support running VMs without being connected to a pool so I wonder if we shouldn't just drop all the reconnect logic and be done with it (esp. as what we're trying to do is get rid of the pool here). We will need to automatically connect to domains later on (acquire lock space) to cut on startup time, but that is pretty much a different issue entirely. Line 693: self.id = hostID Line 694: self.scsiKey = scsiKey Line 695: # Make sure SDCache doesn't have stale data (it can be in case of FC) Line 696: sdCache.invalidateStorage() Line 741: Line 742: @unsecured Line 743: def getPoolParams(self, hostID, scsiKey, msdUUID, masterVersion): Line 744: if all(hostID, scsiKey, msdUUID, masterVersion): Line 745: return hostID, scsiKey, msdUUID, masterVersion this makes things even more weird. if user passed params - return the params right back. if not - read from disk and return Line 746: Line 747: self.log.info( Line 748: Using saved connection parameters: hostID=%s, scsiKey=%s, Line 749: msdUUID=%s, masterVersion=%s, hostID, scsiKey, msdUUID, Line 752: with open(self._poolFile, r) as f: Line 753: poolInfo = dict((x.strip().split(=, 1) for x in f)) Line 754: Line 755: return tuple(poolInfo.get(x) for x in Line 756: (hostId, scsiKey, msdUUID, masterVersion)) same as patch set 1 or at least explain why I'm wrong? Line 757: Line 758: @unsecured Line 759: def createMaster(self, poolName, domain, masterVersion, leaseParams): Line 760: -- To view, visit http://gerrit.ovirt.org/21424 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia57afc04db02b6c15633d09349a55b3ff5ae7fda Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
