Nir Soffer has uploaded a new change for review.
Change subject: vm: Remove useless volume size monitoring
......................................................................
vm: Remove useless volume size monitoring
We used to check drive volume size every 60 seconds but we do not use
the result of this check for anything. According old comments, the
result was used in the past for extending disks. Since 3.6 we are using
the capacity value from libvirt.
This check may block on storage apis for minutes when using NFS
unresponsive storage, blocking executor workers and harming unrelated
vms using other storage.
Change-Id: Ib1436c2968f3e408ce38a913c6ca3146a25a312d
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/vdsm/config.py.in
M lib/vdsm/virt/periodic.py
M vdsm/virt/vm.py
M vdsm/virt/vmdevices/storage.py
4 files changed, 19 insertions(+), 89 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/59801/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 8bcb09c..ad2f8c7 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -287,9 +287,6 @@
'volume_utilization_percent, set the free space limit. Use higher '
'values to extend in bigger chunks.'),
- ('vol_size_sample_interval', '60',
- 'How often should the volume size be checked (seconds).'),
-
('scsi_rescan_maximal_timeout', '30',
'The maximal number of seconds to wait for scsi scan to return.'),
diff --git a/lib/vdsm/virt/periodic.py b/lib/vdsm/virt/periodic.py
index 8ed5d67..72f33a9 100644
--- a/lib/vdsm/virt/periodic.py
+++ b/lib/vdsm/virt/periodic.py
@@ -71,12 +71,6 @@
return Operation(disp, period, scheduler)
_operations = [
- # needs dispatching becuse updating the volume stats needs the
- # access the storage, thus can block.
- per_vm_operation(
- UpdateVolumes,
- config.getint('irs', 'vol_size_sample_interval')),
-
# needs dispatching becuse access FS and libvirt data
per_vm_operation(
NumaInfoMonitor,
@@ -316,22 +310,6 @@
return '<%s vm=%s at 0x%x>' % (
self.__class__.__name__, self._vm.id, id(self)
)
-
-
-class UpdateVolumes(_RunnableOnVm):
-
- @property
- def required(self):
- return (super(UpdateVolumes, self).required and
- # Avoid queries from storage during recovery process
- self._vm.isDisksStatsCollectionEnabled())
-
- def _execute(self):
- for drive in self._vm.getDiskDevices():
- # TODO: If this block (it is actually possible?)
- # we must make sure we don't overwrite good data
- # with stale old data.
- self._vm.updateDriveVolume(drive)
class NumaInfoMonitor(_RunnableOnVm):
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index b5eec87..db2bec0 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -378,15 +378,6 @@
if 'device' not in drv:
drv['device'] = 'disk'
- if drv['device'] == 'disk':
- volsize = self._getVolumeSize(drv['domainID'],
drv['poolID'],
- drv['imageID'],
drv['volumeID'])
- drv['truesize'] = str(volsize.truesize)
- drv['apparentsize'] = str(volsize.apparentsize)
- else:
- drv['truesize'] = 0
- drv['apparentsize'] = 0
-
def __legacyDrives(self):
"""
Backward compatibility for qa scripts that specify direct paths.
@@ -398,8 +389,7 @@
if path:
legacies.append({'type': hwclass.DISK,
'device': 'disk', 'path': path,
- 'iface': 'ide', 'index': index,
- 'truesize': 0})
+ 'iface': 'ide', 'index':
index})
return legacies
def __removableDrives(self):
@@ -408,8 +398,7 @@
'device': 'cdrom',
'iface': vmdevices.storage.DEFAULT_INTERFACE_FOR_ARCH[self.arch],
'path': self.conf.get('cdrom', ''),
- 'index': 2,
- 'truesize': 0}]
+ 'index': 2}]
floppyPath = self.conf.get('floppy')
if floppyPath:
removables.append({
@@ -417,8 +406,7 @@
'device': 'floppy',
'path': floppyPath,
'iface': 'fdc',
- 'index': 0,
- 'truesize': 0})
+ 'index': 0})
return removables
def _devMapFromDevSpecMap(self, dev_spec_map):
@@ -999,10 +987,9 @@
for drive, volumeID, capacity, alloc, physical in extend:
self.log.info(
- "Requesting extension for volume %s on domain %s (apparent: "
- "%s, capacity: %s, allocated: %s, physical: %s)",
- volumeID, drive.domainID, drive.apparentsize, capacity,
- alloc, physical)
+ "Requesting extension for volume %s on domain %s (capacity: "
+ "%s, allocated: %s, physical: %s)",
+ volumeID, drive.domainID, capacity, alloc, physical)
self.extendDriveVolume(drive, volumeID, physical, capacity)
return len(extend) > 0
@@ -1044,8 +1031,6 @@
raise RuntimeError(
"Volume extension failed for %s (domainID: %s, volumeID: %s)"
%
(volInfo['name'], volInfo['domainID'],
volInfo['volumeID']))
-
- return volSize
def __afterReplicaExtension(self, volInfo):
self.__verifyVolumeExtension(volInfo)
@@ -1093,14 +1078,7 @@
def __afterVolumeExtension(self, volInfo):
# Check if the extension succeeded. On failure an exception is raised
# TODO: Report failure to the engine.
- volSize = self.__verifyVolumeExtension(volInfo)
-
- # Only update apparentsize and truesize if we've resized the leaf
- if not volInfo['internal']:
- vmDrive = self._findDriveByName(volInfo['name'])
- vmDrive.apparentsize = volSize.apparentsize
- vmDrive.truesize = volSize.truesize
-
+ self.__verifyVolumeExtension(volInfo)
try:
self.cont()
except libvirt.libvirtError:
@@ -3235,22 +3213,6 @@
return device
raise LookupError("No such disk %r" % name)
- def updateDriveVolume(self, vmDrive):
- if not vmDrive.device == 'disk' or not isVdsmImage(vmDrive):
- return
-
- try:
- volSize = self._getVolumeSize(
- vmDrive.domainID, vmDrive.poolID, vmDrive.imageID,
- vmDrive.volumeID)
- except StorageUnavailableError as e:
- self.log.error("Unable to update drive %s volume size: %s",
- vmDrive.name, e)
- return
-
- vmDrive.truesize = volSize.truesize
- vmDrive.apparentsize = volSize.apparentsize
-
def updateDriveParameters(self, driveParams):
"""Update the drive with the new volume
information"""
@@ -3259,7 +3221,6 @@
if vmDrive.name == driveParams["name"]:
for k, v in driveParams.iteritems():
setattr(vmDrive, k, v)
- self.updateDriveVolume(vmDrive)
break
else:
self.log.error("Unable to update the drive object for: %s",
@@ -3499,11 +3460,8 @@
# see the XML even with 'info' as default level.
self.log.info(snapxml)
- # We need to stop the collection of the stats for two reasons, one
- # is to prevent spurious libvirt errors about missing drive paths
- # (since we're changing them), and also to prevent to trigger a drive
- # extension for the new volume with the apparent size of the old one
- # (the apparentsize is updated as last step in updateDriveParameters)
+ # Prevent spurious libvirt errors about missing drive paths (since
+ # we're changing them).
self.stopDisksStatsCollection()
try:
diff --git a/vdsm/virt/vmdevices/storage.py b/vdsm/virt/vmdevices/storage.py
index 78bf00c..eb8b2ae 100644
--- a/vdsm/virt/vmdevices/storage.py
+++ b/vdsm/virt/vmdevices/storage.py
@@ -56,13 +56,14 @@
class Drive(Base):
- __slots__ = ('iface', '_path', 'readonly',
'bootOrder', 'domainID',
- 'poolID', 'imageID', 'UUID', 'volumeID',
'format',
- 'propagateErrors', 'address', 'apparentsize',
'volumeInfo',
- 'index', 'name', 'optional', 'shared',
'truesize',
- 'volumeChain', 'baseVolumeID', 'serial',
'reqsize', 'cache',
- '_blockDev', 'extSharedState', 'drv',
'sgio', 'GUID',
- 'diskReplicate', '_diskType', 'hosts',
'protocol', 'auth')
+ __slots__ = (
+ 'iface', '_path', 'readonly', 'bootOrder',
'domainID', 'poolID',
+ 'imageID', 'UUID', 'volumeID', 'format',
'propagateErrors', 'address',
+ 'volumeInfo', 'index', 'name', 'optional',
'shared', 'volumeChain',
+ 'baseVolumeID', 'serial', 'reqsize', 'cache',
'_blockDev',
+ 'extSharedState', 'drv', 'sgio', 'GUID',
'diskReplicate', '_diskType',
+ 'hosts', 'protocol', 'auth'
+ )
VOLWM_CHUNK_SIZE = (config.getint('irs',
'volume_utilization_chunk_mb') *
constants.MEGAB)
VOLWM_FREE_PCT = 100 - config.getint('irs',
'volume_utilization_percent')
@@ -165,8 +166,6 @@
self.device = getattr(self, 'device', 'disk')
# Keep sizes as int
self.reqsize = int(kwargs.get('reqsize', '0')) # Backward
compatible
- self.truesize = int(kwargs.get('truesize', '0'))
- self.apparentsize = int(kwargs.get('apparentsize', '0'))
self.name = makeName(self.iface, self.index)
self.cache = config.get('vars', 'qemu_drive_cache')
@@ -247,10 +246,8 @@
Returns the next volume size in bytes. This value is based on the
volExtensionChunk property and it's the size that should be requested
for the next LV extension. curSize is the current size of the volume
- to be extended. For the leaf volume curSize == self.apparentsize.
- For internal volumes it is discovered by calling irs.getVolumeSize().
- capacity is the maximum size of the volume. It can be discovered using
- libvirt.virDomain.blockInfo() or qemuimg.info().
+ to be extended. capacity is the maximum size of the volume. It can be
+ discovered using libvirt.virDomain.blockInfo() or qemuimg.info().
"""
nextSize = utils.round(curSize + self.volExtensionChunk,
constants.MEGAB)
--
To view, visit
https://gerrit.ovirt.org/59801
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib1436c2968f3e408ce38a913c6ca3146a25a312d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>