)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0c31cafe66d5a8ef38b7dda10501c74ae2aa55c3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"02697972_bcba631c","updated":"2021-11-03 13:48:10.000000000","message":"A nit and some questions inline.","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"391590e6a9597350b304ad1007c77b11d9467a4d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"bff2252f_9af9f6d2","updated":"2021-11-05 16:37:58.000000000","message":"Comment inline.","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"3c87becd9e0e193ee2f24e4e3726941547e845ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f8c051e6_4ff59fa5","updated":"2021-11-22 15:02:07.000000000","message":"One last thing, then I think this is ready to go.","commit_id":"7e99d21b7da7db1a775312790f7d74547a9d9879"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"9074cc2aa0611895c531079a86ee82403dac9800","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"80ebe488_211a0054","updated":"2022-01-10 07:49:58.000000000","message":"LGTM","commit_id":"97cd6052ae434d660be078023096d8cee2acbc14"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"47d10d7729fb133143c4b302bdf23c7b27c176b6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"fc7c7e97_9e91507b","updated":"2022-01-24 13:03:11.000000000","message":"Thanks for the revisions, LGTM.","commit_id":"97cd6052ae434d660be078023096d8cee2acbc14"}],"cinder/volume/drivers/dell_emc/powermax/masking.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"60c148707962043e09aa2626905f563f7dd0b687","unresolved":true,"context_lines":[{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names})"},{"line_number":685,"context_line":"                    else:"},{"line_number":686,"context_line":"                        msg \u003d (\"Found initiator group %(ign)s, but could not \""},{"line_number":687,"context_line":"                               \"find initiator_names %(ins)s in the login \""},{"line_number":688,"context_line":"                               \"table. The contained initiators %(ins_host)s \""},{"line_number":689,"context_line":"                               \"do match up with those in the connector \""}],"source_content_type":"text/x-python","patch_set":3,"id":"d02f7d0b_0a1c231e","line":686,"updated":"2021-09-15 21:34:45.000000000","message":"This code is very confusing as you use a Log.warning() in the first case but then you create a separate \u0027msg\u0027 variable and log that for the other cases.  I think this should be changed to be consistent please.","commit_id":"49ed552d87466867d2e0107eea56195cac2b88ed"},{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"731b43157891e731018d4c71b6f987d8b2a33f80","unresolved":false,"context_lines":[{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names})"},{"line_number":685,"context_line":"                    else:"},{"line_number":686,"context_line":"                        msg \u003d (\"Found initiator group %(ign)s, but could not \""},{"line_number":687,"context_line":"                               \"find initiator_names %(ins)s in the login \""},{"line_number":688,"context_line":"                               \"table. The contained initiators %(ins_host)s \""},{"line_number":689,"context_line":"                               \"do match up with those in the connector \""}],"source_content_type":"text/x-python","patch_set":3,"id":"3ecc2ba2_420c42d9","line":686,"in_reply_to":"cc5a5e5c_fb53fdaa","updated":"2021-10-12 10:37:16.000000000","message":"Ack","commit_id":"49ed552d87466867d2e0107eea56195cac2b88ed"},{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"731b43157891e731018d4c71b6f987d8b2a33f80","unresolved":false,"context_lines":[{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names})"},{"line_number":685,"context_line":"                    else:"},{"line_number":686,"context_line":"                        msg \u003d (\"Found initiator group %(ign)s, but could not \""},{"line_number":687,"context_line":"                               \"find initiator_names %(ins)s in the login \""},{"line_number":688,"context_line":"                               \"table. The contained initiators %(ins_host)s \""},{"line_number":689,"context_line":"                               \"do match up with those in the connector \""}],"source_content_type":"text/x-python","patch_set":3,"id":"bda825fa_4fb0b076","line":686,"in_reply_to":"cc5a5e5c_fb53fdaa","updated":"2021-10-12 10:37:16.000000000","message":"Ack","commit_id":"49ed552d87466867d2e0107eea56195cac2b88ed"},{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"5af1e64b7c8907e17e1f72cc254832138cd76520","unresolved":true,"context_lines":[{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names})"},{"line_number":685,"context_line":"                    else:"},{"line_number":686,"context_line":"                        msg \u003d (\"Found initiator group %(ign)s, but could not \""},{"line_number":687,"context_line":"                               \"find initiator_names %(ins)s in the login \""},{"line_number":688,"context_line":"                               \"table. The contained initiators %(ins_host)s \""},{"line_number":689,"context_line":"                               \"do match up with those in the connector \""}],"source_content_type":"text/x-python","patch_set":3,"id":"cc5a5e5c_fb53fdaa","line":686,"in_reply_to":"d02f7d0b_0a1c231e","updated":"2021-09-16 09:59:54.000000000","message":"Thanks Jay. This method returns a msg string which is why the msg is a variable.  When it is warning, None is returned.","commit_id":"49ed552d87466867d2e0107eea56195cac2b88ed"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0c31cafe66d5a8ef38b7dda10501c74ae2aa55c3","unresolved":true,"context_lines":[{"line_number":672,"context_line":"                         {\u0027init_group_name\u0027: init_group_name})"},{"line_number":673,"context_line":"            else:"},{"line_number":674,"context_line":"                initiator_list \u003d initiator_group.get("},{"line_number":675,"context_line":"                    \u0027initiator\u0027, list()) if initiator_group else list()"},{"line_number":676,"context_line":"                if initiator_list:"},{"line_number":677,"context_line":"                    if set(initiator_list) \u003d\u003d set(initiator_names):"},{"line_number":678,"context_line":"                        LOG.warning("}],"source_content_type":"text/x-python","patch_set":4,"id":"cc85ae9a_76fd58ed","line":675,"range":{"start_line":675,"start_character":41,"end_line":675,"end_character":59},"updated":"2021-11-03 13:48:10.000000000","message":"Do you need this check? If initiator_group is None, you will have taken the branch at line 666 and won\u0027t execute the \u0027else\u0027 block containing this statement.  And actually, now that I think about it, if initiator_group is None, you will already have raised an AttributeError at when you call initiator_group.get() at line 674, so you will never hit this code.","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"},{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"a9b01d706b3d03600c2c4d0c5f90907c59e3046d","unresolved":true,"context_lines":[{"line_number":672,"context_line":"                         {\u0027init_group_name\u0027: init_group_name})"},{"line_number":673,"context_line":"            else:"},{"line_number":674,"context_line":"                initiator_list \u003d initiator_group.get("},{"line_number":675,"context_line":"                    \u0027initiator\u0027, list()) if initiator_group else list()"},{"line_number":676,"context_line":"                if initiator_list:"},{"line_number":677,"context_line":"                    if set(initiator_list) \u003d\u003d set(initiator_names):"},{"line_number":678,"context_line":"                        LOG.warning("}],"source_content_type":"text/x-python","patch_set":4,"id":"e802062e_2bd09b69","line":675,"range":{"start_line":675,"start_character":41,"end_line":675,"end_character":59},"in_reply_to":"cc85ae9a_76fd58ed","updated":"2021-11-04 15:04:27.000000000","message":"You are right, its overkill.","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0c31cafe66d5a8ef38b7dda10501c74ae2aa55c3","unresolved":true,"context_lines":[{"line_number":676,"context_line":"                if initiator_list:"},{"line_number":677,"context_line":"                    if set(initiator_list) \u003d\u003d set(initiator_names):"},{"line_number":678,"context_line":"                        LOG.warning("},{"line_number":679,"context_line":"                            \"Found initiator group %(ign)s, but could not \""},{"line_number":680,"context_line":"                            \"find initiator_names %(ins)s in the login \""},{"line_number":681,"context_line":"                            \"table.  The contained initiators match up \""},{"line_number":682,"context_line":"                            \"with those in the connector object.\","},{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names})"},{"line_number":685,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"d2e92aca_200a9c4b","line":682,"range":{"start_line":679,"start_character":0,"end_line":682,"end_character":66},"updated":"2021-11-03 13:48:10.000000000","message":"I find this message a bit confusing.  I think the warning is that the initiator names are missing from the login table; the rest of the message is saying that you have decided that you have found the correct initiator group anyway ... and are going to use it?  (I think not saying the last part in your message is what\u0027s confusing.)\n\nAlso, I guess this is a warning because even though it\u0027s a recoverable error, it could be a sign that there\u0027s something weird going on in the backend?","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"},{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"fc5584fd98cb978ee1a0caeffb595a1eb87cfc7a","unresolved":true,"context_lines":[{"line_number":676,"context_line":"                if initiator_list:"},{"line_number":677,"context_line":"                    if set(initiator_list) \u003d\u003d set(initiator_names):"},{"line_number":678,"context_line":"                        LOG.warning("},{"line_number":679,"context_line":"                            \"Found initiator group %(ign)s, but could not \""},{"line_number":680,"context_line":"                            \"find initiator_names %(ins)s in the login \""},{"line_number":681,"context_line":"                            \"table.  The contained initiators match up \""},{"line_number":682,"context_line":"                            \"with those in the connector object.\","},{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names})"},{"line_number":685,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"c2bf1352_745d7e7a","line":682,"range":{"start_line":679,"start_character":0,"end_line":682,"end_character":66},"in_reply_to":"0c5b9e79_d0c2a92d","updated":"2021-11-10 11:31:25.000000000","message":"Not at all Brian.  As always, I appreciate the feedback","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"391590e6a9597350b304ad1007c77b11d9467a4d","unresolved":true,"context_lines":[{"line_number":676,"context_line":"                if initiator_list:"},{"line_number":677,"context_line":"                    if set(initiator_list) \u003d\u003d set(initiator_names):"},{"line_number":678,"context_line":"                        LOG.warning("},{"line_number":679,"context_line":"                            \"Found initiator group %(ign)s, but could not \""},{"line_number":680,"context_line":"                            \"find initiator_names %(ins)s in the login \""},{"line_number":681,"context_line":"                            \"table.  The contained initiators match up \""},{"line_number":682,"context_line":"                            \"with those in the connector object.\","},{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names})"},{"line_number":685,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"0c5b9e79_d0c2a92d","line":682,"range":{"start_line":679,"start_character":0,"end_line":682,"end_character":66},"in_reply_to":"919243e8_ce6561c0","updated":"2021-11-05 16:37:58.000000000","message":"Thanks for the explanation, Helen.\n\nI wonder if this should be a debug-level log instead of a warning?  It sounds like everything is OK, so there\u0027s nothing for the operator to worry about.  Unless a bunch of these happening could be a sign that the array\u0027s health is not good, so you want operators to be aware that it\u0027s happening.\n\nAnd, sorry to bikeshed this, but instead of saying \"therefore there is no reason ign cannot be reused\", maybe be more affirmative and say \"therefore reusing ign\"","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"},{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"051ae8bbf61287d9d64a3af7d78ba8b4b898d0d3","unresolved":false,"context_lines":[{"line_number":676,"context_line":"                if initiator_list:"},{"line_number":677,"context_line":"                    if set(initiator_list) \u003d\u003d set(initiator_names):"},{"line_number":678,"context_line":"                        LOG.warning("},{"line_number":679,"context_line":"                            \"Found initiator group %(ign)s, but could not \""},{"line_number":680,"context_line":"                            \"find initiator_names %(ins)s in the login \""},{"line_number":681,"context_line":"                            \"table.  The contained initiators match up \""},{"line_number":682,"context_line":"                            \"with those in the connector object.\","},{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names})"},{"line_number":685,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"76b14652_4bfa6788","line":682,"range":{"start_line":679,"start_character":0,"end_line":682,"end_character":66},"in_reply_to":"c2bf1352_745d7e7a","updated":"2021-11-22 13:40:12.000000000","message":"Ack","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"},{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"a9b01d706b3d03600c2c4d0c5f90907c59e3046d","unresolved":true,"context_lines":[{"line_number":676,"context_line":"                if initiator_list:"},{"line_number":677,"context_line":"                    if set(initiator_list) \u003d\u003d set(initiator_names):"},{"line_number":678,"context_line":"                        LOG.warning("},{"line_number":679,"context_line":"                            \"Found initiator group %(ign)s, but could not \""},{"line_number":680,"context_line":"                            \"find initiator_names %(ins)s in the login \""},{"line_number":681,"context_line":"                            \"table.  The contained initiators match up \""},{"line_number":682,"context_line":"                            \"with those in the connector object.\","},{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names})"},{"line_number":685,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"919243e8_ce6561c0","line":682,"range":{"start_line":679,"start_character":0,"end_line":682,"end_character":66},"in_reply_to":"d2e92aca_200a9c4b","updated":"2021-11-04 15:04:27.000000000","message":"The comment on this review goes into more detail. This warning summarizes, but based on your comment it is not clear enough.\n\nYou are correct this is recoverable error.  An initiator group (aka host) in powermax can be created but unless it becomes part of a masking view and scanned(FC) logged in (iSCSI) an entry will not be created in the login table.  \nTherefore, if for whatever reason a successful scan/login does not succeed and the cleanup does not successfully delete the initiator group, the initiator group remains and most likely, will have the correct initiators (from the openstack supplied connector object).  Therefore there is no reason why it cannot be reused on a subsequent attach operation.\n\nDo you think something like the following is clearer\n\nLOG.warning(\n    \"Found initiator group %(ign)s, but could not \"\n     \"find initiators %(ins)s in the login \"\n     \"table. The contained initiator(s) are the \"\n     \"same as those supplied by OpenStack, therefore \"\n     \"there is no reason %(ign)s cannot be reused.\",\n     {\u0027ign\u0027: init_group_name,\n      \u0027ins\u0027: initiator_names,\n      \u0027ign\u0027: init_group_name})","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"3c87becd9e0e193ee2f24e4e3726941547e845ee","unresolved":true,"context_lines":[{"line_number":682,"context_line":"                            \"reusing %(ign)s.\","},{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names,"},{"line_number":685,"context_line":"                             \u0027ign\u0027: init_group_name})"},{"line_number":686,"context_line":"                    else:"},{"line_number":687,"context_line":"                        msg \u003d (\"Found initiator group %(ign)s, but could not \""},{"line_number":688,"context_line":"                               \"find initiator_names %(ins)s in the login \""}],"source_content_type":"text/x-python","patch_set":6,"id":"687d4a2c_ff2013a4","line":685,"range":{"start_line":685,"start_character":29,"end_line":685,"end_character":51},"updated":"2021-11-22 15:02:07.000000000","message":"don\u0027t need this one ... this is still a two-element dict since the first and third key are the same (so removing it won\u0027t hurt, because you are already passing a two-element dict to the log function, and the coverage report shows that line 677 is being executed)","commit_id":"7e99d21b7da7db1a775312790f7d74547a9d9879"},{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"ce8a39e8764b2e05794287d60412eba1cbc2da09","unresolved":false,"context_lines":[{"line_number":682,"context_line":"                            \"reusing %(ign)s.\","},{"line_number":683,"context_line":"                            {\u0027ign\u0027: init_group_name,"},{"line_number":684,"context_line":"                             \u0027ins\u0027: initiator_names,"},{"line_number":685,"context_line":"                             \u0027ign\u0027: init_group_name})"},{"line_number":686,"context_line":"                    else:"},{"line_number":687,"context_line":"                        msg \u003d (\"Found initiator group %(ign)s, but could not \""},{"line_number":688,"context_line":"                               \"find initiator_names %(ins)s in the login \""}],"source_content_type":"text/x-python","patch_set":6,"id":"2245951d_4a13bfa6","line":685,"range":{"start_line":685,"start_character":29,"end_line":685,"end_character":51},"in_reply_to":"687d4a2c_ff2013a4","updated":"2021-12-06 14:42:52.000000000","message":"Done","commit_id":"7e99d21b7da7db1a775312790f7d74547a9d9879"}],"releasenotes/notes/powermax-existing-host-092f7daf29053d82.yaml":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"60c148707962043e09aa2626905f563f7dd0b687","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |    "},{"line_number":4,"context_line":"    PowerMax driver : Enhancement to use an existing initiator group even if"},{"line_number":5,"context_line":"    there is no entry for the contained initiator(s) in the login table. This"},{"line_number":6,"context_line":"    is permissable so long as the initiator(s) in the connector object match."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"ada79d1c_59f76136","line":3,"range":{"start_line":3,"start_character":0,"end_line":3,"end_character":9},"updated":"2021-09-15 21:34:45.000000000","message":"Trailing spaces here.","commit_id":"49ed552d87466867d2e0107eea56195cac2b88ed"},{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"731b43157891e731018d4c71b6f987d8b2a33f80","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |    "},{"line_number":4,"context_line":"    PowerMax driver : Enhancement to use an existing initiator group even if"},{"line_number":5,"context_line":"    there is no entry for the contained initiator(s) in the login table. This"},{"line_number":6,"context_line":"    is permissable so long as the initiator(s) in the connector object match."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"0f9d0817_e8fbcc66","line":3,"range":{"start_line":3,"start_character":0,"end_line":3,"end_character":9},"in_reply_to":"ada79d1c_59f76136","updated":"2021-10-12 10:37:16.000000000","message":"Done","commit_id":"49ed552d87466867d2e0107eea56195cac2b88ed"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0c31cafe66d5a8ef38b7dda10501c74ae2aa55c3","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    PowerMax driver : Enhancement to use an existing initiator group even if"},{"line_number":5,"context_line":"    there is no entry for the contained initiator(s) in the login table. This"},{"line_number":6,"context_line":"    is permissable so long as the initiator(s) in the connector object match."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"1e1cddd1_220e4d9d","line":6,"range":{"start_line":6,"start_character":7,"end_line":6,"end_character":18},"updated":"2021-11-03 13:48:10.000000000","message":"nit: permissible","commit_id":"0dcc622c9c2caed21226ebf5f0bface6fec03b80"}]}