)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"8ceea2df9a46a2e687e6aa1fa5d0a4f67fc11872","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fdbfd398_3d07379d","updated":"2022-05-23 18:40:45.000000000","message":"LGTM and NFS jobs passed!","commit_id":"b9b26036f170e417f5a16a2bffe09bb1b4198375"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bd81a9aa30ec54b51bb1e479dd335be49a4853da","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1585b7ba_a3f9cd4d","updated":"2022-11-11 18:24:55.000000000","message":"Question inline.","commit_id":"b9b26036f170e417f5a16a2bffe09bb1b4198375"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"949618286cdbd379894a0f9bffc555883ea5df8f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"85e4c799_257aa381","updated":"2022-11-15 13:17:20.000000000","message":"Response inline.","commit_id":"b9b26036f170e417f5a16a2bffe09bb1b4198375"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f4506608d037bcc967a4696b654fb5e32c04863f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ea22e730_eafbef94","updated":"2022-11-15 08:17:29.000000000","message":"Thanks for the review Brian, please find my reply inline.","commit_id":"b9b26036f170e417f5a16a2bffe09bb1b4198375"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"60130a7f3ef0e0cf4d683081720968767cdcba67","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b16b37d9_aba1447d","updated":"2022-05-23 08:40:04.000000000","message":"recheck","commit_id":"b9b26036f170e417f5a16a2bffe09bb1b4198375"}],"devstack/plugin.sh":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bd81a9aa30ec54b51bb1e479dd335be49a4853da","unresolved":true,"context_lines":[{"line_number":56,"context_line":"    return $enabled"},{"line_number":57,"context_line":"}"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"function configure_cinder_nfs {"},{"line_number":60,"context_line":"    iniset $CINDER_CONF $be_name nas_host localhost"},{"line_number":61,"context_line":"    iniset $CINDER_CONF $be_name nas_share_path ${NFS_EXPORT_DIR}"},{"line_number":62,"context_line":"    iniset $CINDER_CONF $be_name nas_secure_file_operations \\"}],"source_content_type":"text/x-sh","patch_set":2,"id":"1f931c97_b668d610","line":59,"updated":"2022-11-11 18:24:55.000000000","message":"I don\u0027t see how be_name is being set anywhere.  I think what you need here is something like:\n\n  local be_name\u003d\"${1:-nfs}\"\n  \nwhich will give you backward-compatible behavior (if configure_cinder_nfs is called with an argument, it\u0027s used; if it\u0027s called without an argument, \u0027nfs\u0027 is used).","commit_id":"b9b26036f170e417f5a16a2bffe09bb1b4198375"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f4506608d037bcc967a4696b654fb5e32c04863f","unresolved":true,"context_lines":[{"line_number":56,"context_line":"    return $enabled"},{"line_number":57,"context_line":"}"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"function configure_cinder_nfs {"},{"line_number":60,"context_line":"    iniset $CINDER_CONF $be_name nas_host localhost"},{"line_number":61,"context_line":"    iniset $CINDER_CONF $be_name nas_share_path ${NFS_EXPORT_DIR}"},{"line_number":62,"context_line":"    iniset $CINDER_CONF $be_name nas_secure_file_operations \\"}],"source_content_type":"text/x-sh","patch_set":2,"id":"c93408bb_c4528b17","line":59,"in_reply_to":"1f931c97_b668d610","updated":"2022-11-15 08:17:29.000000000","message":"be_name is set in devstack[1] which is the name of backend and is used in all places where a backend name needs to be defined (cinder.conf, volume type etc).\nWhen we set nfs:nfsdriver-1 in local.conf then nfs is the \"be_type\" and nfsdriver-1 is the \"be_name\" which needs to be used here for cinder.conf (instead of hardcoded nfs).\nSince devstack-plugin-nfs is always used along with devstack, be_name will always be set.\nI would agree with your approach but currently configure_cinder_nfs is not called with any argument[2] and not even sure if devstack passes that while calling the plugin so this seems to be the only possible approach.\n\n[1] https://github.com/openstack/devstack/blob/1054f12bdac0208e73f22e16fe77edb87886722d/lib/cinder#L161\n\n[2] https://github.com/openstack/devstack-plugin-nfs/blob/dd12367f90fc86d42bfebe8a0ebb694dc0308810/devstack/plugin.sh#L102","commit_id":"b9b26036f170e417f5a16a2bffe09bb1b4198375"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"949618286cdbd379894a0f9bffc555883ea5df8f","unresolved":true,"context_lines":[{"line_number":56,"context_line":"    return $enabled"},{"line_number":57,"context_line":"}"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"function configure_cinder_nfs {"},{"line_number":60,"context_line":"    iniset $CINDER_CONF $be_name nas_host localhost"},{"line_number":61,"context_line":"    iniset $CINDER_CONF $be_name nas_share_path ${NFS_EXPORT_DIR}"},{"line_number":62,"context_line":"    iniset $CINDER_CONF $be_name nas_secure_file_operations \\"}],"source_content_type":"text/x-sh","patch_set":2,"id":"58e9f8a4_865189fa","line":59,"in_reply_to":"c93408bb_c4528b17","updated":"2022-11-15 13:17:20.000000000","message":"OK, so this function confuses me:\n\nhttps://github.com/openstack/devstack/blob/448036a6ad382cebcf9df15f717e259479be4965/lib/cinder_backends/nfs#L31\n\n... because that one clearly needs a parameter passed in (the local var overrides any current be_name env var).\n\nMy overall objection is that usually in devstack, global vars are ALL_CAPS and locals are lowercase (though it\u0027s not enforced).  So it\u0027s not at all obvious where this be_name is being set and whether it\u0027s local to the file or what.\n\nI won\u0027t block this patch, but there seems to be a code smell here.","commit_id":"b9b26036f170e417f5a16a2bffe09bb1b4198375"}]}