)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"067d3e4f2ed3dcf5e22aa8ae4c3939e64eb92bb5","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Gary Kotton \u003cgkotton@vmware.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-01 07:48:56 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"VMware: detach cinder volume when instance detroyed"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Ensure that the cinder volume is detached when the instance is destroyed."},{"line_number":10,"context_line":"When an instance has a attached cinder volume we need to ensure that"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa041b7_7b21dc9e","line":7,"updated":"2015-04-04 01:20:24.000000000","message":"typo : detroyed -\u003e destroyed","commit_id":"3de34b87d5850cec874c43b53f6cbd7851bf9971"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"36d3999b12adc8763e12e6fe4e8e864f71cc8783","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Gary Kotton \u003cgkotton@vmware.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-01 07:48:56 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"VMware: detach cinder volume when instance detroyed"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Ensure that the cinder volume is detached when the instance is destroyed."},{"line_number":10,"context_line":"When an instance has a attached cinder volume we need to ensure that"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa041b7_7ed2ead3","line":7,"in_reply_to":"baa041b7_7b21dc9e","updated":"2015-04-04 04:23:27.000000000","message":"Done","commit_id":"3de34b87d5850cec874c43b53f6cbd7851bf9971"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"c10e86d3698925c927e933a542f9c7e27cfe18ee","unresolved":false,"context_lines":[{"line_number":7,"context_line":"VMware: detach cinder volume when instance destroyed"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Ensure that the cinder volume is detached when the instance is destroyed."},{"line_number":10,"context_line":"When an instance has a attached cinder volume we need to ensure that"},{"line_number":11,"context_line":"the volume is detached prior to deleting the VM on the backend."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-bug: #1439168"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9aa53dc9_cd323e5f","line":10,"updated":"2015-04-09 14:56:42.000000000","message":"a \u003d\u003e an","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7c6b14dfb7c34a5eb89c8064a7a32951bd0aa12a","unresolved":false,"context_lines":[{"line_number":7,"context_line":"VMware: detach cinder volume when instance destroyed"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Ensure that the cinder volume is detached when the instance is destroyed."},{"line_number":10,"context_line":"When an instance has a attached cinder volume we need to ensure that"},{"line_number":11,"context_line":"the volume is detached prior to deleting the VM on the backend."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-bug: #1439168"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a0c5dc1_a00df93f","line":10,"in_reply_to":"9aa53dc9_cd323e5f","updated":"2015-05-13 19:01:48.000000000","message":"Done","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"}],"nova/tests/unit/virt/vmwareapi/test_driver_api.py":[{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"c10e86d3698925c927e933a542f9c7e27cfe18ee","unresolved":false,"context_lines":[{"line_number":701,"context_line":"                \u0027disk_bus\u0027: \u0027fake-bus\u0027,"},{"line_number":702,"context_line":"                \u0027device_name\u0027: \u0027fake-name\u0027,"},{"line_number":703,"context_line":"                \u0027mount_device\u0027: \u0027/dev/sdb\u0027}]"},{"line_number":704,"context_line":"        bdi \u003d {\u0027block_device_mapping\u0027: bdm, \u0027root_device_name\u0027: \u0027/dev/sda\u0027}"},{"line_number":705,"context_line":"        self.conn.destroy(self.context, self.instance, self.network_info,"},{"line_number":706,"context_line":"                          block_device_info\u003dbdi)"},{"line_number":707,"context_line":"        mock_detach_volume.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa53dc9_adfc9a72","line":704,"updated":"2015-04-09 14:56:42.000000000","message":"I would suggest using: \n\n    from nova.tests.unit import fake_block_device\n\nso, it would be like this:\n\n    bdm \u003d fake_block_device.AnonFakeDbBlockDeviceDict({\u0027source_type\u0027: \u0027volume\u0027})\n    bdm[\u0027connection_info\u0027] \u003d connection_info\n    bdi \u003d {\u0027block_device_mapping\u0027: bdm, \u0027root_device_name\u0027: \u0027/dev/sda\u0027}","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7c6b14dfb7c34a5eb89c8064a7a32951bd0aa12a","unresolved":false,"context_lines":[{"line_number":701,"context_line":"                \u0027disk_bus\u0027: \u0027fake-bus\u0027,"},{"line_number":702,"context_line":"                \u0027device_name\u0027: \u0027fake-name\u0027,"},{"line_number":703,"context_line":"                \u0027mount_device\u0027: \u0027/dev/sdb\u0027}]"},{"line_number":704,"context_line":"        bdi \u003d {\u0027block_device_mapping\u0027: bdm, \u0027root_device_name\u0027: \u0027/dev/sda\u0027}"},{"line_number":705,"context_line":"        self.conn.destroy(self.context, self.instance, self.network_info,"},{"line_number":706,"context_line":"                          block_device_info\u003dbdi)"},{"line_number":707,"context_line":"        mock_detach_volume.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_2300f300","line":704,"in_reply_to":"9aa53dc9_adfc9a72","updated":"2015-05-13 19:01:48.000000000","message":"i tried and this did not work","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"c10e86d3698925c927e933a542f9c7e27cfe18ee","unresolved":false,"context_lines":[{"line_number":720,"context_line":"                \u0027disk_bus\u0027: \u0027fake-bus\u0027,"},{"line_number":721,"context_line":"                \u0027device_name\u0027: \u0027fake-name\u0027,"},{"line_number":722,"context_line":"                \u0027mount_device\u0027: \u0027/dev/sdb\u0027}]"},{"line_number":723,"context_line":"        bdi \u003d {\u0027block_device_mapping\u0027: bdm, \u0027root_device_name\u0027: \u0027/dev/sda\u0027}"},{"line_number":724,"context_line":"        self.conn.destroy(self.context, self.instance, self.network_info,"},{"line_number":725,"context_line":"                          block_device_info\u003dbdi)"},{"line_number":726,"context_line":"        mock_detach_volume.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa53dc9_2df1aa95","line":723,"updated":"2015-04-09 14:56:42.000000000","message":"ditto","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"}],"nova/virt/vmwareapi/driver.py":[{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"067d3e4f2ed3dcf5e22aa8ae4c3939e64eb92bb5","unresolved":false,"context_lines":[{"line_number":524,"context_line":"        if block_device_info is not None:"},{"line_number":525,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":526,"context_line":"                block_device_info)"},{"line_number":527,"context_line":"            if len(block_device_mapping) \u003e 0:"},{"line_number":528,"context_line":"                for disk in block_device_mapping:"},{"line_number":529,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":530,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa041b7_5b5bb82e","line":527,"range":{"start_line":527,"start_character":19,"end_line":527,"end_character":39},"updated":"2015-04-04 01:20:24.000000000","message":"Do we need to check length? looks like block_device_info_get_mapping is guaranteeing block_device_info_get_mapping will not be None, worst case it will be []. right?","commit_id":"3de34b87d5850cec874c43b53f6cbd7851bf9971"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"36d3999b12adc8763e12e6fe4e8e864f71cc8783","unresolved":false,"context_lines":[{"line_number":524,"context_line":"        if block_device_info is not None:"},{"line_number":525,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":526,"context_line":"                block_device_info)"},{"line_number":527,"context_line":"            if len(block_device_mapping) \u003e 0:"},{"line_number":528,"context_line":"                for disk in block_device_mapping:"},{"line_number":529,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":530,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa041b7_5ed5e6b6","line":527,"in_reply_to":"baa041b7_1b5bf055","updated":"2015-04-04 04:23:27.000000000","message":"Done","commit_id":"3de34b87d5850cec874c43b53f6cbd7851bf9971"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"df70d189485fa698525f8decea3efdccf7512da6","unresolved":false,"context_lines":[{"line_number":524,"context_line":"        if block_device_info is not None:"},{"line_number":525,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":526,"context_line":"                block_device_info)"},{"line_number":527,"context_line":"            if len(block_device_mapping) \u003e 0:"},{"line_number":528,"context_line":"                for disk in block_device_mapping:"},{"line_number":529,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":530,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa041b7_1b5bf055","line":527,"in_reply_to":"baa041b7_5b5bb82e","updated":"2015-04-04 02:31:32.000000000","message":"That is true - see https://github.com/openstack/nova/blob/master/nova/virt/driver.py#L101.  Guess we do not need the extra if.","commit_id":"3de34b87d5850cec874c43b53f6cbd7851bf9971"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"067d3e4f2ed3dcf5e22aa8ae4c3939e64eb92bb5","unresolved":false,"context_lines":[{"line_number":531,"context_line":"                        self.detach_volume(connection_info, instance,"},{"line_number":532,"context_line":"                                           disk.get(\u0027device_name\u0027))"},{"line_number":533,"context_line":"                    except Exception:"},{"line_number":534,"context_line":"                        LOG.debug(\"Unable to detach volume %s\","},{"line_number":535,"context_line":"                                  connection_info,"},{"line_number":536,"context_line":"                                  instance\u003dinstance)"},{"line_number":537,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa041b7_db30e8ea","line":534,"updated":"2015-04-04 01:20:24.000000000","message":"Do we want to log the device_name too?","commit_id":"3de34b87d5850cec874c43b53f6cbd7851bf9971"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"df70d189485fa698525f8decea3efdccf7512da6","unresolved":false,"context_lines":[{"line_number":531,"context_line":"                        self.detach_volume(connection_info, instance,"},{"line_number":532,"context_line":"                                           disk.get(\u0027device_name\u0027))"},{"line_number":533,"context_line":"                    except Exception:"},{"line_number":534,"context_line":"                        LOG.debug(\"Unable to detach volume %s\","},{"line_number":535,"context_line":"                                  connection_info,"},{"line_number":536,"context_line":"                                  instance\u003dinstance)"},{"line_number":537,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa041b7_fb828c93","line":534,"in_reply_to":"baa041b7_db30e8ea","updated":"2015-04-04 02:31:32.000000000","message":"Actually, the device name does not matter because it is not used, see L493.","commit_id":"3de34b87d5850cec874c43b53f6cbd7851bf9971"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"c10e86d3698925c927e933a542f9c7e27cfe18ee","unresolved":false,"context_lines":[{"line_number":524,"context_line":"        if block_device_info is not None:"},{"line_number":525,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":526,"context_line":"                block_device_info)"},{"line_number":527,"context_line":"            if block_device_mapping:"},{"line_number":528,"context_line":"                for disk in block_device_mapping:"},{"line_number":529,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":530,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa53dc9_0dff06ab","line":527,"updated":"2015-04-09 14:56:42.000000000","message":"driver.block_device_info_get_mapping doesn\u0027t return None, it returns [ ]. Which means that this if is not really necessary. You can just for directly.","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7c6b14dfb7c34a5eb89c8064a7a32951bd0aa12a","unresolved":false,"context_lines":[{"line_number":524,"context_line":"        if block_device_info is not None:"},{"line_number":525,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":526,"context_line":"                block_device_info)"},{"line_number":527,"context_line":"            if block_device_mapping:"},{"line_number":528,"context_line":"                for disk in block_device_mapping:"},{"line_number":529,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":530,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_a33bc3d0","line":527,"in_reply_to":"9aa53dc9_0dff06ab","updated":"2015-05-13 19:01:48.000000000","message":"Done","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"ad133bca8f66e5ff1becdd86cbcf6c3c04418bff","unresolved":false,"context_lines":[{"line_number":531,"context_line":"                        self.detach_volume(connection_info, instance,"},{"line_number":532,"context_line":"                                           disk.get(\u0027device_name\u0027))"},{"line_number":533,"context_line":"                    except Exception:"},{"line_number":534,"context_line":"                        LOG.debug(\"Unable to detach volume %s\","},{"line_number":535,"context_line":"                                  connection_info,"},{"line_number":536,"context_line":"                                  instance\u003dinstance)"},{"line_number":537,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1aca2d91_d5d00fcc","line":534,"updated":"2015-05-06 08:58:22.000000000","message":"Do we need to proceed deleting the VM if detach fails? It may delete the volume\u0027s vmdk.","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"},{"author":{"_account_id":9172,"name":"Radoslav Gerganov","email":"rgerganov@vmware.com","username":"rgerganov"},"change_message_id":"e2daf8865c6fd3f8a7c405b07aeff90c28c36408","unresolved":false,"context_lines":[{"line_number":531,"context_line":"                        self.detach_volume(connection_info, instance,"},{"line_number":532,"context_line":"                                           disk.get(\u0027device_name\u0027))"},{"line_number":533,"context_line":"                    except Exception:"},{"line_number":534,"context_line":"                        LOG.debug(\"Unable to detach volume %s\","},{"line_number":535,"context_line":"                                  connection_info,"},{"line_number":536,"context_line":"                                  instance\u003dinstance)"},{"line_number":537,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_778365cd","line":534,"in_reply_to":"1aca2d91_d5d00fcc","updated":"2015-05-13 07:22:02.000000000","message":"I don\u0027t think we have to catch this exception. This only hides the problem and the user doesn\u0027t know something has gone wrong. If we cannot handle the exception (like in this case), we should fail fast.","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7c6b14dfb7c34a5eb89c8064a7a32951bd0aa12a","unresolved":false,"context_lines":[{"line_number":531,"context_line":"                        self.detach_volume(connection_info, instance,"},{"line_number":532,"context_line":"                                           disk.get(\u0027device_name\u0027))"},{"line_number":533,"context_line":"                    except Exception:"},{"line_number":534,"context_line":"                        LOG.debug(\"Unable to detach volume %s\","},{"line_number":535,"context_line":"                                  connection_info,"},{"line_number":536,"context_line":"                                  instance\u003dinstance)"},{"line_number":537,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_6335cbd8","line":534,"in_reply_to":"9a0c5dc1_778365cd","updated":"2015-05-13 19:01:48.000000000","message":"Done","commit_id":"a2a0ebb2dd5bbe1b19e1893cbf72d0154cabbb65"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"7b0c8d4360c65ae73cf2cf9f5b2ba0018f32e255","unresolved":false,"context_lines":[{"line_number":557,"context_line":""},{"line_number":558,"context_line":"        # A resize uses the same instance on the VC. We do not delete that"},{"line_number":559,"context_line":"        # VM in the event of a revert"},{"line_number":560,"context_line":"        if instance.task_state \u003d\u003d task_states.RESIZE_REVERTING:"},{"line_number":561,"context_line":"            return"},{"line_number":562,"context_line":""},{"line_number":563,"context_line":"        # We need to detach attached volumes"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a016987_896bf91d","line":560,"updated":"2015-05-29 13:42:17.000000000","message":"could you also add a test to cover this branch? This should also have been done for vmops.destroy as well.","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"9abf21af2e9e737c5ad01f198ae452d602b9b6f0","unresolved":false,"context_lines":[{"line_number":557,"context_line":""},{"line_number":558,"context_line":"        # A resize uses the same instance on the VC. We do not delete that"},{"line_number":559,"context_line":"        # VM in the event of a revert"},{"line_number":560,"context_line":"        if instance.task_state \u003d\u003d task_states.RESIZE_REVERTING:"},{"line_number":561,"context_line":"            return"},{"line_number":562,"context_line":""},{"line_number":563,"context_line":"        # We need to detach attached volumes"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_98c74512","line":560,"in_reply_to":"7a016987_896bf91d","updated":"2015-06-02 12:50:23.000000000","message":"Done - this already exists - please see - https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/vmwareapi/test_driver_api.py#L1486","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d4ca17935c88f3c22ebf38e40c71766e04066dc","unresolved":false,"context_lines":[{"line_number":560,"context_line":"        if instance.task_state \u003d\u003d task_states.RESIZE_REVERTING:"},{"line_number":561,"context_line":"            return"},{"line_number":562,"context_line":""},{"line_number":563,"context_line":"        # We need to detach attached volumes"},{"line_number":564,"context_line":"        if block_device_info is not None:"},{"line_number":565,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":566,"context_line":"                block_device_info)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_831d9675","line":563,"updated":"2015-06-02 13:18:37.000000000","message":"Why doesn\u0027t this actually happen in vmops?  I thought the driver was just a wrapper over vmops that did the heavy lifting?","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"36a0eab640ed709579a53defb16012181097d711","unresolved":false,"context_lines":[{"line_number":560,"context_line":"        if instance.task_state \u003d\u003d task_states.RESIZE_REVERTING:"},{"line_number":561,"context_line":"            return"},{"line_number":562,"context_line":""},{"line_number":563,"context_line":"        # We need to detach attached volumes"},{"line_number":564,"context_line":"        if block_device_info is not None:"},{"line_number":565,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":566,"context_line":"                block_device_info)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_c874c6d0","line":563,"in_reply_to":"5afe65bd_831d9675","updated":"2015-06-02 14:25:40.000000000","message":"We need to call the driver methods here for the detachment. I feel that this is a far better place to do this. It seems more logical here. Having vmops call driver code is also a bit odd.\n\nIt would actually be better that this was all done in a layer above but that breaks libvirt. see https://review.openstack.org/#/c/183341/","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"7b0c8d4360c65ae73cf2cf9f5b2ba0018f32e255","unresolved":false,"context_lines":[{"line_number":565,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":566,"context_line":"                block_device_info)"},{"line_number":567,"context_line":"            if block_device_mapping:"},{"line_number":568,"context_line":"                self._vmops.power_off(instance)"},{"line_number":569,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":570,"context_line":"                for disk in block_device_mapping:"},{"line_number":571,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a016987_691d5dbd","line":568,"updated":"2015-05-29 13:42:17.000000000","message":"question: doesn\u0027t the instance have to be powered off anyways? I mean, you can\u0027t just destroy a running vm, right? Or this is needed only when you are trying to detach volumes?\n\nIf this comments has any value and the instance must be powered off first, then this should thingy must be commited under another bug and then backported.","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"36a0eab640ed709579a53defb16012181097d711","unresolved":false,"context_lines":[{"line_number":565,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":566,"context_line":"                block_device_info)"},{"line_number":567,"context_line":"            if block_device_mapping:"},{"line_number":568,"context_line":"                self._vmops.power_off(instance)"},{"line_number":569,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":570,"context_line":"                for disk in block_device_mapping:"},{"line_number":571,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_ab1d0468","line":568,"in_reply_to":"5afe65bd_23b14290","updated":"2015-06-02 14:25:40.000000000","message":"Please see https://github.com/openstack/nova/blob/master/nova/virt/vmwareapi/volumeops.py#L504\nI will add in a comment here and hopefully this will be easier for reviewers to understand","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d4ca17935c88f3c22ebf38e40c71766e04066dc","unresolved":false,"context_lines":[{"line_number":565,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":566,"context_line":"                block_device_info)"},{"line_number":567,"context_line":"            if block_device_mapping:"},{"line_number":568,"context_line":"                self._vmops.power_off(instance)"},{"line_number":569,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":570,"context_line":"                for disk in block_device_mapping:"},{"line_number":571,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_23b14290","line":568,"in_reply_to":"5afe65bd_58e8ad7c","updated":"2015-06-02 13:18:37.000000000","message":"Where does that happen?  Is vCenter raising the error when trying to detach a volume on an instance that isn\u0027t powered off?  Or is the driver checking that in volumeops somewhere?  Seems odd that the instance has to be powered off to detach a volume.","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"9abf21af2e9e737c5ad01f198ae452d602b9b6f0","unresolved":false,"context_lines":[{"line_number":565,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":566,"context_line":"                block_device_info)"},{"line_number":567,"context_line":"            if block_device_mapping:"},{"line_number":568,"context_line":"                self._vmops.power_off(instance)"},{"line_number":569,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":570,"context_line":"                for disk in block_device_mapping:"},{"line_number":571,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_58e8ad7c","line":568,"in_reply_to":"7a016987_691d5dbd","updated":"2015-06-02 12:50:23.000000000","message":"the vm needs to be powered off to detach the vm. due to the fact that we are doing it here we need to power off the vm. that is done later in destroy so it is ok.\nin the usual detachment if the user wants to detach and the instance is not off then they will get an exception requiring them to do that","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d4ca17935c88f3c22ebf38e40c71766e04066dc","unresolved":false,"context_lines":[{"line_number":566,"context_line":"                block_device_info)"},{"line_number":567,"context_line":"            if block_device_mapping:"},{"line_number":568,"context_line":"                self._vmops.power_off(instance)"},{"line_number":569,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":570,"context_line":"                for disk in block_device_mapping:"},{"line_number":571,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":572,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_c3038e0e","line":569,"updated":"2015-06-02 13:18:37.000000000","message":"Why is the instance.vm_state changed here?  I don\u0027t see anything in the flow after this that checks this value.","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"efd00dc07003232ee5a778ea6502e7043247c9e8","unresolved":false,"context_lines":[{"line_number":566,"context_line":"                block_device_info)"},{"line_number":567,"context_line":"            if block_device_mapping:"},{"line_number":568,"context_line":"                self._vmops.power_off(instance)"},{"line_number":569,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":570,"context_line":"                for disk in block_device_mapping:"},{"line_number":571,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":572,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_5c8736d7","line":569,"in_reply_to":"5afe65bd_6b26ecbb","updated":"2015-06-08 20:33:28.000000000","message":"Weird, seems volumeops should get the current power state of the VM from the hypervisor and check to see that it\u0027s powered off rather than rely on the nova database.  This is a gross tight-coupling between modules here especially when it\u0027s only for certain types of setups:\n\nvmdk.adapter_type \u003d\u003d constants.ADAPTER_TYPE_IDE","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"36a0eab640ed709579a53defb16012181097d711","unresolved":false,"context_lines":[{"line_number":566,"context_line":"                block_device_info)"},{"line_number":567,"context_line":"            if block_device_mapping:"},{"line_number":568,"context_line":"                self._vmops.power_off(instance)"},{"line_number":569,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":570,"context_line":"                for disk in block_device_mapping:"},{"line_number":571,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":572,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_6b26ecbb","line":569,"in_reply_to":"5afe65bd_c3038e0e","updated":"2015-06-02 14:25:40.000000000","message":"See above","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d4ca17935c88f3c22ebf38e40c71766e04066dc","unresolved":false,"context_lines":[{"line_number":574,"context_line":"                                           disk.get(\u0027device_name\u0027))"},{"line_number":575,"context_line":"                    except Exception:"},{"line_number":576,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":577,"context_line":"                            LOG.error(_LE(\"Unable to detach volume %s\"),"},{"line_number":578,"context_line":"                                      connection_info, instance\u003dinstance)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        self._vmops.destroy(instance, destroy_disks)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_03f6468f","line":577,"updated":"2015-06-02 13:18:37.000000000","message":"Throw the actual exception message in here too.","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"36a0eab640ed709579a53defb16012181097d711","unresolved":false,"context_lines":[{"line_number":574,"context_line":"                                           disk.get(\u0027device_name\u0027))"},{"line_number":575,"context_line":"                    except Exception:"},{"line_number":576,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":577,"context_line":"                            LOG.error(_LE(\"Unable to detach volume %s\"),"},{"line_number":578,"context_line":"                                      connection_info, instance\u003dinstance)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        self._vmops.destroy(instance, destroy_disks)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_2b20f49a","line":577,"in_reply_to":"5afe65bd_03f6468f","updated":"2015-06-02 14:25:40.000000000","message":"Done","commit_id":"344686ded751d5cfa751a94e05f54e791ef5e38b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ef1eab493cc62f05812f3450f6af50b816043f53","unresolved":false,"context_lines":[{"line_number":571,"context_line":"                # plugging. Hence we need to power off the instance and update"},{"line_number":572,"context_line":"                # the instance state."},{"line_number":573,"context_line":"                self._vmops.power_off(instance)"},{"line_number":574,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":575,"context_line":"                for disk in block_device_mapping:"},{"line_number":576,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":577,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_9c992e22","line":574,"updated":"2015-06-08 20:42:21.000000000","message":"Unless it\u0027s totally bonkers, I\u0027d like to see a TODO with this so that the nova vm_state doesn\u0027t need to be checked in https://github.com/openstack/nova/blob/master/nova/virt/vmwareapi/volumeops.py#L504 to tell if it\u0027s OK to detach the volume.  I think it\u0027d be far better for volumeops to get the current power_state of the VM (from vcenter) to see that it\u0027s powered off, rather than it relying on the caller to know it\u0027s doing this check and introduces this tight coupling.\n\nAlso, there is no check in the tests that _vmops.power_off was called on the instance, or that it\u0027s vm_state is changed to STOPPED.","commit_id":"635f6ecd2c793964fa66f0e36e861189b6552a54"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1769ffc6a565de48d38dae09503eaed16a885ff","unresolved":false,"context_lines":[{"line_number":571,"context_line":"                # plugging. Hence we need to power off the instance and update"},{"line_number":572,"context_line":"                # the instance state."},{"line_number":573,"context_line":"                self._vmops.power_off(instance)"},{"line_number":574,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":575,"context_line":"                for disk in block_device_mapping:"},{"line_number":576,"context_line":"                    connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":577,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_0e02966f","line":574,"in_reply_to":"3afb71cf_9c992e22","updated":"2015-06-09 08:23:29.000000000","message":"I will add the TODO. I will post a patch above this one that deals with that too.\n\nThe power off - https://github.com/openstack/nova/blob/master/nova/virt/vmwareapi/vmops.py#L1105 and https://github.com/openstack/nova/blob/master/nova/virt/vmwareapi/vm_util.py#L1469 ensures that the instance is actually powered off. An exception will be raised if this fails.\n\nThe tests have been updated.","commit_id":"635f6ecd2c793964fa66f0e36e861189b6552a54"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ef1eab493cc62f05812f3450f6af50b816043f53","unresolved":false,"context_lines":[{"line_number":581,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":582,"context_line":"                            LOG.error(_LE(\"Unable to detach volume %(info)s. \""},{"line_number":583,"context_line":"                                          \"Exception: %(exc)s\"),"},{"line_number":584,"context_line":"                                      {\u0027info\u0027: connection_info, \u0027exc\u0027: e},"},{"line_number":585,"context_line":"                                      instance\u003dinstance)"},{"line_number":586,"context_line":""},{"line_number":587,"context_line":"        self._vmops.destroy(instance, destroy_disks)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_bcc86a23","line":584,"updated":"2015-06-08 20:42:21.000000000","message":"I believe connection_info can store credentials so you probably shouldn\u0027t log the entire thing here, maybe just use disk.get(\u0027device_name\u0027) or disk.volume_id + disk[\u0027mount_device\u0027] to identify the device for context, like in the compute manager:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4449","commit_id":"635f6ecd2c793964fa66f0e36e861189b6552a54"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1769ffc6a565de48d38dae09503eaed16a885ff","unresolved":false,"context_lines":[{"line_number":581,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":582,"context_line":"                            LOG.error(_LE(\"Unable to detach volume %(info)s. \""},{"line_number":583,"context_line":"                                          \"Exception: %(exc)s\"),"},{"line_number":584,"context_line":"                                      {\u0027info\u0027: connection_info, \u0027exc\u0027: e},"},{"line_number":585,"context_line":"                                      instance\u003dinstance)"},{"line_number":586,"context_line":""},{"line_number":587,"context_line":"        self._vmops.destroy(instance, destroy_disks)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_5d54bffd","line":584,"in_reply_to":"3afb71cf_bcc86a23","updated":"2015-06-09 08:23:29.000000000","message":"Done","commit_id":"635f6ecd2c793964fa66f0e36e861189b6552a54"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ece86a51919304fba8b8d0b53c34163f4d076ddb","unresolved":false,"context_lines":[{"line_number":581,"context_line":"        if not instance.node:"},{"line_number":582,"context_line":"            return"},{"line_number":583,"context_line":""},{"line_number":584,"context_line":"        # A resize uses the same instance on the VC. We do not delete that"},{"line_number":585,"context_line":"        # VM in the event of a revert"},{"line_number":586,"context_line":"        if instance.task_state \u003d\u003d task_states.RESIZE_REVERTING:"},{"line_number":587,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_17060a1d","line":584,"updated":"2015-06-17 14:02:38.000000000","message":"This is moved here so that we don\u0027t detach the volume just to find out later in vmops.destroy() that we aren\u0027t going to destroy the instance.","commit_id":"901d61980dc8136b9393d80659b638e3eb6cddb4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ece86a51919304fba8b8d0b53c34163f4d076ddb","unresolved":false,"context_lines":[{"line_number":595,"context_line":"                # plugging. Hence we need to power off the instance and update"},{"line_number":596,"context_line":"                # the instance state."},{"line_number":597,"context_line":"                self._vmops.power_off(instance)"},{"line_number":598,"context_line":"                # TODO(garyk): update the volumeops to read the state form the"},{"line_number":599,"context_line":"                # VM instead of relying on a instance flag"},{"line_number":600,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":601,"context_line":"                for disk in block_device_mapping:"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_747994c7","line":598,"updated":"2015-06-17 14:02:38.000000000","message":"s/form/from/ in case you need to respin.","commit_id":"901d61980dc8136b9393d80659b638e3eb6cddb4"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"9c5c6f04e33ea5c66132f8ee21ac22b08fc3eac1","unresolved":false,"context_lines":[{"line_number":595,"context_line":"                # plugging. Hence we need to power off the instance and update"},{"line_number":596,"context_line":"                # the instance state."},{"line_number":597,"context_line":"                self._vmops.power_off(instance)"},{"line_number":598,"context_line":"                # TODO(garyk): update the volumeops to read the state form the"},{"line_number":599,"context_line":"                # VM instead of relying on a instance flag"},{"line_number":600,"context_line":"                instance.vm_state \u003d vm_states.STOPPED"},{"line_number":601,"context_line":"                for disk in block_device_mapping:"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_d9f19796","line":598,"in_reply_to":"fa32b979_747994c7","updated":"2015-06-18 04:53:54.000000000","message":"Thanks - followign you suggestion I posted https://review.openstack.org/190627 so this goes away.","commit_id":"901d61980dc8136b9393d80659b638e3eb6cddb4"}]}