)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c4800b50c0f9f187fea95307f1a120008c5fb714","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Cleanup method in libvirt driver is used in"},{"line_number":10,"context_line":"several cases, which make it to generic. This"},{"line_number":11,"context_line":"patch introduced new method instead to cover"},{"line_number":12,"context_line":"cleanup cases for live-migration"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-bug: #1544103"},{"line_number":15,"context_line":"bp: remove-compute-compute-communication"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"3aaa91ec_5e903a95","line":12,"updated":"2016-06-24 18:13:53.000000000","message":"The above commit message doesn\u0027t really make sense. Are you trying to say that the patch consolidates a variety of live migration cleanup actions spread throughout the libvirt virt driver into a single method?","commit_id":"9da06dd539185c94ad5b7994f31f3818acf69caf"}],"nova/compute/manager.py":[{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"506430b1a46e5c84ad9349495433b19cbf7d28a1","unresolved":false,"context_lines":[{"line_number":5457,"context_line":"        if do_cleanup:"},{"line_number":5458,"context_line":"            LOG.debug(\u0027Calling driver.cleanup from _post_live_migration\u0027,"},{"line_number":5459,"context_line":"                      instance\u003dinstance)"},{"line_number":5460,"context_line":"            self.driver.live_migration_cleanup(ctxt, instance, network_info,"},{"line_number":5461,"context_line":"                                migrate_data\u003dmigrate_data)"},{"line_number":5462,"context_line":""},{"line_number":5463,"context_line":"        self.instance_events.clear_events_for_instance(instance)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_43f6b0f9","line":5460,"updated":"2016-07-20 20:34:50.000000000","message":"Shouldn\u0027t you pass here block_device_info? You have such argument in virt interface (virt/driver.py) and looks like libvirt/driver.py strongly relies on this parameter, but it is always None, because you don\u0027t pass it here.","commit_id":"849e7ddf5ac593e8cc581059604979b7cd913d25"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c4800b50c0f9f187fea95307f1a120008c5fb714","unresolved":false,"context_lines":[{"line_number":6367,"context_line":"            if CONF.libvirt.images_type \u003d\u003d \u0027lvm\u0027:"},{"line_number":6368,"context_line":"                self._cleanup_lvm(instance, block_device_info)"},{"line_number":6369,"context_line":"            if CONF.libvirt.images_type \u003d\u003d \u0027rbd\u0027:"},{"line_number":6370,"context_line":"                self._cleanup_rbd(instance)"},{"line_number":6371,"context_line":"        if not is_shared_instance_path:"},{"line_number":6372,"context_line":"            attempts \u003d int(instance.system_metadata.get(\u0027clean_attempts\u0027,"},{"line_number":6373,"context_line":"                                                        \u00270\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"3aaa91ec_3e8bae69","line":6370,"updated":"2016-06-24 18:13:53.000000000","message":"The above block of code from 6366-6370 will have already been executed in _disconnect_volume_during_cleanup(), no?","commit_id":"9da06dd539185c94ad5b7994f31f3818acf69caf"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"506430b1a46e5c84ad9349495433b19cbf7d28a1","unresolved":false,"context_lines":[{"line_number":1020,"context_line":"            if CONF.libvirt.images_type \u003d\u003d \u0027rbd\u0027:"},{"line_number":1021,"context_line":"                self._cleanup_rbd(instance)"},{"line_number":1022,"context_line":""},{"line_number":1023,"context_line":"        is_shared_block_storage \u003d False"},{"line_number":1024,"context_line":"        if migrate_data and \u0027is_shared_block_storage\u0027 in migrate_data:"},{"line_number":1025,"context_line":"            is_shared_block_storage \u003d migrate_data.is_shared_block_storage"},{"line_number":1026,"context_line":"        if destroy_disks or is_shared_block_storage:"},{"line_number":1027,"context_line":"            attempts \u003d int(instance.system_metadata.get(\u0027clean_attempts\u0027,"},{"line_number":1028,"context_line":"                                                        \u00270\u0027))"},{"line_number":1029,"context_line":"            success \u003d self.delete_instance_files(instance)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_0654c6fc","side":"PARENT","line":1026,"range":{"start_line":1023,"start_character":8,"end_line":1026,"end_character":52},"updated":"2016-07-20 20:34:50.000000000","message":"I have some concerns that removing this logic might harm live migration rollbacks. Basically destroy_disks at this point has opposite value to is_shared_block_storage - https://github.com/openstack/nova/blob/e6deb676cdeb630add92663cc59f0680e38ce2a5/nova/compute/manager.py#L5328\nSo basing on this - if block storage is not shared it won\u0027t delete instance files","commit_id":"76020d38a6592ed29fd0136bb4b611ddc0a8a962"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"506430b1a46e5c84ad9349495433b19cbf7d28a1","unresolved":false,"context_lines":[{"line_number":939,"context_line":"    def _disconnect_volume_during_cleanup(self, context, block_device_info,"},{"line_number":940,"context_line":"                                          instance, destroy_disks\u003dFalse):"},{"line_number":941,"context_line":"        block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":942,"context_line":"            block_device_info)"},{"line_number":943,"context_line":"        for vol in block_device_mapping:"},{"line_number":944,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":945,"context_line":"            disk_dev \u003d vol[\u0027mount_device\u0027]"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_0365d8b3","line":942,"range":{"start_line":942,"start_character":0,"end_line":942,"end_character":30},"updated":"2016-07-20 20:34:50.000000000","message":"This is always None, so code below will not execute in any case. You probably should pass block_device_info from compute/manager or retrieve bdi somewhere in driver. Probably passing it to here from compute/manager is better idea because this bdi already exists in manager.","commit_id":"849e7ddf5ac593e8cc581059604979b7cd913d25"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"506430b1a46e5c84ad9349495433b19cbf7d28a1","unresolved":false,"context_lines":[{"line_number":1017,"context_line":"        self._disconnect_volume_during_cleanup(context, block_device_info,"},{"line_number":1018,"context_line":"                                               instance, destroy_disks)"},{"line_number":1019,"context_line":""},{"line_number":1020,"context_line":"        if destroy_disks:"},{"line_number":1021,"context_line":"            # NOTE(haomai): destroy volumes if needed"},{"line_number":1022,"context_line":"            if CONF.libvirt.images_type \u003d\u003d \u0027lvm\u0027:"},{"line_number":1023,"context_line":"                self._cleanup_lvm(instance, block_device_info)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_e649b261","line":1020,"range":{"start_line":1020,"start_character":11,"end_line":1020,"end_character":24},"updated":"2016-07-20 20:34:50.000000000","message":"So this is not enough, I think. You still need to check whether block storage is shared. Otherwise line #1028 will not be executed and it will leave instance files on destination.","commit_id":"849e7ddf5ac593e8cc581059604979b7cd913d25"}]}