)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"99ce1c0f2a5b9ab8eb5f7fd836932d2e267da3e4","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Chris Morrell \u003cchris@solidfire.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-12-02 22:27:37 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Address potential races in SolidFire VAG."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While building the initial volume access group solution I realized there were"},{"line_number":10,"context_line":"race conditions present. This patch attempts to address those by being far"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9a8ffd7b_7d8dd52f","line":7,"updated":"2015-12-03 15:02:19.000000000","message":"Please remove the \u0027.\u0027 from the EOL\n\nhttps://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":16627,"name":"Chris Morrell","email":"gomeler@gmail.com","username":"Chris_Morrell"},"change_message_id":"82edb26a5350c520fa57252448d4b1ab2cc5a715","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Chris Morrell \u003cchris@solidfire.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-12-02 22:27:37 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Address potential races in SolidFire VAG."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While building the initial volume access group solution I realized there were"},{"line_number":10,"context_line":"race conditions present. This patch attempts to address those by being far"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a740942_893fea24","line":7,"in_reply_to":"9a8ffd7b_7d8dd52f","updated":"2015-12-03 18:04:40.000000000","message":"*doh* Thought I had the format right, so hard to break the habit of completing statements with a period. Will do.","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"99ce1c0f2a5b9ab8eb5f7fd836932d2e267da3e4","unresolved":false,"context_lines":[{"line_number":12,"context_line":"detaches around VAG volume removal introduced all sorts of future race"},{"line_number":13,"context_line":"conditions so instead multiattach volumes are treated differently and are only"},{"line_number":14,"context_line":"removed from VAGs on volume removal. Not ideal but it\u0027ll get the job done."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Think I covered all the new branches with unit tests. Complexity was introduced"},{"line_number":17,"context_line":"but it should be worth it due to the safety that was also introduced."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a740942_ced2e334","line":15,"updated":"2015-12-03 15:02:19.000000000","message":"Can you expand here on what you did to prevent the race conditions?","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":16627,"name":"Chris Morrell","email":"gomeler@gmail.com","username":"Chris_Morrell"},"change_message_id":"82edb26a5350c520fa57252448d4b1ab2cc5a715","unresolved":false,"context_lines":[{"line_number":12,"context_line":"detaches around VAG volume removal introduced all sorts of future race"},{"line_number":13,"context_line":"conditions so instead multiattach volumes are treated differently and are only"},{"line_number":14,"context_line":"removed from VAGs on volume removal. Not ideal but it\u0027ll get the job done."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Think I covered all the new branches with unit tests. Complexity was introduced"},{"line_number":17,"context_line":"but it should be worth it due to the safety that was also introduced."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a740942_44bec700","line":15,"in_reply_to":"7a740942_ced2e334","updated":"2015-12-03 18:04:40.000000000","message":"Should whatever comment I post be added to the commit message?\n\nThe initial solution was fraught with issues due to stepping through the VAG creation process by using the discrete API calls around create VAG, add volume to VAG, and add initiator to VAG. The SolidFire API(sfapi) does not enforce VAG names to be unique so with the previous solution two attaches to the same host could yield two VAGs with the same name. However, the sfapi has a weird quirk where an initiator can be part of only one VAG. So one attach would win the race and the other would bomb out. In short, I thought it\u0027d be nice to step through the VAG creation process but that was a terrible idea in hindsight.\n\nNow, one attach will win the race but by using the sfapi create VAG call with the optional initiator and volume inputs, one attach wins the race, the other throws and catches the create exception and retries. The loser will then via the sfapi find the existing VAG and instead add its volume to the VAG.\n\nThere is still a potential issue on detach where two volume detaches to the same host, if they are the last two volumes in the VAG, may both decide to simply remove their volume from the VAG rather than one of them removing the VAG. This will yield cruft VAG entries in the sfapi but we currently have a limit of 1,000 VAGs and with each VAG being initiator specific that\u0027s 1,000 initiators. Cross that scale bridge by beating the sfapp dev team over the head to raise that down the road.\n\nMultiattach with this solution is a pain. I know multiattach isn\u0027t present in Nova yet but I read over the various discussions when trying to figure out how that would affect things. In short, if one volume is multiattached on one host/initiator, I had no safe way to determine the last detach was indeed the last. So, instead I just wait until the volume is deleted, and then purge the volume from the VAG. Clunky but it\u0027ll work.\n\nThink that covers everything. Mostly working through/around limitations of the sfapi and syncing state across two systems without locks. I\u0027m sure there\u0027s still an edge case hanging out waiting to attack.","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"5162d03e612ee979cee5ba0516dd20a41256b7aa","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Think I covered all the new branches with unit tests. Complexity was introduced"},{"line_number":17,"context_line":"but it should be worth it due to the safety that was also introduced."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I80034cdf759295c8d379c02ed9e42659590b41a9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9a8ffd7b_edadbbe9","line":18,"updated":"2015-12-03 01:29:58.000000000","message":"Can you open a bug on this and reference it here?","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":16627,"name":"Chris Morrell","email":"gomeler@gmail.com","username":"Chris_Morrell"},"change_message_id":"82edb26a5350c520fa57252448d4b1ab2cc5a715","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Think I covered all the new branches with unit tests. Complexity was introduced"},{"line_number":17,"context_line":"but it should be worth it due to the safety that was also introduced."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I80034cdf759295c8d379c02ed9e42659590b41a9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a740942_446c879c","line":18,"in_reply_to":"9a8ffd7b_edadbbe9","updated":"2015-12-03 18:04:40.000000000","message":"I\u0027m not following, open a generic bug regarding the additional driver complexity around VAG creation?","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"}],"cinder/volume/drivers/solidfire.py":[{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"5162d03e612ee979cee5ba0516dd20a41256b7aa","unresolved":false,"context_lines":[{"line_number":893,"context_line":"                                    version\u003d\u00277.0\u0027)"},{"line_number":894,"context_line":"            return vag_id"},{"line_number":895,"context_line":"        except exception.SolidFireAPIException as error:"},{"line_number":896,"context_line":"            if \u0027xAlreadyInVolumeAccessGroup\u0027 in error.msg:"},{"line_number":897,"context_line":"                return vag_id"},{"line_number":898,"context_line":"            elif \u0027xVolumeAccessGroupIDDoesNotExist\u0027 in error.msg:"},{"line_number":899,"context_line":"                # No locking means sometimes a VAG can be removed by a parallel"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_c9a0ec44","line":896,"updated":"2015-12-03 01:29:58.000000000","message":"Can you use a constant for \u0027xAlreadyInVolumeAccessGroup\u0027 as it is used in multiple places?","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":16627,"name":"Chris Morrell","email":"gomeler@gmail.com","username":"Chris_Morrell"},"change_message_id":"82edb26a5350c520fa57252448d4b1ab2cc5a715","unresolved":false,"context_lines":[{"line_number":893,"context_line":"                                    version\u003d\u00277.0\u0027)"},{"line_number":894,"context_line":"            return vag_id"},{"line_number":895,"context_line":"        except exception.SolidFireAPIException as error:"},{"line_number":896,"context_line":"            if \u0027xAlreadyInVolumeAccessGroup\u0027 in error.msg:"},{"line_number":897,"context_line":"                return vag_id"},{"line_number":898,"context_line":"            elif \u0027xVolumeAccessGroupIDDoesNotExist\u0027 in error.msg:"},{"line_number":899,"context_line":"                # No locking means sometimes a VAG can be removed by a parallel"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_441a4719","line":896,"in_reply_to":"9a8ffd7b_c9a0ec44","updated":"2015-12-03 18:04:40.000000000","message":"Will do.","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"5162d03e612ee979cee5ba0516dd20a41256b7aa","unresolved":false,"context_lines":[{"line_number":916,"context_line":"        except exception.SolidFireAPIException as error:"},{"line_number":917,"context_line":"            if \u0027xAlreadyInVolumeAccessGroup\u0027 in error.msg:"},{"line_number":918,"context_line":"                return vag_id"},{"line_number":919,"context_line":"            elif \u0027xVolumeAccessGroupIDDoesNotExist\u0027 in error.msg:"},{"line_number":920,"context_line":"                return self._safe_create_vag(iqn, vol_id)"},{"line_number":921,"context_line":"            else:"},{"line_number":922,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_096914f9","line":919,"updated":"2015-12-03 01:29:58.000000000","message":"Same with \u0027xVolumeAccessGroupIDDoesNotExist\u0027.  Suggest using a constant to avoid typo.","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":16627,"name":"Chris Morrell","email":"gomeler@gmail.com","username":"Chris_Morrell"},"change_message_id":"82edb26a5350c520fa57252448d4b1ab2cc5a715","unresolved":false,"context_lines":[{"line_number":916,"context_line":"        except exception.SolidFireAPIException as error:"},{"line_number":917,"context_line":"            if \u0027xAlreadyInVolumeAccessGroup\u0027 in error.msg:"},{"line_number":918,"context_line":"                return vag_id"},{"line_number":919,"context_line":"            elif \u0027xVolumeAccessGroupIDDoesNotExist\u0027 in error.msg:"},{"line_number":920,"context_line":"                return self._safe_create_vag(iqn, vol_id)"},{"line_number":921,"context_line":"            else:"},{"line_number":922,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_24bffbd1","line":919,"in_reply_to":"9a8ffd7b_096914f9","updated":"2015-12-03 18:04:40.000000000","message":"Understandable, will check for other potential constants.","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"5162d03e612ee979cee5ba0516dd20a41256b7aa","unresolved":false,"context_lines":[{"line_number":956,"context_line":""},{"line_number":957,"context_line":"    def _purge_vags(self, limit\u003d10):"},{"line_number":958,"context_line":"        # Purge up to limit number of VAGs that have no active volumes,"},{"line_number":959,"context_line":"        # initiators, and an openstack attribute. Purge oldest VAGs first."},{"line_number":960,"context_line":"        vags \u003d self._base_get_vags()"},{"line_number":961,"context_line":"        targets \u003d [v for v in vags if v[\u0027volumes\u0027] \u003d\u003d [] and"},{"line_number":962,"context_line":"                   v[\u0027initiators\u0027] \u003d\u003d [] and"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_cd3b17d0","line":959,"updated":"2015-12-03 01:29:58.000000000","message":"s/openstack/OpenStack\n\nThere was a patch fixing this, so I want to point it out.","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":16627,"name":"Chris Morrell","email":"gomeler@gmail.com","username":"Chris_Morrell"},"change_message_id":"82edb26a5350c520fa57252448d4b1ab2cc5a715","unresolved":false,"context_lines":[{"line_number":956,"context_line":""},{"line_number":957,"context_line":"    def _purge_vags(self, limit\u003d10):"},{"line_number":958,"context_line":"        # Purge up to limit number of VAGs that have no active volumes,"},{"line_number":959,"context_line":"        # initiators, and an openstack attribute. Purge oldest VAGs first."},{"line_number":960,"context_line":"        vags \u003d self._base_get_vags()"},{"line_number":961,"context_line":"        targets \u003d [v for v in vags if v[\u0027volumes\u0027] \u003d\u003d [] and"},{"line_number":962,"context_line":"                   v[\u0027initiators\u0027] \u003d\u003d [] and"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_649c236a","line":959,"in_reply_to":"9a8ffd7b_cd3b17d0","updated":"2015-12-03 18:04:40.000000000","message":"Will correct, thanks.","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"5162d03e612ee979cee5ba0516dd20a41256b7aa","unresolved":false,"context_lines":[{"line_number":1533,"context_line":"            vol_id \u003d int(self._parse_provider_id_string(provider_id)[0])"},{"line_number":1534,"context_line":""},{"line_number":1535,"context_line":"            if vag and not volume[\u0027multiattach\u0027]:"},{"line_number":1536,"context_line":"                # Multiattach causes problems with removing volumes from VAGs."},{"line_number":1537,"context_line":"                # Compromise solution for now is to remove multiattach volumes"},{"line_number":1538,"context_line":"                # from VAGs during volume deletion."},{"line_number":1539,"context_line":"                vag \u003d vag[0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_8d5f4f0e","line":1536,"updated":"2015-12-03 01:29:58.000000000","message":"Are you testing the \"multiattach\" feature now?  I\u0027m asking because the multiattach support on the Nova side is not merged yet.","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"},{"author":{"_account_id":16627,"name":"Chris Morrell","email":"gomeler@gmail.com","username":"Chris_Morrell"},"change_message_id":"82edb26a5350c520fa57252448d4b1ab2cc5a715","unresolved":false,"context_lines":[{"line_number":1533,"context_line":"            vol_id \u003d int(self._parse_provider_id_string(provider_id)[0])"},{"line_number":1534,"context_line":""},{"line_number":1535,"context_line":"            if vag and not volume[\u0027multiattach\u0027]:"},{"line_number":1536,"context_line":"                # Multiattach causes problems with removing volumes from VAGs."},{"line_number":1537,"context_line":"                # Compromise solution for now is to remove multiattach volumes"},{"line_number":1538,"context_line":"                # from VAGs during volume deletion."},{"line_number":1539,"context_line":"                vag \u003d vag[0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_44e567a6","line":1536,"in_reply_to":"9a8ffd7b_8d5f4f0e","updated":"2015-12-03 18:04:40.000000000","message":"While thinking about the various race conditions, I stumbled across multiattach which completely wrecks this solution and the other proposals I came up with. So I instead address multiattach volume removal from VAGs at volume deletion. Tried to get multiattach working to test via various patches, gave up. This solution should work albeit it\u0027ll leave a bit of cruft on the SolidFire cluster.","commit_id":"33e466490d95abf1cb2c1e892920c672cd0b9d5c"}]}