)]}'
{"cinder/volume/drivers/netapp/dataontap/nfs_base.py":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"8bd85e2d5e46a1041d42cbafc7f4f010b103aeb6","unresolved":false,"context_lines":[{"line_number":533,"context_line":"        extra_specs \u003d na_utils.get_volume_extra_specs(volume)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        try:"},{"line_number":536,"context_line":"            cache_result \u003d self._find_image_in_cache(image_id)"},{"line_number":537,"context_line":"            if cache_result:"},{"line_number":538,"context_line":"                cloned \u003d self._clone_from_cache(volume, image_id, cache_result)"},{"line_number":539,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff0f0b1f_64853a5a","line":536,"range":{"start_line":536,"start_character":32,"end_line":536,"end_character":52},"updated":"2017-05-19 22:19:48.000000000","message":"This method","commit_id":"6102e56f4ef3d31c8572fbdb1b970c13644d913d"},{"author":{"_account_id":5044,"name":"Mate Lakat","email":"mlakat@suse.com","username":"mate-lakat"},"change_message_id":"f2d4976d2321fb8679dd5968d4615f030838f3a2","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        \"\"\"Clones a copy from image cache.\"\"\""},{"line_number":558,"context_line":"        cloned \u003d False"},{"line_number":559,"context_line":"        LOG.info(\u0027Cloning image %s from cache\u0027, image_id)"},{"line_number":560,"context_line":"        path \u003d volume.host.split(\u0027#\u0027)[1]"},{"line_number":561,"context_line":"        for res in cache_result:"},{"line_number":562,"context_line":"            # Repeat tries in other shares if failed in some"},{"line_number":563,"context_line":"            (share, file_name) \u003d res"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f044301_11b905cb","line":560,"range":{"start_line":560,"start_character":8,"end_line":560,"end_character":40},"updated":"2017-05-09 14:46:39.000000000","message":"Are we sure that `volume.host` always contains a `#`? I smell a potential IndexError. I would extract this to a separate function, like: `get_volume_share`, which might return `None` if the above statement failed. I would also give a different name to this variable, as `path` is quite generic. I think it\u0027s meaning is something like `volume_share`?","commit_id":"6102e56f4ef3d31c8572fbdb1b970c13644d913d"},{"author":{"_account_id":2417,"name":"Ben Swartzlander","email":"ben@swartzlander.org","username":"bswartz"},"change_message_id":"459378bcf748bb3f597859d786446331b3d9cffb","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        \"\"\"Clones a copy from image cache.\"\"\""},{"line_number":558,"context_line":"        cloned \u003d False"},{"line_number":559,"context_line":"        LOG.info(\u0027Cloning image %s from cache\u0027, image_id)"},{"line_number":560,"context_line":"        path \u003d volume.host.split(\u0027#\u0027)[1]"},{"line_number":561,"context_line":"        for res in cache_result:"},{"line_number":562,"context_line":"            # Repeat tries in other shares if failed in some"},{"line_number":563,"context_line":"            (share, file_name) \u003d res"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f044301_a16b0a1c","line":560,"range":{"start_line":560,"start_character":8,"end_line":560,"end_character":40},"in_reply_to":"3f044301_11b905cb","updated":"2017-05-10 16:50:14.000000000","message":"In the case of this driver, yes. We always provide pool names and they would always be present here unless some other part of the system changed significantly. We can add some defensive coding, but in any case where there\u0027s no \"#\" or no text after the # we would need to fail immediately.","commit_id":"6102e56f4ef3d31c8572fbdb1b970c13644d913d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"8bd85e2d5e46a1041d42cbafc7f4f010b103aeb6","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        \"\"\"Clones a copy from image cache.\"\"\""},{"line_number":558,"context_line":"        cloned \u003d False"},{"line_number":559,"context_line":"        LOG.info(\u0027Cloning image %s from cache\u0027, image_id)"},{"line_number":560,"context_line":"        path \u003d volume.host.split(\u0027#\u0027)[1]"},{"line_number":561,"context_line":"        for res in cache_result:"},{"line_number":562,"context_line":"            # Repeat tries in other shares if failed in some"},{"line_number":563,"context_line":"            (share, file_name) \u003d res"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff0f0b1f_24cd02b8","line":560,"range":{"start_line":560,"start_character":8,"end_line":560,"end_character":40},"in_reply_to":"3f044301_a16b0a1c","updated":"2017-05-19 22:19:48.000000000","message":"Ben: This method is called by cinder in the \"clone_image\" workflow... so, while working on the bug: https://bugs.launchpad.net/mos/+bug/1632333, realized that you could just stop this in the method where we inspect the \"cache\" instead of here. It\u0027s unintuitive to do it here, because, the flow is:\n\n1) Check if the image has been cached\n2) If the image has been cached, clone the cached image. \n\nSo, in order to correct step 2, it feels like we should fix step 1 so that we check only in the relevant flexvol for a cached image.","commit_id":"6102e56f4ef3d31c8572fbdb1b970c13644d913d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"35b9b8b7de93c5af75c911191b392767e8fea0bc","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        \"\"\"Clones a copy from image cache.\"\"\""},{"line_number":558,"context_line":"        cloned \u003d False"},{"line_number":559,"context_line":"        LOG.info(\u0027Cloning image %s from cache\u0027, image_id)"},{"line_number":560,"context_line":"        path \u003d volume.host.split(\u0027#\u0027)[1]"},{"line_number":561,"context_line":"        for res in cache_result:"},{"line_number":562,"context_line":"            # Repeat tries in other shares if failed in some"},{"line_number":563,"context_line":"            (share, file_name) \u003d res"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff0f0b1f_d4a69fab","line":560,"range":{"start_line":560,"start_character":8,"end_line":560,"end_character":40},"in_reply_to":"ff0f0b1f_04bafedc","updated":"2017-05-23 19:31:04.000000000","message":"Different ways to solve this problem.. As Ben noted below, we should commit this tested surgical fix to stable branches and probably rewrite some of this code in line with changes currently in cinder.","commit_id":"6102e56f4ef3d31c8572fbdb1b970c13644d913d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"7047cf72573c2a025e6b9563a536f4e211afdc4a","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        \"\"\"Clones a copy from image cache.\"\"\""},{"line_number":558,"context_line":"        cloned \u003d False"},{"line_number":559,"context_line":"        LOG.info(\u0027Cloning image %s from cache\u0027, image_id)"},{"line_number":560,"context_line":"        path \u003d volume.host.split(\u0027#\u0027)[1]"},{"line_number":561,"context_line":"        for res in cache_result:"},{"line_number":562,"context_line":"            # Repeat tries in other shares if failed in some"},{"line_number":563,"context_line":"            (share, file_name) \u003d res"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff0f0b1f_04bafedc","line":560,"range":{"start_line":560,"start_character":8,"end_line":560,"end_character":40},"in_reply_to":"ff0f0b1f_24cd02b8","updated":"2017-05-19 22:29:45.000000000","message":"Also, take a look at:\n\n https://github.com/openstack/cinder/blob/1d0a36d/cinder/volume/utils.py#L710\n\nTo get the pool name. In case of NetApp NFS, the pool name contains the full export path for the flexvol, ex:\n\n 192.168.10.12:/my_cinder_flexvol\n\n\nThis will help you avoid the .split(), it\u0027s tested code.","commit_id":"6102e56f4ef3d31c8572fbdb1b970c13644d913d"},{"author":{"_account_id":5044,"name":"Mate Lakat","email":"mlakat@suse.com","username":"mate-lakat"},"change_message_id":"f2d4976d2321fb8679dd5968d4615f030838f3a2","unresolved":false,"context_lines":[{"line_number":564,"context_line":"            if path \u003d\u003d share:"},{"line_number":565,"context_line":"                LOG.debug(\u0027Cache share: %s\u0027, share)"},{"line_number":566,"context_line":"                if (share and"},{"line_number":567,"context_line":"                        self._is_share_clone_compatible(volume, share)):"},{"line_number":568,"context_line":"                    try:"},{"line_number":569,"context_line":"                        self._do_clone_rel_img_cache("},{"line_number":570,"context_line":"                            file_name, volume[\u0027name\u0027], share, file_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f044301_51778d03","line":567,"range":{"start_line":567,"start_character":29,"end_line":567,"end_character":55},"updated":"2017-05-09 14:46:39.000000000","message":"Thinking a bit more, is it possible that this functionality should really go to `_is_share_clone_compatible` function? It seems from it\u0027s description:\n\"\"\"Checks if share is compatible with volume to host its clone.\"\"\"\n\nThat this responsibility should be taken bu the `_is_share_clone_compatible` method.","commit_id":"6102e56f4ef3d31c8572fbdb1b970c13644d913d"},{"author":{"_account_id":2417,"name":"Ben Swartzlander","email":"ben@swartzlander.org","username":"bswartz"},"change_message_id":"459378bcf748bb3f597859d786446331b3d9cffb","unresolved":false,"context_lines":[{"line_number":564,"context_line":"            if path \u003d\u003d share:"},{"line_number":565,"context_line":"                LOG.debug(\u0027Cache share: %s\u0027, share)"},{"line_number":566,"context_line":"                if (share and"},{"line_number":567,"context_line":"                        self._is_share_clone_compatible(volume, share)):"},{"line_number":568,"context_line":"                    try:"},{"line_number":569,"context_line":"                        self._do_clone_rel_img_cache("},{"line_number":570,"context_line":"                            file_name, volume[\u0027name\u0027], share, file_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f044301_81a92e25","line":567,"range":{"start_line":567,"start_character":29,"end_line":567,"end_character":55},"in_reply_to":"3f044301_51778d03","updated":"2017-05-10 16:50:14.000000000","message":"The intention of the is_share_compatible() method is more about figuring out if a clone would fail for some NetApp-specific reason.\n\nYou\u0027ve identified that there\u0027s a logic problem here and you\u0027re right. This part of the code is extremely old and predates the existence of pools. What should have happened when pool support was added is hat this whole loop should have been removed and there should have been a simple check of the volume in question.\n\nWe could make that change now, but it would be far less surgical than the bugfix Marc submitted.","commit_id":"6102e56f4ef3d31c8572fbdb1b970c13644d913d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"5b3c4d5e5fb106d13d1177d0bfc69d4bbe5b5197","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                        volume[\u0027provider_location\u0027] \u003d share"},{"line_number":573,"context_line":"                        break"},{"line_number":574,"context_line":"                    except Exception:"},{"line_number":575,"context_line":"                        LOG.warning(\u0027Unexpected exception during\u0027"},{"line_number":576,"context_line":"                                    \u0027 image cloning in share %s\u0027, share)"},{"line_number":577,"context_line":"        return cloned"},{"line_number":578,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_b6794594","line":575,"updated":"2017-07-25 09:50:05.000000000","message":"Please, add more details why it happen","commit_id":"bd89b4216b1a1a9ab0bf57043866c012b4a4cf34"}]}