)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"04cfe1858aba70ea24717ff403830f7ee521329a","unresolved":false,"context_lines":[{"line_number":1549,"context_line":"        \"\"\"Handle cases where the virt-layer had to detach non-working volumes"},{"line_number":1550,"context_line":"        in order to complete an operation."},{"line_number":1551,"context_line":"        \"\"\""},{"line_number":1552,"context_line":"        for bdm in block_device_info[\u0027block_device_mapping\u0027]:"},{"line_number":1553,"context_line":"            if bdm.get(\u0027mount_device\u0027) in bad_devices:"},{"line_number":1554,"context_line":"                try:"},{"line_number":1555,"context_line":"                    volume_id \u003d bdm[\u0027connection_info\u0027][\u0027data\u0027][\u0027volume_id\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAN3%2F%2Fz4k%3D","line":1552,"updated":"2013-03-11 19:14:41.000000000","message":"could block_device_info[\u0027block_device_mapping\u0027] ever be None?","commit_id":"40feb35898ed0a6d57b1f481c165e683796b045c"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"6e19d3a04939bc784325b34f2bcf552b1fae7bc8","unresolved":false,"context_lines":[{"line_number":1549,"context_line":"        \"\"\"Handle cases where the virt-layer had to detach non-working volumes"},{"line_number":1550,"context_line":"        in order to complete an operation."},{"line_number":1551,"context_line":"        \"\"\""},{"line_number":1552,"context_line":"        for bdm in block_device_info[\u0027block_device_mapping\u0027]:"},{"line_number":1553,"context_line":"            if bdm.get(\u0027mount_device\u0027) in bad_devices:"},{"line_number":1554,"context_line":"                try:"},{"line_number":1555,"context_line":"                    volume_id \u003d bdm[\u0027connection_info\u0027][\u0027data\u0027][\u0027volume_id\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAN3%2F%2FzrY%3D","line":1552,"in_reply_to":"AAAAN3%2F%2Fz4k%3D","updated":"2013-03-11 21:38:02.000000000","message":"I don\u0027t think it can since the function that ultimately creates it (_get_volume_block_device_info) only returns a list.","commit_id":"40feb35898ed0a6d57b1f481c165e683796b045c"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"023be821cad24f415d1e8b4bcc441013a633fc57","unresolved":false,"context_lines":[{"line_number":948,"context_line":"                    instance\u003dinstance)"},{"line_number":949,"context_line":"                self._session.call_xenapi(\u0027VM.start\u0027, vm_ref, False, False)"},{"line_number":950,"context_line":"                return"},{"line_number":951,"context_line":"            elif details[0] \u003d\u003d \u0027SR_BACKEND_FAILURE_46\u0027:"},{"line_number":952,"context_line":"                LOG.warn(_(\"Reboot failed due to bad volumes, detaching bad\""},{"line_number":953,"context_line":"                           \" volumes and starting halted instance\"),"},{"line_number":954,"context_line":"                         instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAN3%2F%2F6hQ%3D","line":951,"updated":"2013-03-07 14:44:36.000000000","message":"Recommend this isn\u0027t an elif - if the VM was already stopped and we try and start it and it doesn\u0027t have the right volumes, then we\u0027d get an exception from the VM.start @949","commit_id":"10a03b7e85488fb25d0d02a01943087530ec6aa9"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"6bcda683c55e633180ca7d97f81ea3901cb67b46","unresolved":false,"context_lines":[{"line_number":948,"context_line":"                    instance\u003dinstance)"},{"line_number":949,"context_line":"                self._session.call_xenapi(\u0027VM.start\u0027, vm_ref, False, False)"},{"line_number":950,"context_line":"                return"},{"line_number":951,"context_line":"            elif details[0] \u003d\u003d \u0027SR_BACKEND_FAILURE_46\u0027:"},{"line_number":952,"context_line":"                LOG.warn(_(\"Reboot failed due to bad volumes, detaching bad\""},{"line_number":953,"context_line":"                           \" volumes and starting halted instance\"),"},{"line_number":954,"context_line":"                         instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAN3%2F%2F4qM%3D","line":951,"in_reply_to":"AAAAN3%2F%2F6hQ%3D","updated":"2013-03-07 17:09:36.000000000","message":"Good point.","commit_id":"10a03b7e85488fb25d0d02a01943087530ec6aa9"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9bb0dd085571273555e125c60720fe0fcde00801","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        # Attached volumes that have become non-responsive will prevent a VM"},{"line_number":281,"context_line":"        # from starting, so scan for these before attempting to start"},{"line_number":282,"context_line":"        bad_devices \u003d self._volumeops.find_bad_volumes(vm_ref)"},{"line_number":283,"context_line":"        for device_name in bad_devices:"},{"line_number":284,"context_line":"            self._volumeops.detach_volume("},{"line_number":285,"context_line":"                    None, instance[\u0027name\u0027], device_name)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAN3%2F%2F0zo%3D","line":282,"updated":"2013-03-11 10:50:31.000000000","message":"I worry a bit about this in the boot from volume case. If the root disk gets disconnected, does that confuse things?\n\nWould it be safer only doing this if VM.start fails, then maybe try remove the volumes and start again? It might not help improve VM start times I guess.\n\nMaybe we should only do this remove when we have an error handler passed in?","commit_id":"b3319e9b97319985311961ee2821c0d9e2836550"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"755090e72849d4d041bc44332924404f0adbc229","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        # Attached volumes that have become non-responsive will prevent a VM"},{"line_number":281,"context_line":"        # from starting, so scan for these before attempting to start"},{"line_number":282,"context_line":"        bad_devices \u003d self._volumeops.find_bad_volumes(vm_ref)"},{"line_number":283,"context_line":"        for device_name in bad_devices:"},{"line_number":284,"context_line":"            self._volumeops.detach_volume("},{"line_number":285,"context_line":"                    None, instance[\u0027name\u0027], device_name)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAN3%2F%2Fz8s%3D","line":282,"in_reply_to":"AAAAN3%2F%2F0zo%3D","updated":"2013-03-11 18:32:27.000000000","message":"\u003e I worry a bit about this in the boot from volume case. If the root disk gets disconnected, does that confuse things?\n\nGood question. Perhaps in the case of boot-volume it shouldn\u0027t auto-detach it; though there isn\u0027t really any harm (I think) in auto-detaching and letting the user reattach and boot. That at least avoids the special case.\n\n\u003e Would it be safer only doing this if VM.start fails, then maybe try remove the volumes and start again? It might not help improve VM start times I guess.\n\nGiven that the happy path doesn\u0027t suffer from the painful timeout, I\u0027d be slightly in favor of just leaving the always-on check there for simplicity, with the idea that in a later optimize-build-times phase, with hard numbers, we could  tweak this.\n\n\u003e Maybe we should only do this remove when we have an error handler passed in?\n\nGood idea.","commit_id":"b3319e9b97319985311961ee2821c0d9e2836550"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9bb0dd085571273555e125c60720fe0fcde00801","unresolved":false,"context_lines":[{"line_number":290,"context_line":""},{"line_number":291,"context_line":"        # Allow higher-layers a chance to detach bad-volumes as well (in order"},{"line_number":292,"context_line":"        # to cleanup BDM entries and detach in Cinder)"},{"line_number":293,"context_line":"        if bad_devices and bad_volumes_callback:"},{"line_number":294,"context_line":"            bad_volumes_callback(bad_devices)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    def _create_disks(self, context, instance, name_label, disk_image_type,"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAN3%2F%2F0zg%3D","line":293,"updated":"2013-03-11 10:50:31.000000000","message":"If VM start fails, I guess the VM goes into an ERROR state, and when removed cinder will clean up the volumes.\n\nI wonder if we should really do this at the same time we do the detach volume? Or is that not really valid.","commit_id":"b3319e9b97319985311961ee2821c0d9e2836550"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"755090e72849d4d041bc44332924404f0adbc229","unresolved":false,"context_lines":[{"line_number":290,"context_line":""},{"line_number":291,"context_line":"        # Allow higher-layers a chance to detach bad-volumes as well (in order"},{"line_number":292,"context_line":"        # to cleanup BDM entries and detach in Cinder)"},{"line_number":293,"context_line":"        if bad_devices and bad_volumes_callback:"},{"line_number":294,"context_line":"            bad_volumes_callback(bad_devices)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    def _create_disks(self, context, instance, name_label, disk_image_type,"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAN3%2F%2Fz8Q%3D","line":293,"in_reply_to":"AAAAN3%2F%2F0zg%3D","updated":"2013-03-11 18:32:27.000000000","message":"Yeah that\u0027s my understanding of how it works too.\n\nI don\u0027t have a preference either way, but given that behavior isn\u0027t changin, I\u0027d prefer to keep it the same for now and have any tweaks to it come as follow-on patches.","commit_id":"b3319e9b97319985311961ee2821c0d9e2836550"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"04cfe1858aba70ea24717ff403830f7ee521329a","unresolved":false,"context_lines":[{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        # Allow higher-layers a chance to detach bad-volumes as well (in order"},{"line_number":296,"context_line":"        # to cleanup BDM entries and detach in Cinder)"},{"line_number":297,"context_line":"        if bad_volumes_callback and bad_devices:"},{"line_number":298,"context_line":"            bad_volumes_callback(bad_devices)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    def _create_disks(self, context, instance, name_label, disk_image_type,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAN3%2F%2Fz40%3D","line":297,"updated":"2013-03-11 19:14:41.000000000","message":"find_bad_volumes above seems to always returns an empty array. Does that mean that bad_volumes_callback is called always even when it has no entries?","commit_id":"40feb35898ed0a6d57b1f481c165e683796b045c"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"6e19d3a04939bc784325b34f2bcf552b1fae7bc8","unresolved":false,"context_lines":[{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        # Allow higher-layers a chance to detach bad-volumes as well (in order"},{"line_number":296,"context_line":"        # to cleanup BDM entries and detach in Cinder)"},{"line_number":297,"context_line":"        if bad_volumes_callback and bad_devices:"},{"line_number":298,"context_line":"            bad_volumes_callback(bad_devices)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    def _create_disks(self, context, instance, name_label, disk_image_type,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAN3%2F%2Fzqk%3D","line":297,"in_reply_to":"AAAAN3%2F%2Fz40%3D","updated":"2013-03-11 21:38:02.000000000","message":"\u003e find_bad_volumes above seems to always returns an empty array. \n\nIt only returns bad (e.g. hung) volumes, so unless you\u0027re severing ISCSI connections to test it (e.g. kill -9 tgtd or something) than the array will (correctly) be empty.\n\n\u003e Does that mean that bad_volumes_callback is called always even when it has no entries?\n\nThe call back is only called if `bad_devices` is truthy, meaning find_bad_volumes detected at least one hung volume.","commit_id":"40feb35898ed0a6d57b1f481c165e683796b045c"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ab670533baea6f895bd8fd8a8c2a5e461c2a516e","unresolved":false,"context_lines":[{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        # Allow higher-layers a chance to detach bad-volumes as well (in order"},{"line_number":296,"context_line":"        # to cleanup BDM entries and detach in Cinder)"},{"line_number":297,"context_line":"        if bad_volumes_callback and bad_devices:"},{"line_number":298,"context_line":"            bad_volumes_callback(bad_devices)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    def _create_disks(self, context, instance, name_label, disk_image_type,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAN3%2F%2FzdI%3D","line":297,"in_reply_to":"AAAAN3%2F%2Fzqk%3D","updated":"2013-03-11 23:48:10.000000000","message":"Ah. thanks.","commit_id":"40feb35898ed0a6d57b1f481c165e683796b045c"}],"nova/virt/xenapi/volumeops.py":[{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"1a76078d9ff30329729e4f41ec343610a5ac71a7","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            sr_ref \u003d volume_utils.find_sr_from_vbd(self._session, vbd_ref)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"            try:"},{"line_number":169,"context_line":"                # TODO(sirp): bug1152401 This relies on a 120 sec timeout"},{"line_number":170,"context_line":"                # within XenServer, update this to fail-fast when this is fixed"},{"line_number":171,"context_line":"                # upstream"},{"line_number":172,"context_line":"                self._session.call_xenapi(\"SR.scan\", sr_ref)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAN3%2F%2Fz%2Fo%3D","line":169,"updated":"2013-03-11 18:19:21.000000000","message":"Could use an eventlet timer if you have a reasonable timeout value here? \n\nwith eventlet.timeout.Timeout(seconds\u003dX):\n  self._session.call_xenapi(\"SR.scan\", sr_ref)","commit_id":"b3319e9b97319985311961ee2821c0d9e2836550"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"03a5d28ffc83d5cc0890db51c6214c20ca5c4a6c","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            sr_ref \u003d volume_utils.find_sr_from_vbd(self._session, vbd_ref)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"            try:"},{"line_number":169,"context_line":"                # TODO(sirp): bug1152401 This relies on a 120 sec timeout"},{"line_number":170,"context_line":"                # within XenServer, update this to fail-fast when this is fixed"},{"line_number":171,"context_line":"                # upstream"},{"line_number":172,"context_line":"                self._session.call_xenapi(\"SR.scan\", sr_ref)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAN3%2F%2Fz6c%3D","line":169,"in_reply_to":"AAAAN3%2F%2Fz%2Fo%3D","updated":"2013-03-11 18:53:27.000000000","message":"That\u0027s def possible, though I\u0027m not sure I like having two different timeouts (the Nova eventlet timeout and the Xenserver ISCSI timeout in play here).\n\nMy pref would be for Citrix to expose the underlying timeout as config that Nova could set. (Perhaps just proxying that value the xenapi down into iscsiadmin or something).","commit_id":"b3319e9b97319985311961ee2821c0d9e2836550"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"07b97d31f2b96b68785e889e73701c84c907f57f","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                # within XenServer, update this to fail-fast when this is fixed"},{"line_number":171,"context_line":"                # upstream"},{"line_number":172,"context_line":"                self._session.call_xenapi(\"SR.scan\", sr_ref)"},{"line_number":173,"context_line":"            except self._session.XenAPI.Failure, exc:"},{"line_number":174,"context_line":"                if exc.details[0] \u003d\u003d \u0027SR_BACKEND_FAILURE_40\u0027:"},{"line_number":175,"context_line":"                    vbd_rec \u003d vbd_rec \u003d self._session.call_xenapi("},{"line_number":176,"context_line":"                            \"VBD.get_record\", vbd_ref)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAN3%2F%2FyU8%3D","line":173,"updated":"2013-03-12 16:33:09.000000000","message":"nit: Use the \u0027as\u0027 clause…","commit_id":"40feb35898ed0a6d57b1f481c165e683796b045c"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"233ab8f44bd6aa25780aa16e9ea5db26a8eae523","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                # within XenServer, update this to fail-fast when this is fixed"},{"line_number":171,"context_line":"                # upstream"},{"line_number":172,"context_line":"                self._session.call_xenapi(\"SR.scan\", sr_ref)"},{"line_number":173,"context_line":"            except self._session.XenAPI.Failure, exc:"},{"line_number":174,"context_line":"                if exc.details[0] \u003d\u003d \u0027SR_BACKEND_FAILURE_40\u0027:"},{"line_number":175,"context_line":"                    vbd_rec \u003d vbd_rec \u003d self._session.call_xenapi("},{"line_number":176,"context_line":"                            \"VBD.get_record\", vbd_ref)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAN3%2F%2FyNQ%3D","line":173,"in_reply_to":"AAAAN3%2F%2FyRE%3D","updated":"2013-03-12 17:17:18.000000000","message":"Heh.  Indeed :)  I think we may only need to use commas in the xen plugins (don\u0027t remember when \u0027as\u0027 was introduced, but it shouldn\u0027t be difficult to look up…)","commit_id":"40feb35898ed0a6d57b1f481c165e683796b045c"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"b1d93ef40bea2bbab52d0e51008a74ad4463e1f9","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                # within XenServer, update this to fail-fast when this is fixed"},{"line_number":171,"context_line":"                # upstream"},{"line_number":172,"context_line":"                self._session.call_xenapi(\"SR.scan\", sr_ref)"},{"line_number":173,"context_line":"            except self._session.XenAPI.Failure, exc:"},{"line_number":174,"context_line":"                if exc.details[0] \u003d\u003d \u0027SR_BACKEND_FAILURE_40\u0027:"},{"line_number":175,"context_line":"                    vbd_rec \u003d vbd_rec \u003d self._session.call_xenapi("},{"line_number":176,"context_line":"                            \"VBD.get_record\", vbd_ref)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAN3%2F%2FyRE%3D","line":173,"in_reply_to":"AAAAN3%2F%2FyU8%3D","updated":"2013-03-12 16:47:27.000000000","message":"Vek:\n\nHa, so I originally was gonna use the \u0027as\u0027 clause, but noticed the code around it didn\u0027t. So in the interest of \u0027local-conformity\u0027, I went with the comma.\n\nI think an excellent follow-on patch would be to update the entire xendriver to use \u0027as\u0027 where appropriate.","commit_id":"40feb35898ed0a6d57b1f481c165e683796b045c"}]}