vdsm-patches May 2014

vdsm-patches@lists.fedorahosted.org

44 participants
645 discussions

Change in vdsm[master]: storage: check if mount specification is a symlink
by Dan Kenigsberg 04 May '14

04 May '14

Dan Kenigsberg has posted comments on this change. Change subject: storage: check if mount specification is a symlink ...................................................................... Patch Set 1: Would you please modify tests/mountTests.py so that it tests for the bug that you are fixing? -- To view, visit http://gerrit.ovirt.org/27321 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32aa019b52c28377c064966684986d20414df6db Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Enrico Tagliavini <enrico.tagliavini(a)gmail.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No

1 0

Change in vdsm[master]: profiling: Add whole application profile
by Nir Soffer 04 May '14

04 May '14

Nir Soffer has uploaded a new change for review. Change subject: profiling: Add whole application profile ...................................................................... profiling: Add whole application profile The built in Python profilers are good for single thread profile but given the excessive multi-threaded nature of vdsm, they are useless for understing where we spend our time. This patch uses yappi <http://code.google.com/p/yappi> to create whole application profile. A new profile_enable configuration is added to enable profiling. Change-Id: I523f52c981f7bb34f3168d3117f00ed5eb6962f8 Relates-to: https://bugzilla.redhat.com/1074097 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M lib/vdsm/config.py.in M vdsm/vdsm 2 files changed, 14 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/26113/1 diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in index fb597a8..e208b2a 100644 --- a/lib/vdsm/config.py.in +++ b/lib/vdsm/config.py.in @@ -29,6 +29,9 @@ ('core_dump_enable', 'true', 'Enable core dump.'), + ('profile_enable', 'false', + 'Enable whole process profiling (requires yappi profiler).'), + ('host_mem_reserve', '256', 'Reserves memory for the host to prevent VMs from using all the ' 'physical pages. The values are in Mbytes.'), diff --git a/vdsm/vdsm b/vdsm/vdsm index 652797c..f89ca1d 100755 --- a/vdsm/vdsm +++ b/vdsm/vdsm @@ -65,6 +65,11 @@ signal.signal(signal.SIGUSR1, sigusr1Handler) zombiereaper.registerSignalHandler() + if config.getboolean('vars', 'profile_enable'): + logging.debug("Starting profiling") + import yappi + yappi.start() + libvirtconnection.start_event_loop() if config.getboolean('irs', 'irs_enable'): @@ -79,6 +84,12 @@ try: while running[0]: signal.pause() + + if config.getboolean('vars', 'profile_enable'): + logging.debug("Stopping profiling") + yappi.stop() + stats = yappi.get_func_stats() + stats.save('/tmp/vdsmd.prof', 'pstat') finally: cif.prepareForShutdown() -- To view, visit http://gerrit.ovirt.org/26113 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I523f52c981f7bb34f3168d3117f00ed5eb6962f8 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

9 48

Change in vdsm[master]: config: Make it easy to create default config
by Nir Soffer 04 May '14

04 May '14

Nir Soffer has uploaded a new change for review. Change subject: config: Make it easy to create default config ...................................................................... config: Make it easy to create default config Previously a default config object was created by invoking a private function, modifying the global config, before reading the configuration from the config file. This design make it impossible to create a default config object for testing, or to reload the configuration while vdsm is running. This patch replace the private function with public function accepting a config object. Now you can create a default config object by creating a new config parser and setting the defaults: fake_config = ConfigParser.ConfigParser() config.set_defaults(fake_config) fake_config.set('irs', 'repository', '/my/fake/repository') Change-Id: I80ec628d1da5c0466af28cb73564c6e63fd5ca65 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M lib/vdsm/config.py.in 1 file changed, 2 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/04/25504/1 diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in index fb597a8..f01743c 100644 --- a/lib/vdsm/config.py.in +++ b/lib/vdsm/config.py.in @@ -318,9 +318,7 @@ ] -def __prepare_config(): - global config - +def set_defaults(config): for section, keylist in parameters: config.add_section(section) for key, value, comment in keylist: @@ -343,7 +341,7 @@ print config = ConfigParser.ConfigParser() -__prepare_config() +set_defaults(config) config.read([os.path.join('@CONFDIR@', 'vdsm.conf')]) if __name__ == '__main__': -- To view, visit http://gerrit.ovirt.org/25504 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I80ec628d1da5c0466af28cb73564c6e63fd5ca65 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

4 6

Change in vdsm[ovirt-3.4]: core: fixing attribute error on fenceSpmStorage
by laravot＠redhat.com 04 May '14

04 May '14

Hello Nir Soffer, Federico Simoncelli, Allon Mureinik, I'd like you to do a code review. Please visit http://gerrit.ovirt.org/27341 to review the following change. Change subject: core: fixing attribute error on fenceSpmStorage ...................................................................... core: fixing attribute error on fenceSpmStorage In change I75493d1db60e51cccd5231b516f963c970d24c99 the pool metadata operations were moved to the pool backend, while the call to invalidateMetadata in fenceSpmStorage was left as it was, leading to an attribute error. This patch fixes it by executing the operation on the pool backend object rather than on the pool, as the metadata operation are implemented only in StoragePoolDiskBackend - there's also a check for the backend type before executing it. Though it's not the nicest way for doing so, it's the pattern already used in hsm.py,therefore in the meanwhile it can be fixed the same way. Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1082365 Signed-off-by: Liron Aravot <laravot(a)redhat.com> Reviewed-on: http://gerrit.ovirt.org/26358 Reviewed-by: Allon Mureinik <amureini(a)redhat.com> Reviewed-by: Nir Soffer <nsoffer(a)redhat.com> Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M vdsm/storage/hsm.py M vdsm/storage/spbackends.py 2 files changed, 5 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/41/27341/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 15cfe01..7e4dc15 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -3544,9 +3544,10 @@ se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s" % (spUUID, lastOwner, lastLver))) pool = self.getPool(spUUID) - pool.invalidateMetadata() - vars.task.getExclusiveLock(STORAGE, spUUID) - pool.forceFreeSpm() + if isinstance(pool.getBackend(), StoragePoolDiskBackend): + pool.getBackend().invalidateMetadata() + vars.task.getExclusiveLock(STORAGE, spUUID) + pool.getBackend().forceFreeSpm() return dict(spm_st=self._getSpmStatusInfo(pool)) @public diff --git a/vdsm/storage/spbackends.py b/vdsm/storage/spbackends.py index ed72373..819287e 100644 --- a/vdsm/storage/spbackends.py +++ b/vdsm/storage/spbackends.py @@ -327,7 +327,7 @@ # DO NOT USE, STUPID, HERE ONLY FOR BC # TODO: SCSI Fence the 'lastOwner' self.setSpmStatus(LVER_INVALID, SPM_ID_FREE, __securityOverride=True) - self.spmRole = SPM_FREE + self.pool.spmRole = SPM_FREE @classmethod def _getPoolMD(cls, domain): -- To view, visit http://gerrit.ovirt.org/27341 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.4 Gerrit-Owner: Liron Ar <laravot(a)redhat.com> Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com> Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>

3 4

Change in vdsm[ovirt-3.4]: core: return lver/spm id from pool metadata
by laravot＠redhat.com 04 May '14

04 May '14

Hello Nir Soffer, Federico Simoncelli, I'd like you to do a code review. Please visit http://gerrit.ovirt.org/27340 to review the following change. Change subject: core: return lver/spm id from pool metadata ...................................................................... core: return lver/spm id from pool metadata Currently the lver/spm id are being returned as they appear in the cluster lock if supported. The fenceSpmStorage verb is currently used to manually confirm that there's no current spm on the responsibillity of the caller. This verb updates the lver/spm id in the pool metadata to indicate that the spm role is free. This won't help us currently as getSpmStatus will return the info from the cluster lock (if supported) which wasn't edited at all. As currently we can't edit the information return from cluster lock, we can return the same results as in case it wasn't supported by the cluster lock. Right now the solution is implemented in StoragePoolDiskBackend only as a bug fix, as StoragePoolMemoryBackend isn't being used at the moment. Change-Id: I460801329a9a1c5ee940bce22566ad3d29b351de Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1092631 Signed-off-by: Liron Aravot <laravot(a)redhat.com> Reviewed-on: http://gerrit.ovirt.org/27226 Reviewed-by: Nir Soffer <nsoffer(a)redhat.com> Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M vdsm/storage/spbackends.py 1 file changed, 8 insertions(+), 19 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/40/27340/1 diff --git a/vdsm/storage/spbackends.py b/vdsm/storage/spbackends.py index 819dc53..ed72373 100644 --- a/vdsm/storage/spbackends.py +++ b/vdsm/storage/spbackends.py @@ -26,7 +26,6 @@ import sd import storage_exception as se -from clusterlock import InquireNotSupportedError from persistentDict import DictValidator from persistentDict import unicodeDecoder from persistentDict import unicodeEncoder @@ -217,26 +216,16 @@ @unsecured def getSpmStatus(self): - try: - # If the cluster lock implements inquire (e.g. sanlock) then we - # can fetch the spmId and the lVer from it. - lVer, spmId = self.masterDomain.inquireClusterLock() - lVer, spmId = lVer or LVER_INVALID, spmId or SPM_ID_FREE - except InquireNotSupportedError: - # Legacy implementation for cluster locks that are not able to - # return the spmId and the lVer. + poolMeta = self._getPoolMD(self.masterDomain) + + # if we claim that we were the SPM (but we're currently not) we + # have to make sure that we're not returning stale data + if (poolMeta[PMDK_SPM_ID] == self.id + and not self.spmRole == SPM_ACQUIRED): + self.invalidateMetadata() poolMeta = self._getPoolMD(self.masterDomain) - # if we claim that we were the SPM (but we're currently not) we - # have to make sure that we're not returning stale data - if (poolMeta[PMDK_SPM_ID] == self.id - and not self.spmRole == SPM_ACQUIRED): - self.invalidateMetadata() - poolMeta = self._getPoolMD(self.masterDomain) - - lVer, spmId = poolMeta[PMDK_LVER], poolMeta[PMDK_SPM_ID] - - return lVer, spmId + return poolMeta[PMDK_LVER], poolMeta[PMDK_SPM_ID] def setSpmStatus(self, lVer=None, spmId=None): self.invalidateMetadata() -- To view, visit http://gerrit.ovirt.org/27340 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I460801329a9a1c5ee940bce22566ad3d29b351de Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.4 Gerrit-Owner: Liron Ar <laravot(a)redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com> Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>

3 4

Change in vdsm[master]: sp: fix spm start when failing to produce domain
by laravot＠redhat.com 04 May '14

04 May '14

Liron Ar has uploaded a new change for review. Change subject: sp: fix spm start when failing to produce domain ...................................................................... sp: fix spm start when failing to produce domain During spmStart we attept to go over all the domains and set their role to be regular in case it's not to avoid situation in which there are two domains with the master role. If we fail to produce any of the domains (which might happen, the domains returned by getDomains(activeOnly=True) we shouldn't fail, as we attempt to produce all the domains that are marked as active in the metadata, but it doesn't mean that they are actually reachable and that calling produce() for the domain won't fail. In case the we fail to call produce() on domain with StorageDomainDoesNotExist exception, it doesn't mean that spmStart should fail - otherwise we might never succeed to start spm. I believe that we should ignore any exception to avoid the same case in different scenarios (for example - failure to alter the metadata), but as reviewers requested, let's start with StorageDomainDoesNotExist exception only. Change-Id: I0d30bc653e3282f2de3dfdc18fadd03cdf42bbb1 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1072900 Signed-off-by: Liron Aravot <laravot(a)redhat.com> --- M vdsm/storage/sp.py 1 file changed, 8 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/18/27118/1 diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 78ff568..a78a54d 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -201,12 +201,16 @@ for sdUUID in self.getDomains(activeOnly=True): if sdUUID == self.masterDomain.sdUUID: continue + try: + domain = sdCache.produce(sdUUID) - domain = sdCache.produce(sdUUID) - if domain.getDomainRole() == sd.REGULAR_DOMAIN: - continue + if domain.getDomainRole() == sd.REGULAR_DOMAIN: + continue - self._backend.setDomainRegularRole(domain) + self._backend.setDomainRegularRole(domain) + except se.StorageDomainDoesNotExist: + self.log.error("Error when trying to find domain %s, ignoring", + sdUUID, exc_info=True) @unsecured def startSpm(self, prevID, prevLVER, maxHostID, expectedDomVersion=None): -- To view, visit http://gerrit.ovirt.org/27118 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0d30bc653e3282f2de3dfdc18fadd03cdf42bbb1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Ar <laravot(a)redhat.com>

5 8

Change in vdsm[master]: core: fixing attribute error on fenceSpmStorage
by laravot＠redhat.com 04 May '14

04 May '14

Liron Ar has uploaded a new change for review. Change subject: core: fixing attribute error on fenceSpmStorage ...................................................................... core: fixing attribute error on fenceSpmStorage In change I75493d1db60e51cccd5231b516f963c970d24c99 the pool metadata operations were moved to the pool backend, while the call to invalidateMetadata in fenceSpmStorage was left as it was, leading to an attribute error. This patch fixes it by executing the operation on the pool backend object rather than on the pool, as the metadata operation are implemented only in StoragePoolDiskBackend - there's also a check for the backend type before executing it. Though it's not the nicest way for doing so, it's the pattern already used in hsm.py,therefore in the meanwhile it can be fixed the same way. Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1082365 Signed-off-by: Liron Aravot <laravot(a)redhat.com> --- M vdsm/storage/hsm.py 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/58/26358/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 0ebe129..d9697ef 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -3529,7 +3529,8 @@ se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s" % (spUUID, lastOwner, lastLver))) pool = self.getPool(spUUID) - pool.invalidateMetadata() + if isinstance(pool.getBackend(), StoragePoolDiskBackend): + pool.getBackend().invalidateMetadata() vars.task.getExclusiveLock(STORAGE, spUUID) pool.forceFreeSpm() return dict(spm_st=self._getSpmStatusInfo(pool)) -- To view, visit http://gerrit.ovirt.org/26358 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5ae4c371e2dc97686621ea16b8cef971183a6ab2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Ar <laravot(a)redhat.com>

6 31

Change in vdsm[master]: storage: check if mount specification is a symlink
by Nir Soffer 03 May '14

03 May '14

Nir Soffer has posted comments on this change. Change subject: storage: check if mount specification is a symlink ...................................................................... Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/27321/1/vdsm/storage/mount.py File vdsm/storage/mount.py: Line 264: for record in _iterMountRecords(): Line 265: realSpec = self.fs_spec Line 266: # if this is a symlink we need the real path Line 267: # to check against /proc/mounts Line 268: if os.path.lexists(realSpec): Your comment contradict the code; lexists is not a check for symlink, but a check that the path exists (without resolving symlinks). If you like to detect a symlink, you should use islink(). if os.path.islink(realSpec): realSpec = os.path.realpath(self.fs_spec) Line 269: realSpec = os.path.realpath(self.fs_spec) Line 270: if (record.fs_spec == realSpec and Line 271: record.fs_file == self.fs_file): Line 272: return record -- To view, visit http://gerrit.ovirt.org/27321 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32aa019b52c28377c064966684986d20414df6db Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Enrico Tagliavini <enrico.tagliavini(a)gmail.com> Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes

1 0

Change in vdsm[master]: tests: update libvirt fake object
by fromani＠redhat.com 03 May '14

03 May '14

Francesco Romani has uploaded a new change for review. Change subject: tests: update libvirt fake object ...................................................................... tests: update libvirt fake object commit eb2ef555345a32a4f0153ac0341b4c422cdacf8c requires the 'close' method to be present. This patch updates the tests to let 'make check' run cleanly again. Change-Id: I1bb271d90ca8d20f83ab0af38e96598991115ad2 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M tests/libvirtconnectionTests.py 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/23/27323/1 diff --git a/tests/libvirtconnectionTests.py b/tests/libvirtconnectionTests.py index 32a6fd4..1ac60a0 100644 --- a/tests/libvirtconnectionTests.py +++ b/tests/libvirtconnectionTests.py @@ -73,6 +73,9 @@ else: return '' + def close(self): + pass + class virDomain(object): pass -- To view, visit http://gerrit.ovirt.org/27323 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1bb271d90ca8d20f83ab0af38e96598991115ad2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

4 5

Change in vdsm[master]: tool.nwfilter: do not close a connection twice
by Dan Kenigsberg 03 May '14

03 May '14

Dan Kenigsberg has uploaded a new change for review. Change subject: tool.nwfilter: do not close a connection twice ...................................................................... tool.nwfilter: do not close a connection twice My commit eb2ef55534 caused all libvirtconnections to be closed when the calling process exists. As reported by Nir on http://gerrit.ovirt.org/27313 that commit has a bug and was sloppily verified. As of eb2ef55534, no one outside the libvirtconnection module should close a libvirt connection. Change-Id: I9dfe4b98401fb2c352f87952e182fb8df06fd0e3 Signed-off-by: Dan Kenigsberg <danken(a)redhat.com> --- M lib/vdsm/tool/nwfilter.py 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/27315/1 diff --git a/lib/vdsm/tool/nwfilter.py b/lib/vdsm/tool/nwfilter.py index 25febc0..2c78219 100755 --- a/lib/vdsm/tool/nwfilter.py +++ b/lib/vdsm/tool/nwfilter.py @@ -32,7 +32,6 @@ """ conn = libvirtconnection.get(None, False) NoMacSpoofingFilter().defineNwFilter(conn) - conn.close() class NwFilter(object): -- To view, visit http://gerrit.ovirt.org/27315 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9dfe4b98401fb2c352f87952e182fb8df06fd0e3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>

5 11

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches May 2014