)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c5d38d1f0e3322fff38a3607fee1140cffbc702","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The nvmet target was using the nvmetcli command to manage the NVMe-oF"},{"line_number":10,"context_line":"targets, but the command has a big limitation when it comes to"},{"line_number":11,"context_line":"controlling it\u0027s behaviour through the command line, as it can only"},{"line_number":12,"context_line":"restore all its configuration and small parts, such as ports or"},{"line_number":13,"context_line":"subsystems, cannot be directly modified without entering into the"},{"line_number":14,"context_line":"interactive mode."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"a8d42aad_8a846715","line":11,"range":{"start_line":11,"start_character":12,"end_line":11,"end_character":16},"updated":"2023-01-13 14:17:00.000000000","message":"nit: its","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c5d38d1f0e3322fff38a3607fee1140cffbc702","unresolved":true,"context_lines":[{"line_number":9,"context_line":"The nvmet target was using the nvmetcli command to manage the NVMe-oF"},{"line_number":10,"context_line":"targets, but the command has a big limitation when it comes to"},{"line_number":11,"context_line":"controlling it\u0027s behaviour through the command line, as it can only"},{"line_number":12,"context_line":"restore all its configuration and small parts, such as ports or"},{"line_number":13,"context_line":"subsystems, cannot be directly modified without entering into the"},{"line_number":14,"context_line":"interactive mode."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9a72b6f6_c31cd360","line":12,"range":{"start_line":12,"start_character":30,"end_line":12,"end_character":33},"updated":"2023-01-13 14:17:00.000000000","message":"nit: so","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c5d38d1f0e3322fff38a3607fee1140cffbc702","unresolved":true,"context_lines":[{"line_number":13,"context_line":"subsystems, cannot be directly modified without entering into the"},{"line_number":14,"context_line":"interactive mode."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Due to this limitation the nvmet target would:"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"- Save current nvmet configuration"},{"line_number":19,"context_line":"- Make changes to the json data"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"74fcd556_cfd745c9","line":16,"range":{"start_line":16,"start_character":27,"end_line":16,"end_character":32},"updated":"2023-01-13 14:17:00.000000000","message":"nit: Cinder nvmet","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ae7286233320d47bb7f49b36d1b6fdd542ee4259","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"09433b6e_8a5aea84","updated":"2022-09-16 10:23:53.000000000","message":"Although i don\u0027t understand 100% of the changes, I\u0027m sure that this is tested with the os-brick changes for nvme connector which should be good enough to verify it.\nLGTM.","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d584aa2e53590753cc2001dfc60fb7bdca1bb794","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"92c41766_4a17657e","updated":"2022-09-21 05:30:08.000000000","message":"I think we can hold this patch for RC2 and merge it in Antelope with the rest of the series. If needed this can be backported and released with the next stable zed release. This is just my opinion, if anyone differs, I will remove my W-1.","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9f533fcfeaf3ba02b9ed95ae0ec2b44fd99cd27f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"33b419aa_1befa564","updated":"2022-09-21 02:21:01.000000000","message":"This is a clever way to use the functionality that nvmetcli only exposes via its interactive mode.  The code at http://git.infradead.org/users/hch/nvmetcli.git looks stable enough even though it\u0027s not \"officially\" exposed as a library, so I think your approach is fine.\n\nA few nits and a question about how to specify nvmetcli as a requirement are inline.  Nothing that can\u0027t be addressed in a followup, though.  The requirement question is what\u0027s making me hesitate about the +A on this, so just +2ing at the moment; Rajat or Gorka feel free to +A if it\u0027s not really something to worry about.\n\n","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"872ff87cf48aeb3b75d1482cddf7675d62cb732c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"34c5b3ec_4874d3ec","updated":"2022-10-03 23:02:04.000000000","message":"This seems okay.","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c5d38d1f0e3322fff38a3607fee1140cffbc702","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"704fa397_bb1e5660","updated":"2023-01-13 14:17:00.000000000","message":"recheck","commit_id":"00793ac09ba125c6af43bbbfeadc483de042815b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"282696a7edb999269a5abdc4942c9deb120b19de","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"74b08600_a3bce7b9","updated":"2023-01-16 11:58:01.000000000","message":"recheck","commit_id":"00793ac09ba125c6af43bbbfeadc483de042815b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7b2948ffd2502598025c6c5cb5d9d012c421b741","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"91c8af2d_843dbf50","updated":"2023-01-16 17:34:39.000000000","message":"still LGTM.","commit_id":"00793ac09ba125c6af43bbbfeadc483de042815b"}],"cinder/privsep/targets/nvmet.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9f533fcfeaf3ba02b9ed95ae0ec2b44fd99cd27f","unresolved":true,"context_lines":[{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":"import os"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"import nvmet"},{"line_number":30,"context_line":"from oslo_log import log as logging"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"from cinder import exception"}],"source_content_type":"text/x-python","patch_set":5,"id":"2ea5d231_3b53dd8d","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":12},"updated":"2022-09-21 02:21:01.000000000","message":"This is supplied by the nvmetcli package on RHEL-type distros.  Do we need to list it in the bindep.txt, or can we just assume that no one in their right mind will try to use nvmeof without installing nvmetcli, so it will be available?","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0ae4bb06c796e1fb3c3faa933364c2da90ee53a5","unresolved":false,"context_lines":[{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":"import os"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"import nvmet"},{"line_number":30,"context_line":"from oslo_log import log as logging"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"from cinder import exception"}],"source_content_type":"text/x-python","patch_set":5,"id":"2930a6d3_4576829e","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":12},"in_reply_to":"2c81d1c4_06c75e67","updated":"2023-01-13 14:25:55.000000000","message":"I believe that is an unrelated change, because before this patch we had the exact same requirement, nvmetcli needed to be installed.","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c7386c55e9a86ed09c80a84a1939bb4a469addcd","unresolved":true,"context_lines":[{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":"import os"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"import nvmet"},{"line_number":30,"context_line":"from oslo_log import log as logging"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"from cinder import exception"}],"source_content_type":"text/x-python","patch_set":5,"id":"2c81d1c4_06c75e67","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":12},"in_reply_to":"2ea5d231_3b53dd8d","updated":"2022-09-21 05:55:57.000000000","message":"Sounds like a good idea, adding it as a dependency is rather safe than assuming people will have the package installed.\n\n\u003e\u003e\u003e import nvmet\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\nModuleNotFoundError: No module named \u0027nvmet\u0027\n\u003e\u003e\u003e \n\n$ sudo dnf install nvmetcli\n\n\u003e\u003e\u003e import nvmet\n\u003e\u003e\u003e","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9f533fcfeaf3ba02b9ed95ae0ec2b44fd99cd27f","unresolved":true,"context_lines":[{"line_number":44,"context_line":"    \"\"\"Serialize parameters, specially nvmet instances."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    The idea is to be able to pass an nvmet instance to privsep methods, since"},{"line_number":47,"context_line":"    they are sometimes required as parameters (ie: port.setup) and also to pass"},{"line_number":48,"context_line":"    the instance where do_privsep_call has to call a specific method."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Instances are passed as a tuple, with the name of the class as the first"}],"source_content_type":"text/x-python","patch_set":5,"id":"b69ee507_16ecf47e","line":47,"range":{"start_line":47,"start_character":47,"end_line":47,"end_character":49},"updated":"2022-09-21 02:21:01.000000000","message":"nit: i think you mean eg here","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0ae4bb06c796e1fb3c3faa933364c2da90ee53a5","unresolved":true,"context_lines":[{"line_number":44,"context_line":"    \"\"\"Serialize parameters, specially nvmet instances."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    The idea is to be able to pass an nvmet instance to privsep methods, since"},{"line_number":47,"context_line":"    they are sometimes required as parameters (ie: port.setup) and also to pass"},{"line_number":48,"context_line":"    the instance where do_privsep_call has to call a specific method."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Instances are passed as a tuple, with the name of the class as the first"}],"source_content_type":"text/x-python","patch_set":5,"id":"be8a391c_968d49fb","line":47,"range":{"start_line":47,"start_character":47,"end_line":47,"end_character":49},"in_reply_to":"b69ee507_16ecf47e","updated":"2023-01-13 14:25:55.000000000","message":"yes","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9f533fcfeaf3ba02b9ed95ae0ec2b44fd99cd27f","unresolved":true,"context_lines":[{"line_number":54,"context_line":"    To differentiate nvmet instances from tuples there is a \u0027tuple\u0027 value that"},{"line_number":55,"context_line":"    can be passed in the first element of the tuple to differentiate them."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    All other instances as passed unaltered."},{"line_number":58,"context_line":"    \"\"\""},{"line_number":59,"context_line":"    if isinstance(instance, nvmet.Root):"},{"line_number":60,"context_line":"        return (\u0027Root\u0027, {})"}],"source_content_type":"text/x-python","patch_set":5,"id":"10bbc057_76a9960c","line":57,"range":{"start_line":57,"start_character":24,"end_line":57,"end_character":26},"updated":"2022-09-21 02:21:01.000000000","message":"nit: are","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ae7286233320d47bb7f49b36d1b6fdd542ee4259","unresolved":true,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    if isinstance(instance, nvmet.ANAGroup):"},{"line_number":80,"context_line":"        return (\u0027ANAGroup\u0027, {\u0027grpid\u0027: instance.grpid,"},{"line_number":81,"context_line":"                             \u0027port\u0027: serialize(instance.port),"},{"line_number":82,"context_line":"                             \u0027mode\u0027: \u0027lookup\u0027})"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    if isinstance(instance, tuple):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bc5090ac_1eaefcd2","line":81,"range":{"start_line":81,"start_character":37,"end_line":81,"end_character":46},"updated":"2022-09-16 10:23:53.000000000","message":"kind of scared of all these recursive calls potentially generating stack overflow","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0ae4bb06c796e1fb3c3faa933364c2da90ee53a5","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    if isinstance(instance, nvmet.ANAGroup):"},{"line_number":80,"context_line":"        return (\u0027ANAGroup\u0027, {\u0027grpid\u0027: instance.grpid,"},{"line_number":81,"context_line":"                             \u0027port\u0027: serialize(instance.port),"},{"line_number":82,"context_line":"                             \u0027mode\u0027: \u0027lookup\u0027})"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    if isinstance(instance, tuple):"}],"source_content_type":"text/x-python","patch_set":5,"id":"63c0ca32_88ff89b7","line":81,"range":{"start_line":81,"start_character":37,"end_line":81,"end_character":46},"in_reply_to":"6b0adb19_f6e8ec53","updated":"2023-01-13 14:25:55.000000000","message":"This is going to be called from within Cinder itself, so fortunately there can\u0027t be an OpenStack user trying to feed it something weird.","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"872ff87cf48aeb3b75d1482cddf7675d62cb732c","unresolved":true,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    if isinstance(instance, nvmet.ANAGroup):"},{"line_number":80,"context_line":"        return (\u0027ANAGroup\u0027, {\u0027grpid\u0027: instance.grpid,"},{"line_number":81,"context_line":"                             \u0027port\u0027: serialize(instance.port),"},{"line_number":82,"context_line":"                             \u0027mode\u0027: \u0027lookup\u0027})"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    if isinstance(instance, tuple):"}],"source_content_type":"text/x-python","patch_set":5,"id":"6b0adb19_f6e8ec53","line":81,"range":{"start_line":81,"start_character":37,"end_line":81,"end_character":46},"in_reply_to":"bc5090ac_1eaefcd2","updated":"2022-10-03 23:02:04.000000000","message":"I don\u0027t understand where the concern originates. It\u0027s a tree of serialization. If port is an integer or string, surely serialize() would just fall through and return its argument?","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ae7286233320d47bb7f49b36d1b6fdd542ee4259","unresolved":true,"context_lines":[{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    # Parameters for the instantiation of the class can be nvmet objects"},{"line_number":104,"context_line":"    # themselves."},{"line_number":105,"context_line":"    params \u003d {name: deserialize(value) for name, value in cls_params.items()}"},{"line_number":106,"context_line":"    # We don\u0027t want the classes from the nvmet method but ours instead"},{"line_number":107,"context_line":"    instance \u003d getattr(nvmet, cls_name)(**params)"},{"line_number":108,"context_line":"    return instance"}],"source_content_type":"text/x-python","patch_set":5,"id":"21c303db_c3b12f71","line":105,"range":{"start_line":105,"start_character":20,"end_line":105,"end_character":31},"updated":"2022-09-16 10:23:53.000000000","message":"I\u0027m assuming the value is always a tuple (or it\u0027s class is a tuple) else we might have a endless recursion case here","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"872ff87cf48aeb3b75d1482cddf7675d62cb732c","unresolved":true,"context_lines":[{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    # Parameters for the instantiation of the class can be nvmet objects"},{"line_number":104,"context_line":"    # themselves."},{"line_number":105,"context_line":"    params \u003d {name: deserialize(value) for name, value in cls_params.items()}"},{"line_number":106,"context_line":"    # We don\u0027t want the classes from the nvmet method but ours instead"},{"line_number":107,"context_line":"    instance \u003d getattr(nvmet, cls_name)(**params)"},{"line_number":108,"context_line":"    return instance"}],"source_content_type":"text/x-python","patch_set":5,"id":"aecc707a_efbab4f5","line":105,"range":{"start_line":105,"start_character":20,"end_line":105,"end_character":31},"in_reply_to":"21c303db_c3b12f71","updated":"2022-10-03 23:02:04.000000000","message":"Look a few lines up, where it checks for tuple. If the value is not a tuple, then indeed we call ourselves again, but return immediately.","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0ae4bb06c796e1fb3c3faa933364c2da90ee53a5","unresolved":false,"context_lines":[{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    # Parameters for the instantiation of the class can be nvmet objects"},{"line_number":104,"context_line":"    # themselves."},{"line_number":105,"context_line":"    params \u003d {name: deserialize(value) for name, value in cls_params.items()}"},{"line_number":106,"context_line":"    # We don\u0027t want the classes from the nvmet method but ours instead"},{"line_number":107,"context_line":"    instance \u003d getattr(nvmet, cls_name)(**params)"},{"line_number":108,"context_line":"    return instance"}],"source_content_type":"text/x-python","patch_set":5,"id":"61510d35_2f89171b","line":105,"range":{"start_line":105,"start_character":20,"end_line":105,"end_character":31},"in_reply_to":"aecc707a_efbab4f5","updated":"2023-01-13 14:25:55.000000000","message":"Yes, that\u0027s the way it would go, it value is a tuple then it gets processed, if it\u0027s not, then that value is used.","commit_id":"19bf9e0ef68555086059f06393ec01998521cd95"}]}