vdsm-patches May 2014

vdsm-patches@lists.fedorahosted.org

44 participants
645 discussions

Change in vdsm[master]: vm: graphdev: update the _devices field
by fromani＠redhat.com 12 Aug '14

12 Aug '14

Francesco Romani has uploaded a new change for review. Change subject: vm: graphdev: update the _devices field ...................................................................... vm: graphdev: update the _devices field this patch let the Graphic Devices instances in _devices be updated from the fresh data from libvirt. Change-Id: Ie6a2133742a5515dc1077cd935ad7d1e37410e44 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M vdsm/virt/vm.py 1 file changed, 10 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/27932/1 diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 2c4ad7e..e977ff3 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -4945,13 +4945,20 @@ updated = False for gxml in graphicsXml: graphicsType = gxml.getAttribute('type') + port = gxml.getAttribute('port') + tlsPort = gxml.getAttribute('tlsPort') + + for d in self._devices[GRAPHICS_DEVICES]: + if d.device == graphicsType: + d.port = port + d.tlsPort = tlsPort + break for dev in self.conf['devices']: - if dev.get('device') == graphicsType: - port = gxml.getAttribute('port') + if (dev.get('type') == GRAPHICS_DEVICES and + dev.get('device') == graphicsType): if port: dev['port'] = port - tlsPort = gxml.getAttribute('tlsPort') if tlsPort: dev['tlsPort'] = tlsPort if not updated: -- To view, visit http://gerrit.ovirt.org/27932 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie6a2133742a5515dc1077cd935ad7d1e37410e44 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

3 16

Change in vdsm[master]: vm: graphdev: rename a parameter for clarity
by fromani＠redhat.com 12 Aug '14

12 Aug '14

Francesco Romani has uploaded a new change for review. Change subject: vm: graphdev: rename a parameter for clarity ...................................................................... vm: graphdev: rename a parameter for clarity The multiple updateDevice API for grpahics device has not clients yet, so this patch changes the naming to make one parameter more self-explanatory. As nice side effect, this allows a cleaner and shorter implementation. Change-Id: I3716b48a5fbe5beebb819754d0f500b097f04677 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M tests/vmTests.py M vdsm/virt/vm.py M vdsm_api/vdsmapi-schema.json 3 files changed, 6 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/36/28036/1 diff --git a/tests/vmTests.py b/tests/vmTests.py index 804a9a2..896cb7e 100644 --- a/tests/vmTests.py +++ b/tests/vmTests.py @@ -968,7 +968,8 @@ 'graphicsType': device['device'], 'password': '***', 'ttl': 0, - 'existingConnAction': 'disconnect'}) + 'existingConnAction': 'disconnect', + 'hookParams': {}}) self.assertEquals(fake._dom.devXml, devXml) diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 878a144..93e10c7 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -3244,13 +3244,9 @@ def _updateGraphicsDevice(self, params): graphics = self._findGraphicsDeviceXMLByType(params['graphicsType']) if graphics: - params.pop('deviceType') - params.pop('graphicsType') - password = params.pop('password') - ttl = params.pop('ttl') - connAct = params.pop('existingConnAction') return self._setTicketForGraphicDev( - graphics, password, ttl, connAct, params) + graphics, params['password'], params['ttl'], + params['existingConnAction'], params['hookParams']) else: return errCode['updateDevice'] diff --git a/vdsm_api/vdsmapi-schema.json b/vdsm_api/vdsmapi-schema.json index 01a42b6..4812f9b 100644 --- a/vdsm_api/vdsmapi-schema.json +++ b/vdsm_api/vdsmapi-schema.json @@ -6256,14 +6256,14 @@ # # @existingConnAction: Indicate what to do with any existing connections # -# @params: Arbitrary key:val pairs that will be passed to hooks +# @hookParams: Arbitrary key:val pairs that will be passed to hooks # # Since: 4.15.0 ## {'type': 'vmUpdateGraphicsDeviceParams', 'data': {'graphicsType': 'VmGraphicsDeviceType', 'password': 'str', 'ttl': 'int', 'existingConnAction': 'VmTicketConflictAction', - 'params': 'StringMap'}} + 'hookParams': 'StringMap'}} ## # @VM.updateDevice: -- To view, visit http://gerrit.ovirt.org/28036 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3716b48a5fbe5beebb819754d0f500b097f04677 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

4 21

Change in vdsm[master]: tests: fix wrong use of assertions
by ykleinbe＠redhat.com 05 Aug '14

05 Aug '14

Yoav Kleinberger has uploaded a new change for review. Change subject: tests: fix wrong use of assertions ...................................................................... tests: fix wrong use of assertions Previously, the test used assertEquals for checking return code of subprocesses spawned by the test itself. This resulted in a resported test failure, when it should have reported an error. To fix this, I replaced execCmd which does not belong in test code (we should not use the vdsm code to write tests that check vdsm) with a new testrunner.runCommand which is easier to use, and simplified the test. Change-Id: I693923e6dcaa05bc1479db814c0b7696b3536c9c Signed-off-by: Yoav Kleinberger <ykleinbe(a)redhat.com> --- M tests/functional/storageTests.py M tests/testrunner.py 2 files changed, 34 insertions(+), 20 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/28124/1 diff --git a/tests/functional/storageTests.py b/tests/functional/storageTests.py index 80ba312..0a19776 100644 --- a/tests/functional/storageTests.py +++ b/tests/functional/storageTests.py @@ -32,6 +32,9 @@ from testrunner import VdsmTestCase as TestCaseBase from testrunner import permutations, expandPermutations from testrunner import TEMPDIR +from testrunner import runCommand +from testrunner import TestRunnerError + try: import rtslib except ImportError: @@ -40,7 +43,6 @@ import storage.sd import storage.storage_exception as se import storage.volume -from storage.misc import execCmd from storage.mount import Mount from vdsm.config import config @@ -372,9 +374,7 @@ if "rtslib" not in globals().keys(): raise SkipTest("python-rtslib is not installed.") - cmd = [_modprobe.cmd, "iscsi_target_mod"] - rc, out, err = execCmd(cmd, sudo=True) - asserts.assertEquals(rc, 0) + runCommand(_modprobe.cmd, "iscsi_target_mod") # mount the configfs for rtslib if it is not mounted m = Mount('configfs', '/sys/kernel/config') @@ -522,21 +522,15 @@ def exportNFS(path): - rc, out, err = execCmd([_exportfs.cmd, '-o', 'rw,insecure,sync', - '127.0.0.1:%s' % path]) - return rc + runCommand(_exportfs.cmd, '-o', 'rw,insecure,sync', '127.0.0.1:%s' % path) def unexportNFS(path): - rc, out, err = execCmd([_exportfs.cmd, '-u', '127.0.0.1:%s' % path]) - return rc + runCommand(_exportfs.cmd, '-u', '127.0.0.1:%s' % path) def listNFS(): - rc, out, err = execCmd([_exportfs.cmd]) - if rc != 0: - raise RuntimeError("Can not list NFS export: %s\n" % err) - return out + return runCommand([_exportfs.cmd]) def cleanNFSLeftovers(pathPrefix): @@ -545,10 +539,13 @@ for line in exports: export = line.split(" ", 1)[0] if fnmatch.fnmatch(export, pathPattern): - if unexportNFS(export) == 0: - shutil.rmtree(export, ignore_errors=True) + try: + unexportNFS(export) + except TestRunnerError as e: + logging.warning("Failed to unexport NFS entry %s: %s", + export, e) else: - logging.warning("Failed to unexport NFS entry %s", export) + shutil.rmtree(export, ignore_errors=True) class NFSServer(BackendServer): @@ -574,10 +571,8 @@ rollback.prependDefer(undo) os.chown(path, uid, gid) os.chmod(path, 0o775) - self.asserts.assertEquals(0, exportNFS(path)) - undo = lambda path=path: self.asserts.assertEquals( - 0, unexportNFS(path)) - rollback.prependDefer(undo) + exportNFS(path) + rollback.prependDefer(unexportNFS, path) connections[uuid] = {'type': 'nfs', 'params': {'export': '127.0.0.1:%s' % path}} diff --git a/tests/testrunner.py b/tests/testrunner.py index ac0f90f..6f99a96 100644 --- a/tests/testrunner.py +++ b/tests/testrunner.py @@ -26,12 +26,21 @@ import shutil import tempfile from contextlib import contextmanager +import subprocess from vdsm import utils from nose import config from nose import core from nose import result + + +class TestRunnerError(Exception): + """ + This is an error in the test itself (e.g. could not cleanup some temproray + file). It is not an error in the tested code. + """ + # Monkey patch pthreading in necessary if sys.version_info[0] == 2: @@ -383,6 +392,16 @@ raise AssertionError(msg) +def runCommand(*args): + process = subprocess.Popen(args, stderr=subprocess.PIPE, + stdout=subprocess.PIPE) + output, error = process.communicate() + if process.returncode != 0: + raise TestRunnerError("subprocerss failed: returncode=%s: stderr=%r" % + (process.returncode, error)) + return output + + if __name__ == '__main__': if "--help" in sys.argv: print("testrunner options:\n" -- To view, visit http://gerrit.ovirt.org/28124 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I693923e6dcaa05bc1479db814c0b7696b3536c9c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yoav Kleinberger <ykleinbe(a)redhat.com>

10 33

Change in vdsm[master]: multipath: Fix devices rescanning
by Nir Soffer 03 Aug '14

03 Aug '14

Nir Soffer has uploaded a new change for review. Change subject: multipath: Fix devices rescanning ...................................................................... multipath: Fix devices rescanning When rescanning devices, we used to update ISCSI devices, and then run multipath to update device mapping. However, we never rescanned FC devices. Since commit dbf2089488 (Jul 9 2013) multipath call was change to use the -r flag, forcing a reload of the device map. This was tested to fix a case where new lun is created on the storage server, while a host was connected, and the new device is not available when issuing the getDeviceList command. According to a comment on gerrit, the change was tested for ISCSI and FC storage types, but there is no documentation of the testing procedure. The related bug has no information about what was tested. We currently have two bugs related multipath rescanning: - Bug 1078879 tell us that invoking multipath with the -r flag sometimes triggers a segfault in the multipathd daemon. In the bug, multipath developer suggests that as long as multipathd daemon is running, there is no need to invoke multipath to detect new devices, and "multipath -r really isn't useful for much of anything". - Bug 1071654 tell us that devices rescanning is broken on FC storage domains (although the -r flag is used). The bug suggest to use issue_lip sysfs api, which is also recommended in the RHEL storage administration manual. This patch removes the -r flag, which seem to be useless currently, and adds the missing FC rescan using the recommended sysfs api. To be on the safe side, I left the multipath call as it was since the first multipath commit in 2009. We will work with kernel and multipath developers further on removing this call if it is indeed unneeded. Bug-Url: https://bugzilla.redhat.com/1078879 Bug-Url: https://bugzilla.redhat.com/1071654 Relates-to: http://gerrit.ovirt.org/17263 Change-Id: I7699504f9771232ee0b880f9c83a51fd5b90f40e Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm/storage/hba.py M vdsm/storage/multipath.py M vdsm/supervdsmServer 3 files changed, 34 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/27122/1 diff --git a/vdsm/storage/hba.py b/vdsm/storage/hba.py index da3feef..050bec5 100644 --- a/vdsm/storage/hba.py +++ b/vdsm/storage/hba.py @@ -36,6 +36,31 @@ NODE_NAME = "node_name" +def rescan(): + """ + Rescan HBAs connections, updating available devices. + + This operation performs a Loop Initialization Protocol (LIP) and then + scans the interconnect and causes the SCSI layer to be updated to reflect + the devices currently on the bus. A LIP is, essentially, a bus reset, and + will cause device addition and removal. + + Bear in mind that issue_lip is an asynchronous operation. The command may + complete before the entire scan has completed. + + Note: Must be executed as root. + TODO: Some drivers do not support this operation. + """ + log.info("Rescanning HBAs") + for path in glob.glob(FC_HOST_MASK + '/issue_lip'): + log.debug("Issuing lip %s", path) + try: + with open(path, 'w') as f: + f.write('1') + except IOError as e: + logging.error("Error issuing lip: %s", e) + + def getiSCSIInitiators(): """ Get iSCSI initiator name from the default location. diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py index 82d435d..ba98866 100644 --- a/vdsm/storage/multipath.py +++ b/vdsm/storage/multipath.py @@ -102,12 +102,12 @@ Should only be called from hsm._rescanDevices() """ - # First ask iSCSI to rescan all its sessions + # First rescan iSCSI and FCP connections iscsi.rescan() + supervdsm.getProxy().hbaRescan() # Now let multipath daemon pick up new devices - cmd = [constants.EXT_MULTIPATH, "-r"] - misc.execCmd(cmd, sudo=True) + misc.execCmd([constants.EXT_MULTIPATH], sudo=True) def isEnabled(): diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer index ffcedb8..dc6f602 100755 --- a/vdsm/supervdsmServer +++ b/vdsm/supervdsmServer @@ -58,6 +58,8 @@ from network import sourceroutethread from network.api import (addNetwork, delNetwork, editNetwork, setupNetworks, setSafeNetworkConfig) + +from storage import hba from network.tc import setPortMirroring, unsetPortMirroring from storage.multipath import getScsiSerial as _getScsiSerial from storage.iscsi import getDevIscsiInfo as _getdeviSCSIinfo @@ -253,6 +255,10 @@ return setSafeNetworkConfig() @logDecorator + def hbaRescan(self): + return hba.rescan() + + @logDecorator def udevTrigger(self, guid): self.__udevReloadRules(guid) cmd = [EXT_UDEVADM, 'trigger', '--verbose', '--action', 'change', -- To view, visit http://gerrit.ovirt.org/27122 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7699504f9771232ee0b880f9c83a51fd5b90f40e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

7 24

Change in vdsm[master]: domainMonitor: Extract domain monitoring methods
by Nir Soffer 01 Aug '14

01 Aug '14

Nir Soffer has uploaded a new change for review. Change subject: domainMonitor: Extract domain monitoring methods ...................................................................... domainMonitor: Extract domain monitoring methods The _domainMonitor method was extremely long and was doing too much. This patch breaks it to multiple small methods to make it easier to understand and modify. One of the planned changes is to allow _domainMonitor to abort in the middle if a monitor is stopped. Implementing this in the original method would only make it worse. This patch (hopefully) does change the behavior of the monitor. Change-Id: I87ac6a82e560bc4360a3bc3f6f6fd94623678cc2 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm/storage/domainMonitor.py 1 file changed, 111 insertions(+), 73 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/14/27714/1 diff --git a/vdsm/storage/domainMonitor.py b/vdsm/storage/domainMonitor.py index 8a50b4f..8057910 100644 --- a/vdsm/storage/domainMonitor.py +++ b/vdsm/storage/domainMonitor.py @@ -174,66 +174,20 @@ self.log.debug("Stopping domain monitor for %s", self.sdUUID) - # If this is an ISO domain we didn't acquire the host id and releasing - # it is superfluous. - if self.domain and not self.isIsoDomain: - try: - self.domain.releaseHostId(self.hostId, unused=True) - except: - self.log.debug("Unable to release the host id %s for domain " - "%s", self.hostId, self.sdUUID, exc_info=True) + if self._shouldReleaseHostId(): + self._releaseHostId() def _monitorDomain(self): self.nextStatus.clear() - if time() - self.lastRefresh > self.refreshTime: - # Refreshing the domain object in order to pick up changes as, - # for example, the domain upgrade. - self.log.debug("Refreshing domain %s", self.sdUUID) - sdCache.manuallyRemoveDomain(self.sdUUID) - self.lastRefresh = time() + if self._shouldRefreshDomain(): + self._refreshDomain() try: - # We should produce the domain inside the monitoring loop because - # it might take some time and we don't want to slow down the thread - # start (and anything else that relies on that as for example - # updateMonitoringThreads). It also needs to be inside the loop - # since it might fail and we want keep trying until we succeed or - # the domain is deactivated. - if self.domain is None: - self.domain = sdCache.produce(self.sdUUID) - - if self.isIsoDomain is None: - # The isIsoDomain assignment is delayed because the isoPrefix - # discovery might fail (if the domain suddenly disappears) and - # we could risk to never try to set it again. - isIsoDomain = self.domain.isISO() - if isIsoDomain: - self.isoPrefix = self.domain.getIsoDomainImagesDir() - self.isIsoDomain = isIsoDomain - - self.domain.selftest() - - self.nextStatus.readDelay = self.domain.getReadDelay() - - stats = self.domain.getStats() - self.nextStatus.diskUtilization = (stats["disktotal"], - stats["diskfree"]) - - self.nextStatus.vgMdUtilization = (stats["mdasize"], - stats["mdafree"]) - - self.nextStatus.vgMdHasEnoughFreeSpace = stats["mdavalid"] - self.nextStatus.vgMdFreeBelowThreashold = stats["mdathreshold"] - - masterStats = self.domain.validateMaster() - self.nextStatus.masterValid = masterStats['valid'] - self.nextStatus.masterMounted = masterStats['mount'] - - self.nextStatus.hasHostId = self.domain.hasHostId(self.hostId) - self.nextStatus.isoPrefix = self.isoPrefix - self.nextStatus.version = self.domain.getVersion() - + self._initializeDomain() + self._checkDomain() + self._checkReadDelay() + self._collectStats() except Exception as e: self.log.error("Error while collecting domain %s monitoring " "information", self.sdUUID, exc_info=True) @@ -243,29 +197,113 @@ self.nextStatus.valid = (self.nextStatus.error is None) if self._statusDidChange(): - self.log.debug("Domain %s changed its status to %s", self.sdUUID, - "Valid" if self.nextStatus.valid else "Invalid") - - try: - self.domainMonitor.onDomainStateChange.emit( - self.sdUUID, self.nextStatus.valid) - except: - self.log.warn("Could not emit domain state change event", - exc_info=True) - + self._notifyStatusChanges() self.firstChange = False - # An ISO domain can be shared by multiple pools - if (not self.isIsoDomain and self.nextStatus.valid - and self.nextStatus.hasHostId is False): - try: - self.domain.acquireHostId(self.hostId, async=True) - except: - self.log.debug("Unable to issue the acquire host id %s " - "request for domain %s", self.hostId, - self.sdUUID, exc_info=True) + if self._shouldAcquireHostId(): + self._acquireHostId() self.status.update(self.nextStatus) + # Notifiying status changes + def _statusDidChange(self): return self.firstChange or self.status.valid != self.nextStatus.valid + + def _notifyStatusChanges(self): + self.log.debug("Domain %s changed its status to %s", self.sdUUID, + "Valid" if self.nextStatus.valid else "Invalid") + try: + self.domainMonitor.onDomainStateChange.emit( + self.sdUUID, self.nextStatus.valid) + except: + self.log.warn("Could not emit domain state change event", + exc_info=True) + + # Refreshing domain + + def _shouldRefreshDomain(self): + return time() - self.lastRefresh > self.refreshTime + + def _refreshDomain(self): + # Refreshing the domain object in order to pick up changes as, + # for example, the domain upgrade. + self.log.debug("Refreshing domain %s", self.sdUUID) + sdCache.manuallyRemoveDomain(self.sdUUID) + self.lastRefresh = time() + + # Deferred initialization + + def _initializeDomain(self): + # We should produce the domain inside the monitoring loop because + # it might take some time and we don't want to slow down the thread + # start (and anything else that relies on that as for example + # updateMonitoringThreads). It also needs to be inside the loop + # since it might fail and we want keep trying until we succeed or + # the domain is deactivated. + if self.domain is None: + self.domain = sdCache.produce(self.sdUUID) + + if self.isIsoDomain is None: + # The isIsoDomain assignment is delayed because the isoPrefix + # discovery might fail (if the domain suddenly disappears) and + # we could risk to never try to set it again. + isIsoDomain = self.domain.isISO() + if isIsoDomain: + self.isoPrefix = self.domain.getIsoDomainImagesDir() + self.isIsoDomain = isIsoDomain + + # Collecting monitoring info + + def _checkDomain(self): + self.domain.selftest() + + def _checkReadDelay(self): + self.nextStatus.readDelay = self.domain.getReadDelay() + + def _collectStats(self): + stats = self.domain.getStats() + self.nextStatus.diskUtilization = (stats["disktotal"], + stats["diskfree"]) + + self.nextStatus.vgMdUtilization = (stats["mdasize"], + stats["mdafree"]) + + self.nextStatus.vgMdHasEnoughFreeSpace = stats["mdavalid"] + self.nextStatus.vgMdFreeBelowThreashold = stats["mdathreshold"] + + masterStats = self.domain.validateMaster() + self.nextStatus.masterValid = masterStats['valid'] + self.nextStatus.masterMounted = masterStats['mount'] + + self.nextStatus.hasHostId = self.domain.hasHostId(self.hostId) + self.nextStatus.isoPrefix = self.isoPrefix + self.nextStatus.version = self.domain.getVersion() + + # Managing host id + + def _shouldAcquireHostId(self): + # An ISO domain can be shared by multiple pools + return (not self.isIsoDomain and + self.nextStatus.valid and + self.nextStatus.hasHostId is False) + + def _shouldReleaseHostId(self): + # If this is an ISO domain we didn't acquire the host id and releasing + # it is superfluous. + return self.domain and not self.isIsoDomain + + def _acquireHostId(self): + try: + self.domain.acquireHostId(self.hostId, async=True) + except: + self.log.debug("Unable to issue the acquire host id %s " + "request for domain %s", self.hostId, + self.sdUUID, exc_info=True) + + def _releaseHostId(self): + try: + self.domain.releaseHostId(self.hostId, unused=True) + except: + self.log.debug("Unable to release the host id %s for domain " + "%s", self.hostId, self.sdUUID, exc_info=True) -- To view, visit http://gerrit.ovirt.org/27714 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I87ac6a82e560bc4360a3bc3f6f6fd94623678cc2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

7 33

Change in vdsm[master]: domainMonitor: Stop domain monitors concurrently
by Nir Soffer 01 Aug '14

01 Aug '14

Nir Soffer has uploaded a new change for review. Change subject: domainMonitor: Stop domain monitors concurrently ...................................................................... domainMonitor: Stop domain monitors concurrently Deactivating a host connected to lot of storage domains takes too much time. This is caused by stopping each monitor thread and waiting until the thread exists before stopping the next. Looking at the log, we can see that while we stop one monitor thread, other monitor threads wake up and continue to monitor their domains. If you try to stop vdsm after moving a host to maintenance, vdsm will be killed after 10 seconds. If some domain monitores id not exit, you will have stale leases held by sanlock. This will lead to failure to upgrade this host, and requires reboot. This patch changes the way monitors are stopped, so we stop all monitors without waiting, which will wake up all sleeping monitors at once, and signal busy monitor to abort. Then we wait until all monitor threads exit. To help a monitor thread exit soon as possible, we check if monitor was stopped in key points in the _monitorDomain method. I tested this patch on Fedora 19 machines connected to 30 storage domains. With this patch, the time to complete disconnectStoragePool dropped from 70 to 13 seconds. Most of this time is spent in stopping monitoring. Stopping all 30 monitor threads took 10 milliseconds, and waiting until monitors threads exist took 10 seconds. Note that this change cannot guarantee that all monitor threads will exit in a timely manner, since some of them may be blocked, reading from inaccessible storage. But it does ensure that in the common case monitors stop and exit immediately. Change-Id: Ia12f137f1ed055316767e1a9384d8982720bc564 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm/storage/domainMonitor.py M vdsm/storage/sp.py 2 files changed, 81 insertions(+), 33 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/73/27573/1 diff --git a/vdsm/storage/domainMonitor.py b/vdsm/storage/domainMonitor.py index 8a50b4f..7b82777 100644 --- a/vdsm/storage/domainMonitor.py +++ b/vdsm/storage/domainMonitor.py @@ -104,9 +104,9 @@ # The domain should be added only after it succesfully started self._domains[sdUUID] = domainThread - def stopMonitoring(self, sdUUID): + def stopMonitoring(self, sdUUID, wait=True): # The domain monitor issues events that might become raceful if - # stopMonitoring doesn't stop until the thread exits. + # you don't wait until a monitor thread exit. # Eg: when a domain is detached the domain monitor is stopped and # the host id is released. If the monitor didn't actually exit it # might respawn a new acquire host id. @@ -114,17 +114,33 @@ try: self._domains[sdUUID].stop() except KeyError: - return + self.log.warning("No monitor for %s", sdUUID) + else: + if wait: + self.waitForMonitor(sdUUID) - del self._domains[sdUUID] + def waitForMonitor(self, sdUUID): + # If you are stopping multiple monitors, you want to invoke + # stopMonitoring with wait=False, and wait on them after all of the + # monitors are stopped. + self.log.info("Waiting for %s", sdUUID) + try: + self._domains[sdUUID].wait() + except KeyError: + self.log.warning("No monitor for %s", sdUUID) + else: + del self._domains[sdUUID] def getStatus(self, sdUUID): return self._domains[sdUUID].getStatus() def close(self): self.log.info("Stopping domain monitors") - for sdUUID in self._domains.keys(): - self.stopMonitoring(sdUUID) + domains = self._domains.keys() + for sdUUID in domains: + self.stopMonitoring(sdUUID, wait=False) + for sdUUID in domains: + self.waitForMonitor(sdUUID) class DomainMonitorThread(object): @@ -152,10 +168,11 @@ def start(self): self.thread.start() - def stop(self, wait=True): + def stop(self): self.stopEvent.set() - if wait: - self.thread.join() + + def wait(self): + self.thread.join() def getStatus(self): return self.status.copy() @@ -164,7 +181,7 @@ def _monitorLoop(self): self.log.debug("Starting domain monitor for %s", self.sdUUID) - while not self.stopEvent.is_set(): + while self._running(): try: self._monitorDomain() except: @@ -189,6 +206,11 @@ if time() - self.lastRefresh > self.refreshTime: # Refreshing the domain object in order to pick up changes as, # for example, the domain upgrade. + + # Don't try to refresh if stopoped + if not self._running(): + return + self.log.debug("Refreshing domain %s", self.sdUUID) sdCache.manuallyRemoveDomain(self.sdUUID) self.lastRefresh = time() @@ -212,9 +234,21 @@ self.isoPrefix = self.domain.getIsoDomainImagesDir() self.isIsoDomain = isIsoDomain + # Don't wait for storage if stopped + if not self._running(): + return + self.domain.selftest() + # Don't wait for storage if stopped + if not self._running(): + return + self.nextStatus.readDelay = self.domain.getReadDelay() + + # I could have been stopped while waiting for storage + if not self._running(): + return stats = self.domain.getStats() self.nextStatus.diskUtilization = (stats["disktotal"], @@ -242,6 +276,10 @@ self.nextStatus.checkTime = time() self.nextStatus.valid = (self.nextStatus.error is None) + # Do not emmit notifications if stopped + if not self._running(): + return + if self._statusDidChange(): self.log.debug("Domain %s changed its status to %s", self.sdUUID, "Valid" if self.nextStatus.valid else "Invalid") @@ -254,6 +292,10 @@ exc_info=True) self.firstChange = False + + # Do not try to acquire host id if stopped + if not self._running(): + return # An ISO domain can be shared by multiple pools if (not self.isIsoDomain and self.nextStatus.valid @@ -269,3 +311,6 @@ def _statusDidChange(self): return self.firstChange or self.status.valid != self.nextStatus.valid + + def _running(self): + return not self.stopEvent.is_set() diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 4a07c6a..7cd77ca 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -641,8 +641,11 @@ @unsecured def stopMonitoringDomains(self): - for sdUUID in self.domainMonitor.poolMonitoredDomains: - self.domainMonitor.stopMonitoring(sdUUID) + domains = self.domainMonitor.poolMonitoredDomains + for sdUUID in domains: + self.domainMonitor.stopMonitoring(sdUUID, wait=False) + for sdUUID in domains: + self.domainMonitor.waitForMonitor(sdUUID) return True @unsecured @@ -1389,29 +1392,29 @@ def updateMonitoringThreads(self): # domain list it's list of sdUUID:status # sdUUID1:status1,sdUUID2:status2,... - activeDomains = self.getDomains(activeOnly=True) - monitoredDomains = self.domainMonitor.poolMonitoredDomains + activeDomains = set(self.getDomains(activeOnly=True)) + monitoredDomains = set(self.domainMonitor.poolMonitoredDomains) - for sdUUID in monitoredDomains: - if sdUUID not in activeDomains: - try: - self.domainMonitor.stopMonitoring(sdUUID) - self.log.debug("Storage Pool `%s` stopped monitoring " - "domain `%s`", self.spUUID, sdUUID) - except se.StorageException: - self.log.error("Unexpected error while trying to stop " - "monitoring domain `%s`", sdUUID, - exc_info=True) + monitorsToStop = monitoredDomains - activeDomains - for sdUUID in activeDomains: - if sdUUID not in monitoredDomains: - try: - self.domainMonitor.startMonitoring(sdUUID, self.id) - self.log.debug("Storage Pool `%s` started monitoring " - "domain `%s`", self.spUUID, sdUUID) - except se.StorageException: - self.log.error("Unexpected error while trying to monitor " - "domain `%s`", sdUUID, exc_info=True) + for sdUUID in monitorsToStop: + self.domainMonitor.stopMonitoring(sdUUID, wait=False) + + for sdUUID in monitorsToStop: + self.domainMonitor.waitForMonitor(sdUUID) + self.log.debug("Storage Pool `%s` stopped monitoring " + "domain `%s`", self.spUUID, sdUUID) + + monitorsToStart = activeDomains - monitoredDomains + + for sdUUID in monitorsToStart: + try: + self.domainMonitor.startMonitoring(sdUUID, self.id) + self.log.debug("Storage Pool `%s` started monitoring " + "domain `%s`", self.spUUID, sdUUID) + except se.StorageException: + self.log.error("Unexpected error while trying to monitor " + "domain `%s`", sdUUID, exc_info=True) @unsecured def getDomains(self, activeOnly=False): -- To view, visit http://gerrit.ovirt.org/27573 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia12f137f1ed055316767e1a9384d8982720bc564 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

8 33

Change in vdsm[master]: Move multipath configuration to vdsm-tool configurator
by ykaplan＠redhat.com 31 Jul '14

31 Jul '14

Yeela Kaplan has uploaded a new change for review. Change subject: Move multipath configuration to vdsm-tool configurator ...................................................................... Move multipath configuration to vdsm-tool configurator Previously multipathe is recofigured on each vdsm service restart. Now it will be reconfigured only on user demand. Change-Id: I40f802477e39000c5cae01a496ac2d9f879ebfa8 Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com> --- M lib/vdsm/tool/configurator.py M lib/vdsm/utils.py M tests/miscTests.py M tests/utilsTests.py M vdsm/caps.py M vdsm/storage/hsm.py M vdsm/storage/misc.py M vdsm/storage/multipath.py M vdsm/storage/sd.py M vdsm/supervdsmServer 10 files changed, 301 insertions(+), 281 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/23/26123/1 diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py index 869d774..d97af61 100644 --- a/lib/vdsm/tool/configurator.py +++ b/lib/vdsm/tool/configurator.py @@ -21,11 +21,10 @@ import sys import grp import argparse +import tempfile -from .. import utils +from .. import utils, constants from . import service, expose -from ..constants import P_VDSM_EXEC, DISKIMAGE_GROUP -from ..constants import QEMU_PROCESS_GROUP, VDSM_GROUP class _ModuleConfigure(object): @@ -68,7 +67,7 @@ rc, out, err = utils.execCmd( ( os.path.join( - P_VDSM_EXEC, + constants.P_VDSM_EXEC, 'libvirt_configure.sh' ), action, @@ -126,7 +125,7 @@ '/usr/sbin/usermod', '-a', '-G', - '%s,%s' % (QEMU_PROCESS_GROUP, VDSM_GROUP), + '%s,%s' % (constants.QEMU_PROCESS_GROUP, constants.VDSM_GROUP), 'sanlock' ), raw=True, @@ -156,7 +155,7 @@ break else: raise RuntimeError("Unable to find sanlock service groups") - ret = grp.getgrnam(DISKIMAGE_GROUP)[2] in groups + ret = grp.getgrnam(constants.DISKIMAGE_GROUP)[2] in groups except IOError as e: if e.errno == os.errno.ENOENT: sys.stdout.write("sanlock service is not running\n") @@ -172,9 +171,149 @@ return ret +MPATH_CONF = "/etc/multipath.conf" + +STRG_MPATH_CONF = ( + "\n\n" + "defaults {\n" + " polling_interval 5\n" + " getuid_callout \"%(scsi_id_path)s --whitelisted " + "--replace-whitespace --device=/dev/%%n\"\n" + " no_path_retry fail\n" + " user_friendly_names no\n" + " flush_on_last_del yes\n" + " fast_io_fail_tmo 5\n" + " dev_loss_tmo 30\n" + " max_fds 4096\n" + "}\n" + "\n" + "devices {\n" + "device {\n" + " vendor \"HITACHI\"\n" + " product \"DF.*\"\n" + " getuid_callout \"%(scsi_id_path)s --whitelisted " + "--replace-whitespace --device=/dev/%%n\"\n" + "}\n" + "device {\n" + " vendor \"COMPELNT\"\n" + " product \"Compellent Vol\"\n" + " no_path_retry fail\n" + "}\n" + "}" +) + +OLD_TAGS = ["# RHAT REVISION 0.2", "# RHEV REVISION 0.3", + "# RHEV REVISION 0.4", "# RHEV REVISION 0.5", + "# RHEV REVISION 0.6", "# RHEV REVISION 0.7", + "# RHEV REVISION 0.8", "# RHEV REVISION 0.9"] +MPATH_CONF_TAG = "# RHEV REVISION 1.0" +MPATH_CONF_PRIVATE_TAG = "# RHEV PRIVATE" + +MPATH_CONF_TEMPLATE = MPATH_CONF_TAG + STRG_MPATH_CONF + +MAX_CONF_COPIES = 5 + +_scsi_id = utils.CommandPath("scsi_id", + "/sbin/scsi_id", # EL6 + "/usr/lib/udev/scsi_id", # Fedora + "/lib/udev/scsi_id", # Ubuntu + ) + + +class MultipathModuleConfigure(_ModuleConfigure): + + def __init__(self): + super(MultipathModuleConfigure, self).__init__() + + def getName(self): + return 'multipath' + + def getServices(self): + return ["multipathd"] + + def configure(self): + """ + Set up the multipath daemon configuration to the known and + supported state. The original configuration, if any, is saved + """ + if os.getuid() != 0: + raise UserWarning("Must run as root") + if self.isconfigured(): + return + if os.path.exists(MPATH_CONF): + utils.rotateFiles( + os.path.dirname(MPATH_CONF), + os.path.basename(MPATH_CONF), MAX_CONF_COPIES, + cp=True, persist=True) + with tempfile.NamedTemporaryFile() as f: + f.write(MPATH_CONF_TEMPLATE % {'scsi_id_path': _scsi_id.cmd}) + f.flush() + cmd = [constants.EXT_CP, f.name, MPATH_CONF] + rc, out, err = utils.execCmd(cmd) + + if rc != 0: + raise RuntimeError("Failed to perform Multipath config.") + utils.persistFile(MPATH_CONF) + + # Flush all unused multipath device maps + utils.execCmd([constants.EXT_MULTIPATH, "-F"]) + + cmd = [constants.EXT_VDSM_TOOL, "service-reload", "multipathd"] + rc, out, err = utils.execCmd(cmd) + if rc != 0: + raise RuntimeError("Failed to reload Multipath.") + + + def isconfigured(self, *args): + """ + Check the multipath daemon configuration. The configuration file + /etc/multipath.conf should contain private tag in form + "RHEV REVISION X.Y" for this check to succeed. + If the tag above is followed by tag "RHEV PRIVATE" the configuration + should be preserved at all cost. + + """ + if os.getuid() != 0: + raise UserWarning("Must run as root") + + if os.path.exists(MPATH_CONF): + first = second = '' + with open(MPATH_CONF) as f: + mpathconf = [x.strip("\n") for x in f.readlines()] + try: + first = mpathconf[0] + second = mpathconf[1] + except IndexError: + pass + if MPATH_CONF_PRIVATE_TAG in second: + sys.stdout.write("Manual override for multipath.conf detected" + " - preserving current configuration") + if MPATH_CONF_TAG not in first: + sys.stdout.write("This manual override for multipath.conf " + "was based on downrevved template. " + "You are strongly advised to " + "contact your support representatives") + return True + + if MPATH_CONF_TAG in first: + sys.stdout.write("Current revision of multipath.conf detected," + " preserving") + return True + + for tag in OLD_TAGS: + if tag in first: + sys.stdout.write("Downrev multipath.conf detected, " + "upgrade required") + return False + + sys.stdout.write("multipath Defaulting to False") + return False + + __configurers = ( LibvirtModuleConfigure(), SanlockModuleConfigure(), + MultipathModuleConfigure(), ) diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index b072bd4..b6a2ecc 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -79,6 +79,15 @@ HIGH = 19 +class OSName: + UNKNOWN = 'unknown' + OVIRT = 'oVirt Node' + RHEL = 'RHEL' + FEDORA = 'Fedora' + RHEVH = 'RHEV Hypervisor' + DEBIAN = 'Debian' + + class GeneralException(Exception): code = 100 message = "General Exception" @@ -144,6 +153,62 @@ logging.warning("Directory: %s already removed", directoryToRemove) else: raise + + +def rotateFiles(directory, prefixName, gen, cp=False, persist=False): + logging.debug("dir: %s, prefixName: %s, versions: %s" % + (directory, prefixName, gen)) + gen = int(gen) + files = os.listdir(directory) + files = glob.glob("%s*" % prefixName) + fd = {} + for fname in files: + name = fname.rsplit('.', 1) + try: + ind = int(name[1]) + except ValueError: + name[0] = fname + ind = 0 + except IndexError: + ind = 0 + except: + continue + if ind < gen: + fd[ind] = {'old': fname, 'new': name[0] + '.' + str(ind + 1)} + + keys = fd.keys() + keys.sort(reverse=True) + logging.debug("versions found: %s" % (keys)) + + for key in keys: + oldName = os.path.join(directory, fd[key]['old']) + newName = os.path.join(directory, fd[key]['new']) + if isOvirtNode() and persist and not cp: + try: + execCmd([constants.EXT_UNPERSIST, oldName], logErr=False, + sudo=True) + execCmd([constants.EXT_UNPERSIST, newName], logErr=False, + sudo=True) + except: + pass + try: + if cp: + execCmd([constants.EXT_CP, oldName, newName], sudo=True) + if isOvirtNode() and persist and not os.path.exists(newName): + execCmd([constants.EXT_PERSIST, newName], logErr=False, + sudo=True) + + else: + os.rename(oldName, newName) + except: + pass + if isOvirtNode() and persist and not cp: + try: + execCmd([constants.EXT_PERSIST, newName], logErr=False, + sudo=True) + except: + pass + IPXMLRPCRequestHandler = SimpleXMLRPCRequestHandler @@ -826,6 +891,31 @@ return functools.partial(self.__call__, obj) +@memoized +def getos(): + if os.path.exists('/etc/rhev-hypervisor-release'): + return OSName.RHEVH + elif glob.glob('/etc/ovirt-node-*-release'): + return OSName.OVIRT + elif os.path.exists('/etc/fedora-release'): + return OSName.FEDORA + elif os.path.exists('/etc/redhat-release'): + return OSName.RHEL + elif os.path.exists('/etc/debian_version'): + return OSName.DEBIAN + else: + return OSName.UNKNOWN + + +def isOvirtNode(): + return getos() in (OSName.RHEVH, OSName.OVIRT) + + +def persistFile(name): + if isOvirtNode(): + execCmd([constants.EXT_PERSIST, name], sudo=True) + + def validateMinimalKeySet(dictionary, reqParams): if not all(key in dictionary for key in reqParams): raise ValueError diff --git a/tests/miscTests.py b/tests/miscTests.py index fb1191b..1932a43 100644 --- a/tests/miscTests.py +++ b/tests/miscTests.py @@ -242,63 +242,6 @@ self.assertRaises(ValueError, misc.itmap(int, data, 0).next) -class RotateFiles(TestCaseBase): - def testNonExistingDir(self, persist=False): - """ - Tests that the method fails correctly when given a non existing dir. - """ - self.assertRaises(OSError, misc.rotateFiles, "/I/DONT/EXIST", "prefix", - 2, persist=persist) - - def testEmptyDir(self, persist=False): - """ - Test that when given an empty dir the rotator works correctly. - """ - prefix = "prefix" - dir = tempfile.mkdtemp() - - misc.rotateFiles(dir, prefix, 0, persist=persist) - - os.rmdir(dir) - - def testFullDir(self, persist=False): - """ - Test that rotator does it's basic functionality. - """ - #Prepare - prefix = "prefix" - stubContent = ('"Multiple exclamation marks", ' - 'he went on, shaking his head, ' - '"are a sure sign of a diseased mind."') - # (C) Terry Pratchet - Small Gods - dir = tempfile.mkdtemp() - gen = 10 - - expectedDirContent = [] - for i in range(gen): - fname = "%s.txt.%d" % (prefix, i + 1) - expectedDirContent.append("%s.txt.%d" % (prefix, i + 1)) - f = open(os.path.join(dir, fname), "wb") - f.write(stubContent) - f.flush() - f.close() - - #Rotate - misc.rotateFiles(dir, prefix, gen, persist=persist) - - #Test result - currentDirContent = os.listdir(dir) - expectedDirContent.sort() - currentDirContent.sort() - try: - self.assertEquals(currentDirContent, expectedDirContent) - finally: - #Clean - for f in os.listdir(dir): - os.unlink(os.path.join(dir, f)) - os.rmdir(dir) - - class ParseHumanReadableSize(TestCaseBase): def testValidInput(self): """ diff --git a/tests/utilsTests.py b/tests/utilsTests.py index c5b250d..1e202d2 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -22,6 +22,7 @@ import contextlib import errno import logging +import tempfile from testrunner import VdsmTestCase as TestCaseBase from vdsm import utils @@ -302,3 +303,60 @@ def handle(self, record): assert self.record is None self.record = record + + +class RotateFiles(TestCaseBase): + def testNonExistingDir(self, persist=False): + """ + Tests that the method fails correctly when given a non existing dir. + """ + self.assertRaises(OSError, utils.rotateFiles, "/I/DONT/EXIST", + "prefix", 2, persist=persist) + + def testEmptyDir(self, persist=False): + """ + Test that when given an empty dir the rotator works correctly. + """ + prefix = "prefix" + dir = tempfile.mkdtemp() + + utils.rotateFiles(dir, prefix, 0, persist=persist) + + os.rmdir(dir) + + def testFullDir(self, persist=False): + """ + Test that rotator does it's basic functionality. + """ + #Prepare + prefix = "prefix" + stubContent = ('"Multiple exclamation marks", ' + 'he went on, shaking his head, ' + '"are a sure sign of a diseased mind."') + # (C) Terry Pratchet - Small Gods + dir = tempfile.mkdtemp() + gen = 10 + + expectedDirContent = [] + for i in range(gen): + fname = "%s.txt.%d" % (prefix, i + 1) + expectedDirContent.append("%s.txt.%d" % (prefix, i + 1)) + f = open(os.path.join(dir, fname), "wb") + f.write(stubContent) + f.flush() + f.close() + + #Rotate + utils.rotateFiles(dir, prefix, gen, persist=persist) + + #Test result + currentDirContent = os.listdir(dir) + expectedDirContent.sort() + currentDirContent.sort() + try: + self.assertEquals(currentDirContent, expectedDirContent) + finally: + #Clean + for f in os.listdir(dir): + os.unlink(os.path.join(dir, f)) + os.rmdir(dir) diff --git a/vdsm/caps.py b/vdsm/caps.py index 4f3d6f9..cdf5e1a 100644 --- a/vdsm/caps.py +++ b/vdsm/caps.py @@ -27,7 +27,6 @@ import logging import time import linecache -import glob import libvirt import rpm @@ -55,14 +54,6 @@ except ImportError: _glusterEnabled = False - -class OSName: - UNKNOWN = 'unknown' - OVIRT = 'oVirt Node' - RHEL = 'RHEL' - FEDORA = 'Fedora' - RHEVH = 'RHEV Hypervisor' - DEBIAN = 'Debian' RNG_SOURCES = {'random': '/dev/random', 'hwrng': '/dev/hwrng'} @@ -275,32 +266,16 @@ @utils.memoized -def getos(): - if os.path.exists('/etc/rhev-hypervisor-release'): - return OSName.RHEVH - elif glob.glob('/etc/ovirt-node-*-release'): - return OSName.OVIRT - elif os.path.exists('/etc/fedora-release'): - return OSName.FEDORA - elif os.path.exists('/etc/redhat-release'): - return OSName.RHEL - elif os.path.exists('/etc/debian_version'): - return OSName.DEBIAN - else: - return OSName.UNKNOWN - - -(a)utils.memoized def osversion(): version = release = '' - osname = getos() + osname = utils.getos() try: - if osname == OSName.RHEVH or osname == OSName.OVIRT: + if osname == utils.OSName.RHEVH or osname == utils.OSName.OVIRT: d = _parseKeyVal(file('/etc/default/version')) version = d.get('VERSION', '') release = d.get('RELEASE', '') - elif osname == OSName.DEBIAN: + elif osname == utils.OSName.DEBIAN: version = linecache.getline('/etc/debian_version', 1).strip("\n") release = "" # Debian just has a version entry else: @@ -403,7 +378,8 @@ pkgs = {'kernel': kernelDict()} - if getos() in (OSName.RHEVH, OSName.OVIRT, OSName.FEDORA, OSName.RHEL): + if utils.getos() in (utils.OSName.RHEVH, utils.OSName.OVIRT, + utils.OSName.FEDORA, utils.OSName.RHEL): KEY_PACKAGES = {'qemu-kvm': ('qemu-kvm', 'qemu-kvm-rhev'), 'qemu-img': ('qemu-img', 'qemu-img-rhev'), 'vdsm': ('vdsm',), @@ -434,7 +410,7 @@ except: logging.error('', exc_info=True) - elif getos() == OSName.DEBIAN and python_apt: + elif utils.getos() == utils.OSName.DEBIAN and python_apt: KEY_PACKAGES = {'qemu-kvm': 'qemu-kvm', 'qemu-img': 'qemu-utils', 'vdsm': 'vdsmd', 'spice-server': 'libspice-server1', 'libvirt': 'libvirt0', 'mom': 'mom'} @@ -454,7 +430,3 @@ logging.error('', exc_info=True) return pkgs - - -def isOvirtNode(): - return getos() in (OSName.RHEVH, OSName.OVIRT) diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 6bae0bd..0cd40f6 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -350,9 +350,6 @@ self._preparedVolumes = defaultdict(list) - if not multipath.isEnabled(): - multipath.setupMultipath() - self.__validateLvmLockingType() self.domainStateChangeCallbacks = set() diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py index 48020b6..1cc73ee 100644 --- a/vdsm/storage/misc.py +++ b/vdsm/storage/misc.py @@ -55,7 +55,6 @@ import storage_exception as se import fileUtils import logUtils -from caps import isOvirtNode IOUSER = "vdsm" DIRECTFLAG = "direct" @@ -477,66 +476,6 @@ if n < 0: raise se.InvalidParameterException(name, number) return n - - -def rotateFiles(directory, prefixName, gen, cp=False, persist=False): - log.debug("dir: %s, prefixName: %s, versions: %s" % - (directory, prefixName, gen)) - gen = int(gen) - files = os.listdir(directory) - files = glob.glob("%s*" % prefixName) - fd = {} - for fname in files: - name = fname.rsplit('.', 1) - try: - ind = int(name[1]) - except ValueError: - name[0] = fname - ind = 0 - except IndexError: - ind = 0 - except: - continue - if ind < gen: - fd[ind] = {'old': fname, 'new': name[0] + '.' + str(ind + 1)} - - keys = fd.keys() - keys.sort(reverse=True) - log.debug("versions found: %s" % (keys)) - - for key in keys: - oldName = os.path.join(directory, fd[key]['old']) - newName = os.path.join(directory, fd[key]['new']) - if isOvirtNode() and persist and not cp: - try: - execCmd([constants.EXT_UNPERSIST, oldName], logErr=False, - sudo=True) - execCmd([constants.EXT_UNPERSIST, newName], logErr=False, - sudo=True) - except: - pass - try: - if cp: - execCmd([constants.EXT_CP, oldName, newName], sudo=True) - if isOvirtNode() and persist and not os.path.exists(newName): - execCmd([constants.EXT_PERSIST, newName], logErr=False, - sudo=True) - - else: - os.rename(oldName, newName) - except: - pass - if isOvirtNode() and persist and not cp: - try: - execCmd([constants.EXT_PERSIST, newName], logErr=False, - sudo=True) - except: - pass - - -def persistFile(name): - if isOvirtNode(): - execCmd([constants.EXT_PERSIST, name], sudo=True) def parseHumanReadableSize(size): diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py index 29851f3..4ffcd1d 100644 --- a/vdsm/storage/multipath.py +++ b/vdsm/storage/multipath.py @@ -25,7 +25,6 @@ import os import errno from glob import glob -import tempfile import logging import re from collections import namedtuple @@ -37,53 +36,11 @@ import supervdsm import devicemapper -import storage_exception as se - DEV_ISCSI = "iSCSI" DEV_FCP = "FCP" DEV_MIXED = "MIXED" -MAX_CONF_COPIES = 5 - TOXIC_CHARS = '()*+?|^$.\\' - -MPATH_CONF = "/etc/multipath.conf" - -OLD_TAGS = ["# RHAT REVISION 0.2", "# RHEV REVISION 0.3", - "# RHEV REVISION 0.4", "# RHEV REVISION 0.5", - "# RHEV REVISION 0.6", "# RHEV REVISION 0.7", - "# RHEV REVISION 0.8", "# RHEV REVISION 0.9"] -MPATH_CONF_TAG = "# RHEV REVISION 1.0" -MPATH_CONF_PRIVATE_TAG = "# RHEV PRIVATE" -STRG_MPATH_CONF = ( - "\n\n" - "defaults {\n" - " polling_interval 5\n" - " getuid_callout \"%(scsi_id_path)s --whitelisted " - "--replace-whitespace --device=/dev/%%n\"\n" - " no_path_retry fail\n" - " user_friendly_names no\n" - " flush_on_last_del yes\n" - " fast_io_fail_tmo 5\n" - " dev_loss_tmo 30\n" - " max_fds 4096\n" - "}\n" - "\n" - "devices {\n" - "device {\n" - " vendor \"HITACHI\"\n" - " product \"DF.*\"\n" - " getuid_callout \"%(scsi_id_path)s --whitelisted " - "--replace-whitespace --device=/dev/%%n\"\n" - "}\n" - "device {\n" - " vendor \"COMPELNT\"\n" - " product \"Compellent Vol\"\n" - " no_path_retry fail\n" - "}\n" - "}" -) -MPATH_CONF_TEMPLATE = MPATH_CONF_TAG + STRG_MPATH_CONF log = logging.getLogger("Storage.Multipath") @@ -108,76 +65,6 @@ # Now let multipath daemon pick up new devices cmd = [constants.EXT_MULTIPATH, "-r"] misc.execCmd(cmd, sudo=True) - - -def isEnabled(): - """ - Check the multipath daemon configuration. The configuration file - /etc/multipath.conf should contain private tag in form - "RHEV REVISION X.Y" for this check to succeed. - If the tag above is followed by tag "RHEV PRIVATE" the configuration - should be preserved at all cost. - - """ - if os.path.exists(MPATH_CONF): - first = second = '' - svdsm = supervdsm.getProxy() - mpathconf = svdsm.readMultipathConf() - try: - first = mpathconf[0] - second = mpathconf[1] - except IndexError: - pass - if MPATH_CONF_PRIVATE_TAG in second: - log.info("Manual override for multipath.conf detected - " - "preserving current configuration") - if MPATH_CONF_TAG not in first: - log.warning("This manual override for multipath.conf " - "was based on downrevved template. " - "You are strongly advised to " - "contact your support representatives") - return True - - if MPATH_CONF_TAG in first: - log.debug("Current revision of multipath.conf detected, " - "preserving") - return True - - for tag in OLD_TAGS: - if tag in first: - log.info("Downrev multipath.conf detected, upgrade required") - return False - - log.debug("multipath Defaulting to False") - return False - - -def setupMultipath(): - """ - Set up the multipath daemon configuration to the known and - supported state. The original configuration, if any, is saved - """ - if os.path.exists(MPATH_CONF): - misc.rotateFiles( - os.path.dirname(MPATH_CONF), - os.path.basename(MPATH_CONF), MAX_CONF_COPIES, - cp=True, persist=True) - with tempfile.NamedTemporaryFile() as f: - f.write(MPATH_CONF_TEMPLATE % {'scsi_id_path': _scsi_id.cmd}) - f.flush() - cmd = [constants.EXT_CP, f.name, MPATH_CONF] - rc = misc.execCmd(cmd, sudo=True)[0] - if rc != 0: - raise se.MultipathSetupError() - misc.persistFile(MPATH_CONF) - - # Flush all unused multipath device maps - misc.execCmd([constants.EXT_MULTIPATH, "-F"], sudo=True) - - cmd = [constants.EXT_VDSM_TOOL, "service-reload", "multipathd"] - rc = misc.execCmd(cmd, sudo=True)[0] - if rc != 0: - raise se.MultipathReloadError() def deduceType(a, b): diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py index 3249343..877f527 100644 --- a/vdsm/storage/sd.py +++ b/vdsm/storage/sd.py @@ -30,7 +30,7 @@ import resourceFactories from resourceFactories import IMAGE_NAMESPACE, VOLUME_NAMESPACE import resourceManager as rm -from vdsm import constants +from vdsm import constants, utils import clusterlock import outOfProcess as oop from persistentDict import unicodeEncoder, unicodeDecoder @@ -702,7 +702,7 @@ def setMetadata(self, newMetadata): # Backup old md (rotate old backup files) - misc.rotateFiles(self.mdBackupDir, self.sdUUID, self.mdBackupVersions) + utils.rotateFiles(self.mdBackupDir, self.sdUUID, self.mdBackupVersions) oldMd = ["%s=%s\n" % (key, value) for key, value in self.getMetadata().copy().iteritems()] open(os.path.join(self.mdBackupDir, self.sdUUID), diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer index 59df478..a902b09 100755 --- a/vdsm/supervdsmServer +++ b/vdsm/supervdsmServer @@ -142,11 +142,6 @@ return _getLsBlk(*args, **kwargs) @logDecorator - def readMultipathConf(self): - with open(MPATH_CONF) as f: - return [x.strip("\n") for x in f.readlines()] - - @logDecorator def getScsiSerial(self, *args, **kwargs): return _getScsiSerial(*args, **kwargs) -- To view, visit http://gerrit.ovirt.org/26123 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I40f802477e39000c5cae01a496ac2d9f879ebfa8 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>

8 21

Change in vdsm[master]: virt: Move all guest agents related cleanup together
by Vinzenz Feenstra 28 Jul '14

28 Jul '14

Vinzenz Feenstra has uploaded a new change for review. Change subject: virt: Move all guest agents related cleanup together ...................................................................... virt: Move all guest agents related cleanup together This patch moves the qemu guest agent socket cleanup together with the ovirt guest agent related cleanup. Change-Id: I99da4b5b9dcb160974205ebbb3c6d6727415e617 Signed-off-by: Vinzenz Feenstra <vfeenstr(a)redhat.com> --- M vdsm/virt/vm.py 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/80/26280/1 diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 944558e..a409752 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -2777,9 +2777,9 @@ except Exception: pass - def _cleanupGuestAgent(self): + def _cleanupGuestAgents(self): """ - Try to stop the guest agent and clean up its socket + Try to stop the guest agent and clean up all guest agent sockets """ try: self.guestAgent.stop() @@ -2787,6 +2787,7 @@ pass self._guestSockCleanup(self._guestSocketFile) + self._guestSockCleanup(self._qemuguestSocketFile) def setDownStatus(self, code, exitReasonCode, exitMessage=''): if not exitMessage: @@ -3073,9 +3074,8 @@ """ self._cleanupDrives() self._cleanupFloppy() - self._cleanupGuestAgent() + self._cleanupGuestAgents() utils.rmFile(self._recoveryFile) - self._guestSockCleanup(self._qemuguestSocketFile) def updateGuestCpuRunning(self): self._guestCpuRunning = (self._dom.info()[0] == -- To view, visit http://gerrit.ovirt.org/26280 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I99da4b5b9dcb160974205ebbb3c6d6727415e617 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com>

6 18

Change in vdsm[master]: block: add reduceStorageDomain command
by Federico Simoncelli 28 Jul '14

28 Jul '14

Federico Simoncelli has uploaded a new change for review. Change subject: block: add reduceStorageDomain command ...................................................................... block: add reduceStorageDomain command Change-Id: I467fc12d3787929b9c0e35f8806402f72f493368 Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M client/vdsClient.py M vdsm/API.py M vdsm/BindingXMLRPC.py M vdsm/storage/blockSD.py M vdsm/storage/hsm.py M vdsm/storage/lvm.py M vdsm/storage/sp.py M vdsm/storage/storage_exception.py M vdsm_api/vdsmapi-schema.json 9 files changed, 86 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/26574/1 diff --git a/client/vdsClient.py b/client/vdsClient.py index b324eb1..ccbdb15 100644 --- a/client/vdsClient.py +++ b/client/vdsClient.py @@ -678,6 +678,15 @@ return dom['status']['code'], dom['status']['message'] return 0, '' + def reduceStorageDomain(self, args): + sdUUID = args[0] + spUUID = args[1] + devList = args[2].split(',') + dom = self.s.reduceStorageDomain(sdUUID, spUUID, devList) + if dom['status']['code']: + return dom['status']['code'], dom['status']['message'] + return 0, '' + def discoverST(self, args): portal = args[0].split(":") ip = portal[0] @@ -2067,6 +2076,11 @@ 'Extend the Storage Domain by adding devices' ' devlist (list of dev GUIDs)' )), + 'reduceStorageDomain': (serv.reduceStorageDomain, ( + '<sdUUID> <spUUID> <devlist>', + 'Reduce the Storage Domain by removing devices devlist (list of ' + 'dev GUIDs)' + )), 'discoverST': (serv.discoverST, ('ip[:port] [username password]', 'Discover the available iSCSI targetnames on a ' diff --git a/vdsm/API.py b/vdsm/API.py index 94b39b6..af22e50 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -961,6 +961,9 @@ return self._irs.extendStorageDomain(self._UUID, spUUID, devlist, force) + def reduce(self, spUUID, devlist): + return self._irs.reduceStorageDomain(self._UUID, spUUID, devlist) + def format(self, autoDetach): return self._irs.formatStorageDomain(self._UUID, autoDetach) diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py index 76251f5..4def574 100644 --- a/vdsm/BindingXMLRPC.py +++ b/vdsm/BindingXMLRPC.py @@ -546,6 +546,10 @@ domain = API.StorageDomain(sdUUID) return domain.extend(spUUID, devlist, force) + def domainReduce(self, sdUUID, spUUID, devlist, options=None): + domain = API.StorageDomain(sdUUID) + return domain.reduce(spUUID, devlist) + def domainFormat(self, sdUUID, autoDetach=False, options=None): domain = API.StorageDomain(sdUUID) @@ -949,6 +953,7 @@ (self.domainDetach, 'detachStorageDomain'), (self.domainDetachForced, 'forcedDetachStorageDomain'), (self.domainExtend, 'extendStorageDomain'), + (self.domainReduce, 'reduceStorageDomain'), (self.domainFormat, 'formatStorageDomain'), (self.domainGetFileStats, 'getFileStats'), (self.domainGetImages, 'getImagesList'), diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index f807d3e..c31bb97 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -734,6 +734,10 @@ else: return self.getFreeMetadataSlot(slotSize) + def reduce(self, devlist): + with self._extendlock: + lvm.reduceVG(self.sdUUID, devlist) + def _getOccupiedMetadataSlots(self): stripPrefix = lambda s, pfx: s[len(pfx):] occupiedSlots = [] diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 0ebe129..93ec28c 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -753,6 +753,14 @@ pool.extendSD(sdUUID, dmDevs, force) @public + def reduceStorageDomain(self, sdUUID, spUUID, guids): + vars.task.getSharedLock(STORAGE, sdUUID) + pool = self.getPool(spUUID) + dmDevs = tuple(os.path.join(devicemapper.DMPATH_PREFIX, guid) for guid + in guids) + pool.reduceSD(sdUUID, dmDevs) + + @public def forcedDetachStorageDomain(self, sdUUID, spUUID, options=None): """Forced detach a storage domain from a storage pool. This removes the storage domain entry in the storage pool meta-data diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py index d36c505..4b6015e 100644 --- a/vdsm/storage/lvm.py +++ b/vdsm/storage/lvm.py @@ -49,7 +49,7 @@ LVM_DEFAULT_TTL = 100 PV_FIELDS = ("uuid,name,size,vg_name,vg_uuid,pe_start,pe_count," - "pe_alloc_count,mda_count,dev_size") + "pe_alloc_count,mda_count,dev_size,pv_mda_used_count") VG_FIELDS = ("uuid,name,attr,size,free,extent_size,extent_count,free_count," "tags,vg_mda_size,vg_mda_free,lv_count,pv_count,pv_name") LV_FIELDS = "uuid,name,vg_name,attr,size,seg_start_pe,devices,tags" @@ -966,6 +966,28 @@ raise se.VolumeGroupExtendError(vgName, pvs) +def reduceVG(vgName, devices): + pvs = [pdev for pdev in _normalizeargs(devices)] + vgpvs = _lvminfo._getVGDevs((vgName,)) + + for pv in pvs: + if pv not in vgpvs: + raise se.BlockDeviceActionError( + 'Phisical device %s not in vg %s' % (pv, vgName)) + if int(getPV(pv).pv_mda_used_count) != 0: + raise se.BlockDeviceActionError( + 'Phisical device %s contains the lvm metadata') + + cmd = ["vgreduce", vgName] + pvs + rc, out, err = _lvminfo.cmd(cmd, vgpvs) + if rc == 0: + _lvminfo._invalidatepvs(vgpvs) + _lvminfo._invalidatevgs(vgName) + log.debug("Cache after reducing vg %s", _lvminfo._vgs) + else: + raise se.VolumeGroupReduceError(vgName, pvs) + + def chkVG(vgName): cmd = ["vgck", vgName] rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, ))) diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 338232f..f4d4987 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -1862,6 +1862,10 @@ self.validatePoolSD(sdUUID) sdCache.produce(sdUUID).extend(devlist, force) + def reduceSD(self, sdUUID, devlist): + self.validatePoolSD(sdUUID) + sdCache.produce(sdUUID).reduce(devlist) + def setSDDescription(self, sd, description): self.validatePoolSD(sd.sdUUID) sd.setDescription(descr=description) diff --git a/vdsm/storage/storage_exception.py b/vdsm/storage/storage_exception.py index b4f3b66..226508a 100644 --- a/vdsm/storage/storage_exception.py +++ b/vdsm/storage/storage_exception.py @@ -1504,6 +1504,13 @@ issue and how to resolve it""" +class VolumeGroupReduceError(StorageException): + def __init__(self, vgname, devname): + self.value = "vgname=%s, devname=%s" % (vgname, devname) + code = 503 + message = "Cannot reduce Volume Group" + + ################################################# # SPM/HSM Exceptions ################################################# diff --git a/vdsm_api/vdsmapi-schema.json b/vdsm_api/vdsmapi-schema.json index 31bd869..61fc66c 100644 --- a/vdsm_api/vdsmapi-schema.json +++ b/vdsm_api/vdsmapi-schema.json @@ -4169,6 +4169,24 @@ 'devlist': ['str'], '*force': 'bool'}} ## +# @StorageDomain.reduce: +# +# Reduce a block-based Storage Domain freeing block devices that are not in +# use (no lvm metadata and lvs). +# +# @storagedomainID: The UUID of the Storage Domain +# +# @storagepoolID: The UUID of the Storage Pool +# +# @devlist: An array of block device names to remove from the domain +# +# Since: 4.14.0 +## +{'command': {'class': 'StorageDomain', 'name': 'reduce'}, + 'data': {'storagedomainID': 'UUID', 'storagepoolID': 'UUID', + 'devlist': ['str']}} + +## # @StorageDomain.format: # # Format a storage domain and erase all of its data. -- To view, visit http://gerrit.ovirt.org/26574 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I467fc12d3787929b9c0e35f8806402f72f493368 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>

3 3

Change in vdsm[master]: ifcfg: Log unhandled exception for new Thread
by Maor Lipchuk 28 Jul '14

28 Jul '14

Maor Lipchuk has uploaded a new change for review. Change subject: ifcfg: Log unhandled exception for new Thread ...................................................................... ifcfg: Log unhandled exception for new Thread Ading a traceback log for unhandled exceptions, when openning a new thread, so it will not die silently. Since the log in ifcfg instance is inaccessible from the decorator, we use the default root logger. Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba Signed-off-by: Maor Lipchuk <mlipchuk(a)redhat.com> --- M vdsm/netconf/ifcfg.py 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/85/23085/1 diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py index 52b95b1..8431e80 100644 --- a/vdsm/netconf/ifcfg.py +++ b/vdsm/netconf/ifcfg.py @@ -738,6 +738,7 @@ def ifup(iface, async=False): "Bring up an interface" + @utils.traceback() def _ifup(netIf): rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False) -- To view, visit http://gerrit.ovirt.org/23085 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com>

6 8

← Newer
1
...
5
6
7
8
9
10
11
...
65
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches May 2014