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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg 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: 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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 Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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 Gerrit-Reviewer: Ayal Baron 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 Gerrit-Reviewer: Ayal Baron 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli 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: 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 Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Vinzenz Feenstra 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli 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]: [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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Haim Ateya Gerrit-Reviewer: Yeela Kaplan 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 Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Yeela Kaplan 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 Gerrit-Reviewer: Ayal Baron 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]: 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 Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Roy Golan Gerrit-Reviewer: Yaniv Bronhaim 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]: Remove redundant check that causes lvm cache to refresh ever...
Ayal Baron has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yeela Kaplan 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]: vm: extend shared property to support locking
Ayal Baron has posted comments on this change. Change subject: vm: extend shared property to support locking .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/17714 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9429ead45caac1178957a33393642817db59508f Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Sandro Bonazzola Gerrit-Reviewer: Yeela Kaplan 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]: vm: extend shared property to support locking
Ayal Baron has submitted this change and it was merged. Change subject: vm: extend shared property to support locking .. vm: extend shared property to support locking The drives "shared" property has been extended (from True/False) to support sanlock locking. The new values are: - "none": no infomation (locking disabled) - "exclusive": the disk is used by the VM in exclusive mode - "shared":the disk might be used by multiple VMs For backward compatibility the old values are mapped as follows: - True: "shared" - False: "none" (or "exclusive" if use_volume_leases is True) Change-Id: I9429ead45caac1178957a33393642817db59508f Signed-off-by: Federico Simoncelli Reviewed-on: http://gerrit.ovirt.org/17714 Reviewed-by: Ayal Baron --- M client/vdsClient.py M tests/vmTests.py M vdsm/storage/hsm.py M vdsm/vm.py M vdsm_api/vdsmapi-schema.json 5 files changed, 146 insertions(+), 53 deletions(-) Approvals: Ayal Baron: Looks good to me, approved Federico Simoncelli: Verified -- To view, visit http://gerrit.ovirt.org/17714 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9429ead45caac1178957a33393642817db59508f Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Sandro Bonazzola Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: One shot prepare.
Ayal Baron has posted comments on this change. Change subject: One shot prepare. .. Patch Set 15: (2 comments) Commit Message Line 9: The number of storage accesses is reduced to 1 or none instead of Line 10: the recursive volume images produces. Line 11: Requires a new selinux policy. Line 12: Line 13: Related to BZ#960952, BZ#769502, BZ#990509. this solves bug 960952. not related to Line 14: Line 15: Change-Id: Id47db9c53199385dd5d08586e4782ea23885eb72 File vdsm/storage/fileSD.py Line 423: Creates a compatiblity layer for vdsm and qemu-img use. Line 424: Line 425: srcImgPath: Dir where the image volumes are. Line 426: """ Line 427: sdRunDir = os.path.join("/var/run/vdsm/storage", self.sdUUID) /var/run/vdsm hard coded here? Line 428: fileUtils.createdir(sdRunDir) Line 429: imgRunDir = os.path.join(sdRunDir, imgUUID) Line 430: try: Line 431: os.symlink(srcImgPath, imgRunDir) -- To view, visit http://gerrit.ovirt.org/4220 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id47db9c53199385dd5d08586e4782ea23885eb72 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Haim Ateya Gerrit-Reviewer: Igor Lvovsky Gerrit-Reviewer: Sandro Bonazzola Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: mount: Reassign mount specification in case of backup option
Ayal Baron has posted comments on this change. Change subject: mount: Reassign mount specification in case of backup option .. Patch Set 2: (1 comment) Commit Message Line 7: mount: Reassign mount specification in case of backup option Line 8: Line 9: When using glusterfs volumes the backup option Line 10: 'backupvolfile-server' allows mount to an alternative volume Line 11: when mount source not available. Please add something like: Gluster supports the ability to mount using a failback server in case mounting using the primary server fails. This ability is specified by the 'backupvolfile-server' mount option. Since mount can end up using a different server than the one specified in the main uri, vdsm needs to take this possibility into account. This patch adds support for this mount option. Line 12: Line 13: Change-Id: I3166c6863dffa297bc0adcdeb4c22f810d18de8e Line 14: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=922744 -- To view, visit http://gerrit.ovirt.org/16534 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3166c6863dffa297bc0adcdeb4c22f810d18de8e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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]: One shot prepare.
Ayal Baron has posted comments on this change. Change subject: One shot prepare. .. Patch Set 16: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/4220 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id47db9c53199385dd5d08586e4782ea23885eb72 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Haim Ateya Gerrit-Reviewer: Igor Lvovsky Gerrit-Reviewer: Sandro Bonazzola Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: Remove redundant check that causes lvm cache to refresh ever...
Ayal Baron has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 6: (1 comment) File vdsm/storage/fileVolume.py Line 90: metaVolPath = cls.__metaVolumePath(volPath) Line 91: cls.log.info("Halfbaked volume rollback for volPath=%s", volPath) Line 92: Line 93: if oop.getProcessPool(sdUUID).fileUtils.pathExists(volPath) and not \ Line 94: oop.getProcessPool(sdUUID).fileUtils.pathExists(metaVolPath): why don't we need a similar check in blockVolume.py ? Line 95: oop.getProcessPool(sdUUID).os.unlink(volPath) Line 96: Line 97: @classmethod Line 98: def validateCreateVolumeParams(cls, volFormat, preallocate, srcVolUUID): -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yeela Kaplan 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]: More precise catch in block volume create.
Ayal Baron has posted comments on this change. Change subject: More precise catch in block volume create. .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18883 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5ac32eb0d58404c3e066dc73488ae0101560a919 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Yeela Kaplan 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]: Read pool metadata once in StoragePool.getInfo()
Ayal Baron has posted comments on this change. Change subject: Read pool metadata once in StoragePool.getInfo() .. Patch Set 4: (1 comment) File vdsm/storage/sp.py Line 1495: poolInfo = { Line 1496: 'type': msdInfo['type'], Line 1497: 'name': pmd[PMDK_POOL_DESCRIPTION], Line 1498: 'domains': domainListEncoder(pmd[PMDK_DOMAINS]), Line 1499: 'master_uuid': self.masterDomain.sdUUID, nit: why change this from msdUUID? (e.g. msdUUID could be a property and in any event is shorter). Anyway, function should be consistent. Line 1500: 'master_ver': pmd[PMDK_MASTER_VER], Line 1501: 'lver': pmd[PMDK_LVER], Line 1502: 'spm_id': pmd[PMDK_SPM_ID], Line 1503: 'pool_status': 'uninitialized', -- To view, visit http://gerrit.ovirt.org/14672 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41a79662a4bd01fc310aa5554c38a16f3f8ba546 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: Yeela Kaplan 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]: Fix getStorageDomainInfo() logic.
Ayal Baron has posted comments on this change. Change subject: Fix getStorageDomainInfo() logic. .. Patch Set 3: Code-Review-1 (3 comments) Commit Message Line 6: Line 7: Fix getStorageDomainInfo() logic. Line 8: Line 9: Domains with 'role' == MASTER_DOMAIN should be attached to a Line 10: pool at any time. s/at any time/all the time/ ? Line 11: Detect 'stale' master domains. Line 12: Not swallowing errors anymore. Line 13: Catching specific errors. Line 14: Line 8: Line 9: Domains with 'role' == MASTER_DOMAIN should be attached to a Line 10: pool at any time. Line 11: Detect 'stale' master domains. Line 12: Not swallowing errors anymore. s/swallowing/hiding/ Line 13: Catching specific errors. Line 14: Line 15: Required for making repoStats pool independent. Line 16: File vdsm/storage/hsm.py Line 2752:exc_info=True) Line 2753: else: Line 2754: # make sure it's THE master of this pool Line 2755: if pool.masterDomain.sdUUID != sdUUID: Line 2756: self.log.error("Domain %s is marked as master but actual " this is not an error, *at most* it's a warning. Line 2757:"master is %s", Line 2758:sdUUID, pool.masterDomain.sdUUID) Line 2759: else: Line 2760: poolInfo = pool.getInfo() -- To view, visit http://gerrit.ovirt.org/14671 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Yeela Kaplan 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]: tests: skip mkimageTests when mkisofs and mkfs.msdos don't e...
Ayal Baron has posted comments on this change. Change subject: tests: skip mkimageTests when mkisofs and mkfs.msdos don't exist .. Patch Set 1: (1 comment) File tests/mkimageTests.py Line 135: def _check_path(self, cmdpath): Line 136: """ Line 137: Check whether execute file exists Line 138: """ Line 139: try: try...except here is redundant. Line 140: if cmdpath.cmd == None: Line 141: raise SkipTest("cannot execute %s" % cmdpath.name) Line 142: except: Line 143: raise SkipTest("cannot execute %s" % cmdpath.name) -- To view, visit http://gerrit.ovirt.org/18984 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I192707f466203786cd0864c8ca7c9e5070867523 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Jarod.w Gerrit-Reviewer: Ayal Baron 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]: Removing validate sd from spm verbs which calls ckvg and ref...
Ayal Baron has posted comments on this change. Change subject: Removing validate sd from spm verbs which calls ckvg and refreshes the domain metadata on the spm even though the spm already controls the changes (i.e. a lot of redundant time is wasted) .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18275 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf597a9e53dad04ad7a01fc1d5ceb8a180a542f2 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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]: Normalize lvm._buildFilter().
Ayal Baron has posted comments on this change. Change subject: Normalize lvm._buildFilter(). .. Patch Set 1: (1 comment) File vdsm/storage/lvm.py Line 133: dmPaths = [] Line 134: for dev in strippeds: Line 135: dmPath = os.path.join(PV_PREFIX, dev) Line 136: dmPaths.append(dmPath.replace(r'\x', r'\\x')) Line 137: filt = '|'.join(dmPaths) I think this whole part could be replaced with: filt = '|'.join(sorted(set(dmPaths))) Line 138: if len(filt) > 0: Line 139: filt = "'a|" + filt + "|', " Line 140: Line 141: filt = "filter = [ " + filt + "'r|.*|' ]" -- To view, visit http://gerrit.ovirt.org/17966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c0a5c61ba6754db996b2c88b32bedee852df22f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Yeela Kaplan 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]: Remove redundant check that causes lvm cache to refresh ever...
Ayal Baron has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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]: Removing validate sd from spm verbs which calls ckvg and ref...
Ayal Baron has posted comments on this change. Change subject: Removing validate sd from spm verbs which calls ckvg and refreshes the domain metadata on the spm even though the spm already controls the changes (i.e. a lot of redundant time is wasted) .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18275 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf597a9e53dad04ad7a01fc1d5ceb8a180a542f2 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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]: [WIP]Avoid prepare template by using unsafe qemu-img rebase
Ayal Baron has posted comments on this change. Change subject: [WIP]Avoid prepare template by using unsafe qemu-img rebase .. Patch Set 4: (4 comments) File vdsm/storage/volume.py Line 383: raise se.IncorrectType(type2name(preallocate)) Line 384: Line 385: @classmethod Line 386: def createVolume(cls, imgUUID, volUUID, size, volFmt, preallocate, Line 387: volParent, srcImgUUID, srcVolUUID, imgPath, volPath): In patch set 2 Saggi commented: "You removed prepare\teardown calls from the original method. Why don't we need those anymore?" I didn't see any reply to this comment, so replying now. Removal of prepare and teardown is the whole purpose of this patch (should be stated in the commit message). It is not needed since 'qemu-img rebase -u' does not access the parent file/device as opposed to 'qemu-img create -b' which does (redundantly). So this means that we no longer need to activate the parent LV before creating the new LV which speeds things up nicely and totally avoids any prepare/teardown races between concurrent creates and the like. Line 388: if not volParent: Line 389: cls.log.info("Request to create %s volume %s with size = %s " Line 390: "sectors", type2name(volFmt), volPath, size) Line 391: Line 384: Line 385: @classmethod Line 386: def createVolume(cls, imgUUID, volUUID, size, volFmt, preallocate, Line 387: volParent, srcImgUUID, srcVolUUID, imgPath, volPath): Line 388: if not volParent: why not reverse this to: if volParent: ... elif volFmt == COW_FORMAT: ... Line 389: cls.log.info("Request to create %s volume %s with size = %s " Line 390: "sectors", type2name(volFmt), volPath, size) Line 391: Line 392: if volFmt == COW_FORMAT: Line 385: @classmethod Line 386: def createVolume(cls, imgUUID, volUUID, size, volFmt, preallocate, Line 387: volParent, srcImgUUID, srcVolUUID, imgPath, volPath): Line 388: if not volParent: Line 389: cls.log.info("Request to create %s volume %s with size = %s " I'd move the log under the next if since we're not actually doing anything if it's raw Line 390: "sectors", type2name(volFmt), volPath, size) Line 391: Line 392: if volFmt == COW_FORMAT: Line 393: _createVolume(None, None, volPath, size, volFmt, preallocate) Line 388: if not volParent: Line 389: cls.log.info("Request to create %s volume %s with size = %s " Line 390: "sectors", type2name(volFmt), volPath, size) Line 391: Line 392: if volFmt == COW_FORMAT: Replying to Saggi's comment: "I know it's a WIP but I'm just pointing out that the method is incomplete sicne if the format is raw the method doesn't create anything" Actually this is complete. This method doesn't need to create anything in case of raw since the container (file/lv) has already been created and there is nothing more to do. Line 393: _createVolume(None, None, volPath, size, volFmt, preallocate) Line 394: else: Line 395: # Create hardlink to template and its meta file Line 396: cls.log.info("Request to create snapshot %s/%s of volume %s/%s", -- To view, visit http://gerrit.ovirt.org/18793 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I959e49315b2539cdb9111edd1786590a22023907 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Saggi Mizrahi 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]: Add iproute2 configurator
Ayal Baron has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: (4 comments) File vdsm/netconf/iproute2.py Line 39: def __init__(self): Line 40: super(Iproute2, self).__init__(ConfigApplier()) Line 41: Line 42: def begin(self): Line 43: # To be implemented in a following patch. why not introduce the API where it's used? (i.e. in the subsequent patch) Line 44: pass Line 45: Line 46: def commit(self): Line 47: # To be implemented in a following patch. Line 81: if bond.areOptionsApplied(): Line 82: nicsToAdd = nicsToSet - currentNics Line 83: nicsToRemove = currentNics - nicsToSet Line 84: else: Line 85: nicsToAdd = nicsToSet nit: you could move these 2 lines above the 'if' and then if optionsApplied: nicsToAdd -= currentNics... Line 86: nicsToRemove = currentNics Line 87: Line 88: for nic in nicsToRemove: Line 89: slave = Nic(nic, self, _netinfo=_netinfo) Line 121: if toBeRemoved: Line 122: if iface.master is None: Line 123: self.configApplier.removeIpConfig(iface) Line 124: Line 125: if destroy: now the API has parameters (destroy, destroyAction) which are only honoured if this nic has no users, otherwise these options are ignored. imo this will lead to bugs Line 126: destroyAction(iface) Line 127: else: Line 128: self.configApplier.setIfaceMtu(iface.name, Line 129:netinfo.DEFAULT_MTU) Line 217: Line 218: def removeVlan(self, vlan): Line 219: ipwrapper.linkDel(vlan.name) Line 220: Line 221: def addBond(self, bond): all of these methods are static, why are they aggregated under a class? (even the name of the class suggests that the aggregation is arbitrary) Line 222: if bond.name not in netinfo.bondings(): Line 223: logging.debug('Add new bonding %s', bond) Line 224: with open(netinfo.BONDING_MASTERS, 'w') as f: Line 225: f.write('+%s' % bond.name) -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Saggi Mizrahi 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]: Fix getStorageDomainInfo() logic.
Ayal Baron has posted comments on this change. Change subject: Fix getStorageDomainInfo() logic. .. Patch Set 3: (3 comments) File vdsm/storage/hsm.py Line 2740: if info['role'] == sd.MASTER_DOMAIN: Line 2741: try: Line 2742: # Verify that the host is connected to the same pool which Line 2743: # the SD is attached to. Line 2744: pool = self.getPool(info['pool'][0]) write the code and you'll see it makes it a lot uglier and does not make it more comprehensible (i.e. comment would still be needed) Line 2745: except IndexError: Line 2746: self.log.error("Domain %s is marked as master but is not " Line 2747:"attached to any pool", sdUUID, exc_info=True) Line 2748: except se.StoragePoolUnknown: Line 2743: # the SD is attached to. Line 2744: pool = self.getPool(info['pool'][0]) Line 2745: except IndexError: Line 2746: self.log.error("Domain %s is marked as master but is not " Line 2747:"attached to any pool", sdUUID, exc_info=True) exc_info is not adding any relevant data here and is just trashing the log Line 2748: except se.StoragePoolUnknown: Line 2749: self.log.error("Domain %s, marked as master, is attached to " Line 2750:"pool %s but this host is connected to pool %s", Line 2751:sdUUID, info['pool'], self.pools[0].spUUID, Line 2752:exc_info=True) Line 2753: else: Line 2754: # make sure it's THE master of this pool Line 2755: if pool.masterDomain.sdUUID != sdUUID: Line 2756: self.log.error("Domain %s is marked as master but actual " fake data? seriously? Line 2757:"master is %s", Line 2758:sdUUID, pool.masterDomain.sdUUID) Line 2759: else: Line 2760: poolInfo = pool.getInfo() -- To view, visit http://gerrit.ovirt.org/14671 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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: fix sloppy rebase of one shot prepare
Ayal Baron has submitted this change and it was merged. Change subject: hsm: fix sloppy rebase of one shot prepare .. hsm: fix sloppy rebase of one shot prepare A sloppy rebase of c072945 "One shot prepare" removed the changes introduced by cef2d5b "vm: extend shared property to support locking". Change-Id: I8f0de78bb9865c93dc40f51c1fcc9b6b29cef516 Signed-off-by: Federico Simoncelli Reviewed-on: http://gerrit.ovirt.org/19049 Reviewed-by: Ayal Baron --- M vdsm/storage/hsm.py 1 file changed, 6 insertions(+), 7 deletions(-) Approvals: Ayal Baron: Looks good to me, approved Federico Simoncelli: Verified -- To view, visit http://gerrit.ovirt.org/19049 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I8f0de78bb9865c93dc40f51c1fcc9b6b29cef516 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: fix sloppy rebase of one shot prepare
Ayal Baron has posted comments on this change. Change subject: hsm: fix sloppy rebase of one shot prepare .. Patch Set 1: Code-Review+2 Merging since Dan is awol and this is needed to make master work again -- To view, visit http://gerrit.ovirt.org/19049 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8f0de78bb9865c93dc40f51c1fcc9b6b29cef516 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli 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]: Make getRepoStats() a hsm method.
Ayal Baron has posted comments on this change. Change subject: Make getRepoStats() a hsm method. .. Patch Set 6: Code-Review+2 It was hard, but after running with meld to compare the changes between sp and hsm this is indeed only moving repoStats into hsm (as it should have been to begin with -- To view, visit http://gerrit.ovirt.org/14673 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0273611a23f29b5c6be0354a4c6b2d6526a9b574 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yeela Kaplan 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]: vdsClient: Fix str/unicode concatenation issue.
Ayal Baron has posted comments on this change. Change subject: vdsClient: Fix str/unicode concatenation issue. .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/19032 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I44dce25bb167da8ade579b2d67eb34a0e05f491b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amador Pahim Gerrit-Reviewer: Amador Pahim Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Douglas Schilling Landgraf 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]: vdsClient: Fix str/unicode concatenation issue.
Ayal Baron has posted comments on this change. Change subject: vdsClient: Fix str/unicode concatenation issue. .. Patch Set 3: (1 comment) Commit Message Line 39: is 'message' variable as type unicode when infos['vmlist'][entry] is unicode. Line 40: Line 41: Also, to avoid stdout redir encode error, the messages print is encoded as UTF-8. Line 42: Line 43: Change-Id: I44dce25bb167da8ade579b2d67eb34a0e05f491b please reference relevant Bug-url -- To view, visit http://gerrit.ovirt.org/19032 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I44dce25bb167da8ade579b2d67eb34a0e05f491b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amador Pahim Gerrit-Reviewer: Amador Pahim Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Douglas Schilling Landgraf 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: update lease parameters on masterMigrate
Ayal Baron has posted comments on this change. Change subject: sp: update lease parameters on masterMigrate .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/19176 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e5935e8a7081c391702f63d450621650769bb92 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli 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]: Fix parent volume rollback after failure to create snapshot
Ayal Baron has posted comments on this change. Change subject: Fix parent volume rollback after failure to create snapshot .. Patch Set 6: (1 comment) File vdsm/storage/volume.py Line 326: " pvolUUID=%s" % (sdUUID, pimgUUID, pvolUUID)) Line 327: if pvolUUID != BLANK_UUID and pimgUUID != BLANK_UUID: Line 328: pvol = sdCache.produce(sdUUID).produceVolume(pimgUUID, Line 329: pvolUUID) Line 330: pvol.prepare() is this prepare fixing a bug? (there was no prepare previously) Line 331: try: Line 332: pvol.recheckIfLeaf() Line 333: except Exception: Line 334: cls.log.error("Unexpected error", exc_info=True) -- To view, visit http://gerrit.ovirt.org/18205 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11948aacad21008ffbb4cb23013bf363e60bdb9b Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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]: Fix parent volume rollback after failure to create snapshot
Ayal Baron has posted comments on this change. Change subject: Fix parent volume rollback after failure to create snapshot .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18205 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11948aacad21008ffbb4cb23013bf363e60bdb9b Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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]: lib: add rebase to qemuImg module
Ayal Baron has posted comments on this change. Change subject: lib: add rebase to qemuImg module .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/19229 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I78e5584e59deaa5c302b1da67f5e3b840f6aa222 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli 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]: volume: Avoid prepare template by using unsafe qemu-img rebase
Ayal Baron has posted comments on this change. Change subject: volume: Avoid prepare template by using unsafe qemu-img rebase .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18793 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I959e49315b2539cdb9111edd1786590a22023907 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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]: Refactor StoragePool.getPoolParams()
Ayal Baron has posted comments on this change. Change subject: Refactor StoragePool.getPoolParams() .. Patch Set 2: Code-Review-1 (5 comments) Commit Message Line 3: AuthorDate: 2013-09-13 12:40:03 +0300 Line 4: Commit: Eduardo Warszawski Line 5: CommitDate: 2013-09-15 15:05:25 +0300 Line 6: Line 7: Refactor StoragePool.getPoolParams() why? Line 8: Line 9: Change-Id: Ibbeaaaf67a39f3ca8b27252fc631a91f266d1adc File vdsm/storage/hsm.py Line 379: # exist under pool link as well (for hsm tasks) Line 380: self.taskMng.loadDumpedTasks(self.tasksDir) Line 381: self.taskMng.recoverDumpedTasks() Line 382: Line 383: poolDirs = os.listdir(self._poolsTmpDir) where did self._poolsTmpDir come from? (bug?) Line 384: for spUUID in poolDirs: Line 385: poolPath = os.path.join(self.storage_repository, spUUID) Line 386: poolParamFile = os.path.join(self._poolsTmpDir, spUUID) Line 387: if os.path.exists(poolPath): Line 383: poolDirs = os.listdir(self._poolsTmpDir) Line 384: for spUUID in poolDirs: Line 385: poolPath = os.path.join(self.storage_repository, spUUID) Line 386: poolParamFile = os.path.join(self._poolsTmpDir, spUUID) Line 387: if os.path.exists(poolPath): return early, avoid indentation. I hate this pattern. if not os.path.exists(poolPath): continue Line 388: try: Line 389: hostID, scsiKey, msdUUID, masterVersion = \ Line 390: sp.getPoolParams(poolParamFile) Line 391: except OSError: Line 400: self.log.error("Connection to SP %s failed", Line 401:spUUID, exc_info=True) Line 402: else: Line 403: # TODO Once we support simultaneous connection to Line 404: # multiple pools, remove following line (break) why leave this comment? this will never happen... no reason to separate break from _connectStoragePool Line 405: break Line 406: Line 407: storageRefreshThread = threading.Thread(target=storageRefresh, Line 408: name="storageRefresh") Line 1043: pool = self.getPool(spUUID) Line 1044: except se.StoragePoolUnknown: Line 1045: pass # pool not connected yet Line 1046: else: Line 1047: misc.validateN(hostID, 'hostID') this will probably fail as you now pass hostID as string instead of int (and if not here, possibly elsewhere since you've changed the API) Line 1048: # Idem. See above. Line 1049: pool.getMasterDomain(msdUUID=msdUUID, Line 1050: masterVersion=masterVersion) Line 1051: return -- To view, visit http://gerrit.ovirt.org/19232 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibbeaaaf67a39f3ca8b27252fc631a91f266d1adc Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Gadi Ickowicz Gerrit-Reviewer: Meital Bourvine Gerrit-Reviewer: Yeela Kaplan 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]: getVolumeInfo() new implementation.
Ayal Baron has posted comments on this change. Change subject: getVolumeInfo() new implementation. .. Patch Set 3: Code-Review-1 (2 comments) Commit Message Line 5: CommitDate: 2013-09-16 04:46:09 +0300 Line 6: Line 7: getVolumeInfo() new implementation. Line 8: Line 9: BC compatibility preserved but in broken behaviours. I don't understand the meaning of this sentence Line 10: Line 11: Required to reduce the volume size functions proliferation. Line 12: Line 13: Change-Id: Iedcfd84cd0848fbe3aca9f9af45c44c17722055e File vdsm/storage/volume.py Line 895: else: Line 896: info = self.metadata2info(meta) Line 897: info["capacity"] = str(int(info["size"]) * BLOCK_SIZE) # Virtual Line 898: del info["size"] # Remove the virtual (by volMD) size [blocks] Line 899: if info['mtime'] == "" and self.__class__.__name__ == "FileVolume": move this into getMetadata (i.e. include mtime in the returned dictionary) Line 900: info['mtime'] = self.oop.os.stat(self.volumePath).st_mtime Line 901: # If image was set to illegal, mark the status same Line 902: # (because of VDC constraints) Line 903: if info['legality'] == ILLEGAL_VOL: -- To view, visit http://gerrit.ovirt.org/18233 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iedcfd84cd0848fbe3aca9f9af45c44c17722055e Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Yeela Kaplan 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]: Making getVSize and getVTrueSize SD methods.
Ayal Baron has posted comments on this change. Change subject: Making getVSize and getVTrueSize SD methods. .. Patch Set 4: Code-Review-1 (8 comments) Commit Message Line 5: CommitDate: 2013-09-16 04:46:07 +0300 Line 6: Line 7: Making getVSize and getVTrueSize SD methods. Line 8: Line 9: Units sanitized. The two methods returns now size in bytes, s/returns/return/ Line 10: like st_size. Line 11: getVTrueSize() renamed to getAllocSize(). Line 12: Line 13: TODO: Line 10: like st_size. Line 11: getVTrueSize() renamed to getAllocSize(). Line 12: Line 13: TODO: Line 14: This two method should be unified in a future patch since the s/This/These/ s/method/methods/ Line 15: two values are returned in the same call for file volumes and Line 16: the two values are identical for block volumes. Line 17: Line 18: Required to reduce the volume size functions proliferation. Line 12: Line 13: TODO: Line 14: This two method should be unified in a future patch since the Line 15: two values are returned in the same call for file volumes and Line 16: the two values are identical for block volumes. so unify them now while you're refactoring. I see no reason to wait. Line 17: Line 18: Required to reduce the volume size functions proliferation. Line 19: Line 20: Change-Id: I3d9772cedd74431e71d6068399546e1b4aae69e9 File vdsm/storage/blockSD.py Line 132: return LVM_ENC_ESCAPE.sub(lambda c: unichr(int(c.groups()[0])), s) Line 133: Line 134: Line 135: def _get_st_size(devPath): Line 136: """ Size in bytes of a dm device workaround. ??? Line 137: Line 138: stat.st_size (in /sys/block/dm-*/stat) is identically 0. Line 139: """ Line 140: with open(devPath, "rb") as f: Line 134: Line 135: def _get_st_size(devPath): Line 136: """ Size in bytes of a dm device workaround. Line 137: Line 138: stat.st_size (in /sys/block/dm-*/stat) is identically 0. ??? Line 139: """ Line 140: with open(devPath, "rb") as f: Line 141: f.seek(0, 2) Line 142: return f.tell() Line 620: return False Line 621: return True Line 622: Line 623: def getVSize(self, imgUUID, volUUID): Line 624: """ Return the block volume size in bytes. Like st_size.""" s/Like st_size// Line 625: try: Line 626: size = _get_st_size(lvm.lvPath(self.sdUUID, volUUID)) Line 627: except IOError as e: Line 628: if e.errno == os.errno.ENOENT: Line 634: raise Line 635: Line 636: return size Line 637: Line 638: getVAllocSize = getVSize redundant, unify the methods/ Line 639: Line 640: @classmethod Line 641: def validateCreateVolumeParams(cls, volFormat, preallocate, srcVolUUID): Line 642: """ File vdsm/storage/volume.py Line 477: # match the apparent size (eg: physical extent granularity in LVM) Line 478: # we need to update the size value so that the metadata reflects Line 479: # the correct state. Line 480: if volFormat == RAW_FORMAT: Line 481: # TODO: fix size in [sectors]! "I tawt I taw a puddy tat!" sub/incorrect quote//g And please spare us of redundant comments (both in code and in reviews) Line 482: apparentSize = int(dom.getVSize(imgUUID, volUUID) / BLOCK_SIZE) Line 483: if apparentSize < size: Line 484: cls.log.error("The volume %s apparent size %s is smaller " Line 485: "than the requested size %s", -- To view, visit http://gerrit.ovirt.org/18202 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3d9772cedd74431e71d6068399546e1b4aae69e9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Yeela Kaplan 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]: Don't re-produce the same SD again.
Ayal Baron has posted comments on this change. Change subject: Don't re-produce the same SD again. .. Patch Set 4: (1 comment) With a proper commit message I'd +2 this in a jiffy Commit Message Line 5: CommitDate: 2013-09-16 04:46:06 +0300 Line 6: Line 7: Don't re-produce the same SD again. Line 8: Line 9: Anyway a similar SD is expected. meaningless commit message Line 10: Line 11: Change-Id: I7f5c267c8afa961bd5cde5e2b67d598a1fb5e486 -- To view, visit http://gerrit.ovirt.org/18201 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7f5c267c8afa961bd5cde5e2b67d598a1fb5e486 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Yeela Kaplan 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: when creating fake template get template params before ...
Ayal Baron has posted comments on this change. Change subject: hsm: when creating fake template get template params before deleting .. Patch Set 1: Code-Review-1 (1 comment) File vdsm/storage/hsm.py Line 1536: if misc.parseBool(postZero): Line 1537: # postZero implies block domain. Backup domains are always NFS Line 1538: # hence no need to create fake template if postZero is true. Line 1539: self._spmSchedule(spUUID, "zeroImage_%s" % imgUUID, dom.zeroImage, Line 1540: sdUUID, imgUUID, volsByImg) what's creating the fake template in postZero case? Line 1541: else: Line 1542: if fakeTUUID: Line 1543: tParams = dom.produceVolume(imgUUID, fakeTUUID).\ Line 1544: getVolumeParams() -- To view, visit http://gerrit.ovirt.org/19286 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I535f29f1f956afaf28f6f496a240aa75abf1ed70 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Yeela Kaplan 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: when creating fake template get template params before ...
Ayal Baron has posted comments on this change. Change subject: hsm: when creating fake template get template params before deleting .. Patch Set 1: Code-Review+1 While reviewing now it looks like although this is a bit better than current situation, we are not handling the postZero case at all (unless I'm missing something). -- To view, visit http://gerrit.ovirt.org/19286 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I535f29f1f956afaf28f6f496a240aa75abf1ed70 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Yeela Kaplan 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: when creating fake template get template params before ...
Ayal Baron has posted comments on this change. Change subject: hsm: when creating fake template get template params before deleting .. Patch Set 1: (1 comment) Please merge this with the previous patch as they are not standalone. File vdsm/storage/hsm.py Line 1536: if misc.parseBool(postZero): Line 1537: # postZero implies block domain. Backup domains are always NFS Line 1538: # hence no need to create fake template if postZero is true. Line 1539: self._spmSchedule(spUUID, "zeroImage_%s" % imgUUID, dom.zeroImage, Line 1540: sdUUID, imgUUID, volsByImg) Ok, reading the comment above explains this, nm. Line 1541: else: Line 1542: if fakeTUUID: Line 1543: tParams = dom.produceVolume(imgUUID, fakeTUUID).\ Line 1544: getVolumeParams() -- To view, visit http://gerrit.ovirt.org/19286 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I535f29f1f956afaf28f6f496a240aa75abf1ed70 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Yeela Kaplan 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]: Use the full prefix when removing file images.
Ayal Baron has posted comments on this change. Change subject: Use the full prefix when removing file images. .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/19107 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie42dfb17eff1cbbf054de0cab493321567ea7e25 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Gadi Ickowicz Gerrit-Reviewer: Meital Bourvine Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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]: Avoid redundant volume produces.
Ayal Baron has posted comments on this change. Change subject: Avoid redundant volume produces. .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/17991 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3ad53a7e8a66d7f9bdd62048f2bf1f722a490c5c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yeela Kaplan 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]: Rename *Volume.extend method to *Volume.enlarge
Ayal Baron has posted comments on this change. Change subject: Rename *Volume.extend method to *Volume.enlarge .. Patch Set 1: Code-Review-2 I see no point for this change (which is not explained in the commit message either). The new name is not even as good as the current one, let alone better. -- To view, visit http://gerrit.ovirt.org/17802 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7b88067fa3fe2c19faab31d0c882b4494f0bc12 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Yeela Kaplan 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]: Avoid hsm image deletions.
Ayal Baron has posted comments on this change. Change subject: Avoid hsm image deletions. .. Patch Set 4: Code-Review-2 -2 for visibility since you keep on rebasing without addressing the comments -- To view, visit http://gerrit.ovirt.org/17193 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ec2ea8793a4ad63453559bc5f663b65f9b9336 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Yeela Kaplan 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]: Fix parent volume rollback after failure to create snapshot
Ayal Baron has posted comments on this change. Change subject: Fix parent volume rollback after failure to create snapshot .. Patch Set 6: (1 comment) File vdsm/storage/volume.py Line 258:(self.volumePath, dst_path)) Line 259: self.prepare(rw=False) Line 260: try: Line 261: size = int(self.getMetaParam(SIZE)) Line 262: parent = self.getVolumePath() Actually current volume *is* the parent of the volume we're creating so this is the path of the parent Line 263: parent_format = fmt2str(self.getFormat()) Line 264: # We should use parent's relative path instead of full path Line 265: parent = os.path.join(os.path.basename(os.path.dirname(parent)), Line 266: os.path.basename(parent)) -- To view, visit http://gerrit.ovirt.org/18205 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11948aacad21008ffbb4cb23013bf363e60bdb9b Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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: deleteImage fails because of wrong dictionary use
Ayal Baron has posted comments on this change. Change subject: hsm: deleteImage fails because of wrong dictionary use .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/17383 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I81f9a5aa63c0914e3b934046454df64ccd39c269 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yeela Kaplan 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]: Introduce lvm short filters.
Ayal Baron has posted comments on this change. Change subject: Introduce lvm short filters. .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/17968 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I26f3a7c712b6d7009f5f4e945374b62f18483889 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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]: Refactor StoragePool.getPoolParams()
Ayal Baron has posted comments on this change. Change subject: Refactor StoragePool.getPoolParams() .. Patch Set 3: Code-Review-1 (1 comment) Commit Message Line 3: AuthorDate: 2013-09-13 12:40:03 +0300 Line 4: Commit: Eduardo Warszawski Line 5: CommitDate: 2013-09-16 11:04:44 +0300 Line 6: Line 7: Refactor StoragePool.getPoolParams() the 'why' should be in the commit message Line 8: Line 9: Change-Id: Ibbeaaaf67a39f3ca8b27252fc631a91f266d1adc -- To view, visit http://gerrit.ovirt.org/19232 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibbeaaaf67a39f3ca8b27252fc631a91f266d1adc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Gadi Ickowicz Gerrit-Reviewer: Meital Bourvine Gerrit-Reviewer: Yeela Kaplan 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]: Fix getStorageDomainInfo() logic.
Ayal Baron has posted comments on this change. Change subject: Fix getStorageDomainInfo() logic. .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/14671 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Oved Ourfali Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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]: Read pool metadata once in StoragePool.getInfo()
Ayal Baron has posted comments on this change. Change subject: Read pool metadata once in StoragePool.getInfo() .. Patch Set 5: Code-Review+2 The subsequent patch addresses my comment so +2 -- To view, visit http://gerrit.ovirt.org/14672 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41a79662a4bd01fc310aa5554c38a16f3f8ba546 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: Yeela Kaplan 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]: Normalize lvm._buildFilter().
Ayal Baron has posted comments on this change. Change subject: Normalize lvm._buildFilter(). .. Patch Set 2: (1 comment) File vdsm/storage/lvm.py Line 131: strippeds = set(d.strip() for d in devices) Line 132: strippeds.discard('') # Who has put a blank here? Line 133: strippeds = sorted(strippeds) Line 134: dmPaths = [dev.replace(r'\x', r'\\x') for dev in strippeds] Line 135: filt = '|'.join(dmPaths) filt = '|'.join(set(sorted(dmPaths)) Line 136: if len(filt) > 0: Line 137: filt = "'a|" + filt + "|', " Line 138: Line 139: filt = "filter = [ " + filt + "'r|.*|' ]" -- To view, visit http://gerrit.ovirt.org/17966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c0a5c61ba6754db996b2c88b32bedee852df22f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Yeela Kaplan 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]: supervdsmServer was not passing kwargs to fuser
Ayal Baron has posted comments on this change. Change subject: supervdsmServer was not passing kwargs to fuser .. Patch Set 1: +1 on what Dan said. unexpected extra arguments should throw an exception so that such bugs would be caught during coding time and not as an after thought. -- To view, visit http://gerrit.ovirt.org/19250 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I55eaedc884d30e8ab0476558351e91bdd58d9ef7 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Better Saggi Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: Fix parent volume rollback after failure to create snapshot
Ayal Baron has posted comments on this change. Change subject: Fix parent volume rollback after failure to create snapshot .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18205 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11948aacad21008ffbb4cb23013bf363e60bdb9b Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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]: blockVolume: round up volume size for createVolume
Ayal Baron has posted comments on this change. Change subject: blockVolume: round up volume size for createVolume .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/19390 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5ae3d918ea4e7ca91383c3a3184a338cb40c4c27 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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]: Read pool metadata once in StoragePool.getInfo()
Ayal Baron has posted comments on this change. Change subject: Read pool metadata once in StoragePool.getInfo() .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/14672 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41a79662a4bd01fc310aa5554c38a16f3f8ba546 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Vered Volansky Gerrit-Reviewer: Yeela Kaplan 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]: Make getRepoStats() a hsm method.
Ayal Baron has posted comments on this change. Change subject: Make getRepoStats() a hsm method. .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/14673 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0273611a23f29b5c6be0354a4c6b2d6526a9b574 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yeela Kaplan 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]: iscsiadm: Filter out IPv6 addresses when discovering targets
Ayal Baron has posted comments on this change. Change subject: iscsiadm: Filter out IPv6 addresses when discovering targets .. Patch Set 1: Code-Review+2 Saggi, filtering at hsm level is useless and incorrect as it would imply that the underlying layer supports ipv6 when in fact it does not (e.g. login would fail). There is no BC issue since we can just return the 'full' dataset in a new dictionary but then we'd risk having an ipv6 target returned and user actually expecting it to work. What is really required is to add support for ipv6 targets. In the meantime (until such support is properly added) there is no reason to fail for non ipv6 targets when ipv6 targets are present and this patch is minimal in allowing that, so I disagree with you. -- To view, visit http://gerrit.ovirt.org/19431 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2f6d60e780e9ac50041cdcaa0c45394e392b52f4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Better Saggi Gerrit-Reviewer: Saggi Mizrahi 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]: vm: add the transient disk support
Ayal Baron has posted comments on this change. Change subject: vm: add the transient disk support .. Patch Set 7: (10 comments) File lib/vdsm/tool/transient.py Line 29: SELINUX_VIRT_IMAGE_LABEL = "system_u:object_r:virt_image_t:s0" Line 30: Line 31: _fuser = CommandPath( Line 32: "fuser", Line 33: "/sbin/fuser", # Fedora, EL6 fuser is already defined elsewhere, why redefine here? Line 34: ) Line 35: Line 36: Line 37: @expose("setup-transient-repository") File vdsm/vm.py Line 2492: with self._volPrepareLock: Line 2493: for drive in drives: Line 2494: try: Line 2495: self._removeTransientDisk(drive) Line 2496: except: except OSError ? Line 2497: self.log.error("Drive transient volume deletion failed " Line 2498:"for drive %s", drive, exc_info=True) Line 2499: try: Line 2500: self.cif.teardownVolumePath(drive) Line 2493: for drive in drives: Line 2494: try: Line 2495: self._removeTransientDisk(drive) Line 2496: except: Line 2497: self.log.error("Drive transient volume deletion failed " this should be a warning, not error Line 2498:"for drive %s", drive, exc_info=True) Line 2499: try: Line 2500: self.cif.teardownVolumePath(drive) Line 2501: except: Line 3299: qemuImg.FORMAT.RAW Line 3300: ) Line 3301: Line 3302: transientHandle, transientPath = tempfile.mkstemp( Line 3303: dir=constants.P_VDSM_TRANSIENT, this should probably be configurable in some way to allow the user to define an area for temporary data (not necessarily in this patch). Line 3304: prefix="%s-%s." % (drive['domainID'], drive['volumeID'])) Line 3305: Line 3306: try: Line 3307: qemuImg.create(transientPath, format=qemuImg.FORMAT.QCOW2, Line 3303: dir=constants.P_VDSM_TRANSIENT, Line 3304: prefix="%s-%s." % (drive['domainID'], drive['volumeID'])) Line 3305: Line 3306: try: Line 3307: qemuImg.create(transientPath, format=qemuImg.FORMAT.QCOW2, What would it take to support 'createing' a transient disk on the fly without a backing file? (doesn't have to be in this patch) Line 3308:backing=drive['path'], backingFormat=driveFormat) Line 3309: os.fchmod(transientHandle, 0660) Line 3310: except: Line 3311: os.unlink(transientPath) # Closing after deletion is correct Line 3384: # We lookup the drive by UUIDs since the image path could Line 3385: # be transient. Line 3386: drive = self._findDriveByUUIDs(diskParams) Line 3387: else: Line 3388: drive = self._findDriveByPath(diskParams['path']) can't you lookup by guid to allow for transient lun hotunplug? Line 3389: except LookupError: Line 3390: self.log.error("Hotunplug disk failed - Disk not found: %s", Line 3391:diskParams) Line 3392: return {'status': { Line 3694: Line 3695: if vmDrive.hasVolumeLeases: Line 3696: return errCode['noimpl'] Line 3697: Line 3698: if vmDrive.transientDisk: how can vmDrive be transient if vm has no transient disks? (you checked on line 3670) Line 3699: return errCode['transientErr'] Line 3700: Line 3701: vmDevName = vmDrive.name Line 3702: Line 3866: mergeStatus['status'] = MERGESTATUS.NOT_STARTED Line 3867: Line 3868: if not mergeDrive or not hasattr(mergeDrive, 'volumeChain'): Line 3869: mergeStatus['status'] = MERGESTATUS.DRIVE_NOT_FOUND Line 3870: elif mergeDrive.hasVolumeLeases or mergeDrive.transientDisk: not related to patch: hasVolumeLeases is the correct test here after recent changes? Line 3871: mergeStatus['status'] = MERGESTATUS.DRIVE_NOT_SUPPORTED Line 3872: else: Line 3873: for volume in mergeDrive.volumeChain: Line 3874: # qemu-kvm looks up for the backing file path looking at Line 3967: if srcDrive.hasVolumeLeases: Line 3968: return errCode['noimpl'] Line 3969: Line 3970: if srcDrive.transientDisk: Line 3971: return errCode['transientErr'] why prevent this? wouldn't this be a way to make data here permanent and be able to migrate disk, etc? (it would probably require a new flow in engine, but that is besides the point) Line 3972: Line 3973: try: Line 3974:
Change in vdsm[master]: fileVolume: improve size checks in _extendSizeRaw
Ayal Baron has posted comments on this change. Change subject: fileVolume: improve size checks in _extendSizeRaw .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/19508 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87695d67bd912f084e99314cff4ce42fd9d4cd2c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron 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]: Fix getStorageDomainInfo() logic.
Ayal Baron has posted comments on this change. Change subject: Fix getStorageDomainInfo() logic. .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/14671 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Oved Ourfali Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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]: iscsiadm: Filter out IPv6 addresses when discovering targets
Ayal Baron has posted comments on this change. Change subject: iscsiadm: Filter out IPv6 addresses when discovering targets .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/19431 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2f6d60e780e9ac50041cdcaa0c45394e392b52f4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Better Saggi Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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]: vm: add the transient disk support
Ayal Baron has posted comments on this change. Change subject: vm: add the transient disk support .. Patch Set 7: (1 comment) File vdsm/vm.py Line 3866: mergeStatus['status'] = MERGESTATUS.NOT_STARTED Line 3867: Line 3868: if not mergeDrive or not hasattr(mergeDrive, 'volumeChain'): Line 3869: mergeStatus['status'] = MERGESTATUS.DRIVE_NOT_FOUND Line 3870: elif mergeDrive.hasVolumeLeases or mergeDrive.transientDisk: just looked at hasVolumeLeases, you can ignore this comment Line 3871: mergeStatus['status'] = MERGESTATUS.DRIVE_NOT_SUPPORTED Line 3872: else: Line 3873: for volume in mergeDrive.volumeChain: Line 3874: # qemu-kvm looks up for the backing file path looking at -- To view, visit http://gerrit.ovirt.org/18326 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Deepak C Shetty Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Liron Ar Gerrit-Reviewer: Timothy Asir 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]: vm: shared attribute backward compatibility
Ayal Baron has posted comments on this change. Change subject: vm: shared attribute backward compatibility .. Patch Set 5: (2 comments) File vdsm/vm.py Line 1347: Line 1348: # Backward compatibility with the old values (true, false) Line 1349: if shared == 'true': Line 1350: self.extSharedState = DRIVE_SHARED_TYPE.SHARED Line 1351: return how about removing 'return' statements and set 'shared' to the relevant value (and elif instead of if) and then the last 'if shared in' would do the assignment? Line 1352: Line 1353: if shared == 'false': Line 1354: if config.getboolean('irs', 'use_volume_leases'): Line 1355: self.extSharedState = DRIVE_SHARED_TYPE.EXCLUSIVE Line 1350: self.extSharedState = DRIVE_SHARED_TYPE.SHARED Line 1351: return Line 1352: Line 1353: if shared == 'false': Line 1354: if config.getboolean('irs', 'use_volume_leases'): isn't 'use_volume_leases' deprecated? Line 1355: self.extSharedState = DRIVE_SHARED_TYPE.EXCLUSIVE Line 1356: else: Line 1357: self.extSharedState = DRIVE_SHARED_TYPE.NONE Line 1358: return -- To view, visit http://gerrit.ovirt.org/19509 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d377635b0687baccc69b203cb3fbe8dbf573169 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg 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]: vm: add the transient disk support
Ayal Baron has posted comments on this change. Change subject: vm: add the transient disk support .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18326 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Deepak C Shetty Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Liron Ar Gerrit-Reviewer: Timothy Asir 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]: getIsoList() returns dict with files metadata
Ayal Baron has posted comments on this change. Change subject: getIsoList() returns dict with files metadata .. Patch Set 5: Code-Review-1 (1 comment) File vdsm/storage/hsm.py Line 2261: found = isoDom.getFileList(pattern='*.' + extension, Line 2262:caseSensitive=False) Line 2263: Line 2264: # Filter out files without proper permissions Line 2265: filtered = dict((key, value) for key, value in found.items() why are you filtering out the files with wrong permissions from the dict? We now have the opportunity to return these together with this information so that user (engine) would be able to know why a file is not usable... Line 2266: if value['status'] == 0) Line 2267: Line 2268: return {'isolist': filtered.keys(), 'fileStats': filtered} Line 2269: -- To view, visit http://gerrit.ovirt.org/19544 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b28488a81cec756188ed763e4489b8a39b2b05d Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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]: fileVolume: fix volume type check in _extendSizeRaw
Ayal Baron has posted comments on this change. Change subject: fileVolume: fix volume type check in _extendSizeRaw .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/19508 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87695d67bd912f084e99314cff4ce42fd9d4cd2c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg 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]: Adding [start|stop]MonitoringDomain().
Ayal Baron has posted comments on this change. Change subject: Adding [start|stop]MonitoringDomain(). .. Patch Set 1: (1 comment) File vdsm/storage/sp.py Line 1545: # {sdUUID1: status1, sdUUID2: status2, ...} Line 1546: self.invalidateMetadata() Line 1547: poolDoms = self.getDomains() Line 1548: activeDomains = tuple(sdUUID for sdUUID in poolDoms Line 1549: if poolDoms[sdUUID] == sd.DOM_ACTIVE_STATUS) this change does not belong here. Line 1550: monitoredDomains = self.domainMonitor.monitoredDomains Line 1551: Line 1552: for sdUUID in poolDoms: Line 1553: if sdUUID not in activeDomains: -- To view, visit http://gerrit.ovirt.org/19762 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I983d49b0a42cc06428ec75b7795d23abaa6ab84c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Gadi Ickowicz Gerrit-Reviewer: Yeela Kaplan 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]: Make hsm.getVolumesList() pool independent.
Ayal Baron has posted comments on this change. Change subject: Make hsm.getVolumesList() pool independent. .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/15766 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib810ca24818fa4a77905032694a05c0d86ef75e2 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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]: getIsoList() returns dict with files metadata
Ayal Baron has posted comments on this change. Change subject: getIsoList() returns dict with files metadata .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/19544 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b28488a81cec756188ed763e4489b8a39b2b05d Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yeela Kaplan 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 premature selinux dependency
Ayal Baron has posted comments on this change. Change subject: revert premature selinux dependency .. Patch Set 1: Code-Review-1 (1 comment) Commit Message Line 12: EL6, so we'd better not require it now. Line 13: Line 14: Note that using /var/run/vdsm/storage is problematic not only due to Line 15: selinux, but also because it breaks cross-version migration. Hence, the Line 16: universal usage of this directory is soon to be changed. not entirely correct. For BC reasons we need to pass to libvirt the /rhev/datacenter path. That is not to say we will not be using /var/run underneath The idea is that on new vdsm hosts we will always use /var/run, but add a link in /rhev/datacenter if SPUUID is passed We also want to stop passing spuuid from engine on run vm in 3.3 cluster level and thus enjoy the benefits of moving to /var/run In addition, using /var/run is mandatory for HE. Line 17: Line 18: Change-Id: Iea625d7c39035055b246357030372430b4bddf68 -- To view, visit http://gerrit.ovirt.org/19807 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea625d7c39035055b246357030372430b4bddf68 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Ohad Basan Gerrit-Reviewer: Toshio くらとみ 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]: vm: Pause vm in case volume allocation is greater than lv size
Ayal Baron has posted comments on this change. Change subject: vm: Pause vm in case volume allocation is greater than lv size .. Patch Set 2: (1 comment) File vdsm/vm.py Line 511: if physical - alloc < 0: Line 512: self._vm.pause(pauseCode='EOTHER') Line 513: self._log.error('vm: %s paused due to the corrupt volume: %s', Line 514: self._vm.id, vmDrive.path) Line 515: continue I wonder if we should not in fact avoid any extend operations on this entire VM in this case since something went awfully wrong here and I don't think we should be doing anything in this state. If we go down this path that would also mean that extendDriveVolume should be called outside of the for loop only on valid drives and only if we did not hit this case (so collect list of disks to extend and raise here if encounter error or something) Line 516: Line 517: if physical - alloc >= vmDrive.watermarkLimit: Line 518: continue Line 519: -- To view, visit http://gerrit.ovirt.org/19719 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id95aa1ed7508218d8ae6a878606cd2d251ac2b05 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Yeela Kaplan 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]: Enable VM migration to old vdsm's.
Ayal Baron has posted comments on this change. Change subject: Enable VM migration to old vdsm's. .. Patch Set 2: (1 comment) File vdsm/storage/blockSD.py Line 1035: try: Line 1036: os.symlink(imgPath, dst) Line 1037: except OSError as e: Line 1038: if e.errno == errno.EEXIST: Line 1039: self.log.debug("img run dir already exists: %s", dst) is this the correct behaviour? shouldn't we overwrite it in this case? (in case the link is wrong) Line 1040: else: Line 1041: self.log.error("Failed to create img run dir: %s", dst) Line 1042: raise Line 1043: return dst -- To view, visit http://gerrit.ovirt.org/19825 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Gadi Ickowicz Gerrit-Reviewer: Yeela Kaplan 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]: Enable VM migration to old vdsm's.
Ayal Baron has posted comments on this change. Change subject: Enable VM migration to old vdsm's. .. Patch Set 2: Code-Review-1 (1 comment) File vdsm/storage/blockSD.py Line 1035: try: Line 1036: os.symlink(imgPath, dst) Line 1037: except OSError as e: Line 1038: if e.errno == errno.EEXIST: Line 1039: self.log.debug("img run dir already exists: %s", dst) What? I have a stale link there to the wrong image dir (from a previous run VM which wasn't cleaned because host crashed or whatever) and since then user ran liveStorageMigration and moved the disk to a new sd. I think this is a bug. Also, this is not runVM context, we're in blockSD Please fix Line 1040: else: Line 1041: self.log.error("Failed to create img run dir: %s", dst) Line 1042: raise Line 1043: return dst -- To view, visit http://gerrit.ovirt.org/19825 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Gadi Ickowicz Gerrit-Reviewer: Yeela Kaplan 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]: image: Calculation of chain to remove is inaccurate
Ayal Baron has posted comments on this change. Change subject: image: Calculation of chain to remove is inaccurate .. Patch Set 1: Code-Review+2 I believe engine only ever passes a single volume so possibly it would be better to just remove the support for removal of subchain and significantly simplify the code. This does not detract from the correctness of this patch though so +2. -- To view, visit http://gerrit.ovirt.org/19852 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I248c51727584a35d1f439e57c93613caa9f369a9 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo 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]: image: Remove redundant activate and deactivate of the same ...
Ayal Baron has posted comments on this change. Change subject: image: Remove redundant activate and deactivate of the same volume .. Patch Set 1: Code-Review-1 (2 comments) File vdsm/storage/image.py Line 918: tmpVol.prepare(rw=True, justme=True, setrw=True) Line 919: Line 920: # We should prepare/teardown volume for every single rebase. Line 921: # The reason is recheckIfLeaf at the end of the rebase, that change Line 922: # volume permissions to RO for internal volumes. Please read this comment Line 923: srcVol.prepare(rw=True, chainrw=True, setrw=True) Line 924: try: Line 925: # Step 2: Rebase successor on top of tmpVol Line 926: # qemu-img rebase -b tmpBackingFile -F backingFormat -f srcFormat Line 926: # qemu-img rebase -b tmpBackingFile -F backingFormat -f srcFormat Line 927: # src Line 928: backingVolPath = os.path.join('..', srcVolParams['imgUUID'], Line 929: newUUID) Line 930: srcVol.rebase(newUUID, backingVolPath, volParams['volFormat'], Since unsafe rebase doesn't require access to the parent, you can probably replace the second prepare with: srcVol.prepare(rw=True, justme=True, setrw=True). Line 931: unsafe=False, rollback=True) Line 932: Line 933: # Step 3: Remove pointer to backing file from the successor by Line 934: # 'unsafed' rebase qemu-img rebase -u -b "" -F -- To view, visit http://gerrit.ovirt.org/19854 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3d615fd8e9b199cef1340360c7f68b19919b0caa Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo 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]: Enable VM migration to old vdsm's.
Ayal Baron has posted comments on this change. Change subject: Enable VM migration to old vdsm's. .. Patch Set 3: Code-Review+1 (2 comments) File vdsm/storage/blockSD.py Line 1035: try: Line 1036: os.symlink(imgPath, dst) Line 1037: except OSError as e: Line 1038: if e.errno == errno.EEXIST: Line 1039: self.log.debug("path to img directory already exists: %s", dst) s/img/image/ Line 1040: else: Line 1041: self.log.error("Failed to create path to img dir: %s", dst) Line 1042: raise Line 1043: return dst Line 1037: except OSError as e: Line 1038: if e.errno == errno.EEXIST: Line 1039: self.log.debug("path to img directory already exists: %s", dst) Line 1040: else: Line 1041: self.log.error("Failed to create path to img dir: %s", dst) ditto Line 1042: raise Line 1043: return dst Line 1044: Line 1045: def createImageLinks(self, srcImgPath, imgUUID, volUUIDs): -- To view, visit http://gerrit.ovirt.org/19825 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia778ad743a11b4c1d212857d8f25c81eb4c0defe Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Gadi Ickowicz Gerrit-Reviewer: Yeela Kaplan 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]: Fix getStorageDomainInfo() logic.
Ayal Baron has posted comments on this change. Change subject: Fix getStorageDomainInfo() logic. .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/14671 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b0b2ad3dca19cf203d937c1a9f6a12ab0f1095f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Paikov Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Oved Ourfali Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan 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]: image: Return the new volume after merge
Ayal Baron has posted comments on this change. Change subject: image: Return the new volume after merge .. Patch Set 1: (1 comment) File vdsm/storage/image.py Line 1132: self.log.error("Failure to remove subchain %s -> %s in image %s", Line 1133:ancestor, successor, imgUUID, exc_info=True) Line 1134: Line 1135: try: Line 1136: srcVol.shrinkToOptimalSize() wouldn't it be simpler and safer just to produce the new volume here? We just finished merging volumes so the overhead is negligible and this way we wouldn't care about any implementation changes above (also, it would be 1 line without changing internal APIs etc). Line 1137: except qemuImg.QImgError: Line 1138: self.log.warning("Auto shrink after merge failed", exc_info=True) Line 1139: Line 1140: self.log.info("Merge src=%s with dst=%s was successfully finished.", -- To view, visit http://gerrit.ovirt.org/19883 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b0969c3144b51ff22a7f5eb756563cf178ffb36 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli 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: unify lease params
Ayal Baron has posted comments on this change. Change subject: sd: unify lease params .. Patch Set 3: (3 comments) File vdsm/storage/fileSD.py Line 214: # initialize domain metadata content Line 215: # FIXME : This is 99% like the metadata in block SD Line 216: # Do we really need to keep the EXPORT_PATH? Line 217: # no one uses it Line 218: meta = {} if you're already changing the name, why not make it consistent with blockSD? Line 219: meta.update({ Line 220: sd.DMDK_VERSION: version, Line 221: sd.DMDK_SDUUID: sdUUID, Line 222: sd.DMDK_TYPE: storageType, Line 223: sd.DMDK_CLASS: domClass, Line 224: sd.DMDK_DESCRIPTION: domainName, Line 225: sd.DMDK_ROLE: sd.REGULAR_DOMAIN, Line 226: sd.DMDK_POOLS: [], Line 227: sd.DMDK_LOCK_POLICY: '', why not unify all of the above as well? Line 228: REMOTE_PATH: remotePath Line 229: }) Line 230: meta.update(sd.SUB_DEFAULT_LEASE_PARAMS) Line 231: md.update(meta) File vdsm/storage/sd.py Line 120: DMDK_LEASE_TIME_SEC = 'LEASETIMESEC' Line 121: DMDK_IO_OP_TIMEOUT_SEC = 'IOOPTIMEOUTSEC' Line 122: DMDK_LEASE_RETRIES = 'LEASERETRIES' Line 123: Line 124: SUB_DEFAULT_LEASE_PARAMS = {DMDK_LEASE_RETRIES: 3, I don't understand the name of this dict Line 125: DMDK_LEASE_TIME_SEC: 60, Line 126: DMDK_LOCK_RENEWAL_INTERVAL_SEC: 5, Line 127: DMDK_IO_OP_TIMEOUT_SEC: 10} Line 128: -- To view, visit http://gerrit.ovirt.org/16318 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id65d5dd2a0130a19234d7fb699854a09f5748566 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli 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]: image: produce volume in order to shrink after merge
Ayal Baron has posted comments on this change. Change subject: image: produce volume in order to shrink after merge .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/19883 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b0969c3144b51ff22a7f5eb756563cf178ffb36 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli 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: Create AsyncProcessOperation
Ayal Baron has posted comments on this change. Change subject: utils: Create AsyncProcessOperation .. Patch Set 9: (5 comments) File lib/vdsm/utils.py Line 914: Line 915: time.sleep(sleep) Line 916: Line 917: Line 918: class AsyncProcessOperation(object): why is this a separate class and not part of AsyncProc? Line 919: def __init__(self, proc, resultParser=None): Line 920: """Wraps a running process operation. Line 921: Line 922: resultParser should be of type callback(rc, out, err) and can return Line 933: the condition has been met""" Line 934: return self._proc.wait(timeout, cond) Line 935: Line 936: def stop(self): Line 937: """Stops the running operaions, effectively sending a kill signal to s/operaions/operations/ Line 938: the process""" Line 939: self._proc.kill() Line 940: Line 941: def result(self): Line 938: the process""" Line 939: self._proc.kill() Line 940: Line 941: def result(self): Line 942: """Returns the result in the as a tuple of (result, error). 'in the' what? Line 943: if the operation is still running it will block until it returns. Line 944: Line 945: The if no resultParser has been set the default result Line 946: is (rc, out, err) """ Line 941: def result(self): Line 942: """Returns the result in the as a tuple of (result, error). Line 943: if the operation is still running it will block until it returns. Line 944: Line 945: The if no resultParser has been set the default result 'The if no'? Line 946: is (rc, out, err) """ Line 947: with self._lock: Line 948: if self._result is None: Line 949: out, err = self._proc.communicate() Line 956: self._result = (None, e) Line 957: else: Line 958: self._result = ((rc, out, err), None) Line 959: Line 960: self._done = True this is never used Line 961: Line 962: return self._result Line 963: Line 964: def __del__(self): -- To view, visit http://gerrit.ovirt.org/19254 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79d0eefc9d917a4a93916d52867fb4f1e793c60e Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: multipath: Move all calls to multipath exe to a single method
Ayal Baron has posted comments on this change. Change subject: multipath: Move all calls to multipath exe to a single method .. Patch Set 9: (1 comment) Commit Message Line 5: CommitDate: 2013-10-06 16:49:08 +0300 Line 6: Line 7: multipath: Move all calls to multipath exe to a single method Line 8: Line 9: This makes the code a bit cleaner cleaner how? why is this dependent on AsyncProcessOperation (which seems totally unrelated to me). Afaiu this should be a part of a new topic branch? i.e. this should be one of many patches Line 10: Line 11: Change-Id: I52afc07a07a925ed7572eb369deb7c203edb04cd -- To view, visit http://gerrit.ovirt.org/19255 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I52afc07a07a925ed7572eb369deb7c203edb04cd Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: iscsi: Iscsi rescan cleanup
Ayal Baron has posted comments on this change. Change subject: iscsi: Iscsi rescan cleanup .. Patch Set 11: (2 comments) File vdsm/storage/iscsi.py Line 391: Line 392: log.debug("Performing SCSI scan, this will take up to %s seconds", Line 393: maxTimeout) Line 394: Line 395: rescanOp = iscsiadm.session_rescan_async() iscsiadm rescan does not hang for long periods of time afaik, so I'm not sure what's the point of this here. i.e. instead of all this fancy code, just remove this function and keep the previous 'rescan' and possibly add a minWait sleep to it, to give the host some time to add devices Line 396: time.sleep(minTimeout) Line 397: rescanOp.wait(timeout=(maxTimeout - minTimeout)) Line 398: Line 399: File vdsm/storage/iscsiadm.py Line 307: return AsyncProcessOperation(proc, parse_result) Line 308: Line 309: Line 310: def session_rescan(): Line 311: aop = session_rescan_async() did you mean apo? that's an implementation detail (that the object returned is of AsyncProcessOperation type). Line 312: return aop.result() Line 313: Line 314: Line 315: def session_logout(sessionId): -- To view, visit http://gerrit.ovirt.org/19256 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I842eb37cea3bd5e8cd357a310dd966081b242abd Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: Rescan FC when rescanning for new multipath devices
Ayal Baron has posted comments on this change. Change subject: Rescan FC when rescanning for new multipath devices .. Patch Set 5: (2 comments) File lib/vdsm/utils.py Line 798: if append: Line 799: cmd.append("--append") Line 800: Line 801: cmd.extend(outputFiles) Line 802: proc = execCmd(cmd, sync=False) I thought you disapprove of direct usage of execCmd? Line 803: proc.stdin.write(data) Line 804: Line 805: def parser(rc, out, err): Line 806: if rc != 0: File vdsm/storage/multipath.py Line 107: Should only be called from hsm._rescanDevices() Line 108: """ Line 109: Line 110: # First ask fc and iSCSI to rescan all of their sessions Line 111: fc.rescan() I thought the premise of the previous patch is that iscsiadm -m session -R already does this? Line 112: iscsi.rescan() Line 113: Line 114: # Now let multipath daemon pick up new devices Line 115: -- To view, visit http://gerrit.ovirt.org/19539 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idec939222676a24452e8825b36db68839bfd2bbc Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Barak Azulay Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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: protect deleteImage with the spm lock
Ayal Baron has posted comments on this change. Change subject: hsm: protect deleteImage with the spm lock .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/19795 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iac5b4b0f71de6a12de34513d4fafb295b701306c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli 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]: WIP: sd.py: Remove sds from sdCache when connecting
Ayal Baron has posted comments on this change. Change subject: WIP: sd.py: Remove sds from sdCache when connecting .. Patch Set 1: Code-Review+2 Our purpose in this patch is to stabilize oVirt 3.3. Eduardo's suggestion of removing the domain path from the domain object would be anything but that, so although long term we should strive in that direction, in order to correct current flows without introducing to much risk we need to do something minimal. Wrt FC domains, although technically correct that this change would not hold for such domains, the problematic use cases that have been found (see both the bz and the closed duplicate of it) are around editing connections which by definition does not apply to FC domains. -- To view, visit http://gerrit.ovirt.org/19995 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic0e0d0e970ce55acf92f7e39ec9cf2170e948274 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Allon Mureinik Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli 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]: New getChildrenList implementation.
Ayal Baron has posted comments on this change. Change subject: New getChildrenList implementation. .. Patch Set 8: (4 comments) Commit Message Line 7: New getChildrenList implementation. Line 8: Line 9: This implementation returns children of any image on the SD. Line 10: Avoids to produce each volume in the domain. Line 11: Implemented like one shot operation. what do you mean implemented like one shot operation? Line 12: Line 13: Change-Id: I584cd5d1b03d3965457f12c3d67de95455d1de24 Line 14: Related-to: BZ#960952 File lib/vdsm/utils.py Line 166: Line 167: def grepCmd(pattern, paths): Line 168: cmd = [constants.EXT_GREP, '-E', '-H', pattern] Line 169: cmd.extend(paths) Line 170: rc, out, err = execCmd(cmd) I thought you agree with Saggi that execCmd should not be called directly? Line 171: if rc == 0: Line 172: matches = out # A list of matching lines Line 173: elif rc == 1: Line 174: matches = [] # pattern not found File vdsm/storage/fileVolume.py Line 374: Line 375: Children can be found in any image of the volume SD. Line 376: """ Line 377: domPath = self.imagePath.split('images')[0] Line 378: gPath = os.path.join(domPath, 'images', '*', '*.meta') gPath? (I hate repeating comments). If it wasn't clear, I have no idea what this name means or represents Line 379: metaPaths = oop.getProcessPool(self.sdUUID).glob.glob(gPath) Line 380: pattern = "%s.*%s" % (volume.PUUID, self.volUUID) Line 381: matches = grepCmd(pattern, metaPaths) Line 382: if matches: Line 378: gPath = os.path.join(domPath, 'images', '*', '*.meta') Line 379: metaPaths = oop.getProcessPool(self.sdUUID).glob.glob(gPath) Line 380: pattern = "%s.*%s" % (volume.PUUID, self.volUUID) Line 381: matches = grepCmd(pattern, metaPaths) Line 382: if matches: you can remove the 'if matches'. i.e.: s/if matches.*/ children = [] for line ... return tuple(children) Line 383: children = [] Line 384: for line in matches: Line 385: volMeta = os.path.basename(line.split(':')[0]) Line 386: children.append(os.path.splitext(volMeta)[0]) # volUUID -- To view, visit http://gerrit.ovirt.org/15765 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I584cd5d1b03d3965457f12c3d67de95455d1de24 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Gadi Ickowicz Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: New getChildrenList implementation.
Ayal Baron has posted comments on this change. Change subject: New getChildrenList implementation. .. Patch Set 8: (2 comments) Commit Message Line 7: New getChildrenList implementation. Line 8: Line 9: This implementation returns children of any image on the SD. Line 10: Avoids to produce each volume in the domain. Line 11: Implemented like one shot operation. 1. that is still not explaining it 2. I wasn't asking for a reply, I was expecting an explanation in the commit message Line 12: Line 13: Change-Id: I584cd5d1b03d3965457f12c3d67de95455d1de24 Line 14: Related-to: BZ#960952 File vdsm/storage/fileVolume.py Line 374: Line 375: Children can be found in any image of the volume SD. Line 376: """ Line 377: domPath = self.imagePath.split('images')[0] Line 378: gPath = os.path.join(domPath, 'images', '*', '*.meta') But it is not passed to grep, it's a glob path. Here is a Better* name: metaPattern? * 'Better' is a registered trademark by Saggi, All rights reserved. Line 379: metaPaths = oop.getProcessPool(self.sdUUID).glob.glob(gPath) Line 380: pattern = "%s.*%s" % (volume.PUUID, self.volUUID) Line 381: matches = grepCmd(pattern, metaPaths) Line 382: if matches: -- To view, visit http://gerrit.ovirt.org/15765 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I584cd5d1b03d3965457f12c3d67de95455d1de24 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Elad Ben Aharon Gerrit-Reviewer: Gadi Ickowicz Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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.py: Remove sds from sdCache when connecting
Ayal Baron has posted comments on this change. Change subject: sd.py: Remove sds from sdCache when connecting .. Patch Set 2: Code-Review+2 I just discussed this with Federico and the truth is the original patch seems better since it also solves any problems with FC domains. The flow is that user activates the domain in engine: this calls activateStorageDomain on SPM and then calls refreshStoragePool on all the HSMs. refreshStoragePool already removes the domains from the cache (since we got a hint that something changed and we need to refresh it) and activateStorageDomain should not assume the object is ok since up until now the domain was not active, i.e. we haven't been monitoring it and it may have changed etc. The flow in engine of activate is: connectStorageServer (on spm) - not called in case of FC activateSD connectStorageServer (on all hosts) - not called in case of FC refreshStoragePool The previous patch was (imo wrongly) nack'd by Edu. I'm acking this one, but I would really prefer the original one. -- To view, visit http://gerrit.ovirt.org/19995 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic0e0d0e970ce55acf92f7e39ec9cf2170e948274 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Allon Mureinik Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli 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]: Fill volume children info.
Ayal Baron has posted comments on this change. Change subject: Fill volume children info. .. Patch Set 1: Code-Review-2 This is causing a performance hit for something nobody uses, we don't need, and plan on totally removing. -- To view, visit http://gerrit.ovirt.org/20004 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf374e6abe81962619baecf96fffa0f817ce5dcb Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Yeela Kaplan 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