)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11297,"name":"Nisha Agarwal","email":"agarwalnisha1980@gmail.com","username":"Nisha"},"change_message_id":"876e4d91b024d60907b719aaaffa52eca46fe6cc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b5a39639_12be5a96","updated":"2021-10-25 13:52:13.000000000","message":"@Ajia, Thanks for the review.\n\n\u003e\u003eIs this sufficient to get it working for all interfaces?\n[NISHA]What do you mean by all interfaces. The scope of the patch is with RAID interface only.\n\u003e\u003e Or if it is not supported for all RAID interfaces, should additional validation added to avoid confusing users?\n[NISHA]This should work with all hardware managers supporting RAID configuration with \u0027agent\u0027 as the raid interface. This is tested well with Redfish driver on SuperdomeFlex 280 Hardware.(This hardware is tested with redfish driver regularly by us).\n\n\u003e\u003eOne thing, there are Ironic common constants here [1], should add nvme.\nAdded.\n\n\u003e\u003eMore things for generic Redfish implementation:\n\n\u003e\u003eSushy mapping at [2]. \n[NISHA] This is for Storage discovery as far as i understand the code.\n\nAnd if nvme is not supported by redfish RAID, then should update its validation at [3]. (I think it should be supported).\n[NISHA] Already the code is update in new patch.","commit_id":"94307e30dc12e9a11e3005eae991c5be76002b18"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"d8ef05added9c62793841b31ee94d0e0edf7868b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4bbe9209_6d2e77a8","updated":"2021-10-22 12:11:19.000000000","message":"thanks for the patch. Is this sufficient to get it working for all interfaces? Or if it is not supported for all RAID interfaces, should additional validation added to avoid confusing users?\n\nOne thing, there are Ironic common constants here [1], should add nvme.\n\nMore things for generic Redfish implementation:\n\nSushy mapping at [2]. \nAnd if nvme is not supported by redfish RAID, then should update its validation at [3]. (I think it should be supported).\n \n[1] https://opendev.org/openstack/ironic/src/commit/dc3fefe6a0f6101cb2568e5f20ed1e7a0474f155/ironic/common/raid.py#L25-L32\n[2] https://opendev.org/openstack/ironic/src/commit/dc3fefe6a0f6101cb2568e5f20ed1e7a0474f155/ironic/drivers/modules/redfish/raid.py#L94-L98\n[3] https://opendev.org/openstack/ironic/src/commit/dc3fefe6a0f6101cb2568e5f20ed1e7a0474f155/ironic/common/raid.py#L25-L32","commit_id":"94307e30dc12e9a11e3005eae991c5be76002b18"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"40651179b9226023b4a87efefa48d2b63443d4be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b00285be_b0c0df7e","in_reply_to":"069804ed_9884adbb","updated":"2021-10-26 16:53:14.000000000","message":"I missed this comment, yes we should not have any side effects.","commit_id":"94307e30dc12e9a11e3005eae991c5be76002b18"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"bc917bc0aa8b93f14293f7a8386beffdb7484434","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"09859134_d685f2a7","in_reply_to":"b00285be_b0c0df7e","updated":"2021-10-29 11:29:36.000000000","message":"maybe the way forward could be discussed in Monday\u0027s meeting?","commit_id":"94307e30dc12e9a11e3005eae991c5be76002b18"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"af9d5bd1a20d2e7afa2ade740b924c52a364e3f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"069804ed_9884adbb","in_reply_to":"b5a39639_12be5a96","updated":"2021-10-26 09:05:09.000000000","message":"Thanks for the reply. I see that commit message is updated to note only `agent` support. As the change in RAID json schema is affecting all implementations, I still have questions how this is affecting other RAID implementations.\n\n \n\u003e \u003e\u003eIs this sufficient to get it working for all interfaces?\n\u003e [NISHA]What do you mean by all interfaces. The scope of the patch is with RAID interface only.\n\nI meant all other eligible RAID implementations [1].\n\nAs an example I gave `redfish` case, but other should be assessed too. Maybe they work out of the box, maybe not. For `redfish` I don\u0027t think it works out of the box because of the missing mapping and possibly other adjustments.\n\nWithout this change, when user would try to use `nvme` with RAID interface `redfish` it would get the same error about `nvme` not supported and that would happen at the very early stage - when setting target RAID config.\n\nWith this change there is nothing restricting user trying out `nvme` with `redfish`. User sets `nvme` as `interface_type` runs RAID cleaning step and that step fails with `KeyError`. I see this is a regression in user experience and this would look like a bug in `redfish` RAID implementation.\n\nSo, I\u0027m just wondering, have other implementation been considered and assessed?\n\n[1] https://opendev.org/openstack/ironic/src/commit/2ff7f553c08ab74c4b09763110e43168b44d638c/setup.cfg#L140-L150","commit_id":"94307e30dc12e9a11e3005eae991c5be76002b18"},{"author":{"_account_id":11297,"name":"Nisha Agarwal","email":"agarwalnisha1980@gmail.com","username":"Nisha"},"change_message_id":"90223c011514152a0da0cf9303b57a05e016433a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d2a5e310_bdec7b8a","updated":"2021-10-26 03:12:32.000000000","message":"recheck","commit_id":"7e3f5bc1021b6be521b94d01d59c152e72ccd1ca"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"4a48cedffbca93e462f9c78be75bd7dddbe41278","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"62c98e91_e6e233b1","updated":"2021-10-26 15:31:31.000000000","message":"As I understand scope of this fix is only for \u0027agent\u0027 and more changes would needed to support \u0027RedfishRAID\u0027. Please add releasenotes for the same. ","commit_id":"9f715a2f30ca9687278b7d1c768db66a6fb9c6ab"},{"author":{"_account_id":11297,"name":"Nisha Agarwal","email":"agarwalnisha1980@gmail.com","username":"Nisha"},"change_message_id":"751c47d244d18e30f62a267ef32e72c08c32c9a1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"72c3917e_86979547","updated":"2021-10-26 08:51:46.000000000","message":"recheck","commit_id":"9f715a2f30ca9687278b7d1c768db66a6fb9c6ab"}]}