)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4886df2add00c5b60e54f210b5b7942940da7a2c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add deactivate step to extend_lv"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Extending a linear LVM volume that has a snapshot requires"},{"line_number":10,"context_line":"that the LV be deactivated explciitly prior to extending."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This adds a deactivate_lv method and calls it prior to"},{"line_number":13,"context_line":"issuing the extend call.  The LV should be auto reactivated"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba3cc151_72c6166a","line":10,"updated":"2015-07-10 09:41:26.000000000","message":"s/explciitly/explicitly","commit_id":"08cc7b373a0fbedb3e573ca0902c805884824232"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"313d87f653aa5bfb817380cb426a949c2b67b610","unresolved":false,"context_lines":[{"line_number":10,"context_line":"that the LV be deactivated explciitly prior to extending."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This adds a deactivate_lv method and calls it prior to"},{"line_number":13,"context_line":"issuing the extend call.  The LV should be auto reactivated"},{"line_number":14,"context_line":"after the extend completes, and also NOTE this doesn\u0027t apply"},{"line_number":15,"context_line":"to thin provisioned LV\u0027s, they don\u0027t require the deactivation"},{"line_number":16,"context_line":"step."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba3cc151_d5f473d0","line":13,"updated":"2015-07-07 03:00:14.000000000","message":"should be auto reactivated, meaning Cinder driver doesn\u0027t have to do it explicitly?","commit_id":"08cc7b373a0fbedb3e573ca0902c805884824232"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4886df2add00c5b60e54f210b5b7942940da7a2c","unresolved":false,"context_lines":[{"line_number":10,"context_line":"that the LV be deactivated explciitly prior to extending."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This adds a deactivate_lv method and calls it prior to"},{"line_number":13,"context_line":"issuing the extend call.  The LV should be auto reactivated"},{"line_number":14,"context_line":"after the extend completes, and also NOTE this doesn\u0027t apply"},{"line_number":15,"context_line":"to thin provisioned LV\u0027s, they don\u0027t require the deactivation"},{"line_number":16,"context_line":"step."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba3cc151_b5f6c88e","line":13,"in_reply_to":"ba3cc151_106ab4ed","updated":"2015-07-10 09:41:26.000000000","message":"Are you sure this is auto-activated?  Because I don\u0027t see where this is being configured, and if I missed it and we are, shouldn\u0027t we change our activation to use -aay instead of -ay to ensure this is properly configured.","commit_id":"08cc7b373a0fbedb3e573ca0902c805884824232"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"22522188cec6ef28b1d305436ffa549fd83b78f4","unresolved":false,"context_lines":[{"line_number":10,"context_line":"that the LV be deactivated explciitly prior to extending."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This adds a deactivate_lv method and calls it prior to"},{"line_number":13,"context_line":"issuing the extend call.  The LV should be auto reactivated"},{"line_number":14,"context_line":"after the extend completes, and also NOTE this doesn\u0027t apply"},{"line_number":15,"context_line":"to thin provisioned LV\u0027s, they don\u0027t require the deactivation"},{"line_number":16,"context_line":"step."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba3cc151_b2fa4142","line":13,"in_reply_to":"ba3cc151_7ce7bd46","updated":"2015-07-10 19:03:34.000000000","message":"Ok, apparently this would only be a problem if they have defined the auto_activation_volume_list in lvm.conf and the VG or LV is not in there.\n\n    # If auto_activation_volume_list is defined, each LV that is to be\n    # activated with the autoactivation option (--activate ay/-a ay) is\n    # first checked against the list. There are two scenarios in which\n    # the autoactivation option is used:\n    #\n    #   - automatic activation of volumes based on incoming PVs. If all the\n    #     PVs making up a VG are present in the system, the autoactivation\n    #     is triggered. This requires lvmetad (global/use_lvmetad\u003d1) and udev\n    #     to be running. In this case, \"pvscan --cache -aay\" is called\n    #     automatically without any user intervention while processing\n    #     udev events. Please, make sure you define auto_activation_volume_list\n    #     properly so only the volumes you want and expect are autoactivated.\n    #\n    #   - direct activation on command line with the autoactivation option.\n    #     In this case, the user calls \"vgchange --activate ay/-a ay\" or\n    #     \"lvchange --activate ay/-a ay\" directly.\n    #\n    # By default, the auto_activation_volume_list is not defined and all\n    # volumes will be activated either automatically or by using --activate ay/-a ay.\n\nI think a note on this commit message is in order and I don\u0027t know if this should also be documented somewhere.","commit_id":"08cc7b373a0fbedb3e573ca0902c805884824232"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"b5bdbe69fed2b315b61342340d66f9919a3aa4e3","unresolved":false,"context_lines":[{"line_number":10,"context_line":"that the LV be deactivated explciitly prior to extending."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This adds a deactivate_lv method and calls it prior to"},{"line_number":13,"context_line":"issuing the extend call.  The LV should be auto reactivated"},{"line_number":14,"context_line":"after the extend completes, and also NOTE this doesn\u0027t apply"},{"line_number":15,"context_line":"to thin provisioned LV\u0027s, they don\u0027t require the deactivation"},{"line_number":16,"context_line":"step."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba3cc151_7ce7bd46","line":13,"in_reply_to":"ba3cc151_b5f6c88e","updated":"2015-07-10 16:43:47.000000000","message":"@Gorka\nWell, so it\u0027s not something Cinder does at all.  Just my observation in testing, it appears that once the extend completes LVM is re-activating the LV.\n\nI can certainly look more closely at this and update the commit message.  Or even add it explicitly as I don\u0027t \"think\" it will hurt anything.","commit_id":"08cc7b373a0fbedb3e573ca0902c805884824232"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"ba3fa98822c59c7258f7dee6f92ed1b1840ec08b","unresolved":false,"context_lines":[{"line_number":10,"context_line":"that the LV be deactivated explciitly prior to extending."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This adds a deactivate_lv method and calls it prior to"},{"line_number":13,"context_line":"issuing the extend call.  The LV should be auto reactivated"},{"line_number":14,"context_line":"after the extend completes, and also NOTE this doesn\u0027t apply"},{"line_number":15,"context_line":"to thin provisioned LV\u0027s, they don\u0027t require the deactivation"},{"line_number":16,"context_line":"step."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba3cc151_106ab4ed","line":13,"in_reply_to":"ba3cc151_d5f473d0","updated":"2015-07-07 23:57:52.000000000","message":"correct, I can update commit message to clarify that.  I\u0027m also wanting to make sure my interpretation of the activate/deactivate process in LVM is accurate.","commit_id":"08cc7b373a0fbedb3e573ca0902c805884824232"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"692d88234549f37bb9c98f2515e96814551356e4","unresolved":false,"context_lines":[{"line_number":15,"context_line":"automatically after the extend.  If auto_activation_volume_list"},{"line_number":16,"context_line":"is defined, then volumes that should be automatically"},{"line_number":17,"context_line":"reactivated should be added to the auto_activation_volume_list"},{"line_number":18,"context_line":"or they won\u0027t be activated automatically."},{"line_number":19,"context_line":"NOTE: This doesn\u0027t apply to thin provisioned LVs as they don\u0027t"},{"line_number":20,"context_line":"require the deactivation step."},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3a50d1a3_d66736e7","line":18,"updated":"2015-07-22 17:00:15.000000000","message":"Above note is worth documenting, so a DocImpact flag is need.","commit_id":"400bb7e56d2e5fde6d51918efc87c3b50f146296"}],"cinder/brick/local_dev/lvm.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4886df2add00c5b60e54f210b5b7942940da7a2c","unresolved":false,"context_lines":[{"line_number":589,"context_line":"            return name"},{"line_number":590,"context_line":"        return \u0027_\u0027 + name"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"    def deactivate_lv(self, name, is_snapshot\u003dFalse):"},{"line_number":593,"context_line":"        if is_snapshot and not self.supports_snapshot_lv_activation:"},{"line_number":594,"context_line":"            return"},{"line_number":595,"context_line":"        lv_path \u003d self.vg_name + \u0027/\u0027 + self._mangle_lv_name(name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba3cc151_f26a26ed","line":592,"updated":"2015-07-10 09:41:26.000000000","message":"On commit message you mention this is not needed for thin provisioned LVs, shouldn\u0027t you be checking if this LV is thin provisioned and return if it is?","commit_id":"08cc7b373a0fbedb3e573ca0902c805884824232"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"b5bdbe69fed2b315b61342340d66f9919a3aa4e3","unresolved":false,"context_lines":[{"line_number":589,"context_line":"            return name"},{"line_number":590,"context_line":"        return \u0027_\u0027 + name"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"    def deactivate_lv(self, name, is_snapshot\u003dFalse):"},{"line_number":593,"context_line":"        if is_snapshot and not self.supports_snapshot_lv_activation:"},{"line_number":594,"context_line":"            return"},{"line_number":595,"context_line":"        lv_path \u003d self.vg_name + \u0027/\u0027 + self._mangle_lv_name(name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba3cc151_1c6471e3","line":592,"in_reply_to":"ba3cc151_f26a26ed","updated":"2015-07-10 16:43:47.000000000","message":"Indeed, you are correct.  It doesn\u0027t appear it\u0027s needed, don\u0027t think it hurts but regardless.\n\nSo full disclosure,  I find the whole activate/deactivate business an absolute pain and rather confusing and most importantly I don\u0027t pretend to fully understand all of the nuances and little gotchas; particularly as there seems to be different behaviors between some distros.","commit_id":"08cc7b373a0fbedb3e573ca0902c805884824232"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"2b6a3711889779104fb04cc95f8941e1d63515a8","unresolved":false,"context_lines":[{"line_number":589,"context_line":"            return name"},{"line_number":590,"context_line":"        return \u0027_\u0027 + name"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"    def deactivate_lv(self, name, is_snapshot\u003dFalse):"},{"line_number":593,"context_line":"        if is_snapshot and not self.supports_snapshot_lv_activation:"},{"line_number":594,"context_line":"            return"},{"line_number":595,"context_line":"        lv_path \u003d self.vg_name + \u0027/\u0027 + self._mangle_lv_name(name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba3cc151_285d7679","line":592,"in_reply_to":"ba3cc151_f26a26ed","updated":"2015-07-10 13:52:42.000000000","message":"What\u0027s the purpose of this \u0027is_snapshot\u0027 variable ?\n\nIt probably mimics the activate_lv signature but the is_snapshot argument of the activate_lv is actually used.","commit_id":"08cc7b373a0fbedb3e573ca0902c805884824232"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"df7b1d947eac323365b5b5c2da1112d44bf9ce81","unresolved":false,"context_lines":[{"line_number":589,"context_line":"            return name"},{"line_number":590,"context_line":"        return \u0027_\u0027 + name"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"    def deactivate_lv(self, name, is_snapshot\u003dFalse):"},{"line_number":593,"context_line":"        lv_path \u003d self.vg_name + \u0027/\u0027 + self._mangle_lv_name(name)"},{"line_number":594,"context_line":"        cmd \u003d [\u0027lvchange\u0027, \u0027-a\u0027, \u0027n\u0027]"},{"line_number":595,"context_line":"        cmd.append(lv_path)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a50d1a3_09b891e4","line":592,"updated":"2015-07-21 01:45:26.000000000","message":"is_snapshot is not used in the function, is it ok to keep it here?","commit_id":"459e78b45e8f0114e3182480a82042a448be09b7"},{"author":{"_account_id":12608,"name":"Curt Bruns","email":"curt.e.bruns@intel.com","username":"cebruns"},"change_message_id":"a3e85ac745e245d3f8fb170d150ef9c09be9ae16","unresolved":false,"context_lines":[{"line_number":589,"context_line":"            return name"},{"line_number":590,"context_line":"        return \u0027_\u0027 + name"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"    def deactivate_lv(self, name, is_snapshot\u003dFalse):"},{"line_number":593,"context_line":"        lv_path \u003d self.vg_name + \u0027/\u0027 + self._mangle_lv_name(name)"},{"line_number":594,"context_line":"        cmd \u003d [\u0027lvchange\u0027, \u0027-a\u0027, \u0027n\u0027]"},{"line_number":595,"context_line":"        cmd.append(lv_path)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a50d1a3_71322912","line":592,"in_reply_to":"3a50d1a3_09b891e4","updated":"2015-07-21 18:49:50.000000000","message":"Good catch - it\u0027s not needed in here, so I\u0027ll remove it.","commit_id":"459e78b45e8f0114e3182480a82042a448be09b7"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"ad5da5aee7b5b2c217013db175d99b98af08b94f","unresolved":false,"context_lines":[{"line_number":598,"context_line":"                          root_helper\u003dself._root_helper,"},{"line_number":599,"context_line":"                          run_as_root\u003dTrue)"},{"line_number":600,"context_line":"        except putils.ProcessExecutionError as err:"},{"line_number":601,"context_line":"            LOG.exception(_LE(\u0027Error deactivating LV\u0027))"},{"line_number":602,"context_line":"            LOG.error(_LE(\u0027Cmd     :%s\u0027), err.cmd)"},{"line_number":603,"context_line":"            LOG.error(_LE(\u0027StdOut  :%s\u0027), err.stdout)"},{"line_number":604,"context_line":"            LOG.error(_LE(\u0027StdErr  :%s\u0027), err.stderr)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_f4b0678d","line":601,"updated":"2015-07-27 16:30:10.000000000","message":"\u0027.\u0027 at EOL","commit_id":"52e643b7dbaa612ad9b71b8a959674a1c76ff81b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"636e8cbe10448baaba6b8fa2796ed334ae96f60a","unresolved":false,"context_lines":[{"line_number":711,"context_line":""},{"line_number":712,"context_line":"    def extend_volume(self, lv_name, new_size):"},{"line_number":713,"context_line":"        \"\"\"Extend the size of an existing volume.\"\"\""},{"line_number":714,"context_line":"        if self.lv_has_snapshot(lv_name):"},{"line_number":715,"context_line":"            self.deactivate_lv(lv_name)"},{"line_number":716,"context_line":"        try:"},{"line_number":717,"context_line":"            self._execute(\u0027lvextend\u0027, \u0027-L\u0027, new_size,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_966b001e","line":714,"updated":"2015-07-28 12:04:43.000000000","message":"You mentioned we didn\u0027t need to call deactivate for thin volumes, so why aren\u0027t we excluding them?\n\n if (self.lv_has_snapshot(lv_name) and\n     not self.vg_thin_pool):","commit_id":"52e643b7dbaa612ad9b71b8a959674a1c76ff81b"},{"author":{"_account_id":12608,"name":"Curt Bruns","email":"curt.e.bruns@intel.com","username":"cebruns"},"change_message_id":"9aaee737ff5cdca023ec761d0fdeb34d4c7068ba","unresolved":false,"context_lines":[{"line_number":711,"context_line":""},{"line_number":712,"context_line":"    def extend_volume(self, lv_name, new_size):"},{"line_number":713,"context_line":"        \"\"\"Extend the size of an existing volume.\"\"\""},{"line_number":714,"context_line":"        if self.lv_has_snapshot(lv_name):"},{"line_number":715,"context_line":"            self.deactivate_lv(lv_name)"},{"line_number":716,"context_line":"        try:"},{"line_number":717,"context_line":"            self._execute(\u0027lvextend\u0027, \u0027-L\u0027, new_size,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_eb7936fd","line":714,"in_reply_to":"3a50d1a3_966b001e","updated":"2015-07-28 14:49:48.000000000","message":"The \u0027lv_has_snapshot\u0027 method checks for \u0027o\u0027 or \u0027O\u0027 in the Attributes flags to see if it has a snap.  Thin volumes with Snaps will have \u0027V\u0027 (Virtual) in their attribute flags so they will not be deactivated.  So it does work, but I agree that it\u0027s not totally obvious.  I can add a comment or use your suggestion to be more explicit.","commit_id":"52e643b7dbaa612ad9b71b8a959674a1c76ff81b"},{"author":{"_account_id":12608,"name":"Curt Bruns","email":"curt.e.bruns@intel.com","username":"cebruns"},"change_message_id":"65a9589dc6b1b54c24ff3ad74b801b72e182847a","unresolved":false,"context_lines":[{"line_number":711,"context_line":""},{"line_number":712,"context_line":"    def extend_volume(self, lv_name, new_size):"},{"line_number":713,"context_line":"        \"\"\"Extend the size of an existing volume.\"\"\""},{"line_number":714,"context_line":"        if self.lv_has_snapshot(lv_name):"},{"line_number":715,"context_line":"            self.deactivate_lv(lv_name)"},{"line_number":716,"context_line":"        try:"},{"line_number":717,"context_line":"            self._execute(\u0027lvextend\u0027, \u0027-L\u0027, new_size,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_6c03f210","line":714,"in_reply_to":"3a50d1a3_b1f95482","updated":"2015-07-28 18:51:24.000000000","message":"Done in next patchset - although the comment is rather wordy.  :)","commit_id":"52e643b7dbaa612ad9b71b8a959674a1c76ff81b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b8acb3d330eaeb2acf8fa1fca7f5ad33ef2d36dc","unresolved":false,"context_lines":[{"line_number":711,"context_line":""},{"line_number":712,"context_line":"    def extend_volume(self, lv_name, new_size):"},{"line_number":713,"context_line":"        \"\"\"Extend the size of an existing volume.\"\"\""},{"line_number":714,"context_line":"        if self.lv_has_snapshot(lv_name):"},{"line_number":715,"context_line":"            self.deactivate_lv(lv_name)"},{"line_number":716,"context_line":"        try:"},{"line_number":717,"context_line":"            self._execute(\u0027lvextend\u0027, \u0027-L\u0027, new_size,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_b1f95482","line":714,"in_reply_to":"3a50d1a3_eb7936fd","updated":"2015-07-28 17:06:59.000000000","message":"Good to know!!\n\nI think a comment would be enough to help the untrained eye.  ;-)","commit_id":"52e643b7dbaa612ad9b71b8a959674a1c76ff81b"}],"cinder/tests/unit/brick/test_brick_lvm.py":[{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"0e2c78bfc53b9aa0afc241996c2e64bab5dd8d7d","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        self.vg.deactivate_lv.reset_mock()"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        # Extend lv without snapshot so deactivate should not be called"},{"line_number":370,"context_line":"        self.assertEqual(None, self.vg.create_volume(\"test\", \"1G\"))"},{"line_number":371,"context_line":"        self.vg.vg_name \u003d \"test-volumes\""},{"line_number":372,"context_line":"        self.vg.extend_volume(\"test\", \"2G\")"},{"line_number":373,"context_line":"        self.assertFalse(self.vg.deactivate_lv.called)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a50d1a3_fe3160ed","line":370,"updated":"2015-07-20 21:07:15.000000000","message":"assertIsNone is the preferred method.","commit_id":"459e78b45e8f0114e3182480a82042a448be09b7"},{"author":{"_account_id":12608,"name":"Curt Bruns","email":"curt.e.bruns@intel.com","username":"cebruns"},"change_message_id":"a3e85ac745e245d3f8fb170d150ef9c09be9ae16","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        self.vg.deactivate_lv.reset_mock()"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"        # Extend lv without snapshot so deactivate should not be called"},{"line_number":370,"context_line":"        self.assertEqual(None, self.vg.create_volume(\"test\", \"1G\"))"},{"line_number":371,"context_line":"        self.vg.vg_name \u003d \"test-volumes\""},{"line_number":372,"context_line":"        self.vg.extend_volume(\"test\", \"2G\")"},{"line_number":373,"context_line":"        self.assertFalse(self.vg.deactivate_lv.called)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a50d1a3_b130b102","line":370,"in_reply_to":"3a50d1a3_fe3160ed","updated":"2015-07-21 18:49:50.000000000","message":"Thanks Xing.  Doing the assertEqual or AssertIsNone isn\u0027t really catching anything additional, so I\u0027ll just remove it to line up with other tests.","commit_id":"459e78b45e8f0114e3182480a82042a448be09b7"}]}