)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"d0e9790b778e59ea6d6a5afce25823c8a0d34429","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d0a5537e_9a58fcee","updated":"2021-11-22 06:09:55.000000000","message":"Hi Arne. I had the first stab at \"hybrid\" NVMe cleaning patch. It\u0027s nowhere near ready yet, I wanted to see if it passes test in CI (having some local issues) and more importantly have an early discussion about the general approach. WDYT? Thanks!","commit_id":"8133f4d4169b31e6f7192eb9d4a860edec12ee96"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"03692e02e450d851a646d4a18d62e78567dafbc6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"542fac74_c8617f9a","updated":"2021-11-29 05:28:57.000000000","message":"need to add test","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"dafe130b5c932073a39bfe8ea37931769e3b4e75","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4ad5fc25_a3f1a209","updated":"2022-03-04 04:33:57.000000000","message":"Thank you for your review Dmitry. Addressed the minor comments for starters, will continue working on the remaining ones. Will keep you posted.","commit_id":"de2555d94d4a761232e6f34999869ade900862cb"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"28ef275e9a13e805082ad267a011d61987e2922e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9a070de2_66d8941c","updated":"2022-03-04 06:26:34.000000000","message":"Thank you for your comment Dmitry - my response in-line. Let me know your thoughts. Thank you.","commit_id":"b82ac6a1f8de17b001de329de3aa09400e8654b6"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"bc4de5de_eba2b5b6","updated":"2022-03-10 11:57:51.000000000","message":"Thank you for your reviews Dmitry and Riccardo. I addressed your comments, hope I haven\u0027t missed anything.","commit_id":"477e7099d081bc8f66d0a7ac9934b98f5658f9b3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"f2725099b83b224d408a5c290c7cb6f21dbc90ba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"7db1ac8c_6ee1785c","updated":"2022-03-11 04:40:39.000000000","message":"Tested the latest revision on a suitable bare metal - to my best knowledge all devices have been erased as expected.\n\n[root@localhost ~]# lsscsi \n[0:0:0:0]    disk    TOSHIBA  AL15SEB120NY     EF03  /dev/sdb \n[0:0:1:0]    disk    TOSHIBA  AL15SEB120NY     EF03  /dev/sdc \n[0:0:2:0]    disk    TOSHIBA  AL15SEB120NY     EF03  /dev/sdd \n[0:0:3:0]    disk    TOSHIBA  AL15SEB120NY     EF03  /dev/sde \n[0:0:4:0]    disk    TOSHIBA  AL15SEB120NY     EF03  /dev/sdf \n[0:0:5:0]    disk    TOSHIBA  AL15SEB120NY     EF03  /dev/sdg \n[0:0:6:0]    disk    TOSHIBA  AL15SEB120NY     EF03  /dev/sdh \n[0:0:7:0]    disk    TOSHIBA  AL15SEB120NY     EF03  /dev/sdi \n[0:0:8:0]    enclosu DP       BP14G+EXP        2.41  -        \n[15:0:0:0]   disk    ATA      DELLBOSS VD      00-0  /dev/sda \n[17:0:0:0]   process Marvell  Console          1.01  -        \n[N:0:0:1]    disk    Dell Express Flash NVMe P4600 1.6TB SF__1  /dev/nvme0n1\n[N:1:0:1]    disk    Dell Express Flash NVMe P4500 1.0TB SF__1  /dev/nvme1n1\n[root@localhost ~]# grep \"Disk metadata\" /var/log/messages \nMar 11 04:26:52 localhost ironic-python-agent[1662]: 2022-03-11 04:26:52.824 1662 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdi successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 04:26:55 localhost ironic-python-agent[1662]: 2022-03-11 04:26:55.116 1662 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdh successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 04:26:57 localhost ironic-python-agent[1662]: 2022-03-11 04:26:57.390 1662 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdg successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 04:26:59 localhost ironic-python-agent[1662]: 2022-03-11 04:26:59.704 1662 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdf successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 04:27:02 localhost ironic-python-agent[1662]: 2022-03-11 04:27:02.000 1662 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sde successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 04:27:04 localhost ironic-python-agent[1662]: 2022-03-11 04:27:04.276 1662 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdd successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 04:27:06 localhost ironic-python-agent[1662]: 2022-03-11 04:27:06.569 1662 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdc successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 04:27:08 localhost ironic-python-agent[1662]: 2022-03-11 04:27:08.870 1662 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdb successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 04:27:09 localhost ironic-python-agent[1662]: 2022-03-11 04:27:09.925 1662 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sda successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\n[root@localhost ~]# grep nvme-cli /var/log/messages \nMar 11 04:27:13 localhost ironic-python-agent[1662]: 2022-03-11 04:27:13.083 1662 INFO root [-] nvme-cli format for device /dev/nvme1n1 (ses\u003d 2 ) completed successfully.#033[00m\nMar 11 04:27:18 localhost ironic-python-agent[1662]: 2022-03-11 04:27:18.399 1662 INFO root [-] nvme-cli format for device /dev/nvme0n1 (ses\u003d 2 ) completed successfully.#033[00m\n[root@localhost ~]# \n","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"3c526dde43f8a8f13fcba3a25be541f012eb0775","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"c57555c6_94f287b3","updated":"2022-03-11 21:02:18.000000000","message":"Thank you for your comments Arne, I uploaded a new revision addressing these.","commit_id":"3f8a77608747fc8ca809ae015ad21895b28e8b22"},{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"bb4680560f010e8d4382599e75cd1c1db3200b7c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"c591ede9_3a0c4005","updated":"2022-03-11 13:37:59.000000000","message":"Thanks, Jacob, lgtm. Some minor suggestions inline.","commit_id":"3f8a77608747fc8ca809ae015ad21895b28e8b22"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5620cea4f4a41ffceed06a2a82512bbd830d958b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"c5736332_cc5a6f76","updated":"2022-03-12 00:01:03.000000000","message":"Marking -1 for visibility purposes.","commit_id":"fffaae84d8387d30cff567de13856c2c6cb8e3bd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"9c018a2e4aeec652ccca2058ff9f264d4304586d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"3fa4787a_f76475de","updated":"2022-03-12 00:00:36.000000000","message":"Parallelism question inline. I guess I\u0027m concerned that with this we basically take a performance hit, but maybe we might be able to merge this and then treat the lack of support for the treading as a bug.","commit_id":"fffaae84d8387d30cff567de13856c2c6cb8e3bd"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"19658c5eb0372e7dabf08cc237bba035fca7cc89","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"0e982f16_f27cfcfe","updated":"2022-03-15 03:22:00.000000000","message":"recheck","commit_id":"e3131fbb3cf04e700a3b2f0628bc0e65a3a94030"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"a932d7353fca2b70e9821303239a6bd0e17a2712","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"f5549a16_e52b0c52","updated":"2022-03-15 09:03:34.000000000","message":"@Arne I removed the config details from the release note in order to put these in a doco change (coming soon).","commit_id":"c5f7f18bcb2e1876b0712c691996346620ffc920"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"6d542b6f2e9e221b1e141ce66e0010c92239d994","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"f5fc5b78_7784b5a9","updated":"2022-03-15 12:37:38.000000000","message":"Re-tested in the lab after removal of ata_erase, just to make sure everything works as it used to:\n\n[root@localhost ~]# grep \"Disk metadata on\" -i /var/log/messages \nMar 15 12:33:14 localhost ironic-python-agent[1616]: 2022-03-15 12:33:14.708 1616 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdi successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 15 12:33:16 localhost ironic-python-agent[1616]: 2022-03-15 12:33:16.996 1616 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdh successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 15 12:33:19 localhost ironic-python-agent[1616]: 2022-03-15 12:33:19.280 1616 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdg successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 15 12:33:21 localhost ironic-python-agent[1616]: 2022-03-15 12:33:21.577 1616 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdf successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 15 12:33:23 localhost ironic-python-agent[1616]: 2022-03-15 12:33:23.847 1616 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sde successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 15 12:33:26 localhost ironic-python-agent[1616]: 2022-03-15 12:33:26.114 1616 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdd successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 15 12:33:28 localhost ironic-python-agent[1616]: 2022-03-15 12:33:28.384 1616 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdc successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 15 12:33:30 localhost ironic-python-agent[1616]: 2022-03-15 12:33:30.670 1616 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdb successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 15 12:33:32 localhost ironic-python-agent[1616]: 2022-03-15 12:33:32.743 1616 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sda successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\n[root@localhost ~]# \n[root@localhost ~]# grep nvme-cli /var/log/messages \nMar 15 12:33:35 localhost ironic-python-agent[1616]: 2022-03-15 12:33:35.890 1616 INFO root [-] nvme-cli format for device /dev/nvme1n1 (ses\u003d 2 ) completed successfully.#033[00m\nMar 15 12:33:41 localhost ironic-python-agent[1616]: 2022-03-15 12:33:41.192 1616 INFO root [-] nvme-cli format for device /dev/nvme0n1 (ses\u003d 2 ) completed successfully.#033[00m\n[root@localhost ~]# \n","commit_id":"c5f7f18bcb2e1876b0712c691996346620ffc920"},{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"6b8474617be16ec339b5edaf64f4a97bcf70be76","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"f2587e81_4609900f","updated":"2022-03-15 16:09:56.000000000","message":"Thanks, Jacob! This is a very welcome improvement to make disk cleaning smarter!","commit_id":"c5f7f18bcb2e1876b0712c691996346620ffc920"}],"ironic_python_agent/hardware.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ad83fa4579a1202f34e495a8043cba724d95e96d","unresolved":true,"context_lines":[{"line_number":1400,"context_line":"        :raises BlockDeviceEraseError: when there\u0027s an error erasing the"},{"line_number":1401,"context_line":"                block device"},{"line_number":1402,"context_line":"        \"\"\""},{"line_number":1403,"context_line":"        block_devices \u003d self.list_block_devices(include_partitions\u003dTrue)"},{"line_number":1404,"context_line":"        # NOTE(coreywright): Reverse sort by device name so a partition (eg"},{"line_number":1405,"context_line":"        # sda1) is processed before it disappears when its associated disk (eg"},{"line_number":1406,"context_line":"        # sda) has its partition table erased and the kernel notified."},{"line_number":1407,"context_line":"        block_devices.sort(key\u003dlambda dev: dev.name, reverse\u003dTrue)"},{"line_number":1408,"context_line":"        erase_errors \u003d {}"},{"line_number":1409,"context_line":"        info \u003d node.get(\u0027driver_internal_info\u0027, {})"},{"line_number":1410,"context_line":"        for dev in block_devices:"},{"line_number":1411,"context_line":"            if self._is_virtual_media_device(dev):"},{"line_number":1412,"context_line":"                LOG.info(\"Skipping metadata erase of virtual media device %s\","},{"line_number":1413,"context_line":"                         dev.name)"},{"line_number":1414,"context_line":"                continue"},{"line_number":1415,"context_line":"            if self._is_linux_raid_member(dev):"},{"line_number":1416,"context_line":"                LOG.info(\"Skipping metadata erase of RAID member device %s\","},{"line_number":1417,"context_line":"                         dev.name)"},{"line_number":1418,"context_line":"                continue"},{"line_number":1419,"context_line":"            if self._is_read_only_device(dev):"},{"line_number":1420,"context_line":"                LOG.info(\"Skipping metadata erase of read-only device %s\","},{"line_number":1421,"context_line":"                         dev.name)"},{"line_number":1422,"context_line":"                continue"},{"line_number":1423,"context_line":"            try:"},{"line_number":1424,"context_line":"                if self._is_nvme(dev):"},{"line_number":1425,"context_line":"                    execute_nvme_erase \u003d info.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"621b0716_784c5cf3","line":1422,"range":{"start_line":1403,"start_character":1,"end_line":1422,"end_character":24},"updated":"2021-11-30 10:52:56.000000000","message":"Let\u0027s maybe separate all this logic into something like self._list_erasable_devices?","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"227441b9d1bbf26eafdc21437e9dbdf82e4ac22e","unresolved":false,"context_lines":[{"line_number":1400,"context_line":"        :raises BlockDeviceEraseError: when there\u0027s an error erasing the"},{"line_number":1401,"context_line":"                block device"},{"line_number":1402,"context_line":"        \"\"\""},{"line_number":1403,"context_line":"        block_devices \u003d self.list_block_devices(include_partitions\u003dTrue)"},{"line_number":1404,"context_line":"        # NOTE(coreywright): Reverse sort by device name so a partition (eg"},{"line_number":1405,"context_line":"        # sda1) is processed before it disappears when its associated disk (eg"},{"line_number":1406,"context_line":"        # sda) has its partition table erased and the kernel notified."},{"line_number":1407,"context_line":"        block_devices.sort(key\u003dlambda dev: dev.name, reverse\u003dTrue)"},{"line_number":1408,"context_line":"        erase_errors \u003d {}"},{"line_number":1409,"context_line":"        info \u003d node.get(\u0027driver_internal_info\u0027, {})"},{"line_number":1410,"context_line":"        for dev in block_devices:"},{"line_number":1411,"context_line":"            if self._is_virtual_media_device(dev):"},{"line_number":1412,"context_line":"                LOG.info(\"Skipping metadata erase of virtual media device %s\","},{"line_number":1413,"context_line":"                         dev.name)"},{"line_number":1414,"context_line":"                continue"},{"line_number":1415,"context_line":"            if self._is_linux_raid_member(dev):"},{"line_number":1416,"context_line":"                LOG.info(\"Skipping metadata erase of RAID member device %s\","},{"line_number":1417,"context_line":"                         dev.name)"},{"line_number":1418,"context_line":"                continue"},{"line_number":1419,"context_line":"            if self._is_read_only_device(dev):"},{"line_number":1420,"context_line":"                LOG.info(\"Skipping metadata erase of read-only device %s\","},{"line_number":1421,"context_line":"                         dev.name)"},{"line_number":1422,"context_line":"                continue"},{"line_number":1423,"context_line":"            try:"},{"line_number":1424,"context_line":"                if self._is_nvme(dev):"},{"line_number":1425,"context_line":"                    execute_nvme_erase \u003d info.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"d6e96152_60e31f34","line":1422,"range":{"start_line":1403,"start_character":1,"end_line":1422,"end_character":24},"in_reply_to":"621b0716_784c5cf3","updated":"2022-03-04 05:27:23.000000000","message":"Done. Should I switch erase_devices_metadata to use the new function in a separate change?","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ad83fa4579a1202f34e495a8043cba724d95e96d","unresolved":true,"context_lines":[{"line_number":1432,"context_line":"                    if execute_secure_erase and self._ata_erase(dev):"},{"line_number":1433,"context_line":"                        return"},{"line_number":1434,"context_line":"            except errors.BlockDeviceEraseError as e:"},{"line_number":1435,"context_line":"                LOG.info(\u0027Failed to securely erase device \"%(dev)s\". \u0027"},{"line_number":1436,"context_line":"                         \u0027Error: %(error)s, falling back to metadata clean\u0027,"},{"line_number":1437,"context_line":"                         {\u0027dev\u0027: dev.name, \u0027error\u0027: e})"},{"line_number":1438,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"0ee9fb70_c38d743a","line":1435,"updated":"2021-11-30 10:52:56.000000000","message":"s/info/warning/","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"dafe130b5c932073a39bfe8ea37931769e3b4e75","unresolved":false,"context_lines":[{"line_number":1432,"context_line":"                    if execute_secure_erase and self._ata_erase(dev):"},{"line_number":1433,"context_line":"                        return"},{"line_number":1434,"context_line":"            except errors.BlockDeviceEraseError as e:"},{"line_number":1435,"context_line":"                LOG.info(\u0027Failed to securely erase device \"%(dev)s\". \u0027"},{"line_number":1436,"context_line":"                         \u0027Error: %(error)s, falling back to metadata clean\u0027,"},{"line_number":1437,"context_line":"                         {\u0027dev\u0027: dev.name, \u0027error\u0027: e})"},{"line_number":1438,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"515be3a7_fef90018","line":1435,"in_reply_to":"0ee9fb70_c38d743a","updated":"2022-03-04 04:33:57.000000000","message":"Done","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ad83fa4579a1202f34e495a8043cba724d95e96d","unresolved":true,"context_lines":[{"line_number":1440,"context_line":"            except processutils.ProcessExecutionError as e:"},{"line_number":1441,"context_line":"                LOG.error(\u0027Failed to erase the metadata on device \"%(dev)s\". \u0027"},{"line_number":1442,"context_line":"                          \u0027Error: %(error)s\u0027, {\u0027dev\u0027: dev.name, \u0027error\u0027: e})"},{"line_number":1443,"context_line":"                erase_errors[dev.name] \u003d e"},{"line_number":1444,"context_line":""},{"line_number":1445,"context_line":"        if erase_errors:"},{"line_number":1446,"context_line":"            excpt_msg \u003d (\u0027Failed to erase the metadata on the device(s): %s\u0027 %"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fb71270_3892d1b6","line":1443,"updated":"2021-11-30 10:52:56.000000000","message":"I wonder if we could somehow also incorporate the error that happened above with the fast path. WDYT?","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9455b00eb6a6ef2fa19deb0a3f87a7be67dd4007","unresolved":true,"context_lines":[{"line_number":1440,"context_line":"            except processutils.ProcessExecutionError as e:"},{"line_number":1441,"context_line":"                LOG.error(\u0027Failed to erase the metadata on device \"%(dev)s\". \u0027"},{"line_number":1442,"context_line":"                          \u0027Error: %(error)s\u0027, {\u0027dev\u0027: dev.name, \u0027error\u0027: e})"},{"line_number":1443,"context_line":"                erase_errors[dev.name] \u003d e"},{"line_number":1444,"context_line":""},{"line_number":1445,"context_line":"        if erase_errors:"},{"line_number":1446,"context_line":"            excpt_msg \u003d (\u0027Failed to erase the metadata on the device(s): %s\u0027 %"}],"source_content_type":"text/x-python","patch_set":4,"id":"96ddae48_15a9e14d","line":1443,"in_reply_to":"134882c3_058c0313","updated":"2022-03-10 10:49:10.000000000","message":"I rather meant something like:\n\n erase_errors[dev.name] \u003d \"Secure erase failed: %s. Fall back to metadata erase also failed: %s\"\n\nWhere the first error is kept from block 1450.","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"28ef275e9a13e805082ad267a011d61987e2922e","unresolved":true,"context_lines":[{"line_number":1440,"context_line":"            except processutils.ProcessExecutionError as e:"},{"line_number":1441,"context_line":"                LOG.error(\u0027Failed to erase the metadata on device \"%(dev)s\". \u0027"},{"line_number":1442,"context_line":"                          \u0027Error: %(error)s\u0027, {\u0027dev\u0027: dev.name, \u0027error\u0027: e})"},{"line_number":1443,"context_line":"                erase_errors[dev.name] \u003d e"},{"line_number":1444,"context_line":""},{"line_number":1445,"context_line":"        if erase_errors:"},{"line_number":1446,"context_line":"            excpt_msg \u003d (\u0027Failed to erase the metadata on the device(s): %s\u0027 %"}],"source_content_type":"text/x-python","patch_set":4,"id":"134882c3_058c0313","line":1443,"in_reply_to":"3fb71270_3892d1b6","updated":"2022-03-04 06:26:34.000000000","message":"Good question Dmitry. My understanding is that if we just include errors from self._ata_erase(dev) in erase_errors[], with the way the code is currently written, this will cause an exception here even if destroy_disk_metadata runs successfully as the next step. \nI can think of two sample options here:\n1) we assign the error from the fast path to a variable and include this error in the log message if destroy_disk_metadata fails as well (IIUC we can no longer access the attributes of the previous exception in the following one as it is no longer the right scope hence the extra variable).\n2) we change the logic so that we don\u0027t raise an exception if erase_errors are not empty but we do it when destroy_disk_metadata fails instead (given it\u0027s the last thing we try). And then we can log all the errors recorded in erase_errors on failure.\nWhat are your thoughts? My understanding is that _ata_erase(dev) will produce exceptions when called against an unsupported device so we probably don\u0027t want to fail out yet when that happens provided that the following metadata clean works.","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[{"line_number":1440,"context_line":"            except processutils.ProcessExecutionError as e:"},{"line_number":1441,"context_line":"                LOG.error(\u0027Failed to erase the metadata on device \"%(dev)s\". \u0027"},{"line_number":1442,"context_line":"                          \u0027Error: %(error)s\u0027, {\u0027dev\u0027: dev.name, \u0027error\u0027: e})"},{"line_number":1443,"context_line":"                erase_errors[dev.name] \u003d e"},{"line_number":1444,"context_line":""},{"line_number":1445,"context_line":"        if erase_errors:"},{"line_number":1446,"context_line":"            excpt_msg \u003d (\u0027Failed to erase the metadata on the device(s): %s\u0027 %"}],"source_content_type":"text/x-python","patch_set":4,"id":"c133967f_67324fdb","line":1443,"in_reply_to":"96ddae48_15a9e14d","updated":"2022-03-10 11:57:51.000000000","message":"Done","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ad83fa4579a1202f34e495a8043cba724d95e96d","unresolved":true,"context_lines":[{"line_number":1443,"context_line":"                erase_errors[dev.name] \u003d e"},{"line_number":1444,"context_line":""},{"line_number":1445,"context_line":"        if erase_errors:"},{"line_number":1446,"context_line":"            excpt_msg \u003d (\u0027Failed to erase the metadata on the device(s): %s\u0027 %"},{"line_number":1447,"context_line":"                         \u0027; \u0027.join([\u0027\"%s\": %s\u0027 % (k, v)"},{"line_number":1448,"context_line":"                                    for k, v in erase_errors.items()]))"},{"line_number":1449,"context_line":"            raise errors.BlockDeviceEraseError(excpt_msg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"c619e9f9_46a4d6f1","line":1446,"updated":"2021-11-30 10:52:56.000000000","message":"mmm, \"conduct an express erase\"?","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"dafe130b5c932073a39bfe8ea37931769e3b4e75","unresolved":false,"context_lines":[{"line_number":1443,"context_line":"                erase_errors[dev.name] \u003d e"},{"line_number":1444,"context_line":""},{"line_number":1445,"context_line":"        if erase_errors:"},{"line_number":1446,"context_line":"            excpt_msg \u003d (\u0027Failed to erase the metadata on the device(s): %s\u0027 %"},{"line_number":1447,"context_line":"                         \u0027; \u0027.join([\u0027\"%s\": %s\u0027 % (k, v)"},{"line_number":1448,"context_line":"                                    for k, v in erase_errors.items()]))"},{"line_number":1449,"context_line":"            raise errors.BlockDeviceEraseError(excpt_msg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"02bcbc29_6f6c0ae3","line":1446,"in_reply_to":"c619e9f9_46a4d6f1","updated":"2022-03-04 04:33:57.000000000","message":"Done","commit_id":"fce566ecfd10942ce829547022b8c007d0c70278"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9455b00eb6a6ef2fa19deb0a3f87a7be67dd4007","unresolved":true,"context_lines":[{"line_number":1361,"context_line":"        # sda) has its partition table erased and the kernel notified."},{"line_number":1362,"context_line":"        block_devices.sort(key\u003dlambda dev: dev.name, reverse\u003dTrue)"},{"line_number":1363,"context_line":"        erase_errors \u003d {}"},{"line_number":1364,"context_line":"        for dev in block_devices:"},{"line_number":1365,"context_line":"            if self._is_virtual_media_device(dev):"},{"line_number":1366,"context_line":"                LOG.info(\"Skipping metadata erase of virtual media device %s\","},{"line_number":1367,"context_line":"                         dev.name)"}],"source_content_type":"text/x-python","patch_set":12,"id":"a3cd91d2_3aff23d1","line":1364,"updated":"2022-03-10 10:49:10.000000000","message":"Please reuse the new helper here","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[{"line_number":1361,"context_line":"        # sda) has its partition table erased and the kernel notified."},{"line_number":1362,"context_line":"        block_devices.sort(key\u003dlambda dev: dev.name, reverse\u003dTrue)"},{"line_number":1363,"context_line":"        erase_errors \u003d {}"},{"line_number":1364,"context_line":"        for dev in block_devices:"},{"line_number":1365,"context_line":"            if self._is_virtual_media_device(dev):"},{"line_number":1366,"context_line":"                LOG.info(\"Skipping metadata erase of virtual media device %s\","},{"line_number":1367,"context_line":"                         dev.name)"}],"source_content_type":"text/x-python","patch_set":12,"id":"80ea9ac6_621c5a1e","line":1364,"in_reply_to":"a3cd91d2_3aff23d1","updated":"2022-03-10 11:57:51.000000000","message":"Done","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9455b00eb6a6ef2fa19deb0a3f87a7be67dd4007","unresolved":true,"context_lines":[{"line_number":1397,"context_line":"        erasable_devices \u003d []"},{"line_number":1398,"context_line":"        for dev in block_devices:"},{"line_number":1399,"context_line":"            if self._is_virtual_media_device(dev):"},{"line_number":1400,"context_line":"                LOG.info(\"Skipping metadata erase of virtual media device %s\","},{"line_number":1401,"context_line":"                         dev.name)"},{"line_number":1402,"context_line":"                continue"},{"line_number":1403,"context_line":"            if self._is_linux_raid_member(dev):"}],"source_content_type":"text/x-python","patch_set":12,"id":"01ecf201_8a763f0f","line":1400,"updated":"2022-03-10 10:49:10.000000000","message":"s/metadata/???/","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"        erasable_devices \u003d []"},{"line_number":1398,"context_line":"        for dev in block_devices:"},{"line_number":1399,"context_line":"            if self._is_virtual_media_device(dev):"},{"line_number":1400,"context_line":"                LOG.info(\"Skipping metadata erase of virtual media device %s\","},{"line_number":1401,"context_line":"                         dev.name)"},{"line_number":1402,"context_line":"                continue"},{"line_number":1403,"context_line":"            if self._is_linux_raid_member(dev):"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a13534d_1e2b615b","line":1400,"in_reply_to":"01ecf201_8a763f0f","updated":"2022-03-10 11:57:51.000000000","message":"I think I\u0027ll just drop \"metadata\" - this will be used by several erasure modes so maybe just \"erasure\"? WDYT?","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9455b00eb6a6ef2fa19deb0a3f87a7be67dd4007","unresolved":true,"context_lines":[{"line_number":1411,"context_line":"            erasable_devices.append(dev)"},{"line_number":1412,"context_line":""},{"line_number":1413,"context_line":"        LOG.debug(\u0027Erasable devices \"%(devlist)s\" \u0027,"},{"line_number":1414,"context_line":"                  {\u0027devlist\u0027: str(erasable_devices)})"},{"line_number":1415,"context_line":""},{"line_number":1416,"context_line":"        return erasable_devices"},{"line_number":1417,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"b7075d71_efdd6717","line":1414,"range":{"start_line":1414,"start_character":48,"end_line":1414,"end_character":49},"updated":"2022-03-10 10:49:10.000000000","message":"nit: no need for str()","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[{"line_number":1411,"context_line":"            erasable_devices.append(dev)"},{"line_number":1412,"context_line":""},{"line_number":1413,"context_line":"        LOG.debug(\u0027Erasable devices \"%(devlist)s\" \u0027,"},{"line_number":1414,"context_line":"                  {\u0027devlist\u0027: str(erasable_devices)})"},{"line_number":1415,"context_line":""},{"line_number":1416,"context_line":"        return erasable_devices"},{"line_number":1417,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"8cf2e768_1803a15c","line":1414,"range":{"start_line":1414,"start_character":48,"end_line":1414,"end_character":49},"in_reply_to":"b7075d71_efdd6717","updated":"2022-03-10 11:57:51.000000000","message":"I\u0027ll remove this entirely.","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9455b00eb6a6ef2fa19deb0a3f87a7be67dd4007","unresolved":true,"context_lines":[{"line_number":1429,"context_line":"        \"\"\""},{"line_number":1430,"context_line":"        erase_errors \u003d {}"},{"line_number":1431,"context_line":"        info \u003d node.get(\u0027driver_internal_info\u0027, {})"},{"line_number":1432,"context_line":"        # NOTE(janders) this is for debugging only - to be removed."},{"line_number":1433,"context_line":"        erasable_devices \u003d self._list_erasable_devices()"},{"line_number":1434,"context_line":"        LOG.debug(\u0027From list_erasable_devices \"%(devlist)s\" \u0027,"},{"line_number":1435,"context_line":"                  {\u0027devlist\u0027: str(erasable_devices)})"}],"source_content_type":"text/x-python","patch_set":12,"id":"a51fe539_7d0f5cf4","line":1432,"updated":"2022-03-10 10:49:10.000000000","message":"Please do :)","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[{"line_number":1429,"context_line":"        \"\"\""},{"line_number":1430,"context_line":"        erase_errors \u003d {}"},{"line_number":1431,"context_line":"        info \u003d node.get(\u0027driver_internal_info\u0027, {})"},{"line_number":1432,"context_line":"        # NOTE(janders) this is for debugging only - to be removed."},{"line_number":1433,"context_line":"        erasable_devices \u003d self._list_erasable_devices()"},{"line_number":1434,"context_line":"        LOG.debug(\u0027From list_erasable_devices \"%(devlist)s\" \u0027,"},{"line_number":1435,"context_line":"                  {\u0027devlist\u0027: str(erasable_devices)})"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a91fbd9_28ef318d","line":1432,"in_reply_to":"a51fe539_7d0f5cf4","updated":"2022-03-10 11:57:51.000000000","message":"And this :)","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"a68af435c31713f1d771b3b0e96321717235e97b","unresolved":true,"context_lines":[{"line_number":1435,"context_line":"                  {\u0027devlist\u0027: str(erasable_devices)})"},{"line_number":1436,"context_line":"        # NOTE(janders) end debug"},{"line_number":1437,"context_line":""},{"line_number":1438,"context_line":"        for dev in self._list_erasable_devices():"},{"line_number":1439,"context_line":"            try:"},{"line_number":1440,"context_line":"                if self._is_nvme(dev):"},{"line_number":1441,"context_line":"                    execute_nvme_erase \u003d info.get("}],"source_content_type":"text/x-python","patch_set":12,"id":"672dc123_13b026c9","line":1438,"updated":"2022-03-10 06:43:13.000000000","message":"I know it\u0027s a remote possibility, but should we consider the case of an empty list of erasable devices? Something like giving a warning?","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"bbe0511b9b1143bbae7781a0da73f3b20600c0b1","unresolved":true,"context_lines":[{"line_number":1435,"context_line":"                  {\u0027devlist\u0027: str(erasable_devices)})"},{"line_number":1436,"context_line":"        # NOTE(janders) end debug"},{"line_number":1437,"context_line":""},{"line_number":1438,"context_line":"        for dev in self._list_erasable_devices():"},{"line_number":1439,"context_line":"            try:"},{"line_number":1440,"context_line":"                if self._is_nvme(dev):"},{"line_number":1441,"context_line":"                    execute_nvme_erase \u003d info.get("}],"source_content_type":"text/x-python","patch_set":12,"id":"de4d057b_2bb70e92","line":1438,"in_reply_to":"672dc123_13b026c9","updated":"2022-03-10 09:15:37.000000000","message":"Thank you Riccardo. I like the idea. This change really caters for HPC and Big Data users - and they definitely do run diskless. So it may be useful to flag this for cases where there\u0027s something unexpected going on. Will fix this later tonight.","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[{"line_number":1435,"context_line":"                  {\u0027devlist\u0027: str(erasable_devices)})"},{"line_number":1436,"context_line":"        # NOTE(janders) end debug"},{"line_number":1437,"context_line":""},{"line_number":1438,"context_line":"        for dev in self._list_erasable_devices():"},{"line_number":1439,"context_line":"            try:"},{"line_number":1440,"context_line":"                if self._is_nvme(dev):"},{"line_number":1441,"context_line":"                    execute_nvme_erase \u003d info.get("}],"source_content_type":"text/x-python","patch_set":12,"id":"7b0a6386_7b991f7a","line":1438,"in_reply_to":"de4d057b_2bb70e92","updated":"2022-03-10 11:57:51.000000000","message":"Done","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9455b00eb6a6ef2fa19deb0a3f87a7be67dd4007","unresolved":true,"context_lines":[{"line_number":1441,"context_line":"                    execute_nvme_erase \u003d info.get("},{"line_number":1442,"context_line":"                        \u0027agent_enable_nvme_secure_erase\u0027, True)"},{"line_number":1443,"context_line":"                    if execute_nvme_erase and self._nvme_erase(dev):"},{"line_number":1444,"context_line":"                        return"},{"line_number":1445,"context_line":"                else:"},{"line_number":1446,"context_line":"                    execute_secure_erase \u003d info.get("},{"line_number":1447,"context_line":"                        \u0027agent_enable_ata_secure_erase\u0027, True)"}],"source_content_type":"text/x-python","patch_set":12,"id":"a34c97de_9b5a1370","line":1444,"updated":"2022-03-10 10:49:10.000000000","message":"I suspect you meant \u0027continue\u0027 here and below","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[{"line_number":1441,"context_line":"                    execute_nvme_erase \u003d info.get("},{"line_number":1442,"context_line":"                        \u0027agent_enable_nvme_secure_erase\u0027, True)"},{"line_number":1443,"context_line":"                    if execute_nvme_erase and self._nvme_erase(dev):"},{"line_number":1444,"context_line":"                        return"},{"line_number":1445,"context_line":"                else:"},{"line_number":1446,"context_line":"                    execute_secure_erase \u003d info.get("},{"line_number":1447,"context_line":"                        \u0027agent_enable_ata_secure_erase\u0027, True)"}],"source_content_type":"text/x-python","patch_set":12,"id":"c0219908_54d00a6e","line":1444,"in_reply_to":"a34c97de_9b5a1370","updated":"2022-03-10 11:57:51.000000000","message":"Yeah - well spotted, thank you!","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9455b00eb6a6ef2fa19deb0a3f87a7be67dd4007","unresolved":true,"context_lines":[{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"        if erase_errors:"},{"line_number":1462,"context_line":"            excpt_msg \u003d (\u0027Failed to conduct an express erase on \u0027"},{"line_number":1463,"context_line":"                         \u0027the device(s): %s; \u0027.join([\u0027\"%s\": %s\u0027 % (k, v)"},{"line_number":1464,"context_line":"                                                    for k, v in"},{"line_number":1465,"context_line":"                                                    erase_errors.items()]))"},{"line_number":1466,"context_line":"            raise errors.BlockDeviceEraseError(excpt_msg)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bc4e34fe_c29b06e0","line":1463,"updated":"2022-03-10 10:49:10.000000000","message":"I think you meant\n\n \u0027the device(s): %s\u0027 % \u0027\\n\u0027.join(...)\n\n(also no need for [] inside join())","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"        if erase_errors:"},{"line_number":1462,"context_line":"            excpt_msg \u003d (\u0027Failed to conduct an express erase on \u0027"},{"line_number":1463,"context_line":"                         \u0027the device(s): %s; \u0027.join([\u0027\"%s\": %s\u0027 % (k, v)"},{"line_number":1464,"context_line":"                                                    for k, v in"},{"line_number":1465,"context_line":"                                                    erase_errors.items()]))"},{"line_number":1466,"context_line":"            raise errors.BlockDeviceEraseError(excpt_msg)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9ec52cee_7bc17b12","line":1463,"in_reply_to":"bc4e34fe_c29b06e0","updated":"2022-03-10 11:57:51.000000000","message":"Done","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9455b00eb6a6ef2fa19deb0a3f87a7be67dd4007","unresolved":true,"context_lines":[{"line_number":1461,"context_line":"        if erase_errors:"},{"line_number":1462,"context_line":"            excpt_msg \u003d (\u0027Failed to conduct an express erase on \u0027"},{"line_number":1463,"context_line":"                         \u0027the device(s): %s; \u0027.join([\u0027\"%s\": %s\u0027 % (k, v)"},{"line_number":1464,"context_line":"                                                    for k, v in"},{"line_number":1465,"context_line":"                                                    erase_errors.items()]))"},{"line_number":1466,"context_line":"            raise errors.BlockDeviceEraseError(excpt_msg)"},{"line_number":1467,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"b2a22fb0_7108161a","line":1464,"updated":"2022-03-10 10:49:10.000000000","message":"nit: also no need to unpack the tuple\n\n .. join(\u0027\"%s\": %s\u0027 % item for item in erase_errors.items())","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"b9687a28d6db193112727d4188dbdb00bb8d7de6","unresolved":false,"context_lines":[{"line_number":1461,"context_line":"        if erase_errors:"},{"line_number":1462,"context_line":"            excpt_msg \u003d (\u0027Failed to conduct an express erase on \u0027"},{"line_number":1463,"context_line":"                         \u0027the device(s): %s; \u0027.join([\u0027\"%s\": %s\u0027 % (k, v)"},{"line_number":1464,"context_line":"                                                    for k, v in"},{"line_number":1465,"context_line":"                                                    erase_errors.items()]))"},{"line_number":1466,"context_line":"            raise errors.BlockDeviceEraseError(excpt_msg)"},{"line_number":1467,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"cea1afae_52656647","line":1464,"in_reply_to":"b2a22fb0_7108161a","updated":"2022-03-10 11:57:51.000000000","message":"Done","commit_id":"f72e23063d3dd2630a1039efee25120fa4a16fe3"},{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"bb4680560f010e8d4382599e75cd1c1db3200b7c","unresolved":true,"context_lines":[{"line_number":1413,"context_line":"        \"\"\""},{"line_number":1414,"context_line":"        erase_errors \u003d {}"},{"line_number":1415,"context_line":"        info \u003d node.get(\u0027driver_internal_info\u0027, {})"},{"line_number":1416,"context_line":"        if self._list_erasable_devices:"},{"line_number":1417,"context_line":"            for dev in self._list_erasable_devices():"},{"line_number":1418,"context_line":"                try:"},{"line_number":1419,"context_line":"                    if self._is_nvme(dev):"}],"source_content_type":"text/x-python","patch_set":19,"id":"38ac9e0a_fc7f3dfd","line":1416,"range":{"start_line":1416,"start_character":0,"end_line":1416,"end_character":39},"updated":"2022-03-11 13:37:59.000000000","message":"In the interest of limiting the indentation, I would probably do:\n\nif not self._list_erasable_devices:\n    LOG.warn(\"No erasable devices have been found.\")\n    return\n\nfor dev in self._list_erasable_devices():\n    ...","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"3c526dde43f8a8f13fcba3a25be541f012eb0775","unresolved":false,"context_lines":[{"line_number":1413,"context_line":"        \"\"\""},{"line_number":1414,"context_line":"        erase_errors \u003d {}"},{"line_number":1415,"context_line":"        info \u003d node.get(\u0027driver_internal_info\u0027, {})"},{"line_number":1416,"context_line":"        if self._list_erasable_devices:"},{"line_number":1417,"context_line":"            for dev in self._list_erasable_devices():"},{"line_number":1418,"context_line":"                try:"},{"line_number":1419,"context_line":"                    if self._is_nvme(dev):"}],"source_content_type":"text/x-python","patch_set":19,"id":"faba07ab_12895283","line":1416,"range":{"start_line":1416,"start_character":0,"end_line":1416,"end_character":39},"in_reply_to":"38ac9e0a_fc7f3dfd","updated":"2022-03-11 21:02:18.000000000","message":"Done","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"bb4680560f010e8d4382599e75cd1c1db3200b7c","unresolved":true,"context_lines":[{"line_number":1445,"context_line":"                    else:"},{"line_number":1446,"context_line":"                        erase_errors[dev.name] \u003d e"},{"line_number":1447,"context_line":"        else:"},{"line_number":1448,"context_line":"            LOG.warn(\"No erasable devices have been found.\")"},{"line_number":1449,"context_line":""},{"line_number":1450,"context_line":"        if erase_errors:"},{"line_number":1451,"context_line":"            excpt_msg \u003d (\u0027Failed to conduct an express erase on \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"6fe95cc2_adedc5f8","line":1448,"range":{"start_line":1448,"start_character":1,"end_line":1448,"end_character":60},"updated":"2022-03-11 13:37:59.000000000","message":"Shouldn\u0027t this be just a debug message?","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"3c526dde43f8a8f13fcba3a25be541f012eb0775","unresolved":false,"context_lines":[{"line_number":1445,"context_line":"                    else:"},{"line_number":1446,"context_line":"                        erase_errors[dev.name] \u003d e"},{"line_number":1447,"context_line":"        else:"},{"line_number":1448,"context_line":"            LOG.warn(\"No erasable devices have been found.\")"},{"line_number":1449,"context_line":""},{"line_number":1450,"context_line":"        if erase_errors:"},{"line_number":1451,"context_line":"            excpt_msg \u003d (\u0027Failed to conduct an express erase on \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"a114356b_c7863100","line":1448,"range":{"start_line":1448,"start_character":1,"end_line":1448,"end_character":60},"in_reply_to":"6fe95cc2_adedc5f8","updated":"2022-03-11 21:02:18.000000000","message":"Done","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"d88baf63e8fde8d8b2a6a9415faeb7f40b0a4aac","unresolved":true,"context_lines":[{"line_number":1999,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2000,"context_line":"                \u0027abortable\u0027: True"},{"line_number":2001,"context_line":"            },"},{"line_number":2002,"context_line":"            {"},{"line_number":2003,"context_line":"                \u0027step\u0027: \u0027erase_devices_metadata\u0027,"},{"line_number":2004,"context_line":"                \u0027priority\u0027: 99,"},{"line_number":2005,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"},{"line_number":2006,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2007,"context_line":"                \u0027abortable\u0027: True"},{"line_number":2008,"context_line":"            },"},{"line_number":2009,"context_line":"            {"},{"line_number":2010,"context_line":"                \u0027step\u0027: \u0027erase_devices_express\u0027,"},{"line_number":2011,"context_line":"                \u0027priority\u0027: 20,"},{"line_number":2012,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"},{"line_number":2013,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2014,"context_line":"                \u0027abortable\u0027: True"},{"line_number":2015,"context_line":"            },"},{"line_number":2016,"context_line":"            {"},{"line_number":2017,"context_line":"                \u0027step\u0027: \u0027erase_pstore\u0027,"},{"line_number":2018,"context_line":"                \u0027priority\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":19,"id":"85280e48_8d30f980","line":2015,"range":{"start_line":2002,"start_character":0,"end_line":2015,"end_character":14},"updated":"2022-03-11 04:44:52.000000000","message":"@Dmitry @Iury @Arne - I wonder whether we should set priority of erase_devices_express to 0 in the ramdisk and have the operators interested in using the feature set override, e.g.[conductor]/clean_step_priority_override\u003ddeploy.erase_devices_express:5\n?\nThe former-operator in me is thinking given we\u0027re talking about cleaning functionality here (as in - something security sensitive) the default behavior may be best left unchanged. And then only those who need the feature can switch it on for starters. With time, it could become the default option.\nWDYT?","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"f262eadd623a316d9116a07837199b379ec63f38","unresolved":true,"context_lines":[{"line_number":1999,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2000,"context_line":"                \u0027abortable\u0027: True"},{"line_number":2001,"context_line":"            },"},{"line_number":2002,"context_line":"            {"},{"line_number":2003,"context_line":"                \u0027step\u0027: \u0027erase_devices_metadata\u0027,"},{"line_number":2004,"context_line":"                \u0027priority\u0027: 99,"},{"line_number":2005,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"},{"line_number":2006,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2007,"context_line":"                \u0027abortable\u0027: True"},{"line_number":2008,"context_line":"            },"},{"line_number":2009,"context_line":"            {"},{"line_number":2010,"context_line":"                \u0027step\u0027: \u0027erase_devices_express\u0027,"},{"line_number":2011,"context_line":"                \u0027priority\u0027: 20,"},{"line_number":2012,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"},{"line_number":2013,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2014,"context_line":"                \u0027abortable\u0027: True"},{"line_number":2015,"context_line":"            },"},{"line_number":2016,"context_line":"            {"},{"line_number":2017,"context_line":"                \u0027step\u0027: \u0027erase_pstore\u0027,"},{"line_number":2018,"context_line":"                \u0027priority\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":19,"id":"e683c02e_a85427c8","line":2015,"range":{"start_line":2002,"start_character":0,"end_line":2015,"end_character":14},"in_reply_to":"85280e48_8d30f980","updated":"2022-03-11 09:06:15.000000000","message":"I think it\u0027s a good idea, especially to avoid the step to kick it automatically right away.","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"d260ea95ebc9e7aa79d414b9f567429bef4aac00","unresolved":false,"context_lines":[{"line_number":1999,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2000,"context_line":"                \u0027abortable\u0027: True"},{"line_number":2001,"context_line":"            },"},{"line_number":2002,"context_line":"            {"},{"line_number":2003,"context_line":"                \u0027step\u0027: \u0027erase_devices_metadata\u0027,"},{"line_number":2004,"context_line":"                \u0027priority\u0027: 99,"},{"line_number":2005,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"},{"line_number":2006,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2007,"context_line":"                \u0027abortable\u0027: True"},{"line_number":2008,"context_line":"            },"},{"line_number":2009,"context_line":"            {"},{"line_number":2010,"context_line":"                \u0027step\u0027: \u0027erase_devices_express\u0027,"},{"line_number":2011,"context_line":"                \u0027priority\u0027: 20,"},{"line_number":2012,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"},{"line_number":2013,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2014,"context_line":"                \u0027abortable\u0027: True"},{"line_number":2015,"context_line":"            },"},{"line_number":2016,"context_line":"            {"},{"line_number":2017,"context_line":"                \u0027step\u0027: \u0027erase_pstore\u0027,"},{"line_number":2018,"context_line":"                \u0027priority\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":19,"id":"b1efc924_ac73daba","line":2015,"range":{"start_line":2002,"start_character":0,"end_line":2015,"end_character":14},"in_reply_to":"e683c02e_a85427c8","updated":"2022-03-11 09:07:14.000000000","message":"Thank you Dmitry, uploading new revision soon.","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"22b2af8f0f3fbf65b8879105cc4f93da65f9472c","unresolved":false,"context_lines":[{"line_number":2008,"context_line":"            },"},{"line_number":2009,"context_line":"            {"},{"line_number":2010,"context_line":"                \u0027step\u0027: \u0027erase_devices_express\u0027,"},{"line_number":2011,"context_line":"                \u0027priority\u0027: 0,"},{"line_number":2012,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"},{"line_number":2013,"context_line":"                \u0027reboot_requested\u0027: False,"},{"line_number":2014,"context_line":"                \u0027abortable\u0027: True"}],"source_content_type":"text/x-python","patch_set":20,"id":"2b58e898_6b5bf376","line":2011,"range":{"start_line":2011,"start_character":0,"end_line":2011,"end_character":30},"updated":"2022-03-11 09:47:00.000000000","message":"Confirmed that it still works fine with a zero here and priority override in place:\n\n[root@localhost ~]# grep \"Disk metadata\" /var/log/messages\nMar 11 09:27:44 localhost ironic-python-agent[1660]: 2022-03-11 09:27:44.174 1660 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdi successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 09:27:46 localhost ironic-python-agent[1660]: 2022-03-11 09:27:46.492 1660 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdh successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 09:27:48 localhost ironic-python-agent[1660]: 2022-03-11 09:27:48.802 1660 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdg successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 09:27:51 localhost ironic-python-agent[1660]: 2022-03-11 09:27:51.121 1660 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdf successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 09:27:53 localhost ironic-python-agent[1660]: 2022-03-11 09:27:53.426 1660 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sde successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 09:27:55 localhost ironic-python-agent[1660]: 2022-03-11 09:27:55.755 1660 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdd successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 09:27:58 localhost ironic-python-agent[1660]: 2022-03-11 09:27:58.060 1660 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdc successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 09:28:00 localhost ironic-python-agent[1660]: 2022-03-11 09:28:00.351 1660 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sdb successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\nMar 11 09:28:01 localhost ironic-python-agent[1660]: 2022-03-11 09:28:01.405 1660 INFO ironic_lib.disk_utils [-] Disk metadata on /dev/sda successfully destroyed for node 4e41df61-84b1-5856-bfb6-6b5f2cd3dd11#033[00m\n[root@localhost ~]# grep nvme-cli /var/log/messages\nMar 11 09:28:04 localhost ironic-python-agent[1660]: 2022-03-11 09:28:04.571 1660 INFO root [-] nvme-cli format for device /dev/nvme1n1 (ses\u003d 2 ) completed successfully.#033[00m\nMar 11 09:28:09 localhost ironic-python-agent[1660]: 2022-03-11 09:28:09.889 1660 INFO root [-] nvme-cli format for device /dev/nvme0n1 (ses\u003d 2 ) completed successfully.#033[00m\n[root@localhost ~]# grep -A 1 \" \u0027step\u0027: \u0027erase_devices_express\u0027\" /opt/ironic-python-agent/lib/python3.6/site-packages/ironic_python_agent/hardware.py \n                \u0027step\u0027: \u0027erase_devices_express\u0027,\n                \u0027priority\u0027: 0,\n[root@localhost ~]#","commit_id":"c806901435d4096675f479ebd5c2a8a6c84bfe65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"9c018a2e4aeec652ccca2058ff9f264d4304586d","unresolved":true,"context_lines":[{"line_number":781,"context_line":"        info \u003d node.get(\u0027driver_internal_info\u0027, {})"},{"line_number":782,"context_line":"        max_pool_size \u003d info.get(\u0027disk_erasure_concurrency\u0027, 1)"},{"line_number":783,"context_line":""},{"line_number":784,"context_line":"        thread_pool \u003d ThreadPool(min(max_pool_size, len(block_devices)))"},{"line_number":785,"context_line":"        for block_device in block_devices:"},{"line_number":786,"context_line":"            params \u003d {\u0027node\u0027: node, \u0027block_device\u0027: block_device}"},{"line_number":787,"context_line":"            erase_results[block_device.name] \u003d thread_pool.apply_async("},{"line_number":788,"context_line":"                dispatch_to_managers, (\u0027erase_block_device\u0027,), params)"},{"line_number":789,"context_line":"        thread_pool.close()"},{"line_number":790,"context_line":"        thread_pool.join()"},{"line_number":791,"context_line":""},{"line_number":792,"context_line":"        for device_name, result in erase_results.items():"},{"line_number":793,"context_line":"            erase_results[device_name] \u003d result.get()"},{"line_number":794,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"7df56929_2cccce64","line":791,"range":{"start_line":784,"start_character":3,"end_line":791,"end_character":0},"updated":"2022-03-12 00:00:36.000000000","message":"This may be useful to reference, specifically because ATA erase and any fallbacks should be parallelizedsince even ATA secure erase, on spinning devices can still be multi-hour operations.... If the device does not support rotating encryption keys or if it just has a null key set to begin with.","commit_id":"fffaae84d8387d30cff567de13856c2c6cb8e3bd"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"49e0e7f508e22f210b47b8798a30015aa134e519","unresolved":true,"context_lines":[{"line_number":781,"context_line":"        info \u003d node.get(\u0027driver_internal_info\u0027, {})"},{"line_number":782,"context_line":"        max_pool_size \u003d info.get(\u0027disk_erasure_concurrency\u0027, 1)"},{"line_number":783,"context_line":""},{"line_number":784,"context_line":"        thread_pool \u003d ThreadPool(min(max_pool_size, len(block_devices)))"},{"line_number":785,"context_line":"        for block_device in block_devices:"},{"line_number":786,"context_line":"            params \u003d {\u0027node\u0027: node, \u0027block_device\u0027: block_device}"},{"line_number":787,"context_line":"            erase_results[block_device.name] \u003d thread_pool.apply_async("},{"line_number":788,"context_line":"                dispatch_to_managers, (\u0027erase_block_device\u0027,), params)"},{"line_number":789,"context_line":"        thread_pool.close()"},{"line_number":790,"context_line":"        thread_pool.join()"},{"line_number":791,"context_line":""},{"line_number":792,"context_line":"        for device_name, result in erase_results.items():"},{"line_number":793,"context_line":"            erase_results[device_name] \u003d result.get()"},{"line_number":794,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"fb608d90_1635f004","line":791,"range":{"start_line":784,"start_character":3,"end_line":791,"end_character":0},"in_reply_to":"7df56929_2cccce64","updated":"2022-03-14 05:22:35.000000000","message":"Thank you Julia, you raised a good point. \n\nI will start with two statements to set the context:\n\n1) I don\u0027t have any first hand experience with devices that support ATA secure erase implemented as a long-running operation so haven\u0027t thought about this particular use case, my main goal was to help with NVMe+HDD scenario. I added ATA erase support thinking of it as an equivalent of NVMe erase for SATA SSDs thinking \"why not help these cases too\". You caught me out on the HDD use case! 😊\n\n2) In the early days of working on this patch I did actually ask myself whether we need this to be parallel or not. Given I am thinking of this feature as a \"hardware assisted\" erase_devices_metadata, I figured I will use the same approach as erase_devices_metadata which doesn\u0027t use parallelism given the runtime is reasonably quick.\n\nI see your point regarding parallelism. For this particular feature I\u0027d be leaning towards saying that if a secure erase operation is long-running, it should not be a part of this cleaning mode whether it\u0027s using parallelism or not (and it belongs in erase_devices). My idea was to perform time-optimised cleaning - use fast, \"hardware-accelerated\" secure erasure methods where available (e.g. NVMe secure erase or ATA secure erase) and fall back to metadata clean if not. If a particular model of a storage device takes say 3 hours to clean with ATA erase, I think this is already breaking the promise of \"express\" erasure. Not using parallelism will make it blow out way more if we have say 16 of such devices in the system just as you pointed out (in which case we may be looking at 48 hours). \n\nIf we are dealing with long-running ATA secure erase, I\u0027d lean towards saying we shouldn\u0027t use it here, even if it\u0027s made run in parallel on each device in the system. The challenge there is - how would we know whether it\u0027s long running or not. With NVMes, IIRC at least some of them do actually report estimated erasure time if queried. Does such feature exist for ATA secure erase and if so do we know if it is dependable enough to build upon it? Any insights on this would be very helpful.\n\nWithout betting on the above information I see a couple potential ways forward:\n\n1) we remove ATA erase from this feature entirely and try NVMe erase and fallback to metadata erase if NVMe erase isn\u0027t available.\nPros: we will not run into an issue where long-running ATA erase operations make erase_devices_express break it\u0027s \"express\" promise\nCons: we may prevent operators with SATA SSDs that support near-instant ATA erase and would *just work* as a part of this feature from fully utilising the features of their hardware\n\n2) we keep it as-is and perhaps add a \"known issue\" with devices which only support long-running ATA secure erase. We could also add parallelism later as a bugfix as you suggested.\nPros: We retain support for SATA devices that are capable of fast and secure data erasure \nCons: Operators may run into the issue where erase_devices_express isn\u0027t that express\n\n3) we could make this behaviour configurable in some shape or form (even today global use of ATA secure erase could be disabled, avoiding this problem). If we have a dedicated option that can be overly complex (but more flexible).\n\nLet me know your thoughts. I would also value feedback from Arne who is a likely user of this feature and from Dmitry and Iury who\u0027ve been assisting me in implementing this.","commit_id":"fffaae84d8387d30cff567de13856c2c6cb8e3bd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"9c018a2e4aeec652ccca2058ff9f264d4304586d","unresolved":true,"context_lines":[{"line_number":1404,"context_line":""},{"line_number":1405,"context_line":"                for NVMe devices, perform NVMe Secure Erase if supported,"},{"line_number":1406,"context_line":"                for ATA devices, attempt ATA Secure Erase"},{"line_number":1407,"context_line":"                for other devices, perform metadata erasure"},{"line_number":1408,"context_line":""},{"line_number":1409,"context_line":"        :param node: Ironic node object"},{"line_number":1410,"context_line":"        :param ports: list of Ironic port objects"}],"source_content_type":"text/x-python","patch_set":22,"id":"7c561c07_385894aa","line":1407,"range":{"start_line":1407,"start_character":8,"end_line":1407,"end_character":15},"updated":"2022-03-12 00:00:36.000000000","message":"Style wise, we don\u0027t generally indent this that much.","commit_id":"fffaae84d8387d30cff567de13856c2c6cb8e3bd"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"49e0e7f508e22f210b47b8798a30015aa134e519","unresolved":false,"context_lines":[{"line_number":1404,"context_line":""},{"line_number":1405,"context_line":"                for NVMe devices, perform NVMe Secure Erase if supported,"},{"line_number":1406,"context_line":"                for ATA devices, attempt ATA Secure Erase"},{"line_number":1407,"context_line":"                for other devices, perform metadata erasure"},{"line_number":1408,"context_line":""},{"line_number":1409,"context_line":"        :param node: Ironic node object"},{"line_number":1410,"context_line":"        :param ports: list of Ironic port objects"}],"source_content_type":"text/x-python","patch_set":22,"id":"352360b2_13f18e41","line":1407,"range":{"start_line":1407,"start_character":8,"end_line":1407,"end_character":15},"in_reply_to":"7c561c07_385894aa","updated":"2022-03-14 05:22:35.000000000","message":"Good pickup, thanks Julia, fixed it now.","commit_id":"fffaae84d8387d30cff567de13856c2c6cb8e3bd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"9c018a2e4aeec652ccca2058ff9f264d4304586d","unresolved":true,"context_lines":[{"line_number":1426,"context_line":"                else:"},{"line_number":1427,"context_line":"                    execute_secure_erase \u003d info.get("},{"line_number":1428,"context_line":"                        \u0027agent_enable_ata_secure_erase\u0027, True)"},{"line_number":1429,"context_line":"                    if execute_secure_erase and self._ata_erase(dev):"},{"line_number":1430,"context_line":"                        continue"},{"line_number":1431,"context_line":"            except errors.BlockDeviceEraseError as e:"},{"line_number":1432,"context_line":"                LOG.warning(\u0027Failed to securely erase device \"%(dev)s\". \u0027"}],"source_content_type":"text/x-python","patch_set":22,"id":"2d3195a9_cdcb2af5","line":1429,"range":{"start_line":1429,"start_character":48,"end_line":1429,"end_character":69},"updated":"2022-03-12 00:00:36.000000000","message":"Do we really want to be triggering _ata_erase one drive at a time?","commit_id":"fffaae84d8387d30cff567de13856c2c6cb8e3bd"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"49e0e7f508e22f210b47b8798a30015aa134e519","unresolved":true,"context_lines":[{"line_number":1426,"context_line":"                else:"},{"line_number":1427,"context_line":"                    execute_secure_erase \u003d info.get("},{"line_number":1428,"context_line":"                        \u0027agent_enable_ata_secure_erase\u0027, True)"},{"line_number":1429,"context_line":"                    if execute_secure_erase and self._ata_erase(dev):"},{"line_number":1430,"context_line":"                        continue"},{"line_number":1431,"context_line":"            except errors.BlockDeviceEraseError as e:"},{"line_number":1432,"context_line":"                LOG.warning(\u0027Failed to securely erase device \"%(dev)s\". \u0027"}],"source_content_type":"text/x-python","patch_set":22,"id":"d4332382_6ece70c1","line":1429,"range":{"start_line":1429,"start_character":48,"end_line":1429,"end_character":69},"in_reply_to":"2d3195a9_cdcb2af5","updated":"2022-03-14 05:22:35.000000000","message":"Good point, I will respond in detail to second comment (the one with parallelism example). Thank you Julia.","commit_id":"fffaae84d8387d30cff567de13856c2c6cb8e3bd"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"a77ad0b5f0313f74e4738b835083697b354b59e7","unresolved":true,"context_lines":[{"line_number":1423,"context_line":"                    if execute_nvme_erase and self._nvme_erase(dev):"},{"line_number":1424,"context_line":"                        continue"},{"line_number":1425,"context_line":"            except errors.BlockDeviceEraseError as e:"},{"line_number":1426,"context_line":"                LOG.warning(\u0027Failed to securely erase device \"%(dev)s\". \u0027"},{"line_number":1427,"context_line":"                            \u0027Error: %(error)s, falling back to metadata \u0027"},{"line_number":1428,"context_line":"                            \u0027clean\u0027, {\u0027dev\u0027: dev.name, \u0027error\u0027: e})"},{"line_number":1429,"context_line":"                secure_erase_error \u003d e"}],"source_content_type":"text/x-python","patch_set":25,"id":"54249682_dbf55e49","line":1426,"range":{"start_line":1426,"start_character":16,"end_line":1426,"end_character":27},"updated":"2022-03-15 08:38:45.000000000","message":"nit: LOG.error ?","commit_id":"e3131fbb3cf04e700a3b2f0628bc0e65a3a94030"}],"releasenotes/notes/add-erase-devices-express-1df107c75f2b3627.yaml":[{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"bb4680560f010e8d4382599e75cd1c1db3200b7c","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds support for express cleaning mode where hardware-assisted, fast and"},{"line_number":5,"context_line":"    secure data erasure is performed on devices that support it, while other"},{"line_number":6,"context_line":"    devices fall back to erase_devices_metadata. The goal of this feature is"},{"line_number":7,"context_line":"    to enable express node cleaning in environments with hybrid storage"},{"line_number":8,"context_line":"    configuration (e.g. NVMe + HDD)"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"a53054c7_a462b356","line":8,"range":{"start_line":4,"start_character":0,"end_line":8,"end_character":35},"updated":"2022-03-11 13:37:59.000000000","message":"How about adding a note to say that this new feature will also need the \"agent_enable_[ata,nvme]_secure_erase\" options to be set?","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":32177,"name":"Jacob Anders","email":"jacob-anders-dev@proton.me","username":"janders"},"change_message_id":"3c526dde43f8a8f13fcba3a25be541f012eb0775","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds support for express cleaning mode where hardware-assisted, fast and"},{"line_number":5,"context_line":"    secure data erasure is performed on devices that support it, while other"},{"line_number":6,"context_line":"    devices fall back to erase_devices_metadata. The goal of this feature is"},{"line_number":7,"context_line":"    to enable express node cleaning in environments with hybrid storage"},{"line_number":8,"context_line":"    configuration (e.g. NVMe + HDD)"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"9d0f23b2_266a61c7","line":8,"range":{"start_line":4,"start_character":0,"end_line":8,"end_character":35},"in_reply_to":"a53054c7_a462b356","updated":"2022-03-11 21:02:18.000000000","message":"These options are enabled by default. But perhaps I add a note stating we need something like clean_step_priority_override\u003ddeploy.erase_devices_express:5 to enable given default priority is zero and mention these as well.","commit_id":"3fd6f0db1ad1baead8183422cf2fdaf1c941595a"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"a77ad0b5f0313f74e4738b835083697b354b59e7","unresolved":true,"context_lines":[{"line_number":6,"context_line":"    while other devices fall back to erase_devices_metadata."},{"line_number":7,"context_line":"    The goal of this feature is to enable express node cleaning in"},{"line_number":8,"context_line":"    environments with hybrid storage configuration (e.g. NVMe + HDD)."},{"line_number":9,"context_line":"    In order to use this feature, following options are recommended:"},{"line_number":10,"context_line":"    `[deploy]/erase_devices_priority\u003d0`"},{"line_number":11,"context_line":"    `[deploy]/erase_devices_metadata_priority\u003d0`"},{"line_number":12,"context_line":"    `[conductor]/clean_step_priority_override\u003ddeploy.erase_devices_express:5`"}],"source_content_type":"text/x-yaml","patch_set":25,"id":"6ba8c3ca_7e56fdf6","line":9,"updated":"2022-03-15 08:38:45.000000000","message":"add this directly to docs? a follow up is ok if we have consensus merging this as it is","commit_id":"e3131fbb3cf04e700a3b2f0628bc0e65a3a94030"}]}
