Change in vdsm[ovirt-3.3]: Read pool metadata once in StoragePool.getInfo()
by Federico Simoncelli
Hello Dan Kenigsberg, Eduardo,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/20191
to review the following change.
Change subject: Read pool metadata once in StoragePool.getInfo()
......................................................................
Read pool metadata once in StoragePool.getInfo()
The behavioural change of spm_id should be safe since
StoragePool.getInfo() call in hsm.getStoragePoolInfo() is under
the (shared) pool lock making startSpm block in this host when
the info is retrieved.
Making repoStats pool independent.
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1019273
Change-Id: I41a79662a4bd01fc310aa5554c38a16f3f8ba546
Signed-off-by: Eduardo <ewarszaw(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/14672
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm/storage/sp.py
1 file changed, 15 insertions(+), 13 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/91/20191/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 8bab500..7dab69d 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -1496,26 +1496,28 @@
raise se.StoragePoolMasterNotFound(self.spUUID, msdUUID)
try:
- poolInfo = {
- 'type': msdInfo['type'],
- 'name': self.getDescription(),
- 'domains': domainListEncoder(self.getDomains()),
- 'master_uuid': msdUUID,
- 'master_ver': self.getMasterVersion(),
- 'lver': self.getMetaParam(PMDK_LVER),
- 'spm_id': self.getSpmId(),
- 'pool_status': 'uninitialized',
- 'version': str(self.getVersion()),
- 'isoprefix': '',
- }
+ pmd = self._getPoolMD(self.masterDomain)
except Exception:
self.log.error("Pool metadata error", exc_info=True)
raise se.StoragePoolActionError(self.spUUID)
+ poolInfo = {
+ 'type': msdInfo['type'],
+ 'name': pmd[PMDK_POOL_DESCRIPTION],
+ 'domains': domainListEncoder(pmd[PMDK_DOMAINS]),
+ 'master_uuid': self.masterDomain.sdUUID,
+ 'master_ver': pmd[PMDK_MASTER_VER],
+ 'lver': pmd[PMDK_LVER],
+ 'spm_id': pmd[PMDK_SPM_ID],
+ 'pool_status': 'uninitialized',
+ 'version': str(msdInfo['version']),
+ 'isoprefix': '',
+ }
+
domInfo = {}
repoStats = self.getRepoStats()
- for sdUUID, sdStatus in self.getDomains().iteritems():
+ for sdUUID, sdStatus in pmd[PMDK_DOMAINS].iteritems():
# Return statistics for active domains only
domInfo[sdUUID] = {'status': sdStatus, 'alerts': []}
--
To view, visit http://gerrit.ovirt.org/20191
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I41a79662a4bd01fc310aa5554c38a16f3f8ba546
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
10 years, 3 months
Change in vdsm[ovirt-3.3]: Make hsm.getVolumesList() pool independent.
by Federico Simoncelli
Hello Ayal Baron, Eduardo,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/20190
to review the following change.
Change subject: Make hsm.getVolumesList() pool independent.
......................................................................
Make hsm.getVolumesList() pool independent.
Related to BZ#960952.
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1019273
Change-Id: Ib810ca24818fa4a77905032694a05c0d86ef75e2
Signed-off-by: Eduardo <ewarszaw(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/15766
Reviewed-by: Ayal Baron <abaron(a)redhat.com>
---
M vdsm/storage/hsm.py
1 file changed, 5 insertions(+), 12 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/90/20190/1
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 061182d..ace6634 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -3281,8 +3281,7 @@
"""
Gets a list of all volumes.
- :param spUUID: The UUID of the storage pool that manages the storage
- domain you want to query.
+ :param spUUID: Unused.
:type spUUID: UUID
:param sdUUID: The UUID of the storage domain you want to query.
:type sdUUID: UUID
@@ -3293,18 +3292,12 @@
"""
vars.task.getSharedLock(STORAGE, sdUUID)
dom = sdCache.produce(sdUUID=sdUUID)
+ vols = dom.getAllVolumes()
if imgUUID == volume.BLANK_UUID:
- images = list(dom.getAllImages())
+ volUUIDs = vols.keys()
else:
- images = [imgUUID]
-
- uuidlist = []
- repoPath = os.path.join(self.storage_repository, spUUID)
- for img in images:
- uuidlist += (dom.getVolumeClass().
- getImageVolumes(repoPath, sdUUID, img))
- self.log.info("List of volumes is %s", uuidlist)
- return dict(uuidlist=uuidlist)
+ volUUIDs = [k for k, v in vols.iteritems() if imgUUID in v.imgs]
+ return dict(uuidlist=volUUIDs)
@public
def getImagesList(self, sdUUID, options=None):
--
To view, visit http://gerrit.ovirt.org/20190
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib810ca24818fa4a77905032694a05c0d86ef75e2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
10 years, 3 months
Change in vdsm[ovirt-3.3]: vm: add the transient disk support
by Federico Simoncelli
Hello Dan Kenigsberg, Deepak C Shetty,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/20189
to review the following change.
Change subject: vm: add the transient disk support
......................................................................
vm: add the transient disk support
A transient disk is a temporary qcow layer based on a (usually
internal) volume that is attached to a VM in read-write mode.
All changes stored in the transient disk will be lost as soon
as the disk is hot-unplugged or when the VM exits.
Its primary use is to access in read-write mode a snapshot
(read-only volume).
Libvirt doesn't support transient disks yet, hence this patch
includes also all the required parts to fill that gap.
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1019205
Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524
Signed-off-by: Deepak C Shetty <deepakcs(a)linux.vnet.ibm.com>
Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/18326
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M init/vdsmd_init_common.sh.in
M lib/vdsm/config.py.in
M lib/vdsm/define.py
M lib/vdsm/tool/Makefile.am
A lib/vdsm/tool/transient.py
M tests/vmTests.py
M vdsm.spec.in
M vdsm/vm.py
8 files changed, 196 insertions(+), 20 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/20189/1
diff --git a/init/vdsmd_init_common.sh.in b/init/vdsmd_init_common.sh.in
index 38da542..4fef62c 100644
--- a/init/vdsmd_init_common.sh.in
+++ b/init/vdsmd_init_common.sh.in
@@ -59,6 +59,12 @@
}
+task_prepare_transient_repository() {
+ "$VDSM_TOOL" setup-transient-repository
+ "$VDSM_TOOL" cleanup-transient-repository
+}
+
+
task_syslog_available() {
if ! [ -S "/dev/log" ]; then
echo "Cannot communicate with syslog deamon for reports. " \
@@ -203,8 +209,9 @@
--pre-start)
run_tasks " \
run_init_hooks gencerts reconfigure_sanlock reconfigure_libvirt \
- syslog_available nwfilter dummybr load_needed_modules \
- tune_system mkdirs test_space test_lo test_conflicting_conf"
+ prepare_transient_repository syslog_available nwfilter dummybr \
+ load_needed_modules tune_system mkdirs test_space test_lo \
+ test_conflicting_conf"
;;
--post-stop)
run_tasks "run_final_hooks"
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index a229998..f244510 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -154,6 +154,9 @@
('libvirt_env_variable_log_outputs', '',
'Specify the output to track libvirt calls'),
+
+ ('transient_disks_repository', '@VDSMLIBDIR@/transient',
+ 'Local path to the transient disks repository.'),
]),
# Section: [ksm]
diff --git a/lib/vdsm/define.py b/lib/vdsm/define.py
index e38fce1..eb78633 100644
--- a/lib/vdsm/define.py
+++ b/lib/vdsm/define.py
@@ -129,6 +129,9 @@
'resizeErr': {'status': {
'code': 58,
'message': 'Wrong resize disk parameter'}},
+ 'transientErr': {'status': {
+ 'code': 59,
+ 'message': 'Action not permitted on a VM with transient disks'}},
'recovery': {'status': {
'code': 99,
'message': 'Recovering from crash or Initializing'}},
diff --git a/lib/vdsm/tool/Makefile.am b/lib/vdsm/tool/Makefile.am
index 9f00984..7a4e095 100644
--- a/lib/vdsm/tool/Makefile.am
+++ b/lib/vdsm/tool/Makefile.am
@@ -43,6 +43,7 @@
sanlock.py \
seboolsetup.py \
service.py \
+ transient.py \
vdsm-id.py \
$(NULL)
diff --git a/lib/vdsm/tool/transient.py b/lib/vdsm/tool/transient.py
new file mode 100644
index 0000000..7452f30
--- /dev/null
+++ b/lib/vdsm/tool/transient.py
@@ -0,0 +1,86 @@
+# Copyright 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+import os
+import glob
+import pwd
+import selinux
+
+from vdsm import constants
+from vdsm.config import config
+from vdsm.tool import expose
+from vdsm.utils import execCmd, CommandPath
+
+
+SELINUX_VIRT_IMAGE_LABEL = "system_u:object_r:virt_image_t:s0"
+TRANSIENT_DISKS_REPO = config.get('vars', 'transient_disks_repository')
+
+_fuser = CommandPath(
+ "fuser",
+ "/sbin/fuser", # Fedora, EL6
+)
+
+
+@expose("setup-transient-repository")
+def setup_transient_repository(*args):
+ """
+ Prepare the transient disks repository
+ """
+ _, _, vdsm_uid, vdsm_gid, _, _, _ = pwd.getpwnam(constants.VDSM_USER)
+
+ try:
+ os.makedirs(TRANSIENT_DISKS_REPO)
+ except OSError as e:
+ if e.errno != os.errno.EEXIST:
+ raise
+
+ os.chown(TRANSIENT_DISKS_REPO, vdsm_uid, vdsm_gid)
+ os.chmod(TRANSIENT_DISKS_REPO, 0750)
+ selinux.chcon(TRANSIENT_DISKS_REPO, SELINUX_VIRT_IMAGE_LABEL)
+
+
+@expose("cleanup-transient-repository")
+def cleanup_transient_repository(*args):
+ """
+ Cleanup the unused transient disks present in the repository.
+ (NOTE: it is recommended to NOT execute this command when the vdsm
+ daemon is running)
+ """
+ transient_images = set(glob.glob(os.path.join(TRANSIENT_DISKS_REPO, "*")))
+
+ if len(transient_images) == 0:
+ return # Nothing to do
+
+ cmd_ret, cmd_out, cmd_err = execCmd([_fuser.cmd] + list(transient_images))
+ # According to: "fuser returns a non-zero return code if none of the
+ # specified files is accessed or in case of a fatal error. If at least
+ # one access has been found, fuser returns zero." we can discard the
+ # return code.
+ # NOTE: the list of open files is printed to cmd_err with an extra ":"
+ # character appended (removed by [:-1]).
+ open_transient_images = set(x[:-1] for x in cmd_err)
+
+ for image_path in transient_images - open_transient_images:
+ # NOTE: This could cause a race with the creation of a virtual
+ # machine with a transient disk (if vdsm is running).
+ try:
+ os.unlink(image_path)
+ except OSError as e:
+ if e.errno != os.errno.ENOENT:
+ raise
diff --git a/tests/vmTests.py b/tests/vmTests.py
index c03cd5f..1f69f0a 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -367,6 +367,7 @@
{},
# New extended values
{'shared': 'exclusive'}, {'shared': 'shared'}, {'shared': 'none'},
+ {'shared': 'transient'},
]
expectedStates = [
@@ -375,7 +376,7 @@
# Missing shared definition
'none',
# New extended values
- 'exclusive', 'shared', 'none',
+ 'exclusive', 'shared', 'none', 'transient',
]
driveConfig = {'index': '0', 'iface': 'virtio', 'device': 'disk'}
diff --git a/vdsm.spec.in b/vdsm.spec.in
index e175ff8..a9e8f7d 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1053,6 +1053,7 @@
%{python_sitearch}/%{vdsm_name}/tool/sanlock.py*
%{python_sitearch}/%{vdsm_name}/tool/seboolsetup.py*
%{python_sitearch}/%{vdsm_name}/tool/service.py*
+%{python_sitearch}/%{vdsm_name}/tool/transient.py*
%{python_sitearch}/%{vdsm_name}/tool/validate_ovirt_certs.py*
%{python_sitearch}/%{vdsm_name}/tool/vdsm-id.py*
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 1a16d53..244e7b0 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -428,11 +428,12 @@
NONE = "none"
EXCLUSIVE = "exclusive"
SHARED = "shared"
+ TRANSIENT = "transient"
@classmethod
def getAllValues(cls):
# TODO: use introspection
- return (cls.NONE, cls.EXCLUSIVE, cls.SHARED)
+ return (cls.NONE, cls.EXCLUSIVE, cls.SHARED, cls.TRANSIENT)
# These strings are representing libvirt virDomainEventType values
@@ -1341,6 +1342,7 @@
self.truesize = int(kwargs.get('truesize', '0'))
self.apparentsize = int(kwargs.get('apparentsize', '0'))
self.name = self._makeName()
+ self.cache = config.get('vars', 'qemu_drive_cache')
if self.device in ("cdrom", "floppy"):
self._blockDev = False
@@ -1441,15 +1443,25 @@
"block device", self.path, exc_info=True)
return self._blockDev
+ @property
+ def transientDisk(self):
+ # Using getattr to handle legacy and removable drives.
+ return getattr(self, 'shared', None) == DRIVE_SHARED_TYPE.TRANSIENT
+
def _customize(self):
- # Customize disk device
- if self.iface == 'virtio':
+ if self.transientDisk:
+ # Force the cache to be writethrough, which is qemu's default.
+ # This is done to ensure that we don't ever use cache=none for
+ # transient disks, since we create them in /var/run/vdsm which
+ # may end up on tmpfs and don't support O_DIRECT, and qemu uses
+ # O_DIRECT when cache=none and hence hotplug might fail with
+ # error that one can take eternity to debug the reason behind it!
+ self.cache = "writethrough"
+ elif self.iface == 'virtio':
try:
self.cache = self.conf['custom']['viodiskcache']
except KeyError:
- self.cache = config.get('vars', 'qemu_drive_cache')
- else:
- self.cache = config.get('vars', 'qemu_drive_cache')
+ pass # Ignore if custom disk cache is missing
def _makeName(self):
devname = {'ide': 'hd', 'scsi': 'sd', 'virtio': 'vd', 'fdc': 'fd'}
@@ -2159,6 +2171,10 @@
# Now we got all the resources we needed
self.startDisksStatsCollection()
+ def _prepareTransientDisks(self, drives):
+ for drive in drives:
+ self._createTransientDisk(drive)
+
def _onQemuDeath(self):
self.log.info('underlying process disconnected')
# Try release VM resources first, if failed stuck in 'Powering Down'
@@ -2482,6 +2498,13 @@
with self._volPrepareLock:
for drive in drives:
try:
+ self._removeTransientDisk(drive)
+ except:
+ self.log.warning("Drive transient volume deletion failed "
+ "for drive %s", drive, exc_info=True)
+ # Skip any exception as we don't want to interrupt the
+ # teardown process for any reason.
+ try:
self.cif.teardownVolumePath(drive)
except:
self.log.error("Drive teardown failure for %s",
@@ -2644,12 +2667,20 @@
def isMigrating(self):
return self._migrationSourceThread.isAlive()
+ def hasTransientDisks(self):
+ for drive in self._devices[DISK_DEVICES]:
+ if drive.transientDisk:
+ return True
+ return False
+
def migrate(self, params):
self._acquireCpuLockWithTimeout()
try:
if self.isMigrating():
self.log.warning('vm already migrating')
return errCode['exist']
+ if self.hasTransientDisks():
+ return errCode['transientErr']
# while we were blocking, another migrationSourceThread could have
# taken self Down
if self._lastStatus == 'Down':
@@ -2871,6 +2902,7 @@
if not 'recover' in self.conf:
devices = self.buildConfDevices()
self.preparePaths(devices[DISK_DEVICES])
+ self._prepareTransientDisks(devices[DISK_DEVICES])
# Update self.conf with updated devices
# For old type vmParams, new 'devices' key will be
# created with all devices info
@@ -3266,6 +3298,40 @@
params=customProps)
return {'status': doneCode, 'vmList': self.status()}
+ def _createTransientDisk(self, diskParams):
+ if diskParams.get('shared', None) != DRIVE_SHARED_TYPE.TRANSIENT:
+ return
+
+ # FIXME: This should be replaced in future the support for transient
+ # disk in libvirt (BZ#832194)
+ driveFormat = (
+ qemuImg.FORMAT.QCOW2 if diskParams['format'] == 'cow' else
+ qemuImg.FORMAT.RAW
+ )
+
+ transientHandle, transientPath = tempfile.mkstemp(
+ dir=config.get('vars', 'transient_disks_repository'),
+ prefix="%s-%s." % (diskParams['domainID'], diskParams['volumeID']))
+
+ try:
+ qemuImg.create(transientPath, format=qemuImg.FORMAT.QCOW2,
+ backing=diskParams['path'],
+ backingFormat=driveFormat)
+ os.fchmod(transientHandle, 0660)
+ except:
+ os.unlink(transientPath) # Closing after deletion is correct
+ self.log.error("Failed to create the transient disk for "
+ "volume %s", diskParams['volumeID'], exc_info=True)
+ finally:
+ os.close(transientHandle)
+
+ diskParams['path'] = transientPath
+ diskParams['format'] = 'cow'
+
+ def _removeTransientDisk(self, drive):
+ if drive.transientDisk:
+ os.unlink(drive.path)
+
def hotplugDisk(self, params):
if self.isMigrating():
return errCode['migInProgress']
@@ -3276,6 +3342,7 @@
if vdsmImg:
self._normalizeVdsmImg(diskParams)
+ self._createTransientDisk(diskParams)
self.updateDriveIndex(diskParams)
drive = Drive(self.conf, self.log, **diskParams)
@@ -3324,17 +3391,15 @@
diskParams = params.get('drive', {})
diskParams['path'] = self.cif.prepareVolumePath(diskParams)
- # Find disk object in vm's drives list
- for drv in self._devices[DISK_DEVICES][:]:
- if drv.path == diskParams['path']:
- drive = drv
- break
- else:
+ try:
+ drive = self._findDriveByUUIDs(diskParams)
+ except LookupError:
self.log.error("Hotunplug disk failed - Disk not found: %s",
diskParams)
- return {'status': {'code': errCode['hotunplugDisk']
- ['status']['code'],
- 'message': "Disk not found"}}
+ return {'status': {
+ 'code': errCode['hotunplugDisk']['status']['code'],
+ 'message': "Disk not found"
+ }}
if drive.hasVolumeLeases:
return errCode['noimpl']
@@ -3350,7 +3415,7 @@
diskDev = None
for dev in self.conf['devices'][:]:
if (dev['type'] == DISK_DEVICES and
- dev['path'] == diskParams['path']):
+ dev['path'] == drive.path):
with self._confLock:
self.conf['devices'].remove(dev)
diskDev = dev
@@ -3600,6 +3665,9 @@
if self.isMigrating():
return errCode['migInProgress']
+ if self.hasTransientDisks():
+ return errCode['transientErr']
+
for drive in snapDrives:
baseDrv, tgetDrv = _normSnapDriveParams(drive)
@@ -3794,7 +3862,7 @@
if not mergeDrive or not hasattr(mergeDrive, 'volumeChain'):
mergeStatus['status'] = MERGESTATUS.DRIVE_NOT_FOUND
- elif mergeDrive.hasVolumeLeases:
+ elif mergeDrive.hasVolumeLeases or mergeDrive.transientDisk:
mergeStatus['status'] = MERGESTATUS.DRIVE_NOT_SUPPORTED
else:
for volume in mergeDrive.volumeChain:
@@ -3894,6 +3962,9 @@
if srcDrive.hasVolumeLeases:
return errCode['noimpl']
+ if srcDrive.transientDisk:
+ return errCode['transientErr']
+
try:
self._setDiskReplica(srcDrive, dstDisk)
except:
@@ -3944,6 +4015,9 @@
if srcDrive.hasVolumeLeases:
return errCode['noimpl']
+ if srcDrive.transientDisk:
+ return errCode['transientErr']
+
if not srcDrive.isDiskReplicationInProgress():
return errCode['replicaErr']
--
To view, visit http://gerrit.ovirt.org/20189
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Deepak C Shetty <deepakcs(a)linux.vnet.ibm.com>
10 years, 3 months
Change in vdsm[ovirt-3.4]: spec: vdsm pkg for EL distro must include pkla
by Douglas Schilling Landgraf
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/24139
to review the following change.
Change subject: spec: vdsm pkg for EL distro must include pkla
......................................................................
spec: vdsm pkg for EL distro must include pkla
vdsm on EL system should ship the .pkla file or libvirt will
keep asking password and vdsm won't be able to start correctly.
Originally the commit f824941b has added into %files session
the polkit files but on a %else conditional for %0{?rhel} and
this code contains %else for %if 0%{?fedora} >= 18 which won't be EL as expected.
Additionaly, a new commit bae85d50 added %with_systemd macro and triggered
this bug inserting new %if 0%{?rhel}/%else which contains %if 0%{?fedora} >= 18
and %else but won't work too for EL.
Change-Id: I298f21ee6a3e5f2fdf78939ea421a2152490972b
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1060524
Signed-off-by: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/24083
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm.spec.in
1 file changed, 8 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/39/24139/1
diff --git a/vdsm.spec.in b/vdsm.spec.in
index d4ebc43..149793e 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -656,19 +656,21 @@
# This is not commonplace, but we want /var/log/core to be a world-writable
# dropbox for core dumps
install -dDm 1777 %{buildroot}%{_localstatedir}/log/core
-%else
+%endif
+
+%if !0%{?rhel}
# Install the configuration sample
install -Dm 0644 lib/vdsm/vdsm.conf.sample \
- %{buildroot}%{_sysconfdir}/vdsm/vdsm.conf
+ %{buildroot}%{_sysconfdir}/%{vdsm_name}/vdsm.conf
+%endif
# Install the polkit for libvirt
-%if 0%{?fedora} >= 18
+%if 0%{?fedora} >= 18 || 0%{?rhel} >= 7
install -Dm 0644 vdsm/vdsm-libvirt-access.rules \
%{buildroot}%{_polkitdir}/10-vdsm-libvirt-access.rules
%else
install -Dm 0644 vdsm/vdsm-libvirt-access.pkla \
%{buildroot}%{_polkitdir}/10-vdsm-libvirt-access.pkla
-%endif
%endif
%check
@@ -1086,12 +1088,11 @@
%{_mandir}/man8/vdsmd.8*
%if 0%{?rhel}
%dir %{_localstatedir}/log/core
-%else
-%if 0%{?fedora} >= 18
+%endif
+%if 0%{?fedora} >= 18 || 0%{?rhel} >= 7
%{_polkitdir}/10-vdsm-libvirt-access.rules
%else
%{_polkitdir}/10-vdsm-libvirt-access.pkla
-%endif
%endif
%defattr(-, %{vdsm_user}, %{qemu_group}, -)
--
To view, visit http://gerrit.ovirt.org/24139
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I298f21ee6a3e5f2fdf78939ea421a2152490972b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
10 years, 3 months
Change in vdsm[master]: spec: vdsm pkg for EL distro must include pkla
by Douglas Schilling Landgraf
Douglas Schilling Landgraf has uploaded a new change for review.
Change subject: spec: vdsm pkg for EL distro must include pkla
......................................................................
spec: vdsm pkg for EL distro must include pkla
vdsm on EL system should ship the .pkla file or libvirt will
keep asking password and vdsm won't be able to start correctly.
Change-Id: I298f21ee6a3e5f2fdf78939ea421a2152490972b
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1060524
Signed-off-by: Douglas Schilling Landgraf <dougsland(a)redhat.com>
---
M vdsm.spec.in
1 file changed, 5 insertions(+), 8 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/83/24083/1
diff --git a/vdsm.spec.in b/vdsm.spec.in
index d4ebc43..7f59f5f 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -656,10 +656,11 @@
# This is not commonplace, but we want /var/log/core to be a world-writable
# dropbox for core dumps
install -dDm 1777 %{buildroot}%{_localstatedir}/log/core
-%else
+%endif
+
# Install the configuration sample
install -Dm 0644 lib/vdsm/vdsm.conf.sample \
- %{buildroot}%{_sysconfdir}/vdsm/vdsm.conf
+ %{buildroot}%{_sysconfdir}/%{vdsm_name}/vdsm.conf
# Install the polkit for libvirt
%if 0%{?fedora} >= 18
@@ -668,7 +669,6 @@
%else
install -Dm 0644 vdsm/vdsm-libvirt-access.pkla \
%{buildroot}%{_polkitdir}/10-vdsm-libvirt-access.pkla
-%endif
%endif
%check
@@ -928,6 +928,7 @@
%{_datadir}/%{vdsm_name}/vdsm-store-net-config
%{_datadir}/%{vdsm_name}/vm.py*
+%config(noreplace) %{_sysconfdir}/%{vdsm_name}/vdsm.conf
%config(noreplace) %{_sysconfdir}/%{vdsm_name}/logger.conf
%config(noreplace) %{_sysconfdir}/%{vdsm_name}/svdsm.logger.conf
%config(noreplace) %{_sysconfdir}/%{vdsm_name}/upgrade.logger.conf
@@ -1086,12 +1087,11 @@
%{_mandir}/man8/vdsmd.8*
%if 0%{?rhel}
%dir %{_localstatedir}/log/core
-%else
+%endif
%if 0%{?fedora} >= 18
%{_polkitdir}/10-vdsm-libvirt-access.rules
%else
%{_polkitdir}/10-vdsm-libvirt-access.pkla
-%endif
%endif
%defattr(-, %{vdsm_user}, %{qemu_group}, -)
@@ -1117,9 +1117,6 @@
%files python
%defattr(-, root, root, -)
%{_bindir}/vdsm-tool
-%if !0%{?rhel}
-%config(noreplace) %{_sysconfdir}/%{vdsm_name}/vdsm.conf
-%endif
%dir %{python_sitearch}/%{vdsm_name}
%dir %{python_sitearch}/%{vdsm_name}/tool
%{python_sitearch}/%{vdsm_name}/__init__.py*
--
To view, visit http://gerrit.ovirt.org/24083
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I298f21ee6a3e5f2fdf78939ea421a2152490972b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
10 years, 3 months
Change in vdsm[master]: netinfo: Determine bootproto without ifcfg files
by osvoboda＠redhat.com
Ondřej Svoboda has uploaded a new change for review.
Change subject: netinfo: Determine bootproto without ifcfg files
......................................................................
netinfo: Determine bootproto without ifcfg files
This is an initial take with comments on IPv4/6 and completeness.
Change-Id: I5fbc48a0adf5f40120a72ec2c4cc2fc80b7226b8
Bug-Url: https://bugzilla.redhat.com/??????
Signed-off-by: Ondřej Svoboda <osvoboda(a)redhat.com>
---
M lib/vdsm/netinfo.py
1 file changed, 47 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/98/23098/1
diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py
index 6d0c5ff..2ea3236 100644
--- a/lib/vdsm/netinfo.py
+++ b/lib/vdsm/netinfo.py
@@ -40,7 +40,7 @@
from .ipwrapper import routeShowAllDefaultGateways
from . import libvirtconnection
from .ipwrapper import linkShowDev
-from .utils import anyFnmatch
+from .utils import anyFnmatch, pgrep, getCmdArgs
from .netconfpersistence import RunningConfig
@@ -447,6 +447,44 @@
raise NotImplementedError
+def getBootProtocolDynamic(iface):
+ logging.info('Looking for DHCP client for %s.', iface)
+
+ for pid in pgrep('dhclient'):
+ if iface in getCmdArgs(pid):
+ logging.info('Found dhclient running for %s.', iface)
+ return 'dhcp'
+
+ for pid in pgrep('dhcpcd'):
+ if iface in getCmdArgs(pid):
+ logging.info('Found dhcpcd running for %s.', iface)
+ return 'dhcp'
+
+ # To do/consider (optimization and edge (?) cases):
+ #
+ # There are a bunch of "network managers" around but all of them
+ # hopefully only use either dhclient or dhcpcd
+ # => not worth asking them or is NetworkManager so widespread?
+ #
+ # Walk the process list only once for all interfaces
+ #
+ # Treat IPv4 and IPv6 independently
+ #
+ # Fall back to getBootProtocol?
+ #
+ # Try harder (how much?) in cases a device name is not explicitly specified
+ # on command line / or DHCP client already gone but its lease still valid
+ # This possibly includes: defaulting to all interfaces; dhcpcd's globbing;
+ # explicit/implicit configuration files (of different nature, indeed)
+ #
+ # A last resort: lease files created by daemons _somewhere_
+
+ logging.info('No DHCP client found for %s. '
+ 'Please file a bug if it was configured using DHCP.', iface)
+
+ return 'none'
+
+
def permAddr():
paddr = {}
for b in bondings():
@@ -480,6 +518,7 @@
'gateway': getgateway(gateways, iface),
'ipv6addrs': ipv6addrs,
'ipv6gateway': ipv6routes.get(iface, '::'),
+ 'bootproto4': getBootProtocolDynamic(iface),
'mtu': str(getMtu(iface))})
if qosInbound:
data['qosInbound'] = qosInbound
@@ -527,6 +566,7 @@
return {'addr': ipv4addr,
'cfg': getIfaceCfg(dev),
'ipv6addrs': ipv6addrs,
+ 'bootproto4': getBootProtocolDynamic(dev),
'mtu': str(getMtu(dev)),
'netmask': ipv4netmask}
@@ -538,6 +578,12 @@
ipv6routes = getIPv6Routes()
paddr = permAddr()
+ # To do:
+ #
+ # Determine if interfaces were configured using DHCP in a single loop
+ # over processes run from here / a run over interfaces when resorting
+ # to lease files
+
for net, netAttr in networks().iteritems():
try:
d['networks'][net] = _getNetInfo(netAttr.get('iface', net),
--
To view, visit http://gerrit.ovirt.org/23098
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5fbc48a0adf5f40120a72ec2c4cc2fc80b7226b8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com>
10 years, 3 months
Change in vdsm[master]: utils: Moved pgrep and getCmdArgs from storage/misc
by osvoboda＠redhat.com
Ondřej Svoboda has uploaded a new change for review.
Change subject: utils: Moved pgrep and getCmdArgs from storage/misc
......................................................................
utils: Moved pgrep and getCmdArgs from storage/misc
They will be used elsewhere to find a running DHCP client
and to read an interface name from its cmdline.
Change-Id: Ia9a933bec0df3f7773c0f8e47fc0bf0492149031
Signed-off-by: Ondřej Svoboda <osvoboda(a)redhat.com>
---
M lib/vdsm/utils.py
M tests/miscTests.py
M tests/utilsTests.py
M vdsm/storage/blockSD.py
M vdsm/storage/misc.py
5 files changed, 86 insertions(+), 84 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/40/23040/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index b072bd4..b0eeea8 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -259,6 +259,48 @@
return stat._make(res[:len(fields)])
+def iteratePids():
+ for path in glob.iglob("/proc/[0-9]*"):
+ pid = os.path.basename(path)
+ yield int(pid)
+
+
+def pgrep(name):
+ res = []
+ for pid in iteratePids():
+ try:
+ pid = int(pid)
+ except ValueError:
+ continue
+
+ try:
+ procName = pidStat(pid).comm
+ if procName == name:
+ res.append(pid)
+ except (OSError, IOError):
+ continue
+ return res
+
+
+def _parseCmdLine(pid):
+ with open("/proc/%d/cmdline" % pid, "rb") as f:
+ return tuple(f.read().split("\0")[:-1])
+
+
+def getCmdArgs(pid):
+ res = tuple()
+ # Sometimes cmdline is empty even though the process is not a zombie.
+ # Retrying seems to solve it.
+ while len(res) == 0:
+ # cmdline is empty for zombie processes
+ if pidStat(pid).state in ("Z", "z"):
+ return tuple()
+
+ res = _parseCmdLine(pid)
+
+ return res
+
+
def convertToStr(val):
varType = type(val)
if varType is float:
diff --git a/tests/miscTests.py b/tests/miscTests.py
index fb1191b..1765a57 100644
--- a/tests/miscTests.py
+++ b/tests/miscTests.py
@@ -58,45 +58,6 @@
return ret, out, err
-class PgrepTests(TestCaseBase):
- def test(self):
- sleepProcs = []
- for i in range(3):
- sleepProcs.append(utils.execCmd([EXT_SLEEP, "3"], sync=False,
- sudo=False))
-
- pids = misc.pgrep("sleep")
- for proc in sleepProcs:
- self.assertTrue(proc.pid in pids, "pid %d was not located by pgrep"
- % proc.pid)
-
- for proc in sleepProcs:
- proc.kill()
- proc.wait()
-
-
-class GetCmdArgsTests(TestCaseBase):
- def test(self):
- args = [EXT_SLEEP, "4"]
- sproc = utils.execCmd(args, sync=False, sudo=False)
- try:
- self.assertEquals(misc.getCmdArgs(sproc.pid), tuple(args))
- finally:
- sproc.kill()
- sproc.wait()
-
- def testZombie(self):
- args = [EXT_SLEEP, "0"]
- sproc = utils.execCmd(args, sync=False, sudo=False)
- sproc.kill()
- try:
- test = lambda: self.assertEquals(misc.getCmdArgs(sproc.pid),
- tuple())
- utils.retry(AssertionError, test, tries=10, sleep=0.1)
- finally:
- sproc.wait()
-
-
class EventTests(TestCaseBase):
def testEmit(self):
ev = threading.Event()
diff --git a/tests/utilsTests.py b/tests/utilsTests.py
index c5b250d..1399c93 100644
--- a/tests/utilsTests.py
+++ b/tests/utilsTests.py
@@ -27,6 +27,8 @@
from vdsm import utils
import time
+EXT_SLEEP = "sleep"
+
class RetryTests(TestCaseBase):
def testStopCallback(self):
@@ -66,6 +68,45 @@
sproc.wait()
+class PgrepTests(TestCaseBase):
+ def test(self):
+ sleepProcs = []
+ for i in range(3):
+ sleepProcs.append(utils.execCmd([EXT_SLEEP, "3"], sync=False,
+ sudo=False))
+
+ pids = utils.pgrep("sleep")
+ for proc in sleepProcs:
+ self.assertTrue(proc.pid in pids, "pid %d was not located by pgrep"
+ % proc.pid)
+
+ for proc in sleepProcs:
+ proc.kill()
+ proc.wait()
+
+
+class GetCmdArgsTests(TestCaseBase):
+ def test(self):
+ args = [EXT_SLEEP, "4"]
+ sproc = utils.execCmd(args, sync=False, sudo=False)
+ try:
+ self.assertEquals(utils.getCmdArgs(sproc.pid), tuple(args))
+ finally:
+ sproc.kill()
+ sproc.wait()
+
+ def testZombie(self):
+ args = [EXT_SLEEP, "0"]
+ sproc = utils.execCmd(args, sync=False, sudo=False)
+ sproc.kill()
+ try:
+ test = lambda: self.assertEquals(utils.getCmdArgs(sproc.pid),
+ tuple())
+ utils.retry(AssertionError, test, tries=10, sleep=0.1)
+ finally:
+ sproc.wait()
+
+
class CommandPathTests(TestCaseBase):
def testExisting(self):
cp = utils.CommandPath('sh', 'utter nonsense', '/bin/sh')
diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 55bd796..5179c5a 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -1177,7 +1177,7 @@
@classmethod
def __handleStuckUmount(cls, masterDir):
- umountPids = misc.pgrep("umount")
+ umountPids = utils.pgrep("umount")
try:
masterMount = mount.getMountFromTarget(masterDir)
except OSError as ex:
@@ -1189,7 +1189,7 @@
for umountPid in umountPids:
try:
state = utils.pidStat(umountPid).state
- mountPoint = misc.getCmdArgs(umountPid)[-1]
+ mountPoint = utils.getCmdArgs(umountPid)[-1]
except:
# Process probably exited
continue
diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
index 48020b6..3f2c074 100644
--- a/vdsm/storage/misc.py
+++ b/vdsm/storage/misc.py
@@ -828,48 +828,6 @@
return helper
-def iteratePids():
- for path in glob.iglob("/proc/[0-9]*"):
- pid = os.path.basename(path)
- yield int(pid)
-
-
-def pgrep(name):
- res = []
- for pid in iteratePids():
- try:
- pid = int(pid)
- except ValueError:
- continue
-
- try:
- procName = utils.pidStat(pid).comm
- if procName == name:
- res.append(pid)
- except (OSError, IOError):
- continue
- return res
-
-
-def _parseCmdLine(pid):
- with open("/proc/%d/cmdline" % pid, "rb") as f:
- return tuple(f.read().split("\0")[:-1])
-
-
-def getCmdArgs(pid):
- res = tuple()
- # Sometimes cmdline is empty even though the process is not a zombie.
- # Retrying seems to solve it.
- while len(res) == 0:
- # cmdline is empty for zombie processes
- if utils.pidStat(pid).state in ("Z", "z"):
- return tuple()
-
- res = _parseCmdLine(pid)
-
- return res
-
-
def tmap(func, iterable):
resultsDict = {}
error = [None]
@@ -1008,7 +966,7 @@
def killall(name, signum, group=False):
exception = None
knownPgs = set()
- pidList = pgrep(name)
+ pidList = utils.pgrep(name)
if len(pidList) == 0:
raise OSError(errno.ESRCH,
"Could not find processes named `%s`" % name)
--
To view, visit http://gerrit.ovirt.org/23040
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia9a933bec0df3f7773c0f8e47fc0bf0492149031
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com>
10 years, 3 months
Change in vdsm[master]: netconf.ifcfg: include CONFFILE_HEADER in route/rule files
by Dan Kenigsberg
Dan Kenigsberg has uploaded a new change for review.
Change subject: netconf.ifcfg: include CONFFILE_HEADER in route/rule files
......................................................................
netconf.ifcfg: include CONFFILE_HEADER in route/rule files
Include the
# Generated by VDSM version x.y.z
header in all vdsm-written ifcfg files.
Change-Id: Ia49754b85797168df056ebd9acf81c151ad3980f
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm/netconf/ifcfg.py
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/03/24103/1
diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index 0bc1b96..47354e4 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -515,6 +515,7 @@
'''Backs up the previous contents of the file referenced by fileName
writes the new configuration and sets the specified access mode.'''
self._backup(fileName)
+ configuration = self.CONFFILE_HEADER + '\n' + configuration
logging.debug('Writing to file %s configuration:\n%s' % (fileName,
configuration))
with open(fileName, 'w') as confFile:
@@ -534,9 +535,8 @@
def _createConfFile(self, conf, name, ipconfig, mtu=None, **kwargs):
""" Create ifcfg-* file with proper fields per device """
- cfg = self.CONFFILE_HEADER + '\n'
- cfg += """DEVICE=%s\nONBOOT=yes\n""" % pipes.quote(name)
+ cfg = """DEVICE=%s\nONBOOT=yes\n""" % pipes.quote(name)
cfg += conf
if ipconfig.ipaddr:
cfg = cfg + 'IPADDR=%s\n' % pipes.quote(ipconfig.ipaddr)
--
To view, visit http://gerrit.ovirt.org/24103
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia49754b85797168df056ebd9acf81c151ad3980f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
10 years, 3 months
Change in vdsm[ovirt-3.4]: vm: do not pause during refreshVolume
by Federico Simoncelli
Hello Ayal Baron, Allon Mureinik,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/24038
to review the following change.
Change subject: vm: do not pause during refreshVolume
......................................................................
vm: do not pause during refreshVolume
There's no need to pause the VM during the logical volume refresh as LVM
is taking care of freezing the IO during the dm table update.
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=889097
Change-Id: I896993c00b94c3255ad2007486e0589bf10b3a98
Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/8382
Reviewed-by: Ayal Baron <abaron(a)redhat.com>
Reviewed-by: Allon Mureinik <amureini(a)redhat.com>
---
M vdsm/vm.py
1 file changed, 2 insertions(+), 13 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/24038/1
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 7c2d496..bf64195 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -2489,19 +2489,8 @@
self.__extendDriveVolume(vmDrive, newSize)
def __refreshDriveVolume(self, volInfo):
- """ Stop vm before refreshing LV. """
-
- self._guestCpuLock.acquire()
- try:
- wasRunning = self._guestCpuRunning
- if wasRunning:
- self.pause(guestCpuLocked=True)
- self.cif.irs.refreshVolume(volInfo['domainID'], volInfo['poolID'],
- volInfo['imageID'], volInfo['volumeID'])
- if wasRunning:
- self.cont(guestCpuLocked=True)
- finally:
- self._guestCpuLock.release()
+ self.cif.irs.refreshVolume(volInfo['domainID'], volInfo['poolID'],
+ volInfo['imageID'], volInfo['volumeID'])
def __verifyVolumeExtension(self, volInfo):
self.log.debug("Refreshing drive volume for %s (domainID: %s, "
--
To view, visit http://gerrit.ovirt.org/24038
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I896993c00b94c3255ad2007486e0589bf10b3a98
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
10 years, 3 months
Change in vdsm[master]: fencing: use deathSignal to keep scripts at bay
by Dan Kenigsberg
Dan Kenigsberg has uploaded a new change for review.
Change subject: fencing: use deathSignal to keep scripts at bay
......................................................................
fencing: use deathSignal to keep scripts at bay
Before this patch, the fencing code used a knowingly-raceful code to
wait for pending fencing scripts, and kill them when Vdsm exited.
This patch makes sure that deathSignal is sent to pending scripts if
Vdsm exits or crashes.
We require a newer version of python-cpopen that raises a proper
exception when an executable is missing.
Change-Id: I068acf5ca0ad0813aaef660770de79958bd0b763
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm.spec.in
M vdsm/API.py
2 files changed, 24 insertions(+), 36 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/22997/1
diff --git a/vdsm.spec.in b/vdsm.spec.in
index d42b1b9..2d3a1b4 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -104,7 +104,7 @@
Requires: python-netaddr
Requires: python-inotify
Requires: python-argparse
-Requires: python-cpopen >= 1.2.3-4
+Requires: python-cpopen >= 1.2.4
Requires: python-ethtool >= 0.6-3
Requires: %{name}-python-zombiereaper = %{version}-%{release}
Requires: rpm-python
diff --git a/vdsm/API.py b/vdsm/API.py
index 3cf3133..f76bd7b 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -1078,26 +1078,15 @@
action can be one of (status, on, off, reboot)."""
@utils.traceback(on=self.log.name)
- def waitForPid(p, inp):
- """ Wait until p.pid exits. Kill it if vdsm exists before. """
- p.stdin.write(inp)
- p.stdin.close()
- while p.poll() is None:
- if not self._cif._enabled:
- self.log.debug('killing fence script pid %s', p.pid)
- os.kill(p.pid, signal.SIGTERM)
- time.sleep(1)
- try:
- # improbable race: p.pid may now belong to another
- # process
- os.kill(p.pid, signal.SIGKILL)
- except:
- pass
- return
- time.sleep(1)
- self.log.debug('rc %s inp %s out %s err %s', p.returncode,
- hidePasswd(inp),
- p.stdout.read(), p.stderr.read())
+ def fence(script, inp):
+ # non-status actions are sent asyncronously. deathSignal is set to
+ # make sure that no stray fencing scripts are left behind if Vdsm
+ # crashes.
+ rc, out, err = utils.execCmd(
+ [script], deathSignal=signal.SIGTERM,
+ data=inp)
+ self.log.debug('rc %s inp %s out %s err %s', rc,
+ hidePasswd(inp), out, err)
def hidePasswd(text):
cleantext = ''
@@ -1116,15 +1105,6 @@
script = constants.EXT_FENCE_PREFIX + agent
- try:
- p = subprocess.Popen([script], stdin=subprocess.PIPE,
- stdout=subprocess.PIPE,
- stderr=subprocess.PIPE, close_fds=True)
- except OSError as e:
- if e.errno == os.errno.ENOENT:
- return errCode['fenceAgent']
- raise
-
inp = ('agent=fence_%s\nipaddr=%s\nlogin=%s\noption=%s\n'
'passwd=%s\n') % (agent, addr, username, action, password)
if port != '':
@@ -1132,24 +1112,32 @@
if utils.tobool(secure):
inp += 'secure=yes\n'
inp += options
+
if action == 'status':
- out, err = p.communicate(inp)
- self.log.debug('rc %s in %s out %s err %s', p.returncode,
+ try:
+ rc, out, err = utils.execCmd(
+ [script], deathSignal=signal.SIGTERM,
+ data=inp)
+ except OSError as e:
+ if e.errno == os.errno.ENOENT:
+ return errCode['fenceAgent']
+ raise
+ self.log.debug('rc %s in %s out %s err %s', rc,
hidePasswd(inp), out, err)
- if not 0 <= p.returncode <= 2:
+ if not 0 <= rc <= 2:
return {'status': {'code': 1,
'message': out + err}}
message = doneCode['message']
- if p.returncode == 0:
+ if rc == 0:
power = 'on'
- elif p.returncode == 2:
+ elif rc == 2:
power = 'off'
else:
power = 'unknown'
message = out + err
return {'status': {'code': 0, 'message': message},
'power': power}
- threading.Thread(target=waitForPid, args=(p, inp)).start()
+ threading.Thread(target=fence, args=(script, inp)).start()
return {'status': doneCode}
def ping(self):
--
To view, visit http://gerrit.ovirt.org/22997
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I068acf5ca0ad0813aaef660770de79958bd0b763
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
10 years, 3 months