From 4da54c07861a1542a8e30a768d4506d3e81b5598 Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem.os@gmail.com>
Date: Wed, 10 Oct 2018 15:07:00 -0400
Subject: [PATCH] Fix NoneType error in _notify_volume_usage_detach

If the driver.block_stats() method returns None, like the
libvirt driver will if the guest is gone during an evacuate,
we'll get a NoneType error trying to unpack the return value
from the driver. Instead, simply return as if the driver
raised NotImplementedError.

Since handling None is changing the contract on the virt
driver API, the docstring is updated to explain the acceptable
return values of the driver method.

Change-Id: I98a2785c07f7af02ad83650c72d9e1868290ece4
Closes-Bug: #1796981
---
 nova/compute/manager.py                     |  3 ++-
 nova/tests/unit/compute/test_compute_mgr.py | 17 +++++++++++++++++
 nova/virt/driver.py                         |  7 +++++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 7e4ede30a9..db09a83b0a 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -5544,13 +5544,14 @@ class ComputeManager(manager.Manager):
         if CONF.volume_usage_poll_interval <= 0:
             return
 
-        vol_stats = []
         mp = bdm.device_name
         # Handle bootable volumes which will not contain /dev/
         if '/dev/' in mp:
             mp = mp[5:]
         try:
             vol_stats = self.driver.block_stats(instance, mp)
+            if vol_stats is None:
+                return
         except NotImplementedError:
             return
 
diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index 232a952450..54599967ff 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -4931,6 +4931,23 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
             expected_reqspec_hints, self.compute._get_scheduler_hints(
                 filter_properties, reqspec))
 
+    def test_notify_volume_usage_detach_no_block_stats(self):
+        """Tests the case that the virt driver returns None from the
+        block_stats() method and no notification is sent, similar to the
+        virt driver raising NotImplementedError.
+        """
+        self.flags(volume_usage_poll_interval=60)
+        fake_instance = objects.Instance()
+        fake_bdm = objects.BlockDeviceMapping(device_name='/dev/vda')
+        with mock.patch.object(self.compute.driver, 'block_stats',
+                               return_value=None) as block_stats:
+            # Assert a notification isn't sent.
+            with mock.patch.object(self.compute.notifier, 'info',
+                                   new_callable=mock.NonCallableMock):
+                self.compute._notify_volume_usage_detach(
+                    self.context, fake_instance, fake_bdm)
+        block_stats.assert_called_once_with(fake_instance, 'vda')
+
 
 class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
     def setUp(self):
diff --git a/nova/virt/driver.py b/nova/virt/driver.py
index 2cbd63d0a1..83b6eee522 100644
--- a/nova/virt/driver.py
+++ b/nova/virt/driver.py
@@ -1400,6 +1400,13 @@ class ComputeDriver(object):
         unused.
 
         Note that this function takes an instance ID.
+
+        :param instance: nova.objects.Instance to get block storage statistics
+        :param disk_id: mountpoint name, e.g. "vda"
+        :returns: None if block statistics could not be retrieved, otherwise a
+            list of the form: [rd_req, rd_bytes, wr_req, wr_bytes, errs]
+        :raises: NotImplementedError if the driver does not implement this
+            method
         """
         raise NotImplementedError()