)]}'
{"nova/conductor/manager.py":[{"author":{"_account_id":7366,"name":"Ivan Melnikov","email":"imelnikov@griddynamics.com","username":"imelnikov"},"change_message_id":"8d482a9ad4053460f9efc6948bd11c7633ff12d2","unresolved":false,"context_lines":[{"line_number":851,"context_line":"    def snapshot_instance(self, context, instance, image_id):"},{"line_number":852,"context_line":"        live_snapshot \u003d self.compute_rpcapi.prepare_snapshot(context,"},{"line_number":853,"context_line":"                                            instance, image_id)"},{"line_number":854,"context_line":"        out_path, image_format \u003d self.compute_rpcapi.make_snapshot(context,"},{"line_number":855,"context_line":"                                            instance,"},{"line_number":856,"context_line":"                                            live_snapshot)"},{"line_number":857,"context_line":"        self.compute_rpcapi.upload_snapshot(context, instance, image_format,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8qok%3D","line":854,"updated":"2013-12-23 13:58:42.000000000","message":"I think it is better to return arbitrary data here (like, a dictionary) so that other drivers could pass their specific data from one method to another.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":851,"context_line":"    def snapshot_instance(self, context, instance, image_id):"},{"line_number":852,"context_line":"        live_snapshot \u003d self.compute_rpcapi.prepare_snapshot(context,"},{"line_number":853,"context_line":"                                            instance, image_id)"},{"line_number":854,"context_line":"        out_path, image_format \u003d self.compute_rpcapi.make_snapshot(context,"},{"line_number":855,"context_line":"                                            instance,"},{"line_number":856,"context_line":"                                            live_snapshot)"},{"line_number":857,"context_line":"        self.compute_rpcapi.upload_snapshot(context, instance, image_format,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8mYU%3D","line":854,"in_reply_to":"AAAATn%2F8qok%3D","updated":"2013-12-24 14:45:09.000000000","message":"Agreed.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"e22c0d78cdadee92c618983ca10df69916cfb362","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        self.compute_rpcapi.upload_snapshot(context, instance, settings,"},{"line_number":869,"context_line":"                                            image_id)"},{"line_number":870,"context_line":"        #FIXME: Add new task state CLEANING_SNAPSHOT"},{"line_number":871,"context_line":"        self.compute_rpcapi.cleanup_tmp_snapshot(context, instance, out_path)"},{"line_number":872,"context_line":"        update_task_state(task_state\u003dNone,"},{"line_number":873,"context_line":"                expected_state\u003dtask_states.IMAGE_UPLOADING)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F4sA%3D","line":871,"updated":"2013-12-28 01:40:30.000000000","message":"If \u0027upload_snapshot\u0027 (or make_snapshot) fails don\u0027t u still want to call \u0027cleanup_tmp_snapshot\u0027?","commit_id":"7b69a500415d27c79f79f4397bf9a6d86898689d"}],"nova/virt/driver.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"c635eab646f47f60ddce611cd8315968c59a3c38","unresolved":false,"context_lines":[{"line_number":424,"context_line":"        raise NotImplementedError()"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"    def cleanup_tmp_snapshot(self, instance, path):"},{"line_number":427,"context_line":"        raise NotImplementedError()"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"    def finish_migration(self, context, migration, instance, disk_info,"},{"line_number":430,"context_line":"                         network_info, image_meta, resize_instance,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2B7UU%3D","line":427,"updated":"2014-01-20 17:30:36.000000000","message":"All of these methods need to have API documentation describing their parameters, along with what they actually do, so virt driver maintainers, as well as core reviewers, understand what\u0027s required by an impl.\n\nAlso shouldn\u0027t the existing \u0027snapshot\u0027 method be removed since you\u0027re removing all impls of it ?","commit_id":"d240378afc8fa6a1ca1bd68db2e9335e0cf8c6d7"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"6179db1b76a42a2661ddb986339d610708aeaa32","unresolved":false,"context_lines":[{"line_number":424,"context_line":"        raise NotImplementedError()"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"    def cleanup_tmp_snapshot(self, instance, path):"},{"line_number":427,"context_line":"        raise NotImplementedError()"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"    def finish_migration(self, context, migration, instance, disk_info,"},{"line_number":430,"context_line":"                         network_info, image_meta, resize_instance,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F9vMM%3D","line":427,"in_reply_to":"AAAAUH%2F%2B7UU%3D","updated":"2014-02-04 14:48:23.000000000","message":"I agree that all methods should be well-documented and clear enough. This patch is the first attempt to split snapshot in more atomic operations. And, of course, I would really appreciate any help or remarks on designing convenient interface in this particular case.","commit_id":"d240378afc8fa6a1ca1bd68db2e9335e0cf8c6d7"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"a0898a3fe03143d4e90bf6fa208c2c608195aef5","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"        return state"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"    def _prepare_snapshot(self, context, instance, image_href):"},{"line_number":1400,"context_line":"        try:"},{"line_number":1401,"context_line":"            virt_dom \u003d self._lookup_by_name(instance[\u0027name\u0027])"},{"line_number":1402,"context_line":"        except exception.InstanceNotFound:"},{"line_number":1403,"context_line":"            raise exception.InstanceNotRunning(instance_id\u003dinstance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8o%2BE%3D","line":1400,"updated":"2013-12-23 21:28:59.000000000","message":"It would seem at each stage (prepare, make, upload) that you would want to make sure the instance still exists right? Otherwise u can abort what u are currently doing in that stage.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"8c29d02de7af7d3cc2d5d17e67029fdca4f94603","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"        return state"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"    def _prepare_snapshot(self, context, instance, image_href):"},{"line_number":1400,"context_line":"        try:"},{"line_number":1401,"context_line":"            virt_dom \u003d self._lookup_by_name(instance[\u0027name\u0027])"},{"line_number":1402,"context_line":"        except exception.InstanceNotFound:"},{"line_number":1403,"context_line":"            raise exception.InstanceNotRunning(instance_id\u003dinstance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8lSk%3D","line":1400,"in_reply_to":"AAAATn%2F8mYg%3D","updated":"2013-12-25 04:31:42.000000000","message":"Offline to me is different from \"InstanceNotFound\" right? Instance not found means the VM is deleted, never to come back again? If this is true, then lets say the following happens. Prepare takes 10 seconds, after those 10 seconds but before _make_snapshot happens the VM disappears. Does it make sense to continue the snapshot, what if the same thing happens in \u0027upload_snapshot\u0027 (any IO involving call could cause eventlet to switch, and during that switch another \u0027thread\u0027 could delete the VM). Is there anything stopping this from happening (the delete)? If not then likely each step needs to have the same kind of precondition validations before it does more work.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"        return state"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"    def _prepare_snapshot(self, context, instance, image_href):"},{"line_number":1400,"context_line":"        try:"},{"line_number":1401,"context_line":"            virt_dom \u003d self._lookup_by_name(instance[\u0027name\u0027])"},{"line_number":1402,"context_line":"        except exception.InstanceNotFound:"},{"line_number":1403,"context_line":"            raise exception.InstanceNotRunning(instance_id\u003dinstance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8mYg%3D","line":1400,"in_reply_to":"AAAATn%2F8o%2BE%3D","updated":"2013-12-24 14:45:09.000000000","message":"You mean that if VM is offline, we can skip any other steps and immediately call cleanup_tmp_snapshot?","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"a0898a3fe03143d4e90bf6fa208c2c608195aef5","unresolved":false,"context_lines":[{"line_number":1450,"context_line":"        if state \u003d\u003d power_state.SHUTDOWN:"},{"line_number":1451,"context_line":"            live_snapshot \u003d False"},{"line_number":1452,"context_line":""},{"line_number":1453,"context_line":"        return live_snapshot"},{"line_number":1454,"context_line":""},{"line_number":1455,"context_line":"    def _make_snapshot(self, instance, live_snapshot, update_task_state):"},{"line_number":1456,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8o9w%3D","line":1453,"updated":"2013-12-23 21:28:59.000000000","message":"Probably return a dict, since dicts can be used by other drivers to return more info (and are much more expandable later if new entries are needed).","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":1450,"context_line":"        if state \u003d\u003d power_state.SHUTDOWN:"},{"line_number":1451,"context_line":"            live_snapshot \u003d False"},{"line_number":1452,"context_line":""},{"line_number":1453,"context_line":"        return live_snapshot"},{"line_number":1454,"context_line":""},{"line_number":1455,"context_line":"    def _make_snapshot(self, instance, live_snapshot, update_task_state):"},{"line_number":1456,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8mb0%3D","line":1453,"in_reply_to":"AAAATn%2F8o9w%3D","updated":"2013-12-24 14:45:09.000000000","message":"Agreed.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"a0898a3fe03143d4e90bf6fa208c2c608195aef5","unresolved":false,"context_lines":[{"line_number":1466,"context_line":"                    pci_manager.get_instance_pci_devs(instance))"},{"line_number":1467,"context_line":"                virt_dom.managedSave(0)"},{"line_number":1468,"context_line":""},{"line_number":1469,"context_line":"        instance.task_state \u003d task_states.IMAGE_PENDING_UPLOAD"},{"line_number":1470,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_SNAPSHOT)"},{"line_number":1471,"context_line":"        snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1472,"context_line":"        fileutils.ensure_tree(snapshot_directory)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8o9Y%3D","line":1469,"updated":"2013-12-23 21:28:59.000000000","message":"So mutations of task_state are intersting, any thoughts on what happens to these functions and the associated task_state if a failure occurs. it\u0027d be nice to not have the instance stuck in \u0027IMAGE_PENDING_UPLOAD\u0027 if a later piece of code here fails (_live_snapshot() or other...).","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"8c29d02de7af7d3cc2d5d17e67029fdca4f94603","unresolved":false,"context_lines":[{"line_number":1466,"context_line":"                    pci_manager.get_instance_pci_devs(instance))"},{"line_number":1467,"context_line":"                virt_dom.managedSave(0)"},{"line_number":1468,"context_line":""},{"line_number":1469,"context_line":"        instance.task_state \u003d task_states.IMAGE_PENDING_UPLOAD"},{"line_number":1470,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_SNAPSHOT)"},{"line_number":1471,"context_line":"        snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1472,"context_line":"        fileutils.ensure_tree(snapshot_directory)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8lSc%3D","line":1469,"in_reply_to":"AAAATn%2F8miM%3D","updated":"2013-12-25 04:31:42.000000000","message":"I think it seems to make sense, of course the nova folks would have the final say in this :-)","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":1466,"context_line":"                    pci_manager.get_instance_pci_devs(instance))"},{"line_number":1467,"context_line":"                virt_dom.managedSave(0)"},{"line_number":1468,"context_line":""},{"line_number":1469,"context_line":"        instance.task_state \u003d task_states.IMAGE_PENDING_UPLOAD"},{"line_number":1470,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_SNAPSHOT)"},{"line_number":1471,"context_line":"        snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1472,"context_line":"        fileutils.ensure_tree(snapshot_directory)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8miM%3D","line":1469,"in_reply_to":"AAAATn%2F8o9Y%3D","updated":"2013-12-24 14:45:09.000000000","message":"I think I need to move all logic (error handling, state management, etc)  from driver and nova compute to nova conductor. Current implementation is quite naive and incomplete. What do you think about that moving?","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"a0898a3fe03143d4e90bf6fa208c2c608195aef5","unresolved":false,"context_lines":[{"line_number":1470,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_SNAPSHOT)"},{"line_number":1471,"context_line":"        snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1472,"context_line":"        fileutils.ensure_tree(snapshot_directory)"},{"line_number":1473,"context_line":"        snapshot_name \u003d uuid.uuid4().hex"},{"line_number":1474,"context_line":"        tmpdir \u003d tempfile.mkdtemp(dir\u003dsnapshot_directory)"},{"line_number":1475,"context_line":"        disk_path \u003d libvirt_utils.find_disk(virt_dom)"},{"line_number":1476,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8o8w%3D","line":1473,"updated":"2013-12-23 21:28:59.000000000","message":"The snapshot_name almost seems like this is something u would want in the prepare part (so that u aren\u0027t recreating a new snapshot everytime u resume from this function - when that is later added). Possibly also the mkdtemp and such also belong up in the prepare stage?","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":1470,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_SNAPSHOT)"},{"line_number":1471,"context_line":"        snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1472,"context_line":"        fileutils.ensure_tree(snapshot_directory)"},{"line_number":1473,"context_line":"        snapshot_name \u003d uuid.uuid4().hex"},{"line_number":1474,"context_line":"        tmpdir \u003d tempfile.mkdtemp(dir\u003dsnapshot_directory)"},{"line_number":1475,"context_line":"        disk_path \u003d libvirt_utils.find_disk(virt_dom)"},{"line_number":1476,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8mbE%3D","line":1473,"in_reply_to":"AAAATn%2F8o8w%3D","updated":"2013-12-24 14:45:09.000000000","message":"Agreed.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"ce1f29934b4b456f7c074ea8f04f9f0a14ab59a7","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"                snapshot_backend.snapshot_extract(out_path, image_format)"},{"line_number":1491,"context_line":"        finally:"},{"line_number":1492,"context_line":"            new_dom \u003d None"},{"line_number":1493,"context_line":"            # NOTE(dkang): because previous managedSave is not called"},{"line_number":1494,"context_line":"            #              for LXC, _create_domain must not be called."},{"line_number":1495,"context_line":"            if CONF.libvirt.virt_type !\u003d \u0027lxc\u0027 and not live_snapshot:"},{"line_number":1496,"context_line":"                if state \u003d\u003d power_state.RUNNING:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8omI%3D","line":1493,"updated":"2013-12-23 23:24:43.000000000","message":"This whole one seems like it should be its own method, it seems to be restarting the domain (seems like this should just be a restart method or something?)","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"8c29d02de7af7d3cc2d5d17e67029fdca4f94603","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"                snapshot_backend.snapshot_extract(out_path, image_format)"},{"line_number":1491,"context_line":"        finally:"},{"line_number":1492,"context_line":"            new_dom \u003d None"},{"line_number":1493,"context_line":"            # NOTE(dkang): because previous managedSave is not called"},{"line_number":1494,"context_line":"            #              for LXC, _create_domain must not be called."},{"line_number":1495,"context_line":"            if CONF.libvirt.virt_type !\u003d \u0027lxc\u0027 and not live_snapshot:"},{"line_number":1496,"context_line":"                if state \u003d\u003d power_state.RUNNING:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8lSY%3D","line":1493,"in_reply_to":"AAAATn%2F8mZM%3D","updated":"2013-12-25 04:31:42.000000000","message":"Sure, it seems to almost be its own contained functionality (in a way) that is doing something specifically for when the try path completes, just an idea.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"                snapshot_backend.snapshot_extract(out_path, image_format)"},{"line_number":1491,"context_line":"        finally:"},{"line_number":1492,"context_line":"            new_dom \u003d None"},{"line_number":1493,"context_line":"            # NOTE(dkang): because previous managedSave is not called"},{"line_number":1494,"context_line":"            #              for LXC, _create_domain must not be called."},{"line_number":1495,"context_line":"            if CONF.libvirt.virt_type !\u003d \u0027lxc\u0027 and not live_snapshot:"},{"line_number":1496,"context_line":"                if state \u003d\u003d power_state.RUNNING:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8mZM%3D","line":1493,"in_reply_to":"AAAATn%2F8omI%3D","updated":"2013-12-24 14:45:09.000000000","message":"I haven\u0027t found such method, but it\u0027s a good idea to separate it in a function (at least nested function) and write UT :)","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"ce1f29934b4b456f7c074ea8f04f9f0a14ab59a7","unresolved":false,"context_lines":[{"line_number":1501,"context_line":"                if new_dom is not None:"},{"line_number":1502,"context_line":"                    self._attach_pci_devices(new_dom,"},{"line_number":1503,"context_line":"                        pci_manager.get_instance_pci_devs(instance))"},{"line_number":1504,"context_line":"            LOG.info(_(\"Snapshot extracted, beginning image upload\"),"},{"line_number":1505,"context_line":"                     instance\u003dinstance)"},{"line_number":1506,"context_line":"        return out_path, image_format"},{"line_number":1507,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8omE%3D","line":1504,"updated":"2013-12-23 23:24:43.000000000","message":"This seems like it shouldn\u0027t be logged here, but instead the thing that calls this function should be logging this right?","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"8c29d02de7af7d3cc2d5d17e67029fdca4f94603","unresolved":false,"context_lines":[{"line_number":1501,"context_line":"                if new_dom is not None:"},{"line_number":1502,"context_line":"                    self._attach_pci_devices(new_dom,"},{"line_number":1503,"context_line":"                        pci_manager.get_instance_pci_devs(instance))"},{"line_number":1504,"context_line":"            LOG.info(_(\"Snapshot extracted, beginning image upload\"),"},{"line_number":1505,"context_line":"                     instance\u003dinstance)"},{"line_number":1506,"context_line":"        return out_path, image_format"},{"line_number":1507,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8lSU%3D","line":1504,"in_reply_to":"AAAATn%2F8mhI%3D","updated":"2013-12-25 04:31:42.000000000","message":"Up to u, it just seems odd that this function knows that an image upload is the next step, when really the function has no idea what the next step is, the thing calling this function knows that step (and it may change it depending on what steps it wants to take).","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":1501,"context_line":"                if new_dom is not None:"},{"line_number":1502,"context_line":"                    self._attach_pci_devices(new_dom,"},{"line_number":1503,"context_line":"                        pci_manager.get_instance_pci_devs(instance))"},{"line_number":1504,"context_line":"            LOG.info(_(\"Snapshot extracted, beginning image upload\"),"},{"line_number":1505,"context_line":"                     instance\u003dinstance)"},{"line_number":1506,"context_line":"        return out_path, image_format"},{"line_number":1507,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8mhI%3D","line":1504,"in_reply_to":"AAAATn%2F8omE%3D","updated":"2013-12-24 14:45:09.000000000","message":"Agree, the message is a bit weird, but we can log in both place: here and in function, that calls it. Or it makes some ambiguity?","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"a0898a3fe03143d4e90bf6fa208c2c608195aef5","unresolved":false,"context_lines":[{"line_number":1522,"context_line":"                                                  snapshot[\u0027name\u0027])"},{"line_number":1523,"context_line":"        #update_task_state(task_state\u003dtask_states.IMAGE_UPLOADING,"},{"line_number":1524,"context_line":"        #         expected_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1525,"context_line":"        instance.task_state \u003d task_states.IMAGE_UPLOADING"},{"line_number":1526,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1527,"context_line":""},{"line_number":1528,"context_line":"        with libvirt_utils.file_open(out_path) as image_file:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8o8c%3D","line":1525,"updated":"2013-12-23 21:28:59.000000000","message":"Same weirdness around instance getting stuck in \u0027IMAGE_UPLOADING\u0027 if this fails, be nice to have an idea how to unstick this.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":1522,"context_line":"                                                  snapshot[\u0027name\u0027])"},{"line_number":1523,"context_line":"        #update_task_state(task_state\u003dtask_states.IMAGE_UPLOADING,"},{"line_number":1524,"context_line":"        #         expected_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1525,"context_line":"        instance.task_state \u003d task_states.IMAGE_UPLOADING"},{"line_number":1526,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1527,"context_line":""},{"line_number":1528,"context_line":"        with libvirt_utils.file_open(out_path) as image_file:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8mYY%3D","line":1525,"in_reply_to":"AAAATn%2F8o8c%3D","updated":"2013-12-24 14:45:09.000000000","message":"Answered above.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"8c29d02de7af7d3cc2d5d17e67029fdca4f94603","unresolved":false,"context_lines":[{"line_number":1522,"context_line":"                                                  snapshot[\u0027name\u0027])"},{"line_number":1523,"context_line":"        #update_task_state(task_state\u003dtask_states.IMAGE_UPLOADING,"},{"line_number":1524,"context_line":"        #         expected_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1525,"context_line":"        instance.task_state \u003d task_states.IMAGE_UPLOADING"},{"line_number":1526,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1527,"context_line":""},{"line_number":1528,"context_line":"        with libvirt_utils.file_open(out_path) as image_file:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8lSQ%3D","line":1525,"in_reply_to":"AAAATn%2F8o8c%3D","updated":"2013-12-25 04:31:42.000000000","message":"Ya, this is in the thinking of how do we resume from each of these steps, if the piece of code is running in a daemon (which it usually is), the daemon gets shutdown and restarted (perhaps to upgrade), then instead of having the task_state be stuck in IMAGE_UPLOADING it can at least try to continue going to the next step.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"a0898a3fe03143d4e90bf6fa208c2c608195aef5","unresolved":false,"context_lines":[{"line_number":1526,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1527,"context_line":""},{"line_number":1528,"context_line":"        with libvirt_utils.file_open(out_path) as image_file:"},{"line_number":1529,"context_line":"            image_service.update(context,"},{"line_number":1530,"context_line":"                                 image_href,"},{"line_number":1531,"context_line":"                                 metadata,"},{"line_number":1532,"context_line":"                                 image_file)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8o%2BY%3D","line":1529,"updated":"2013-12-23 21:28:59.000000000","message":"A question, what happens if the same file is updated twice, does it resume from that last upload? This will likely be needed later (to make sure it resumes) so that partially completed uploads can continue.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"71d545cc1d56a6af5a0b93e4f65e0c9ea61533e1","unresolved":false,"context_lines":[{"line_number":1526,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1527,"context_line":""},{"line_number":1528,"context_line":"        with libvirt_utils.file_open(out_path) as image_file:"},{"line_number":1529,"context_line":"            image_service.update(context,"},{"line_number":1530,"context_line":"                                 image_href,"},{"line_number":1531,"context_line":"                                 metadata,"},{"line_number":1532,"context_line":"                                 image_file)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8lRQ%3D","line":1529,"in_reply_to":"AAAATn%2F8mcg%3D","updated":"2013-12-25 04:37:40.000000000","message":"Ya, we might just need to see if the code even supports resuming in the first place, hopefully it does, if not, then this will be hard to resume if glance doesn\u0027t support it in the first place.","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":1526,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1527,"context_line":""},{"line_number":1528,"context_line":"        with libvirt_utils.file_open(out_path) as image_file:"},{"line_number":1529,"context_line":"            image_service.update(context,"},{"line_number":1530,"context_line":"                                 image_href,"},{"line_number":1531,"context_line":"                                 metadata,"},{"line_number":1532,"context_line":"                                 image_file)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8mcg%3D","line":1529,"in_reply_to":"AAAATn%2F8o%2BY%3D","updated":"2013-12-24 14:45:09.000000000","message":"Exciting thing! Good catch!","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"a0898a3fe03143d4e90bf6fa208c2c608195aef5","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"    def _cleanup_tmp_snapshot(self, instance, path):"},{"line_number":1537,"context_line":"        shutil.rmtree(os.path.dirname(path))"},{"line_number":1538,"context_line":"        instance.task_state \u003d None"},{"line_number":1539,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_UPLOADING)"},{"line_number":1540,"context_line":""},{"line_number":1541,"context_line":"    def snapshot(self, context, instance, image_href, update_task_state):"},{"line_number":1542,"context_line":"        \"\"\"Create snapshot from a running VM instance."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8o8U%3D","line":1539,"updated":"2013-12-23 21:28:59.000000000","message":"It\u0027d be nice to have any of the task states that could be set in these stages be alloweable \u0027expected_task_state\u0027 (since any of them may have failed, not just the last stage) and u\u0027d probably like to cleanup from all of the stages (and not just the last one).","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"8c29d02de7af7d3cc2d5d17e67029fdca4f94603","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"    def _cleanup_tmp_snapshot(self, instance, path):"},{"line_number":1537,"context_line":"        shutil.rmtree(os.path.dirname(path))"},{"line_number":1538,"context_line":"        instance.task_state \u003d None"},{"line_number":1539,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_UPLOADING)"},{"line_number":1540,"context_line":""},{"line_number":1541,"context_line":"    def snapshot(self, context, instance, image_href, update_task_state):"},{"line_number":1542,"context_line":"        \"\"\"Create snapshot from a running VM instance."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8lSM%3D","line":1539,"in_reply_to":"AAAATn%2F8mck%3D","updated":"2013-12-25 04:31:42.000000000","message":"Well this is more of thinking what is the cleanup workflow that would be needed at each step (if that step is stopped half-way through). Back to the daemon stop/restart case, any step after the piece of code that created this tmp_dir would need to potentially clean it up (instead of just the last step). With full ability to resume u can try to get to this step, but say the resume fails (for some reason) then u want to be able to cleanup without having to reach this final step (all else fails, cleanup and log a big error). Make sense?","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":7794,"name":"Alexander Gorodnev","email":"agorodnev@griddynamics.com","username":"agorodnev"},"change_message_id":"4be1dbd725af65af5d43625bb9f051a59c8c9bb2","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"    def _cleanup_tmp_snapshot(self, instance, path):"},{"line_number":1537,"context_line":"        shutil.rmtree(os.path.dirname(path))"},{"line_number":1538,"context_line":"        instance.task_state \u003d None"},{"line_number":1539,"context_line":"        instance.save(expected_task_state\u003dtask_states.IMAGE_UPLOADING)"},{"line_number":1540,"context_line":""},{"line_number":1541,"context_line":"    def snapshot(self, context, instance, image_href, update_task_state):"},{"line_number":1542,"context_line":"        \"\"\"Create snapshot from a running VM instance."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F8mck%3D","line":1539,"in_reply_to":"AAAATn%2F8o8U%3D","updated":"2013-12-24 14:45:09.000000000","message":"Just to clarify: You mean that there is not only sequence when cleanup_tmp_snapshot follows right after upload_snapshot and upload_snapshot could be called after any method?","commit_id":"bffcfaf252f84c4a8317a2a72e9ed1a92d58e7ff"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"deb73e614e6f5a58e06cabe296a78146bfffc85e","unresolved":false,"context_lines":[{"line_number":1458,"context_line":""},{"line_number":1459,"context_line":"        return data"},{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"    def make_snapshot(self, instance, capabilities):"},{"line_number":1462,"context_line":"        try:"},{"line_number":1463,"context_line":"            virt_dom \u003d self._lookup_by_name(instance[\u0027name\u0027])"},{"line_number":1464,"context_line":"        except exception.InstanceNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F4sI%3D","line":1461,"updated":"2013-12-28 01:39:12.000000000","message":"Seems like \u0027capabilities\u0027 might not be the right word now. This seems better called \u0027details\u0027 or settings (or something similar?)","commit_id":"7b69a500415d27c79f79f4397bf9a6d86898689d"}],"nova/virt/xenapi/driver.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"c635eab646f47f60ddce611cd8315968c59a3c38","unresolved":false,"context_lines":[{"line_number":263,"context_line":"        pass"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def cleanup_tmp_snapshot(self, instance, path):"},{"line_number":266,"context_line":"        pass"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    def reboot(self, context, instance, network_info, reboot_type,"},{"line_number":269,"context_line":"               block_device_info\u003dNone, bad_volumes_callback\u003dNone):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2B7SU%3D","line":266,"updated":"2014-01-20 17:30:36.000000000","message":"This approach of stuffing everything into the \u0027prepare_snapshot\u0027 method seems rather dubious to me. If this approach is OK, why do we go the trouble of splitting the libvirt impl up into 4 stages, but not split the other drivers.","commit_id":"d240378afc8fa6a1ca1bd68db2e9335e0cf8c6d7"}]}