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 bf98c1fac8119ecd2a2e600c5e527bb51ea17a72..c05be46dec947ddf1260e5660b411e6f55511d30 100644 (file)
--- 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 b951467ae48c2e104796821b86590e40f0022e18..6005f8863ea134f38308fcbec3a8c63b5cca4850 100644 (file)
--- 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 6bf6e796a41903a0e2d98675d8cad502caddb317..1a75bc7da81e1d4051f803e06b8b8399c691a012 100644 (file)
--- 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."""