Merge pull request #1268 from tbzatek/nvme-13

nvme: Persistent DC tests
storaged-project · Mar 19, 2024 · fb68ccc · fb68ccc
2 parents c7900aa + fa6c4af
commit fb68ccc
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 10 deletions.
diff --git a/src/tests/dbus-tests/test_nvme.py b/src/tests/dbus-tests/test_nvme.py
@@ -172,6 +172,7 @@ def setup_nvme_target(dev_paths, subnqn):
 
 class UdisksNVMeTest(udiskstestcase.UdisksTestCase):
     SUBNQN = 'udisks_test_subnqn'
+    DISCOVERY_NQN = 'nqn.2014-08.org.nvmexpress.discovery'
     NUM_NS = 2
     NS_SIZE = 1024**3
 
@@ -507,6 +508,68 @@ def test_fabrics_connect(self):
         with self.assertRaisesRegex(dbus.exceptions.DBusException, r'Object does not exist at path .*|No such interface'):
             self.get_property_raw(ctrl, '.NVMe.Fabrics', 'HostNQN')
 
+    def test_persistent_dc(self):
+        manager = self.get_interface("/Manager", ".Manager.NVMe")
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, 'Invalid value specified for the transport address argument'):
+            manager.Connect(self.str_to_ay(self.DISCOVERY_NQN), "notransport", "", self.no_options)
+        msg = r'Error connecting the controller: failed to write to nvme-fabrics device'
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, msg):
+            manager.Connect(self.str_to_ay(self.DISCOVERY_NQN), "loop", "127.0.0.1", self.no_options)
+
+        ctrl_obj_path = manager.Connect(self.str_to_ay(self.DISCOVERY_NQN), "loop", "", self.no_options)
+        self.addCleanup(self._nvme_disconnect, self.DISCOVERY_NQN, ignore_errors=True)
+
+        ctrl = self.get_object(ctrl_obj_path)
+        self.assertHasIface(ctrl, 'org.freedesktop.UDisks2.NVMe.Controller')
+        self.assertHasIface(ctrl, 'org.freedesktop.UDisks2.NVMe.Fabrics')
+
+        # health information should never be updated on a discovery controller
+        smart_timestamp = self.get_property_raw(ctrl, '.NVMe.Controller', 'SmartUpdated')
+        self.assertEqual(smart_timestamp, 0)
+        nqn = self.get_property_raw(ctrl, '.NVMe.Controller', 'SubsystemNQN')
+        self.assertEqual(nqn, self.str_to_ay(self.DISCOVERY_NQN))
+        transport = self.get_property_raw(ctrl, '.NVMe.Fabrics', 'Transport')
+        self.assertEqual(transport, 'loop')
+        tr_addr = self.get_property_raw(ctrl, '.NVMe.Fabrics', 'TransportAddress')
+        self.assertEqual(len(tr_addr), 1)   # the zero trailing byte
+
+        # make sure that no block device object references this drive object
+        udisks = self.get_object('')
+        objects = udisks.GetManagedObjects(dbus_interface='org.freedesktop.DBus.ObjectManager')
+        block_paths = [p for p in list(objects.keys()) if '/block_devices/' in p]
+        for p in block_paths:
+            self.assertNotEqual(str(objects[p]['org.freedesktop.UDisks2.Block']['Drive']), str(ctrl_obj_path))
+
+        # the following methods should fail on discovery controllers
+        msg = 'NVMe Health Information is only supported on I/O controllers'
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, msg):
+            ctrl.SmartUpdate(self.no_options, dbus_interface=self.iface_prefix + '.NVMe.Controller')
+        msg = 'SMART data not collected'
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, msg):
+            ctrl.SmartGetAttributes(self.no_options, dbus_interface=self.iface_prefix + '.NVMe.Controller')
+
+        msg = 'The NVMe controller has no support for self-test operations'
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, msg):
+            ctrl.SmartSelftestStart('xxx', self.no_options, dbus_interface=self.iface_prefix + '.NVMe.Controller')
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, msg):
+            ctrl.SmartSelftestStart('short', self.no_options, dbus_interface=self.iface_prefix + '.NVMe.Controller')
+        msg = 'NVMe Device Self-test command error: Invalid Command Opcode'
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, msg):
+            ctrl.SmartSelftestAbort(self.no_options, dbus_interface=self.iface_prefix + '.NVMe.Controller')
+
+        msg = r'The NVMe controller has no support for the .* sanitize operation'
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, msg):
+            ctrl.SanitizeStart('block-erase', self.no_options, dbus_interface=self.iface_prefix + '.NVMe.Controller')
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, msg):
+            ctrl.SanitizeStart('crypto-erase', self.no_options, dbus_interface=self.iface_prefix + '.NVMe.Controller')
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, msg):
+            ctrl.SanitizeStart('overwrite', self.no_options, dbus_interface=self.iface_prefix + '.NVMe.Controller')
+
+        ctrl.Disconnect(self.no_options, dbus_interface=self.iface_prefix + '.NVMe.Fabrics')
+        ctrl = self.get_object(ctrl_obj_path)
+        with self.assertRaisesRegex(dbus.exceptions.DBusException, r'Object does not exist at path .*|No such interface'):
+            self.get_property_raw(ctrl, '.NVMe.Fabrics', 'HostNQN')
+
     def test_hostnqn(self):
         HOSTNQN_PATH = '/etc/nvme/hostnqn'
         HOSTID_PATH = '/etc/nvme/hostid'

diff --git a/src/udiskslinuxdriveobject.c b/src/udiskslinuxdriveobject.c
@@ -1116,9 +1116,8 @@ udisks_linux_drive_object_housekeeping (UDisksLinuxDriveObject  *object,
                                         GCancellable            *cancellable,
                                         GError                 **error)
 {
-  gboolean ret;
-
-  ret = FALSE;
+  UDisksLinuxDevice *device = NULL;
+  gboolean ret = FALSE;
 
   /* ATA */
   if (object->iface_drive_ata != NULL &&
@@ -1172,20 +1171,28 @@ udisks_linux_drive_object_housekeeping (UDisksLinuxDriveObject  *object,
     {
       GError *local_error = NULL;
 
-      udisks_info ("Refreshing Health Information on %s",
-                   g_dbus_object_get_object_path (G_DBUS_OBJECT (object)));
-
-      if (!udisks_linux_nvme_controller_refresh_smart_sync (UDISKS_LINUX_NVME_CONTROLLER (object->iface_nvme_ctrl),
-                                                            cancellable, &local_error))
+      /* Only perform health check on I/O controllers */
+      device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */);
+      if (device && device->nvme_ctrl_info &&
+          (device->nvme_ctrl_info->controller_type == BD_NVME_CTRL_TYPE_IO ||
+           device->nvme_ctrl_info->controller_type == BD_NVME_CTRL_TYPE_UNKNOWN))
         {
-          g_propagate_prefixed_error (error, local_error, "Error updating Health Information: ");
-          goto out;
+          udisks_info ("Refreshing Health Information on %s",
+                       g_dbus_object_get_object_path (G_DBUS_OBJECT (object)));
+
+          if (!udisks_linux_nvme_controller_refresh_smart_sync (UDISKS_LINUX_NVME_CONTROLLER (object->iface_nvme_ctrl),
+                                                                cancellable, &local_error))
+            {
+              g_propagate_prefixed_error (error, local_error, "Error updating Health Information: ");
+              goto out;
+            }
         }
     }
 
   ret = TRUE;
 
  out:
+  g_clear_object (&device);
   return ret;
 }
 

diff --git a/src/udiskslinuxnvmecontroller.c b/src/udiskslinuxnvmecontroller.c
@@ -390,6 +390,15 @@ udisks_linux_nvme_controller_refresh_smart_sync (UDisksLinuxNVMeController  *ctr
       g_object_unref (object);
       return FALSE;
     }
+  if (device->nvme_ctrl_info->controller_type != BD_NVME_CTRL_TYPE_UNKNOWN &&
+      device->nvme_ctrl_info->controller_type != BD_NVME_CTRL_TYPE_IO)
+    {
+      g_set_error_literal (error, UDISKS_ERROR, UDISKS_ERROR_FAILED,
+                           "NVMe Health Information is only supported on I/O controllers");
+      g_object_unref (device);
+      g_object_unref (object);
+      return FALSE;
+    }
 
   /* Controller capabilities check - there's no authoritative way to find out which
    * log pages are actually supported, taking controller feature flags in account instead.