)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"4a7520aa1b066a1c6a10a2bb2730223a9e60a5b9","unresolved":false,"context_lines":[{"line_number":11,"context_line":"first removing it. Additionally an empty, unused disk image would also"},{"line_number":12,"context_line":"be placed into the directory."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This was introduced by 29476a6 making a call to imagebackend when"},{"line_number":15,"context_line":"attempting to remove snapshots during a resize cleanup. When made"},{"line_number":16,"context_line":"against volume backed instances this causes the above behaviour, also"},{"line_number":17,"context_line":"blocking any future migration or resize attempts for the instance back"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7aa08908_2caf8f7e","line":14,"updated":"2016-06-16 09:55:48.000000000","message":"Nit: my preference is reference by change id, as gerrit will automatically turn them into hyperlinks, and if the change has been backported it will list all backports. Turns out you can shorten these, too. See: change I328d2c41","commit_id":"e5d5f9ed99f971ca7157918b91ee00bfe8eac8e6"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"02f8b61e56a41985b078d3693b224a80ff23d6b0","unresolved":false,"context_lines":[{"line_number":11,"context_line":"first removing it. Additionally an empty, unused disk image would also"},{"line_number":12,"context_line":"be placed into the directory."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This was introduced by 29476a6 making a call to imagebackend when"},{"line_number":15,"context_line":"attempting to remove snapshots during a resize cleanup. When made"},{"line_number":16,"context_line":"against volume backed instances this causes the above behaviour, also"},{"line_number":17,"context_line":"blocking any future migration or resize attempts for the instance back"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7aa08908_860bf72a","line":14,"in_reply_to":"7aa08908_2caf8f7e","updated":"2016-06-16 12:34:43.000000000","message":"Done","commit_id":"e5d5f9ed99f971ca7157918b91ee00bfe8eac8e6"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"47bd602ad2b2cf99f07c670d1ebfb045e714b84b","unresolved":false,"context_lines":[{"line_number":15077,"context_line":"                                         \u0027image-ref\u0027: None})"},{"line_number":15078,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":15079,"context_line":"        drvr.image_backend \u003d mock.Mock()"},{"line_number":15080,"context_line":"        drvr.image_backend.image.return_value \u003d drvr.image_backend"},{"line_number":15081,"context_line":""},{"line_number":15082,"context_line":"        with test.nested("},{"line_number":15083,"context_line":"                mock.patch.object(os.path, \u0027exists\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_3250ac0a","line":15080,"range":{"start_line":15080,"start_character":8,"end_line":15080,"end_character":66},"updated":"2016-06-15 16:35:26.000000000","message":"I\u0027ll change this to assert that image_backend isn\u0027t called at all in a future PS.","commit_id":"7f54c1ddc4df4f0cd5becb6d5e1dbc1102544777"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"4a7520aa1b066a1c6a10a2bb2730223a9e60a5b9","unresolved":false,"context_lines":[{"line_number":15085,"context_line":"            mock_exists.return_value \u003d False"},{"line_number":15086,"context_line":"            drvr._cleanup_resize(context.get_admin_context(),"},{"line_number":15087,"context_line":"                                 ins_ref, mock.sentinel.netinfo)"},{"line_number":15088,"context_line":"            mock_imagebackend.assert_not_called()"},{"line_number":15089,"context_line":""},{"line_number":15090,"context_line":"    def test_cleanup_resize_snap_backend_image_does_not_exist(self):"},{"line_number":15091,"context_line":"        CONF.set_override(\u0027policy_dirs\u0027, [], group\u003d\u0027oslo_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_7a3949d8","line":15088,"updated":"2016-06-16 09:55:48.000000000","message":"As with the main code, could you just comment somewhere that what we\u0027re actually testing is that we\u0027re not trying to delete a snapshot on a volume? I\u0027m happy that this test covers what we need, but because we\u0027re testing a proxy (a call to backend.image), it might not be immediately obvious to Future Maintainer what you were going for here.","commit_id":"e5d5f9ed99f971ca7157918b91ee00bfe8eac8e6"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"02f8b61e56a41985b078d3693b224a80ff23d6b0","unresolved":false,"context_lines":[{"line_number":15085,"context_line":"            mock_exists.return_value \u003d False"},{"line_number":15086,"context_line":"            drvr._cleanup_resize(context.get_admin_context(),"},{"line_number":15087,"context_line":"                                 ins_ref, mock.sentinel.netinfo)"},{"line_number":15088,"context_line":"            mock_imagebackend.assert_not_called()"},{"line_number":15089,"context_line":""},{"line_number":15090,"context_line":"    def test_cleanup_resize_snap_backend_image_does_not_exist(self):"},{"line_number":15091,"context_line":"        CONF.set_override(\u0027policy_dirs\u0027, [], group\u003d\u0027oslo_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_01289184","line":15088,"in_reply_to":"7aa08908_7a3949d8","updated":"2016-06-16 12:34:43.000000000","message":"Done","commit_id":"e5d5f9ed99f971ca7157918b91ee00bfe8eac8e6"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"8cf49dfbfc84916b8a9a4e94cb30c94a9d846844","unresolved":false,"context_lines":[{"line_number":1057,"context_line":"            utils.execute(\u0027rm\u0027, \u0027-rf\u0027, target, delay_on_retry\u003dTrue,"},{"line_number":1058,"context_line":"                          attempts\u003d5)"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"        if not compute_utils.is_volume_backed_instance(instance):"},{"line_number":1061,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1062,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."},{"line_number":1063,"context_line":"            # It is set to True here to avoid any upgrade issues surrounding"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_85455844","line":1060,"updated":"2016-06-13 14:57:13.000000000","message":"The creator of this snapshot is (until I move it somewhere more appropriate) in _create_image, and guarded by driver._is_booted_from_volume(), so this guard is obviously correct here.\n\nHowever, I\u0027m concerned about edge cases caused by the guards not being identical. Can you also please investigate the possibility of replacing driver._is_booted_from_volume()? If so, I\u0027d greatly appreciate a separate patch to fix that up.","commit_id":"9f8c2aaaaf89491b6d8763079b3f4d747d6e736a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"8a976470cb6ac5c13c4072359e08edf87a2e91c4","unresolved":false,"context_lines":[{"line_number":1057,"context_line":"            utils.execute(\u0027rm\u0027, \u0027-rf\u0027, target, delay_on_retry\u003dTrue,"},{"line_number":1058,"context_line":"                          attempts\u003d5)"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"        if not compute_utils.is_volume_backed_instance(instance):"},{"line_number":1061,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1062,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."},{"line_number":1063,"context_line":"            # It is set to True here to avoid any upgrade issues surrounding"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_ca6f3d29","line":1060,"in_reply_to":"7aa08908_85455844","updated":"2016-06-14 12:42:55.000000000","message":"I\u0027ve switched back to self._is_booted_from_volume() for now as the compute_utils.is_volume_backed_instance() call requires that context is provided so it can do a BDM lookup for the instance. I\u0027ll look into replacing all uses of self._is_booted_from_volume in a separate patch to avoid any issues backporting this fix.","commit_id":"9f8c2aaaaf89491b6d8763079b3f4d747d6e736a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"4a7520aa1b066a1c6a10a2bb2730223a9e60a5b9","unresolved":false,"context_lines":[{"line_number":1057,"context_line":"            utils.execute(\u0027rm\u0027, \u0027-rf\u0027, target, delay_on_retry\u003dTrue,"},{"line_number":1058,"context_line":"                          attempts\u003d5)"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"        # Only call the imagebackend for non volume backend instances"},{"line_number":1061,"context_line":"        if not compute_utils.is_volume_backed_instance(context, instance):"},{"line_number":1062,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1063,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_2f34a148","line":1060,"updated":"2016-06-16 09:55:48.000000000","message":"I don\u0027t think this comment adequately explains why we\u0027re doing this. Also, the reason described here and in the commit message could reasonably go away in the future if we stop unnecessarily creating the instance directory before it\u0027s required, but we would still need this guard.\n\nThe reason we\u0027re adding the guard here is because we didn\u0027t create a snapshot in the first place for a volume-backed instance. Follow finish_migration-\u003e_create_image-\u003e\n\n  if not booted_from_volume:\n    ...\n    if instance.task_state \u003d\u003d task_states.RESIZE_FINISH:\n      backend.create_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME)\n\ni.e. cleanup needs to mirror creation, regardless of weird behaviour by imagebackend.\n\nSide note: the above code shouldn\u0027t be in _create_image. It should be in finish_migration with its own explicit guard. That\u0027s on my todo list, but feel free :)","commit_id":"e5d5f9ed99f971ca7157918b91ee00bfe8eac8e6"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"02f8b61e56a41985b078d3693b224a80ff23d6b0","unresolved":false,"context_lines":[{"line_number":1057,"context_line":"            utils.execute(\u0027rm\u0027, \u0027-rf\u0027, target, delay_on_retry\u003dTrue,"},{"line_number":1058,"context_line":"                          attempts\u003d5)"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"        # Only call the imagebackend for non volume backend instances"},{"line_number":1061,"context_line":"        if not compute_utils.is_volume_backed_instance(context, instance):"},{"line_number":1062,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1063,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_4170d9a4","line":1060,"in_reply_to":"7aa08908_2f34a148","updated":"2016-06-16 12:34:43.000000000","message":"Done, yeah happy to look at that once this lands.","commit_id":"e5d5f9ed99f971ca7157918b91ee00bfe8eac8e6"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"4a7520aa1b066a1c6a10a2bb2730223a9e60a5b9","unresolved":false,"context_lines":[{"line_number":7248,"context_line":""},{"line_number":7249,"context_line":"    def confirm_migration(self, context, migration, instance, network_info):"},{"line_number":7250,"context_line":"        \"\"\"Confirms a resize, destroying the source VM.\"\"\""},{"line_number":7251,"context_line":"        self._cleanup_resize(context, instance, network_info)"},{"line_number":7252,"context_line":""},{"line_number":7253,"context_line":"    @staticmethod"},{"line_number":7254,"context_line":"    def _get_io_devices(xml_doc):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_8c004304","line":7251,"updated":"2016-06-16 09:55:48.000000000","message":"Note: _cleanup_resize has no other callers :/ Might as well just move it here. Not in this patch, though.","commit_id":"e5d5f9ed99f971ca7157918b91ee00bfe8eac8e6"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"02f8b61e56a41985b078d3693b224a80ff23d6b0","unresolved":false,"context_lines":[{"line_number":7248,"context_line":""},{"line_number":7249,"context_line":"    def confirm_migration(self, context, migration, instance, network_info):"},{"line_number":7250,"context_line":"        \"\"\"Confirms a resize, destroying the source VM.\"\"\""},{"line_number":7251,"context_line":"        self._cleanup_resize(context, instance, network_info)"},{"line_number":7252,"context_line":""},{"line_number":7253,"context_line":"    @staticmethod"},{"line_number":7254,"context_line":"    def _get_io_devices(xml_doc):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_019d31bb","line":7251,"in_reply_to":"7aa08908_8c004304","updated":"2016-06-16 12:34:43.000000000","message":"Yeah I\u0027ll sort this out in a follow up, I\u0027ll just remove this method and rename _cleanup_resize to hopefully preserve some form of change history.","commit_id":"e5d5f9ed99f971ca7157918b91ee00bfe8eac8e6"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"766ac8b7edef1d379c669193d064a30cb6ee273d","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":1066,"context_line":"               context, instance.uuid)"},{"line_number":1067,"context_line":"        bdm_info \u003d driver.get_block_device_info(instance, bdms)"},{"line_number":1068,"context_line":"        block_device_mapping \u003d driver.block_device_info_get_mapping(bdm_info)"},{"line_number":1069,"context_line":""},{"line_number":1070,"context_line":"        # NOTE(lyarwood): Duplicate the guard used when first attempting an"},{"line_number":1071,"context_line":"        # instance resize and creating a snapshot of the root disk. Only"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_94903e44","line":1068,"updated":"2016-06-16 13:32:46.000000000","message":"I was ok with the last version, tbh, but it probably makes sense to use the exact same guard as the create path as you\u0027ve done here. When we clean this up we can update both at the same time, which is less likely to have weird edge cases.","commit_id":"c0cec13ac7839cb09a138bb42bd61a88bf427a6d"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"7cea4dbecefb60ee1b602ca983249385100ea54c","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"        # instance resize and creating the snapshot of the root disk. Only"},{"line_number":1135,"context_line":"        # attempting to remove the root disk snapshot if the instance is not"},{"line_number":1136,"context_line":"        # booting from a volume."},{"line_number":1137,"context_line":"        if not compute_utils.is_volume_backed_instance(context, instance):"},{"line_number":1138,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1139,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."},{"line_number":1140,"context_line":"            # It is set to True here to avoid any upgrade issues surrounding"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_cea9d7e1","line":1137,"range":{"start_line":1137,"start_character":0,"end_line":1137,"end_character":74},"updated":"2016-07-27 14:26:35.000000000","message":"this doesn\u0027t fix the same scenario for NFS shared storage. self.image_backend.image(instance, \u0027disk\u0027) will rewrite disk_info, luckily without bugs, but still unwanted side-effect. \n\nAfter walking through imagebackend.py:\nhttps://github.com/openstack/nova/blob/953514c4b28d158f5bc4f069b8603a0cf4663ceb/nova/virt/libvirt/imagebackend.py#L541 - this call is kind of side-effect which causes of all these things. As you just need to instantiate object rather than creating disk.info. I wonder, could we rewrite this section instead?\nP.S. the same should be valid for Ploop.","commit_id":"8fa60ed9f6e7d90b83db40c282ec08f0e4f7cdfe"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"3c6fab4225e5f473c56f3c6160c01cade2163b5f","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"        # instance resize and creating the snapshot of the root disk. Only"},{"line_number":1135,"context_line":"        # attempting to remove the root disk snapshot if the instance is not"},{"line_number":1136,"context_line":"        # booting from a volume."},{"line_number":1137,"context_line":"        if not compute_utils.is_volume_backed_instance(context, instance):"},{"line_number":1138,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1139,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."},{"line_number":1140,"context_line":"            # It is set to True here to avoid any upgrade issues surrounding"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_afbcb6bb","line":1137,"range":{"start_line":1137,"start_character":0,"end_line":1137,"end_character":74},"in_reply_to":"bacf61ea_9e6926fc","updated":"2016-07-31 20:03:52.000000000","message":"Left comment in launchpad. I understand your point, and agree that we need to fix this, but from my sight, it looks like the bug is way broader. So this patch is kind of  partial-fix while root cause is the way Image class objects are instantiated. SI\u0027d prefer fix all possible scenarios instead of only one. Please try to reproduce cases I\u0027ve described here and on launchpad.","commit_id":"8fa60ed9f6e7d90b83db40c282ec08f0e4f7cdfe"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"8a9693364b0291ec19e98cfa114d53688902416a","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"        # instance resize and creating the snapshot of the root disk. Only"},{"line_number":1135,"context_line":"        # attempting to remove the root disk snapshot if the instance is not"},{"line_number":1136,"context_line":"        # booting from a volume."},{"line_number":1137,"context_line":"        if not compute_utils.is_volume_backed_instance(context, instance):"},{"line_number":1138,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1139,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."},{"line_number":1140,"context_line":"            # It is set to True here to avoid any upgrade issues surrounding"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_941bf31b","line":1137,"range":{"start_line":1137,"start_character":0,"end_line":1137,"end_character":74},"in_reply_to":"bacf61ea_afbcb6bb","updated":"2016-08-04 11:55:31.000000000","message":"So, I agree with Timofey that the refactor he describes would be the cleanest thing to do here, and in fact I just wrote a big comment about how to do it. Turns out it wouldn\u0027t be *that* hard. I can share it if you like.\n\nHOWEVER, the persistent image metadata spec, when I finally get it merged, makes disk.info redundant. So the refactor work, while not actually that hard, would have a limited lifespan. It would also require making resolve_driver_format() slightly more complex which, while not a huge deal, isn\u0027t ideal for code whose card is already marked.\n\nBUT, there\u0027s a much simpler fix here. By the time this function completes, the instance directory shouldn\u0027t exist, right? Just move the code block above, which deletes it, down to the bottom. It\u0027s simple, backportable, robust, and doesn\u0027t involve any dead-end work. I\u0027d add some comments to Flat, Qcow2, and Ploop anyway to point out the unintended side-effect (in a separate patch), but I\u0027m not 100% convinced it\u0027s worth the effort to fix right now.","commit_id":"8fa60ed9f6e7d90b83db40c282ec08f0e4f7cdfe"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"cd6d76b552dff7650b115acb19b68a3d1f88e78d","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"        # instance resize and creating the snapshot of the root disk. Only"},{"line_number":1135,"context_line":"        # attempting to remove the root disk snapshot if the instance is not"},{"line_number":1136,"context_line":"        # booting from a volume."},{"line_number":1137,"context_line":"        if not compute_utils.is_volume_backed_instance(context, instance):"},{"line_number":1138,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1139,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."},{"line_number":1140,"context_line":"            # It is set to True here to avoid any upgrade issues surrounding"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_f10e3ae4","line":1137,"range":{"start_line":1137,"start_character":0,"end_line":1137,"end_character":74},"in_reply_to":"bacf61ea_cea9d7e1","updated":"2016-07-27 14:42:50.000000000","message":"IMHO that change in behaviour for the imagebackend is separate to this fix. Why would we even want to waste time checking root_disk.exists() if we know this is a volume backed instance?","commit_id":"8fa60ed9f6e7d90b83db40c282ec08f0e4f7cdfe"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"034f2a1ecd042765905394653ad129d8c2986d20","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"        # instance resize and creating the snapshot of the root disk. Only"},{"line_number":1135,"context_line":"        # attempting to remove the root disk snapshot if the instance is not"},{"line_number":1136,"context_line":"        # booting from a volume."},{"line_number":1137,"context_line":"        if not compute_utils.is_volume_backed_instance(context, instance):"},{"line_number":1138,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1139,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."},{"line_number":1140,"context_line":"            # It is set to True here to avoid any upgrade issues surrounding"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_9e6926fc","line":1137,"range":{"start_line":1137,"start_character":0,"end_line":1137,"end_character":74},"in_reply_to":"bacf61ea_d2033175","updated":"2016-07-31 14:32:17.000000000","message":"Would you mind documenting how you reproduced this in the bug? I\u0027m sure I tested this downstream and didn\u0027t see the same failure to migrate the instance back to the node.\n\nAnyway I\u0027m still adamant that we should move forward with the current bugfix as it is, getting it into stable etc before then looking at re-factoring the imagebackend. I\u0027ll reach out to mdbooth and others who are already looking into this area tomorrow to get their input on this.","commit_id":"8fa60ed9f6e7d90b83db40c282ec08f0e4f7cdfe"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"47158cd762c5ce301880f2e2f0ca6b50e1dda86b","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"        # instance resize and creating the snapshot of the root disk. Only"},{"line_number":1135,"context_line":"        # attempting to remove the root disk snapshot if the instance is not"},{"line_number":1136,"context_line":"        # booting from a volume."},{"line_number":1137,"context_line":"        if not compute_utils.is_volume_backed_instance(context, instance):"},{"line_number":1138,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1139,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."},{"line_number":1140,"context_line":"            # It is set to True here to avoid any upgrade issues surrounding"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_d2033175","line":1137,"range":{"start_line":1137,"start_character":0,"end_line":1137,"end_character":74},"in_reply_to":"bacf61ea_dcb4edd2","updated":"2016-07-27 16:27:25.000000000","message":"so I\u0027ve made one more test and now could confirm that approach you are trying to use isn\u0027t help, if it\u0027s not volume-backed instances, and there is no shared storage, nova leave source node with instance dir, that contains only disk.info. The root cause of that is in my comments above. So please, instead of trying to cover all these cases here, separate image object creation from call to resolve_driver_format().","commit_id":"8fa60ed9f6e7d90b83db40c282ec08f0e4f7cdfe"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"2af5ed68df11423b525763054dbd08fd9a9c4a70","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"        # instance resize and creating the snapshot of the root disk. Only"},{"line_number":1135,"context_line":"        # attempting to remove the root disk snapshot if the instance is not"},{"line_number":1136,"context_line":"        # booting from a volume."},{"line_number":1137,"context_line":"        if not compute_utils.is_volume_backed_instance(context, instance):"},{"line_number":1138,"context_line":"            root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":1139,"context_line":"            # TODO(nic): Set ignore_errors\u003dFalse in a future release."},{"line_number":1140,"context_line":"            # It is set to True here to avoid any upgrade issues surrounding"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_dcb4edd2","line":1137,"range":{"start_line":1137,"start_character":0,"end_line":1137,"end_character":74},"in_reply_to":"bacf61ea_f10e3ae4","updated":"2016-07-27 15:03:08.000000000","message":"Yes, we don\u0027t want. At the same time why we want wasting time by re-creation disk.info if it\u0027s NFS shared storage? \nAt this point we have 2 options: \n1: make this check more complex, trying to figure out all possible questions where we *do not want* to check disk.exists()\n2: refactor Image __init__ to remove this directory re-creation. Still complex refactoring. Need to check all possible use-cases for image object creation.\nI\u0027d prefer to go with 2nd option.","commit_id":"8fa60ed9f6e7d90b83db40c282ec08f0e4f7cdfe"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"4db1ce991e1b4edb48dcf47b6a7930cb00f886ac","unresolved":false,"context_lines":[{"line_number":7359,"context_line":"    def confirm_migration(self, migration, instance, network_info):"},{"line_number":7360,"context_line":"        \"\"\"Confirms a resize, destroying the source VM.\"\"\""},{"line_number":7361,"context_line":""},{"line_number":7362,"context_line":"        # NOTE(wangpan): we get the pre-grizzly instance path firstly,"},{"line_number":7363,"context_line":"        #                so the backup dir of pre-grizzly instance can"},{"line_number":7364,"context_line":"        #                be deleted correctly with grizzly or later nova."},{"line_number":7365,"context_line":"        pre_grizzly_name \u003d libvirt_utils.get_instance_path(instance,"},{"line_number":7366,"context_line":"                                                           forceold\u003dTrue)"},{"line_number":7367,"context_line":"        target \u003d pre_grizzly_name + \u0027_resize\u0027"},{"line_number":7368,"context_line":"        if not os.path.exists(target):"},{"line_number":7369,"context_line":"            target \u003d libvirt_utils.get_instance_path(instance) + \u0027_resize\u0027"},{"line_number":7370,"context_line":""},{"line_number":7371,"context_line":"        root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":7372,"context_line":"        # TODO(nic): Set ignore_errors\u003dFalse in a future release."}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_e100ac04","line":7369,"range":{"start_line":7362,"start_character":8,"end_line":7369,"end_character":74},"updated":"2016-08-04 13:51:29.000000000","message":"this one should be removed, see https://review.openstack.org/#/c/348284/","commit_id":"87dad4065908318d7be2ef8d9bcac7c3555c384c"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"4e57687b44747c984311eeb09459295b3faa75c7","unresolved":false,"context_lines":[{"line_number":7359,"context_line":"    def confirm_migration(self, migration, instance, network_info):"},{"line_number":7360,"context_line":"        \"\"\"Confirms a resize, destroying the source VM.\"\"\""},{"line_number":7361,"context_line":""},{"line_number":7362,"context_line":"        # NOTE(wangpan): we get the pre-grizzly instance path firstly,"},{"line_number":7363,"context_line":"        #                so the backup dir of pre-grizzly instance can"},{"line_number":7364,"context_line":"        #                be deleted correctly with grizzly or later nova."},{"line_number":7365,"context_line":"        pre_grizzly_name \u003d libvirt_utils.get_instance_path(instance,"},{"line_number":7366,"context_line":"                                                           forceold\u003dTrue)"},{"line_number":7367,"context_line":"        target \u003d pre_grizzly_name + \u0027_resize\u0027"},{"line_number":7368,"context_line":"        if not os.path.exists(target):"},{"line_number":7369,"context_line":"            target \u003d libvirt_utils.get_instance_path(instance) + \u0027_resize\u0027"},{"line_number":7370,"context_line":""},{"line_number":7371,"context_line":"        root_disk \u003d self.image_backend.image(instance, \u0027disk\u0027)"},{"line_number":7372,"context_line":"        # TODO(nic): Set ignore_errors\u003dFalse in a future release."}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_edf6dd5b","line":7369,"range":{"start_line":7362,"start_character":8,"end_line":7369,"end_character":74},"in_reply_to":"bacf61ea_e100ac04","updated":"2016-08-04 14:48:27.000000000","message":"Done","commit_id":"87dad4065908318d7be2ef8d9bcac7c3555c384c"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"2b258248921d64d20b5ef8f2906f18afbf8830c4","unresolved":false,"context_lines":[{"line_number":7380,"context_line":"            root_disk.remove_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME,"},{"line_number":7381,"context_line":"                                  ignore_errors\u003dTrue)"},{"line_number":7382,"context_line":""},{"line_number":7383,"context_line":"        # NOTE(lyarwood): Finally remove the instance resize directory"},{"line_number":7384,"context_line":"        if os.path.exists(target):"},{"line_number":7385,"context_line":"            # Deletion can fail over NFS, so retry the deletion as required."},{"line_number":7386,"context_line":"            # Set maximum attempt as 5, most test can remove the directory"},{"line_number":7387,"context_line":"            # for the second time."}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_acc22b83","line":7384,"range":{"start_line":7383,"start_character":8,"end_line":7384,"end_character":34},"updated":"2016-08-04 13:53:42.000000000","message":"afair we are trying to remove instance dir, not _resize one\nAlso, please check Matthew\u0027s proposal on that.","commit_id":"87dad4065908318d7be2ef8d9bcac7c3555c384c"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"4e57687b44747c984311eeb09459295b3faa75c7","unresolved":false,"context_lines":[{"line_number":7380,"context_line":"            root_disk.remove_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME,"},{"line_number":7381,"context_line":"                                  ignore_errors\u003dTrue)"},{"line_number":7382,"context_line":""},{"line_number":7383,"context_line":"        # NOTE(lyarwood): Finally remove the instance resize directory"},{"line_number":7384,"context_line":"        if os.path.exists(target):"},{"line_number":7385,"context_line":"            # Deletion can fail over NFS, so retry the deletion as required."},{"line_number":7386,"context_line":"            # Set maximum attempt as 5, most test can remove the directory"},{"line_number":7387,"context_line":"            # for the second time."}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_0d91b1f1","line":7384,"range":{"start_line":7383,"start_character":8,"end_line":7384,"end_character":34},"in_reply_to":"bacf61ea_acc22b83","updated":"2016-08-04 14:48:27.000000000","message":"Yeah point, further we should only attempt to remove the instance directory when the instance is now remote to the host. I\u0027ll correct this once the above lands. Thanks again.","commit_id":"87dad4065908318d7be2ef8d9bcac7c3555c384c"}]}