From: John Griffith Date: Fri, 6 Sep 2013 19:55:39 +0000 (-0600) Subject: Do not attempt vg.update on uninitialized vg X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d552220a83052daabe19682b531bc6b8e685a834;p=openstack-build%2Fcinder-build.git 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 --- diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index bf98c1fac..c05be46de 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -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 diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index b951467ae..6005f8863 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -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.""" + + 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 - # FIXME(jdg): Fix up the duplicate code between - # LVM, LVMISCSI and ISER starting with this section - LOG.debug(_("Updating volume status")) + 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) diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 6bf6e796a..1a75bc7da 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -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."""