Do not attempt vg.update on uninitialized vg

It's possible for the scheduler to send the periodic
stats update request *before* the volume service/driver
has finished initializing, resulting in an exception trace
being logged.

This change adds a check to verify that the vg is actually
initialized (completed check_for_setup_error) and it also
removes all of the duplication of each LVM class driver.

To deal with other drivers that may have similar issues,
add initialized member to base driver, that member is
set False on __init__ and is updated by the manager
after running check_for_setup_error.  We also skip
update_stats if initialization isn't set True.

Rather than have a separate copy of update_stats in every
driver, we have one in the base LVM class and we set the two
unique variables as member parameters.

Fixes bug 1221874

Change-Id: I159e98a77782b8b2c85a8dd956b150828358fd25
This commit is contained in:
John Griffith 2013-09-06 13:55:39 -06:00
parent 24cbfb3539
commit d552220a83
3 changed files with 52 additions and 80 deletions

View File

@ -109,9 +109,19 @@ class VolumeDriver(object):
self.set_execute(execute)
self._stats = {}
# set True by manager after succesful check_for_setup
self._initialized = False
def set_execute(self, execute):
self._execute = execute
def set_initialized(self):
self._initialized = True
@property
def initialized(self):
return self._initialized
def get_version(self):
"""Get the current version of this driver."""
return self.VERSION

View File

@ -73,6 +73,9 @@ class LVMVolumeDriver(driver.VolumeDriver):
self.configuration.append_config_values(volume_opts)
self.hostname = socket.gethostname()
self.vg = vg_obj
self.backend_name =\
self.configuration.safe_get('volume_backend_name') or 'LVM'
self.protocol = 'local'
def set_execute(self, execute):
self._execute = execute
@ -322,24 +325,29 @@ class LVMVolumeDriver(driver.VolumeDriver):
"""
if refresh:
self._update_volume_status()
self._update_volume_status()
self._update_volume_stats()
return self._stats
def _update_volume_status(self):
"""Retrieve status info from volume group."""
def _update_volume_stats(self):
"""Retrieve stats info from volume group."""
# FIXME(jdg): Fix up the duplicate code between
# LVM, LVMISCSI and ISER starting with this section
LOG.debug(_("Updating volume status"))
LOG.debug(_("Updating volume stats"))
if self.vg is None:
LOG.warning(_('Unable to update stats on non-intialized '
'Volume Group: %s'), self.configuration.volume_group)
return
self.vg.update_volume_group_info()
data = {}
backend_name = self.configuration.safe_get('volume_backend_name')
data["volume_backend_name"] = backend_name or 'LVM'
# Note(zhiteng): These information are driver/backend specific,
# each driver may define these values in its own config options
# or fetch from driver specific configuration file.
data["volume_backend_name"] = self.backend_name
data["vendor_name"] = 'Open Source'
data["driver_version"] = self.VERSION
data["storage_protocol"] = 'local'
data["storage_protocol"] = self.protocol
data['total_capacity_gb'] = float(self.vg.vg_size)
data['free_capacity_gb'] = float(self.vg.vg_free_space)
@ -376,6 +384,9 @@ class LVMISCSIDriver(LVMVolumeDriver, driver.ISCSIDriver):
root_helper = 'sudo cinder-rootwrap %s' % CONF.rootwrap_config
self.tgtadm = iscsi.get_target_admin(root_helper)
super(LVMISCSIDriver, self).__init__(*args, **kwargs)
self.backend_name =\
self.configuration.safe_get('volume_backend_name') or 'LVM_iSCSI'
self.protocol = 'iSCSI'
def set_execute(self, execute):
super(LVMISCSIDriver, self).set_execute(execute)
@ -659,46 +670,6 @@ class LVMISCSIDriver(LVMVolumeDriver, driver.ISCSIDriver):
return (True, model_update)
def get_volume_stats(self, refresh=False):
"""Get volume stats.
If 'refresh' is True, run update the stats first.
"""
if refresh:
self._update_volume_stats()
return self._stats
def _update_volume_stats(self):
"""Retrieve stats info from volume group."""
LOG.debug(_("Updating volume stats"))
self.vg.update_volume_group_info()
data = {}
# Note(zhiteng): These information are driver/backend specific,
# each driver may define these values in its own config options
# or fetch from driver specific configuration file.
backend_name = self.configuration.safe_get('volume_backend_name')
data["volume_backend_name"] = backend_name or 'LVM_iSCSI'
data["vendor_name"] = 'Open Source'
data["driver_version"] = self.VERSION
data["storage_protocol"] = 'iSCSI'
data['total_capacity_gb'] = float(self.vg.vg_size)
data['free_capacity_gb'] = float(self.vg.vg_free_space)
data['reserved_percentage'] = self.configuration.reserved_percentage
data['QoS_support'] = False
data['location_info'] =\
('LVMVolumeDriver:%(hostname)s:%(vg)s'
':%(lvm_type)s:%(lvm_mirrors)s' %
{'hostname': self.hostname,
'vg': self.configuration.volume_group,
'lvm_type': self.configuration.lvm_type,
'lvm_mirrors': self.configuration.lvm_mirrors})
self._stats = data
def _iscsi_location(self, ip, target, iqn, lun=None):
return "%s:%s,%s %s %s" % (ip, self.configuration.iscsi_port,
target, iqn, lun)
@ -727,6 +698,9 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver):
root_helper = 'sudo cinder-rootwrap %s' % CONF.rootwrap_config
self.tgtadm = iser.get_target_admin(root_helper)
LVMVolumeDriver.__init__(self, *args, **kwargs)
self.backend_name =\
self.configuration.safe_get('volume_backend_name') or 'LVM_iSER'
self.protocol = 'iSER'
def set_execute(self, execute):
LVMVolumeDriver.set_execute(self, execute)
@ -855,29 +829,6 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver):
self.tgtadm.remove_iser_target(iser_target, 0, volume['id'],
volume['name'])
def _update_volume_status(self):
"""Retrieve status info from volume group."""
LOG.debug(_("Updating volume status"))
self.vg.update_volume_group_info()
data = {}
# Note(zhiteng): These information are driver/backend specific,
# each driver may define these values in its own config options
# or fetch from driver specific configuration file.
backend_name = self.configuration.safe_get('volume_backend_name')
data["volume_backend_name"] = backend_name or 'LVM_iSER'
data["vendor_name"] = 'Open Source'
data["driver_version"] = self.VERSION
data["storage_protocol"] = 'iSER'
data['total_capacity_gb'] = float(self.vg.vg_size)
data['free_capacity_gb'] = float(self.vg.vg_free_space)
data['reserved_percentage'] = self.configuration.reserved_percentage
data['QoS_support'] = False
self._stats = data
def _iser_location(self, ip, target, iqn, lun=None):
return "%s:%s,%s %s %s" % (ip, self.configuration.iser_port,
target, iqn, lun)

View File

@ -166,8 +166,13 @@ class VolumeManager(manager.SchedulerDependentManager):
LOG.info(_("Starting volume driver %(driver_name)s (%(version)s)") %
{'driver_name': self.driver.__class__.__name__,
'version': self.driver.get_version()})
self.driver.do_setup(ctxt)
self.driver.check_for_setup_error()
try:
self.driver.do_setup(ctxt)
self.driver.check_for_setup_error()
except Exception:
LOG.error(_("Error encountered during "
"initialization of driver: %s"),
self.driver.__class__.__name__)
volumes = self.db.volume_get_all_by_host(ctxt, self.host)
LOG.debug(_("Re-exporting %s volumes"), len(volumes))
@ -188,6 +193,8 @@ class VolumeManager(manager.SchedulerDependentManager):
LOG.info(_('Resuming delete on volume: %s') % volume['id'])
self.delete_volume(ctxt, volume['id'])
self.driver.set_initialized()
# collect and publish service capabilities
self.publish_service_capabilities(ctxt)
@ -745,11 +752,15 @@ class VolumeManager(manager.SchedulerDependentManager):
@periodic_task.periodic_task
def _report_driver_status(self, context):
LOG.info(_("Updating volume status"))
volume_stats = self.driver.get_volume_stats(refresh=True)
if volume_stats:
# This will grab info about the host and queue it
# to be sent to the Schedulers.
self.update_service_capabilities(volume_stats)
if not self.driver.initialized:
LOG.warning(_('Unabled to update stats, driver is '
'uninitialized'))
else:
volume_stats = self.driver.get_volume_stats(refresh=True)
if volume_stats:
# This will grab info about the host and queue it
# to be sent to the Schedulers.
self.update_service_capabilities(volume_stats)
def publish_service_capabilities(self, context):
"""Collect driver status and then publish."""