)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"919fd0270360717efa04d1e1c1a18dc2b21619b7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"57c8f229_c04bcc4d","updated":"2022-09-07 12:34:07.000000000","message":"Is it possible that this may break the case when we have multiple values in backend_names?","commit_id":"7c8d4d2bd60502d2f58e3316fa2933b8167f72c8"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"15783c2046c26e61cd3fe46738ab8bb8bf80ad3a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"017949a5_c7c5a0fb","updated":"2022-09-07 22:46:54.000000000","message":"-1: i think there\u0027s an error in base.py (see comment inline).\n\nAlso some suggestions for restructuring the manager.py code.","commit_id":"bec3e8d550bbfd61401bd3541abb093eddfd4188"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4a051cd8fc1fa6f83bb8da14580fb41ebc072825","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"30d1184d_f343e8db","updated":"2022-09-07 22:47:23.000000000","message":"Forgot to actually vote.","commit_id":"bec3e8d550bbfd61401bd3541abb093eddfd4188"},{"author":{"_account_id":30742,"name":"Soniya Murlidhar Vyas","email":"svyas@redhat.com","username":"svyas"},"change_message_id":"e40d2998a792eae3f7a17795354de65d58237e66","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"01fea538_ce53593c","updated":"2022-10-07 16:00:14.000000000","message":"So volume_backend_name is specified as extra-specs in multi-backend storage[1][2]. Hence, adding backend name for volume type is a good idea.\n\n[1]https://docs.openstack.org/cinder/latest/admin/multi-backend.html\n[2]https://docs.openstack.org/api-ref/block-storage/v3/?expanded\u003dshow-volume-type-detail-detail,create-or-update-a-default-volume-type-detail,list-default-volume-types-detail,create-metadata-for-volume-detail,create-or-update-extra-specs-for-volume-type-detail,show-all-extra-specifications-for-volume-type-detail,create-a-volume-detail#create-or-update-extra-specs-for-volume-type","commit_id":"9154265c1f5d2533aa719306b0f7130e56f8e75c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c5f8a7edf12263100c22b5886646789cbb5c30cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"7dd47050_7389ee04","updated":"2023-07-20 21:58:16.000000000","message":"its been long but I ok with the change if you rebase and add the releasenotes for the new config option. my -1 is for adding the releasenotes","commit_id":"9154265c1f5d2533aa719306b0f7130e56f8e75c"}],"tempest/api/volume/base.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"15783c2046c26e61cd3fe46738ab8bb8bf80ad3a","unresolved":true,"context_lines":[{"line_number":293,"context_line":"        \"\"\"Create a test volume-type\"\"\""},{"line_number":294,"context_line":"        name \u003d name or data_utils.rand_name(cls.__name__ + \u0027-volume-type\u0027)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        e_spec \u003d kwargs.get(\u0027extra_specs\u0027)"},{"line_number":297,"context_line":"        if (not e_spec or all(x not in [\"volume_backend_name\","},{"line_number":298,"context_line":"                                        \"capabilities:volume_backend_name\"]"},{"line_number":299,"context_line":"                              for x in e_spec.keys()))\\"},{"line_number":300,"context_line":"                and CONF.volume.volume_type_backend_name:"},{"line_number":301,"context_line":"            kwargs[\u0027extra_specs\u0027].update({"},{"line_number":302,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name})"},{"line_number":303,"context_line":"        volume_type \u003d cls.admin_volume_types_client.create_volume_type("},{"line_number":304,"context_line":"            name\u003dname, **kwargs)[\u0027volume_type\u0027]"},{"line_number":305,"context_line":"        cls.addClassResourceCleanup(cls.clear_volume_type, volume_type[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"2d811640_a8cd463e","line":302,"range":{"start_line":296,"start_character":0,"end_line":302,"end_character":77},"updated":"2022-09-07 22:46:54.000000000","message":"I think you can make this more efficient ... if the config opt isn\u0027t defined, we don\u0027t need to look at any of the other stuff.  So maybe something like:\n\n  e_spec \u003d kwargs.get(\u0027extra_specs\u0027)\n  if CONF.volume.volume_type_backend_name:\n      if not e_spec:\n          kwargs[\u0027extra_specs\u0027] \u003d {\u0027volume_backend_name\u0027:\n                                    CONF.volume.volume_type_backend_name}\n      elif (\"volume_backend_name\" not in e_spec\n            and \"capabilities:volume_backend_name\" not in e_spec):\n          kwargs[\u0027extra_specs\u0027].update(\n              {\u0027volume_backend_name\u0027:\n               CONF.volume.volume_type_backend_name})\n\n(Note that in your current code, if e_spec is None, you\u0027ll get a KeyError at line 301)","commit_id":"bec3e8d550bbfd61401bd3541abb093eddfd4188"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"2c85766f3963e7c68ebb09367ce120a18f071f5d","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        \"\"\"Create a test volume-type\"\"\""},{"line_number":294,"context_line":"        name \u003d name or data_utils.rand_name(cls.__name__ + \u0027-volume-type\u0027)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        e_spec \u003d kwargs.get(\u0027extra_specs\u0027)"},{"line_number":297,"context_line":"        if (not e_spec or all(x not in [\"volume_backend_name\","},{"line_number":298,"context_line":"                                        \"capabilities:volume_backend_name\"]"},{"line_number":299,"context_line":"                              for x in e_spec.keys()))\\"},{"line_number":300,"context_line":"                and CONF.volume.volume_type_backend_name:"},{"line_number":301,"context_line":"            kwargs[\u0027extra_specs\u0027].update({"},{"line_number":302,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name})"},{"line_number":303,"context_line":"        volume_type \u003d cls.admin_volume_types_client.create_volume_type("},{"line_number":304,"context_line":"            name\u003dname, **kwargs)[\u0027volume_type\u0027]"},{"line_number":305,"context_line":"        cls.addClassResourceCleanup(cls.clear_volume_type, volume_type[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"95700e74_f93e7685","line":302,"range":{"start_line":296,"start_character":0,"end_line":302,"end_character":77},"in_reply_to":"2d811640_a8cd463e","updated":"2022-09-08 03:27:20.000000000","message":"Yes , i see i missed it .... but i think we can fix it this way:\nkwargs.setdefault(\"extra_specs\",{})\nmeans if it does not exist it will create {} empty otherwide ignored","commit_id":"bec3e8d550bbfd61401bd3541abb093eddfd4188"}],"tempest/scenario/manager.py":[{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"8bcaf8ee13f711c9a9d413e932e33d7355a35ace","unresolved":true,"context_lines":[{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        if not kwargs.get(\u0027extra_specs\u0027) and\\"},{"line_number":536,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":537,"context_line":"            kwargs[\u0027extra_specs\u0027] \u003d {"},{"line_number":538,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name}"}],"source_content_type":"text/x-python","patch_set":1,"id":"6205536b_5d50c668","line":535,"updated":"2022-09-07 12:40:14.000000000","message":"if extra_specs is set but volume_backend_name is not set, shouldn\u0027t volume_backend_name be set anyway? \nMaybe moving this after line 540 (extra_specs \u003d kwargs...) and keeping just the volume_type_backend_name condition may work","commit_id":"7c8d4d2bd60502d2f58e3316fa2933b8167f72c8"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"ffb6b0aa517e248f4163fdf94a31ca5b2600ae92","unresolved":true,"context_lines":[{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        if not kwargs.get(\u0027extra_specs\u0027) and\\"},{"line_number":536,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":537,"context_line":"            kwargs[\u0027extra_specs\u0027] \u003d {"},{"line_number":538,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name}"}],"source_content_type":"text/x-python","patch_set":1,"id":"88c1a1bd_2ece9ac3","line":535,"in_reply_to":"6205536b_5d50c668","updated":"2022-09-07 12:45:47.000000000","message":"ack good catch :) , so if extra_spec is \"k1\":\"blabla\" my code will not work!!!\nThanks fixing","commit_id":"7c8d4d2bd60502d2f58e3316fa2933b8167f72c8"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"e95cd50a7b0165bb357e1b957530f9940ea648a3","unresolved":false,"context_lines":[{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        if not kwargs.get(\u0027extra_specs\u0027) and\\"},{"line_number":536,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":537,"context_line":"            kwargs[\u0027extra_specs\u0027] \u003d {"},{"line_number":538,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name}"}],"source_content_type":"text/x-python","patch_set":1,"id":"88f943e8_5831513e","line":535,"in_reply_to":"88c1a1bd_2ece9ac3","updated":"2022-09-07 17:19:54.000000000","message":"Done","commit_id":"7c8d4d2bd60502d2f58e3316fa2933b8167f72c8"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"12032168b2fcf0fd66c77ecf394e8d05a0954695","unresolved":true,"context_lines":[{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        extra_specs \u003d kwargs.pop(\"extra_specs\", {})"},{"line_number":536,"context_line":"        if (not extra_specs or all(x not in ["},{"line_number":537,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":538,"context_line":"            for x in extra_specs.keys())) and\\"},{"line_number":539,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":540,"context_line":"            kwargs[\u0027extra_specs\u0027] \u003d {"},{"line_number":541,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name}"},{"line_number":542,"context_line":"        if backend_name:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7e3e1958_8ea93891","line":539,"range":{"start_line":536,"start_character":8,"end_line":539,"end_character":53},"updated":"2022-09-07 14:10:34.000000000","message":"If I understand correctly, we would like to use the volume_type_backend_name only if all other options aren\u0027t set. Correct me if I\u0027m wrong please, is this code going to be overwritten on line 541? If so, the code should be after line 544.","commit_id":"a8566aa2595cb117b4c2247a89a57316543d3607"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"e95cd50a7b0165bb357e1b957530f9940ea648a3","unresolved":false,"context_lines":[{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        extra_specs \u003d kwargs.pop(\"extra_specs\", {})"},{"line_number":536,"context_line":"        if (not extra_specs or all(x not in ["},{"line_number":537,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":538,"context_line":"            for x in extra_specs.keys())) and\\"},{"line_number":539,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":540,"context_line":"            kwargs[\u0027extra_specs\u0027] \u003d {"},{"line_number":541,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name}"},{"line_number":542,"context_line":"        if backend_name:"}],"source_content_type":"text/x-python","patch_set":2,"id":"e460c724_b467d6b5","line":539,"range":{"start_line":536,"start_character":8,"end_line":539,"end_character":53},"in_reply_to":"7e3e1958_8ea93891","updated":"2022-09-07 17:19:54.000000000","message":"I think the flow should be:\nlet me write it to be on the safe side :\nif extra_spec does not exist -\u003e create it and add the backend name\nif extra_spec exists:\n- if both volume_backend_name\", \"capabilities:volume_backend_name not in the keys means user did not configure it --\u003e create volume_backend_name volume_backend_name\", \"capabilities:volume_backend_name already exists we do not override.\n\nThe reason for that is that means that user run multi_backend and set the params.\n\n  \n\nif extra_spec exists and one of","commit_id":"a8566aa2595cb117b4c2247a89a57316543d3607"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"38be80fd9722e2bb209f5b054803c09eb816f73d","unresolved":true,"context_lines":[{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        extra_specs \u003d kwargs.pop(\"extra_specs\", {})"},{"line_number":536,"context_line":"        if (not extra_specs or all(x not in ["},{"line_number":537,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":538,"context_line":"            for x in extra_specs.keys())) and\\"},{"line_number":539,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":540,"context_line":"            kwargs[\u0027extra_specs\u0027] \u003d {"},{"line_number":541,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name}"},{"line_number":542,"context_line":"        if backend_name:"}],"source_content_type":"text/x-python","patch_set":2,"id":"46e0126b_16542630","line":539,"range":{"start_line":536,"start_character":8,"end_line":539,"end_character":53},"in_reply_to":"7e3e1958_8ea93891","updated":"2022-09-07 14:36:31.000000000","message":"This specific workflow point is correct, because a test can override volume_backend_name (see backend_name), so the default should apply as the last only if backend_name is not set.\nI\u0027m not sure about the rest of the logic though. What happens if extra_specs_keys contains capabilities:volume_backend_name and we add volume_backend_name?","commit_id":"a8566aa2595cb117b4c2247a89a57316543d3607"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"12032168b2fcf0fd66c77ecf394e8d05a0954695","unresolved":true,"context_lines":[{"line_number":537,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":538,"context_line":"            for x in extra_specs.keys())) and\\"},{"line_number":539,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":540,"context_line":"            kwargs[\u0027extra_specs\u0027] \u003d {"},{"line_number":541,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name}"},{"line_number":542,"context_line":"        if backend_name:"},{"line_number":543,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"}],"source_content_type":"text/x-python","patch_set":2,"id":"da447c06_d5053977","line":540,"range":{"start_line":540,"start_character":12,"end_line":540,"end_character":33},"updated":"2022-09-07 14:10:34.000000000","message":"I think you can do `extra_specs.update({\"volume_backend_name\": CONF.volume.volume_type_backend_name})` to keep consistency with the old code.","commit_id":"a8566aa2595cb117b4c2247a89a57316543d3607"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"e95cd50a7b0165bb357e1b957530f9940ea648a3","unresolved":false,"context_lines":[{"line_number":537,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":538,"context_line":"            for x in extra_specs.keys())) and\\"},{"line_number":539,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":540,"context_line":"            kwargs[\u0027extra_specs\u0027] \u003d {"},{"line_number":541,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name}"},{"line_number":542,"context_line":"        if backend_name:"},{"line_number":543,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"}],"source_content_type":"text/x-python","patch_set":2,"id":"0adc5145_6b41ea68","line":540,"range":{"start_line":540,"start_character":12,"end_line":540,"end_character":33},"in_reply_to":"1846b248_fe824225","updated":"2022-09-07 17:19:54.000000000","message":"Done","commit_id":"a8566aa2595cb117b4c2247a89a57316543d3607"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"74cb19b330d9f29eee95c105b587c643a61763b3","unresolved":true,"context_lines":[{"line_number":537,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":538,"context_line":"            for x in extra_specs.keys())) and\\"},{"line_number":539,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":540,"context_line":"            kwargs[\u0027extra_specs\u0027] \u003d {"},{"line_number":541,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name}"},{"line_number":542,"context_line":"        if backend_name:"},{"line_number":543,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"}],"source_content_type":"text/x-python","patch_set":2,"id":"1846b248_fe824225","line":540,"range":{"start_line":540,"start_character":12,"end_line":540,"end_character":33},"in_reply_to":"da447c06_d5053977","updated":"2022-09-07 14:24:24.000000000","message":"yes my mistake .","commit_id":"a8566aa2595cb117b4c2247a89a57316543d3607"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"54c0ee72764bba4f0556d23a4a71bec189afa0c3","unresolved":true,"context_lines":[{"line_number":537,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":538,"context_line":"            for x in extra_specs.keys())) and\\"},{"line_number":539,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":540,"context_line":"            kwargs[\u0027extra_specs\u0027].update({"},{"line_number":541,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name})"},{"line_number":542,"context_line":"        if backend_name:"},{"line_number":543,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"}],"source_content_type":"text/x-python","patch_set":3,"id":"166a1b74_9968d248","line":540,"updated":"2022-09-07 14:40:35.000000000","message":"Shouldn\u0027t you use extra_specs, just like line 543 does? Otherwise it may not work when backend_name is also set (which one is going to win?)","commit_id":"122cd84fd954bc5f1deb0b9e0c7883fbe35c7cda"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"e95cd50a7b0165bb357e1b957530f9940ea648a3","unresolved":false,"context_lines":[{"line_number":537,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":538,"context_line":"            for x in extra_specs.keys())) and\\"},{"line_number":539,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":540,"context_line":"            kwargs[\u0027extra_specs\u0027].update({"},{"line_number":541,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name})"},{"line_number":542,"context_line":"        if backend_name:"},{"line_number":543,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"}],"source_content_type":"text/x-python","patch_set":3,"id":"e92a731d_ab2d4810","line":540,"in_reply_to":"166a1b74_9968d248","updated":"2022-09-07 17:19:54.000000000","message":"ack .. the backend should be first if not set go and check","commit_id":"122cd84fd954bc5f1deb0b9e0c7883fbe35c7cda"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"f18d7b1dc4afeeee80945968bcf13b32847927ff","unresolved":true,"context_lines":[{"line_number":539,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":540,"context_line":"                                   for x in extra_specs.keys())) and \\"},{"line_number":541,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":542,"context_line":"            kwargs[\u0027extra_specs\u0027].update({"},{"line_number":543,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name})"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"        volume_type_resp \u003d client.create_volume_type("}],"source_content_type":"text/x-python","patch_set":4,"id":"d712b6b1_77fdc2a9","line":542,"updated":"2022-09-07 15:02:54.000000000","message":"I would still keep using \nextra_specs.update(...)\nfor consistency","commit_id":"33584806b8b0074f82b6f86c09c17640abc06b2a"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"e95cd50a7b0165bb357e1b957530f9940ea648a3","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            \"volume_backend_name\", \"capabilities:volume_backend_name\"]"},{"line_number":540,"context_line":"                                   for x in extra_specs.keys())) and \\"},{"line_number":541,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":542,"context_line":"            kwargs[\u0027extra_specs\u0027].update({"},{"line_number":543,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name})"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"        volume_type_resp \u003d client.create_volume_type("}],"source_content_type":"text/x-python","patch_set":4,"id":"04638303_9663334e","line":542,"in_reply_to":"d712b6b1_77fdc2a9","updated":"2022-09-07 17:19:54.000000000","message":"Ok , lets let to other review the code and see how it goes. thanks for your time","commit_id":"33584806b8b0074f82b6f86c09c17640abc06b2a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"15783c2046c26e61cd3fe46738ab8bb8bf80ad3a","unresolved":true,"context_lines":[{"line_number":530,"context_line":"            name \u003d data_utils.rand_name(class_name + \u0027-volume-type\u0027)"},{"line_number":531,"context_line":"        randomized_name \u003d data_utils.rand_name(\u0027scenario-type-\u0027 + name)"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        extra_specs \u003d kwargs.pop(\"extra_specs\", {})"},{"line_number":536,"context_line":"        if backend_name:"},{"line_number":537,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"}],"source_content_type":"text/x-python","patch_set":6,"id":"d03a4443_a0b1c57f","line":534,"range":{"start_line":533,"start_character":0,"end_line":534,"end_character":48},"updated":"2022-09-07 22:46:54.000000000","message":"this log message is happening too soon now","commit_id":"bec3e8d550bbfd61401bd3541abb093eddfd4188"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"6df4df9bf3d1cdcdc8cec8edaf1eaea80fdf7bb6","unresolved":true,"context_lines":[{"line_number":530,"context_line":"            name \u003d data_utils.rand_name(class_name + \u0027-volume-type\u0027)"},{"line_number":531,"context_line":"        randomized_name \u003d data_utils.rand_name(\u0027scenario-type-\u0027 + name)"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        extra_specs \u003d kwargs.pop(\"extra_specs\", {})"},{"line_number":536,"context_line":"        if backend_name:"},{"line_number":537,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"}],"source_content_type":"text/x-python","patch_set":6,"id":"6a04ffb8_851e657a","line":534,"range":{"start_line":533,"start_character":0,"end_line":534,"end_character":48},"in_reply_to":"d03a4443_a0b1c57f","updated":"2022-09-12 12:09:51.000000000","message":"Yes i will fix it ... lets wait for more comments","commit_id":"bec3e8d550bbfd61401bd3541abb093eddfd4188"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"15783c2046c26e61cd3fe46738ab8bb8bf80ad3a","unresolved":true,"context_lines":[{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        extra_specs \u003d kwargs.pop(\"extra_specs\", {})"},{"line_number":536,"context_line":"        if backend_name:"},{"line_number":537,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"},{"line_number":538,"context_line":"        elif (not extra_specs or all("},{"line_number":539,"context_line":"                x not in [\"volume_backend_name\","},{"line_number":540,"context_line":"                          \"capabilities:volume_backend_name\"]"},{"line_number":541,"context_line":"                for x in extra_specs.keys())) and \\"},{"line_number":542,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":543,"context_line":"            extra_specs.update({"},{"line_number":544,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name})"},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"        volume_type_resp \u003d client.create_volume_type("},{"line_number":547,"context_line":"            name\u003drandomized_name, extra_specs\u003dextra_specs, **kwargs)"},{"line_number":548,"context_line":"        volume_type \u003d volume_type_resp[\u0027volume_type\u0027]"}],"source_content_type":"text/x-python","patch_set":6,"id":"d744aa6d_5f2312b5","line":545,"range":{"start_line":536,"start_character":0,"end_line":545,"end_character":0},"updated":"2022-09-07 22:46:54.000000000","message":"I suggest rewriting this so that we can log what backend is being used here, because there are several possibilities now.  Something like:\n\n  if backend_name:\n      extra_specs[\u0027volume_backend_name\u0027] \u003d backend_name\n  else:\n      if \u0027volume_backend_name\u0027 in extra_specs:\n          backend_name \u003d extra_specs[\u0027volume_backend_name\u0027]\n      elif \u0027capabilities:volume_backend_name\u0027 in extra_specs:\n          backend_name \u003d extra_specs[\u0027capabilities:volume_backend_name\u0027]\n      else\n          backend_name \u003d CONF.volume.volume_type_backend_name\n          if backend_name:\n              extra_specs[\u0027volume_backend_name\u0027] \u003d backend_name\n          else:\n              backend_name \u003d \u0027\u003cunspecified\u003e\u0027\n\n  LOG.debug(\"Creating a volume type: %s on backend %s\",\n            randomized_name, backend_name)\n\n\nThat being said, is \u0027capabilities:volume_backend_name\u0027 really a thing?\n\n  $ grep -I -R -F -n \u0027capabilities:volume_backend_name\u0027 cinder/*\n  cinder/volume/drivers/dell_emc/powermax/common.py:49:PREFIXBACKENDNAME \u003d \u0027capabilities:volume_backend_name\u0027\n  $ grep -I -R -F -n \u0027PREFIXBACKENDNAME\u0027 cinder/*\n  cinder/volume/drivers/dell_emc/powermax/common.py:49:PREFIXBACKENDNAME \u003d \u0027capabilities:volume_backend_name\u0027\n  \nSo although dell defines it, it doesn\u0027t actually seem to be used anywhere.\n\nThe capabilities filter strips off \u0027capabilities:\u0027 [0], so if the extra_specs have both \u0027v_b_n\u0027 and \u0027cap:v_b_n\u0027 keys and they have different values, the extra_specs will never match anything (because a backend will only have one name).  So if you do think you may get \u0027capabilities:volume_backend_name\u0027 in an extra_spec, we need to handle that in the \u0027if backend_name\u0027 clause (though I\u0027m not sure exactly what we should do).\n\n[0] https://opendev.org/openstack/cinder/src/commit/1a231d3d2e9fcd3945b67cf7b8d7a8dda3c76e2a/cinder/scheduler/filters/capabilities_filter.py#L64","commit_id":"bec3e8d550bbfd61401bd3541abb093eddfd4188"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c5f8a7edf12263100c22b5886646789cbb5c30cd","unresolved":true,"context_lines":[{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        extra_specs \u003d kwargs.pop(\"extra_specs\", {})"},{"line_number":536,"context_line":"        if backend_name:"},{"line_number":537,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"},{"line_number":538,"context_line":"        elif (not extra_specs or all("},{"line_number":539,"context_line":"                x not in [\"volume_backend_name\","},{"line_number":540,"context_line":"                          \"capabilities:volume_backend_name\"]"},{"line_number":541,"context_line":"                for x in extra_specs.keys())) and \\"},{"line_number":542,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":543,"context_line":"            extra_specs.update({"},{"line_number":544,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name})"},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"        volume_type_resp \u003d client.create_volume_type("},{"line_number":547,"context_line":"            name\u003drandomized_name, extra_specs\u003dextra_specs, **kwargs)"},{"line_number":548,"context_line":"        volume_type \u003d volume_type_resp[\u0027volume_type\u0027]"}],"source_content_type":"text/x-python","patch_set":6,"id":"3748dd50_357b9603","line":545,"range":{"start_line":536,"start_character":0,"end_line":545,"end_character":0},"in_reply_to":"8d7e26b5_8920d719","updated":"2023-07-20 21:58:16.000000000","message":"agree to log the backend as mentioned by Brian","commit_id":"bec3e8d550bbfd61401bd3541abb093eddfd4188"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"231aa8b90463637335466629967bfce0804d4b6e","unresolved":true,"context_lines":[{"line_number":533,"context_line":"        LOG.debug(\"Creating a volume type: %s on backend %s\","},{"line_number":534,"context_line":"                  randomized_name, backend_name)"},{"line_number":535,"context_line":"        extra_specs \u003d kwargs.pop(\"extra_specs\", {})"},{"line_number":536,"context_line":"        if backend_name:"},{"line_number":537,"context_line":"            extra_specs.update({\"volume_backend_name\": backend_name})"},{"line_number":538,"context_line":"        elif (not extra_specs or all("},{"line_number":539,"context_line":"                x not in [\"volume_backend_name\","},{"line_number":540,"context_line":"                          \"capabilities:volume_backend_name\"]"},{"line_number":541,"context_line":"                for x in extra_specs.keys())) and \\"},{"line_number":542,"context_line":"                CONF.volume.volume_type_backend_name:"},{"line_number":543,"context_line":"            extra_specs.update({"},{"line_number":544,"context_line":"                \u0027volume_backend_name\u0027: CONF.volume.volume_type_backend_name})"},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"        volume_type_resp \u003d client.create_volume_type("},{"line_number":547,"context_line":"            name\u003drandomized_name, extra_specs\u003dextra_specs, **kwargs)"},{"line_number":548,"context_line":"        volume_type \u003d volume_type_resp[\u0027volume_type\u0027]"}],"source_content_type":"text/x-python","patch_set":6,"id":"8d7e26b5_8920d719","line":545,"range":{"start_line":536,"start_character":0,"end_line":545,"end_character":0},"in_reply_to":"d744aa6d_5f2312b5","updated":"2022-09-08 04:07:47.000000000","message":"The change is based on tempest existing code, \nhttps://github.com/openstack/tempest/blob/master/tempest/api/volume/admin/test_multi_backend.py#L61\n\nIn case we enable multi-backend it should run.","commit_id":"bec3e8d550bbfd61401bd3541abb093eddfd4188"}]}