)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"70ee160d6bce6885d231d831b6fbfd5a5686f75e","unresolved":false,"context_lines":[{"line_number":15,"context_line":"user (if using authx) to allow writing to the pool where vm images"},{"line_number":16,"context_line":"are stored."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Spec: https://review.openstack.org/188244"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint implement-rbd-snapshots-instead-of-qemu-snapshots"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3a50d1a3_a4a6eabe","line":18,"updated":"2015-07-28 21:23:30.000000000","message":"Use a change ID please rather than URL.","commit_id":"a328da74832b41ade90719fc5786a9095ae5f690"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"a584e076641e4a9cad901012942340a58eaa0c9c","unresolved":false,"context_lines":[{"line_number":15,"context_line":"user (if using authx) to allow writing to the pool where vm images"},{"line_number":16,"context_line":"are stored."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Spec: https://review.openstack.org/188244"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint implement-rbd-snapshots-instead-of-qemu-snapshots"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3a50d1a3_5824a55e","line":18,"in_reply_to":"3a50d1a3_a4a6eabe","updated":"2015-07-29 16:30:51.000000000","message":"Done","commit_id":"a328da74832b41ade90719fc5786a9095ae5f690"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"70ee160d6bce6885d231d831b6fbfd5a5686f75e","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Spec: https://review.openstack.org/188244"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint implement-rbd-snapshots-instead-of-qemu-snapshots"},{"line_number":22,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3a50d1a3_247ada24","line":20,"updated":"2015-07-28 21:23:30.000000000","message":"What is the DocImpact?  You should provide information here for the doc writers since they aren\u0027t nova devs so probably don\u0027t know what docs are impacted or what to write about this.","commit_id":"a328da74832b41ade90719fc5786a9095ae5f690"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"70ee160d6bce6885d231d831b6fbfd5a5686f75e","unresolved":false,"context_lines":[{"line_number":18,"context_line":"Spec: https://review.openstack.org/188244"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint implement-rbd-snapshots-instead-of-qemu-snapshots"},{"line_number":22,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3a50d1a3_c4c69667","line":21,"updated":"2015-07-28 21:23:30.000000000","message":"This doesn\u0027t actually link to anything.  Need to fix up the link.","commit_id":"a328da74832b41ade90719fc5786a9095ae5f690"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"a584e076641e4a9cad901012942340a58eaa0c9c","unresolved":false,"context_lines":[{"line_number":18,"context_line":"Spec: https://review.openstack.org/188244"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint implement-rbd-snapshots-instead-of-qemu-snapshots"},{"line_number":22,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3a50d1a3_78272163","line":21,"in_reply_to":"3a50d1a3_c4c69667","updated":"2015-07-29 16:30:51.000000000","message":"Done","commit_id":"a328da74832b41ade90719fc5786a9095ae5f690"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9303455bf4209290106e7adae87237106c7afe99","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Spec: Ia3666d53e663eacf8c65dbffbd4bc847dd948171"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint rbd-instance-snapshots"},{"line_number":22,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"fa69d971_1f9622f0","line":20,"updated":"2016-01-11 19:19:54.000000000","message":"Seems we should have a reno change for this, to advertise the feature functionality, to point out that if efficient snapshots fail there will be errors in the compute log, and that show_image_direct_url needs to be True in glance configs for this to work - and that exposes a security risk.\n\nIt also looks like show_image_direct_url in glance is only used in glance v2, so we should probably also document that limitation.\n\nhttps://github.com/openstack/glance/blob/master/glance/api/v2/images.py#L748\n\nWhich is concerning given that\u0027s not in glance v3. I\u0027m not sure if that\u0027s a conscious decision by the glance team or not.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Spec: Ia3666d53e663eacf8c65dbffbd4bc847dd948171"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint rbd-instance-snapshots"},{"line_number":22,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"fa69d971_82a6320d","line":20,"in_reply_to":"fa69d971_10bb6fe6","updated":"2016-01-11 20:47:43.000000000","message":"Can you provide a link to where it\u0027s documented?","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":14119,"name":"Sam Yaple","email":"sam+git@yaple.net","username":"SamYaple"},"change_message_id":"5ec63199c80f6bb32b1095b402e524426db8818f","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Spec: Ia3666d53e663eacf8c65dbffbd4bc847dd948171"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint rbd-instance-snapshots"},{"line_number":22,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"fa69d971_10bb6fe6","line":20,"in_reply_to":"fa69d971_1f9622f0","updated":"2016-01-11 19:35:45.000000000","message":"To be fair here Matt, to do a CoW clone when launching the instance from glance requires v2 and show_image_direct_url\u003dTrue currently. And that is documented (and documented as a security risk everywhere i have seen it). The recommendation being run a glance server that is only accessible from trusted hosts.\n\nSo this is already a requirement for existing ceph snapshotting.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0fb2a318ee864107cd24a1ea49cc660bd3d675f8","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Spec: Ia3666d53e663eacf8c65dbffbd4bc847dd948171"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint rbd-instance-snapshots"},{"line_number":22,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"da6ed579_60a8dd20","line":20,"in_reply_to":"fa69d971_2b440148","updated":"2016-01-14 18:42:25.000000000","message":"I\u0027d be happy if we had a release note with this change that also points to http://docs.ceph.com/docs/master/rbd/rbd-openstack/. That doc also doesn\u0027t point out that it only works with glance v2.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":14119,"name":"Sam Yaple","email":"sam+git@yaple.net","username":"SamYaple"},"change_message_id":"a4b31a5e6b1dd954346014b801b82f0757a45c02","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Spec: Ia3666d53e663eacf8c65dbffbd4bc847dd948171"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":"Implements: blueprint rbd-instance-snapshots"},{"line_number":22,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"fa69d971_2b440148","line":20,"in_reply_to":"fa69d971_82a6320d","updated":"2016-01-11 21:08:55.000000000","message":"Youve caught me! I dont think there is a docs.openstack.org link with this info. But for that matter there isn\u0027t a docs.openstack.org link that describes how to do CoW snapshots. That info seems to be solely at http://docs.ceph.com/docs/master/rbd/rbd-openstack/\n\nThe line where it says to use \"show_direct_image_url \u003d True\" is followed by the statement\n\n\"Note that this exposes the back end location via Glance’s API, so the endpoint with this option enabled should not be publicly accessible.\"\n\nPerhaps this just means we need better openstack docs for ceph in general? But overall the docs that do exist explain there is a risk involved.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a28df1c38e9a01186682cdac4ec03b73b770cb50","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Spec: Ia3666d53e663eacf8c65dbffbd4bc847dd948171"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"DocImpact: Ceph and Glance configuration changes"},{"line_number":23,"context_line":"Implements: blueprint rbd-instance-snapshots"},{"line_number":24,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"da6ed579_96d0bb20","line":22,"updated":"2016-01-14 18:57:24.000000000","message":"See comments in ps24, but I\u0027d like to see a release note coupled with this change, see:\n\nhttp://docs.openstack.org/developer/nova/code-review.html#release-notes\n\nIt can simply point out the feature, and also refer to http://docs.ceph.com/docs/master/rbd/rbd-openstack/ for configuration information along with a note that right now glance v2 would have to be used for the show_image_direct_url option. It should probably also note that this is the default behavior now and will log any issues and fallback to the previous behavior (sort of like what\u0027s in the commit message).","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"d42787ba746b23e3e5d8d30a120442cbbab4fa97","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Spec: Ia3666d53e663eacf8c65dbffbd4bc847dd948171"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"DocImpact: Ceph and Glance configuration changes"},{"line_number":23,"context_line":"Implements: blueprint rbd-instance-snapshots"},{"line_number":24,"context_line":"Change-Id: If13d25b6c94e247d2505583b524ae3af9c34b5dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"da6ed579_2a8ddbd5","line":22,"in_reply_to":"da6ed579_96d0bb20","updated":"2016-01-14 20:45:14.000000000","message":"Done","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a5575d4039ff5db0f0d419b5c2158388a7f1984d","unresolved":false,"context_lines":[{"line_number":16001,"context_line":"    def test_raw_with_rbd_clone_graceful_fallback(self, mock_rbd, mock_driver):"},{"line_number":16002,"context_line":"        self.flags(images_type\u003d\u0027rbd\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":16003,"context_line":"        rbd \u003d mock_driver.return_value"},{"line_number":16004,"context_line":"        rbd.parent_info \u003d mock.Mock(side_effect\u003dexception.ImageUnacceptable("},{"line_number":16005,"context_line":"            image_id\u003d\u0027fake_id\u0027, reason\u003d\u0027rbd testing\u0027))"},{"line_number":16006,"context_line":"        with test.nested("},{"line_number":16007,"context_line":"                mock.patch.object(libvirt_driver.imagebackend.images,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_2c184084","line":16004,"range":{"start_line":16004,"start_character":8,"end_line":16004,"end_character":76},"updated":"2016-01-20 20:21:10.000000000","message":"Couldn\u0027t we just be mocking that direct_snapshot fails regardless of it\u0027s internals (like the fact that direct_snapshot calls get_parent_info)?","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"4cf4fdab388cba0e55e96c4319cab4d83b2f1468","unresolved":false,"context_lines":[{"line_number":16001,"context_line":"    def test_raw_with_rbd_clone_graceful_fallback(self, mock_rbd, mock_driver):"},{"line_number":16002,"context_line":"        self.flags(images_type\u003d\u0027rbd\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":16003,"context_line":"        rbd \u003d mock_driver.return_value"},{"line_number":16004,"context_line":"        rbd.parent_info \u003d mock.Mock(side_effect\u003dexception.ImageUnacceptable("},{"line_number":16005,"context_line":"            image_id\u003d\u0027fake_id\u0027, reason\u003d\u0027rbd testing\u0027))"},{"line_number":16006,"context_line":"        with test.nested("},{"line_number":16007,"context_line":"                mock.patch.object(libvirt_driver.imagebackend.images,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_89a6f9bf","line":16004,"range":{"start_line":16004,"start_character":8,"end_line":16004,"end_character":76},"in_reply_to":"9a68dd71_2c184084","updated":"2016-01-20 21:49:18.000000000","message":"We could, but I wanted all the other various and sundry checks that self._test_snapshot() performs as well.\n\nI can address this in a follow-on patch if you think the other snapshot tests exercise these code paths adequately enough.","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a5575d4039ff5db0f0d419b5c2158388a7f1984d","unresolved":false,"context_lines":[{"line_number":16019,"context_line":"    def test_raw_with_rbd_clone_eperm(self, mock_rbd, mock_driver):"},{"line_number":16020,"context_line":"        self.flags(images_type\u003d\u0027rbd\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":16021,"context_line":"        rbd \u003d mock_driver.return_value"},{"line_number":16022,"context_line":"        rbd.parent_info \u003d mock.Mock(return_value\u003d[\u0027test-pool\u0027, \u0027\u0027, \u0027\u0027])"},{"line_number":16023,"context_line":"        rbd.parse_url \u003d mock.Mock(return_value\u003d[\u0027a\u0027, \u0027b\u0027, \u0027c\u0027, \u0027d\u0027])"},{"line_number":16024,"context_line":"        rbd.clone \u003d mock.Mock(side_effect\u003dexception.Forbidden("},{"line_number":16025,"context_line":"                image_id\u003d\u0027fake_id\u0027, reason\u003d\u0027rbd testing\u0027))"},{"line_number":16026,"context_line":"        with test.nested("},{"line_number":16027,"context_line":"                mock.patch.object(libvirt_driver.imagebackend.images,"},{"line_number":16028,"context_line":"                                  \u0027convert_image\u0027,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_27829b53","line":16025,"range":{"start_line":16022,"start_character":8,"end_line":16025,"end_character":58},"updated":"2016-01-20 20:21:10.000000000","message":"Same as above, I don\u0027t see why the libvirt driver tests need to mock out the internals of the rbd image backend code to test the failure scenarios in the libvirt driver. All the libvirt driver code cares about is that direct_snapshot raises Forbidden.\n\nI don\u0027t want to nit pick on this too much, but it is kind of a gross tight coupling in the libvirt driver unit tests which goes beyond what\u0027s being tested in the driver - it requires the test to know the internals of a specific image bakend (and in some cases the rbd_utils). Some people don\u0027t mind that but I\u0027m not a fan since if rbd_utils changes then this test has to also be updated.","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a5575d4039ff5db0f0d419b5c2158388a7f1984d","unresolved":false,"context_lines":[{"line_number":16033,"context_line":"                mock.patch.object(fake_libvirt_utils, \u0027disk_type\u0027, new\u003d\u0027rbd\u0027)):"},{"line_number":16034,"context_line":"            self._test_snapshot(disk_format\u003d\u0027raw\u0027)"},{"line_number":16035,"context_line":"            # Ensure that the direct_snapshot attempt was cleaned up"},{"line_number":16036,"context_line":"            rbd.remove_snap.assert_called_with(\u0027c\u0027, \u0027d\u0027, ignore_errors\u003dFalse,"},{"line_number":16037,"context_line":"                                               pool\u003d\u0027b\u0027, force\u003dTrue)"},{"line_number":16038,"context_line":""},{"line_number":16039,"context_line":"    @mock.patch.object(rbd_utils, \u0027RBDDriver\u0027)"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_2737fbb1","line":16036,"range":{"start_line":16036,"start_character":12,"end_line":16036,"end_character":46},"updated":"2016-01-20 20:21:10.000000000","message":"Similar, seems we should just be testing that snapshot_backend.cleanup_direct_snapshot was called, not the internals of it.","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8813a354082c5f9265ea0a48ff28441ddfa8e8a7","unresolved":false,"context_lines":[{"line_number":16052,"context_line":"                mock.patch.object(fake_libvirt_utils, \u0027find_disk\u0027,"},{"line_number":16053,"context_line":"                                  return_value\u003d(\u0027rbd://some/fake/rbd/image\u0027,"},{"line_number":16054,"context_line":"                                                \u0027raw\u0027)),"},{"line_number":16055,"context_line":"                mock.patch.object(fake_libvirt_utils, \u0027file_open\u0027,"},{"line_number":16056,"context_line":"                                  side_effect\u003dIOError()),"},{"line_number":16057,"context_line":"                mock.patch.object(fake_libvirt_utils, \u0027disk_type\u0027, new\u003d\u0027rbd\u0027)):"},{"line_number":16058,"context_line":"            self.assertRaises(IOError, self._test_snapshot, disk_format\u003d\u0027raw\u0027)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9a68dd71_32265269","line":16055,"range":{"start_line":16055,"start_character":16,"end_line":16055,"end_character":66},"updated":"2016-01-20 22:16:01.000000000","message":"This is causing the error inside the fallback block, which is not what the generic Exception is handling on L1566.\n\nJust make rbd.clone raise nova.test.TestingException or whatever it is (or AttributeError/ValueError, whatevs).","commit_id":"a1901ef32c2034f29d8a3d113f3f3e7728cb7c5b"}],"nova/tests/unit/virt/libvirt/test_imagebackend.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a5575d4039ff5db0f0d419b5c2158388a7f1984d","unresolved":false,"context_lines":[{"line_number":1459,"context_line":"            mock_pi.side_effect \u003d exception.ImageUnacceptable(image_id\u003d\u0027test\u0027,"},{"line_number":1460,"context_line":"                                                              reason\u003d\u0027test\u0027)"},{"line_number":1461,"context_line":"            mock_get.return_value \u003d {\u0027locations\u0027: [{\u0027url\u0027: rbd_uri}]}"},{"line_number":1462,"context_line":"            image._get_parent_pool(self.CONTEXT, \u0027fake-image\u0027, self.FSID)"},{"line_number":1463,"context_line":""},{"line_number":1464,"context_line":"    def test_get_parent_pool_non_local_image(self):"},{"line_number":1465,"context_line":"        image \u003d self.image_class(self.INSTANCE, self.NAME)"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_0c3a1c37","line":1462,"range":{"start_line":1462,"start_character":12,"end_line":1462,"end_character":34},"updated":"2016-01-20 20:21:10.000000000","message":"Assert the expected result from this?","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"4cf4fdab388cba0e55e96c4319cab4d83b2f1468","unresolved":false,"context_lines":[{"line_number":1459,"context_line":"            mock_pi.side_effect \u003d exception.ImageUnacceptable(image_id\u003d\u0027test\u0027,"},{"line_number":1460,"context_line":"                                                              reason\u003d\u0027test\u0027)"},{"line_number":1461,"context_line":"            mock_get.return_value \u003d {\u0027locations\u0027: [{\u0027url\u0027: rbd_uri}]}"},{"line_number":1462,"context_line":"            image._get_parent_pool(self.CONTEXT, \u0027fake-image\u0027, self.FSID)"},{"line_number":1463,"context_line":""},{"line_number":1464,"context_line":"    def test_get_parent_pool_non_local_image(self):"},{"line_number":1465,"context_line":"        image \u003d self.image_class(self.INSTANCE, self.NAME)"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_093f69b5","line":1462,"range":{"start_line":1462,"start_character":12,"end_line":1462,"end_character":34},"in_reply_to":"9a68dd71_0c3a1c37","updated":"2016-01-20 21:49:18.000000000","message":"Whoops! Done","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"f2c961ffacf73ee9f6ae0986ebd52da3cac42712","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"            metadata[\u0027location\u0027] \u003d snapshot_backend.direct_snapshot("},{"line_number":1372,"context_line":"                context, snapshot_name, image_format, image_id,"},{"line_number":1373,"context_line":"                instance.image_ref)"},{"line_number":1374,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1375,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1376,"context_line":"            self._snapshot_domain(context, live_snapshot, virt_dom, state,"},{"line_number":1377,"context_line":"                                  instance)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a50d1a3_e02f9e86","line":1374,"updated":"2015-07-30 13:40:17.000000000","message":"If this update fails you are left with an orphaned flattened clone in Glance\u0027s pool- essentially a storage leak.  This call should be put in a try block and if it fails a followup call should be made to the snapshot_backend to remove the clone from the value of metadata[\u0027location\u0027].","commit_id":"5ad9547d716e172760f4320d1ffd6c8f48b5990e"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"dcc2261fdc5221950f0b4e142ceb9fb1fce9028c","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"            metadata[\u0027location\u0027] \u003d snapshot_backend.direct_snapshot("},{"line_number":1372,"context_line":"                context, snapshot_name, image_format, image_id,"},{"line_number":1373,"context_line":"                instance.image_ref)"},{"line_number":1374,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1375,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1376,"context_line":"            self._snapshot_domain(context, live_snapshot, virt_dom, state,"},{"line_number":1377,"context_line":"                                  instance)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a50d1a3_3dcda4b7","line":1374,"in_reply_to":"3a50d1a3_e02f9e86","updated":"2015-07-30 16:56:41.000000000","message":"Done","commit_id":"5ad9547d716e172760f4320d1ffd6c8f48b5990e"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"d468b9dab8154a64a4550d851efe038c2238b2e0","unresolved":false,"context_lines":[{"line_number":1376,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1377,"context_line":"            self._snapshot_domain(context, live_snapshot, virt_dom, state,"},{"line_number":1378,"context_line":"                                  instance)"},{"line_number":1379,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1380,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1381,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1382,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_80348ffb","line":1379,"updated":"2015-07-30 17:52:18.000000000","message":"I\u0027m not sure of all the possible exceptions that _image_api.update would through for any particular glance client or server side failure on that update.  The cleanup_direct_snapshot may be necessary in a separate try-except around that update call.","commit_id":"52b2357c136e1948c0d6eb9d932e2c188a6dfa6a"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"1476908a94a0c6b692d5f7ff11289993d2f72693","unresolved":false,"context_lines":[{"line_number":1376,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1377,"context_line":"            self._snapshot_domain(context, live_snapshot, virt_dom, state,"},{"line_number":1378,"context_line":"                                  instance)"},{"line_number":1379,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1380,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1381,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1382,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_1691a552","line":1379,"in_reply_to":"3a50d1a3_80348ffb","updated":"2015-07-30 18:10:11.000000000","message":"Let\u0027s not over-complicate things: the only error from Glance at this point that we can do anything about is Forbidden, because if it\u0027s any other error condition, the entire operation is about to fail in very strange ways.  This is essentially applying the same standard of care as the QEMU snapshots get.","commit_id":"52b2357c136e1948c0d6eb9d932e2c188a6dfa6a"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"d468b9dab8154a64a4550d851efe038c2238b2e0","unresolved":false,"context_lines":[{"line_number":1380,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1381,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1382,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1383,"context_line":"            snapshot_backend.cleanup_direct_snapshot(failed_snap,"},{"line_number":1384,"context_line":"                                                     ignore_errors\u003dTrue)"},{"line_number":1385,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"},{"line_number":1386,"context_line":"                              expected_state\u003dtask_states.IMAGE_UPLOADING)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_db44c8b8","line":1383,"updated":"2015-07-30 17:52:18.000000000","message":"The location in meta is a full RBD URL to a protected snapshot of a flattened clone including fsid, pool, object name, snap name.\n\nDoes this method\u0027s implementation succesfully remove both the snapshot and the flattened clone as we would need to do when the image update fails?\n\nI\u0027m asking because my knowledge of the underlying RBD commands isn\u0027t that great when it comes to this and I don\u0027t have time to look it up.\n\nBe careful in changing this method to remove the parent of the snapshot, which would be the flattened clone of the VM\u0027s disk.  If this method is re-used to cleanup the temporary snapshot of the VM\u0027s disk during an error on clone/flatten as it currently is in imagebackend, this would likely result in an attempt to remove the VM\u0027s disk if my thinking is correct.\n\nI\u0027m thinking you need two separate cleanup methods, one to cleanup the temporary snapshot of the VM disk that is used to create the clone that is then flattened, and a separate one to cleanup the flattened clone and its protected snapshot that is sent to Glance, if the Glance update fails.\n\nYou could probably get away with 1 method to do the Glance cleanup and just fall back to doing an unprotect/ remove snap on the temp VM disk snapshot in imagebackend.","commit_id":"52b2357c136e1948c0d6eb9d932e2c188a6dfa6a"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"1476908a94a0c6b692d5f7ff11289993d2f72693","unresolved":false,"context_lines":[{"line_number":1380,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1381,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1382,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1383,"context_line":"            snapshot_backend.cleanup_direct_snapshot(failed_snap,"},{"line_number":1384,"context_line":"                                                     ignore_errors\u003dTrue)"},{"line_number":1385,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"},{"line_number":1386,"context_line":"                              expected_state\u003dtask_states.IMAGE_UPLOADING)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_f6c8b958","line":1383,"in_reply_to":"3a50d1a3_db44c8b8","updated":"2015-07-30 18:10:11.000000000","message":"Oh god damn it","commit_id":"52b2357c136e1948c0d6eb9d932e2c188a6dfa6a"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"10e1ec88ebdac0f1a119cf10331908c98c44cbae","unresolved":false,"context_lines":[{"line_number":1380,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1381,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1382,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1383,"context_line":"            snapshot_backend.cleanup_direct_snapshot(failed_snap, destroy\u003dTrue,"},{"line_number":1384,"context_line":"                                                     ignore_errors\u003dTrue)"},{"line_number":1385,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"},{"line_number":1386,"context_line":"                              expected_state\u003dtask_states.IMAGE_UPLOADING)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_06339414","line":1383,"updated":"2015-07-31 12:33:03.000000000","message":"The direct snapshot cleanup should move to a try-finally that is placed around the image_api.update and _snapshot_domain calls above.  Those calls could fail in any number of ways and we need to ensure the allocated storage is cleaned up in all of them.  For the normal snapshot flow below, \"with utils.tempdir(dir\u003dsnapshot_directory)\" is used and the snapshot image is written to that temp directory.  The context manager on tempdir removes the directory and the snapshot file in a finally block so that the storage is released in any error case.","commit_id":"151589e86cb16ab8d4aeb7477c353550296b21d1"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"8bbfdf3f7f391ab78d78358c822548af82d3debb","unresolved":false,"context_lines":[{"line_number":1380,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1381,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1382,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1383,"context_line":"            snapshot_backend.cleanup_direct_snapshot(failed_snap, destroy\u003dTrue,"},{"line_number":1384,"context_line":"                                                     ignore_errors\u003dTrue)"},{"line_number":1385,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"},{"line_number":1386,"context_line":"                              expected_state\u003dtask_states.IMAGE_UPLOADING)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_9e46f979","line":1383,"in_reply_to":"3a50d1a3_06339414","updated":"2015-07-31 17:55:01.000000000","message":"Done","commit_id":"151589e86cb16ab8d4aeb7477c353550296b21d1"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"3cfe5d2258cfd4b87e96ad9b499b51f8f69cb3e4","unresolved":false,"context_lines":[{"line_number":1377,"context_line":"                instance.image_ref)"},{"line_number":1378,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1379,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1380,"context_line":"            self._snapshot_domain(context, live_snapshot, virt_dom, state,"},{"line_number":1381,"context_line":"                                  instance)"},{"line_number":1382,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1383,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""}],"source_content_type":"text/x-python","patch_set":13,"id":"1a4dcd0f_2ee1c0d0","line":1380,"updated":"2015-08-04 20:43:23.000000000","message":"Calling this method after the _image_api.update() call caused weird and random failures with libvirt when we deployed it.  Moving it to beforehand clears things up.  It\u0027s some odd order-of-operations bug.","commit_id":"aedb4194d7288573aed70cf46e066ab74f252dbd"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"51f2f22652557ab59cecc281f92cb89f84e6cae2","unresolved":false,"context_lines":[{"line_number":1392,"context_line":"        #               future."},{"line_number":1393,"context_line":"        if (self._host.has_min_version(MIN_LIBVIRT_LIVESNAPSHOT_VERSION,"},{"line_number":1394,"context_line":"                                       MIN_QEMU_LIVESNAPSHOT_VERSION,"},{"line_number":1395,"context_line":"                                       host.HV_DRIVER_QEMU)"},{"line_number":1396,"context_line":"             and source_format not in (\u0027lvm\u0027, \u0027rbd\u0027)"},{"line_number":1397,"context_line":"             and not CONF.ephemeral_storage_encryption.enabled"},{"line_number":1398,"context_line":"             and not CONF.workarounds.disable_libvirt_livesnapshot):"}],"source_content_type":"text/x-python","patch_set":18,"id":"da20952f_e51bd4c8","line":1395,"updated":"2015-09-03 16:40:16.000000000","message":"Question: Would it be safe to enable live snapshots of instances on RBD-storage with your patch, or do we need more work to make that possible?","commit_id":"f5b860b9581dc07d1fe0aa78e4fd1f18f018674e"},{"author":{"_account_id":6984,"name":"Sébastien Han","email":"seb@redhat.com","username":"leseb"},"change_message_id":"61ef8de74ebdaa39dd734ff56d56faeecebe770a","unresolved":false,"context_lines":[{"line_number":1392,"context_line":"        #               future."},{"line_number":1393,"context_line":"        if (self._host.has_min_version(MIN_LIBVIRT_LIVESNAPSHOT_VERSION,"},{"line_number":1394,"context_line":"                                       MIN_QEMU_LIVESNAPSHOT_VERSION,"},{"line_number":1395,"context_line":"                                       host.HV_DRIVER_QEMU)"},{"line_number":1396,"context_line":"             and source_format not in (\u0027lvm\u0027, \u0027rbd\u0027)"},{"line_number":1397,"context_line":"             and not CONF.ephemeral_storage_encryption.enabled"},{"line_number":1398,"context_line":"             and not CONF.workarounds.disable_libvirt_livesnapshot):"}],"source_content_type":"text/x-python","patch_set":18,"id":"da20952f_830f4f38","line":1395,"in_reply_to":"da20952f_2be508a3","updated":"2015-09-04 09:45:56.000000000","message":"+1 agree, it has been hard enough to get to this point. Further improvements should be addressed by new patches.So let\u0027s have this land first, then we will talk about enhancements :).","commit_id":"f5b860b9581dc07d1fe0aa78e4fd1f18f018674e"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"d2c41b698d9580cacc960689437fab0911073434","unresolved":false,"context_lines":[{"line_number":1392,"context_line":"        #               future."},{"line_number":1393,"context_line":"        if (self._host.has_min_version(MIN_LIBVIRT_LIVESNAPSHOT_VERSION,"},{"line_number":1394,"context_line":"                                       MIN_QEMU_LIVESNAPSHOT_VERSION,"},{"line_number":1395,"context_line":"                                       host.HV_DRIVER_QEMU)"},{"line_number":1396,"context_line":"             and source_format not in (\u0027lvm\u0027, \u0027rbd\u0027)"},{"line_number":1397,"context_line":"             and not CONF.ephemeral_storage_encryption.enabled"},{"line_number":1398,"context_line":"             and not CONF.workarounds.disable_libvirt_livesnapshot):"}],"source_content_type":"text/x-python","patch_set":18,"id":"da20952f_2be508a3","line":1395,"in_reply_to":"da20952f_e51bd4c8","updated":"2015-09-03 23:16:28.000000000","message":"I haven\u0027t looked into it at all, but it seems that even if it were safe to do so, that\u0027s a separate concern than what this patch set addresses, and should be done in a follow-up patch.","commit_id":"f5b860b9581dc07d1fe0aa78e4fd1f18f018674e"},{"author":{"_account_id":6463,"name":"Jackie","email":"guanqiang85@gmail.com","username":"hzguanqiang"},"change_message_id":"590e5d4a504b0a96f2e59747f0d54ad27318b54f","unresolved":false,"context_lines":[{"line_number":1430,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1431,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1432,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1433,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1434,"context_line":"            snapshot_backend.cleanup_direct_snapshot(failed_snap, destroy\u003dTrue,"},{"line_number":1435,"context_line":"                                                     ignore_errors\u003dTrue)"},{"line_number":1436,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba8a016a_0e1ef95e","line":1433,"updated":"2015-11-19 02:50:19.000000000","message":"failed_snap  returned from direct_snapshot() is a string, but in function cleanup_direct_snapshot it will be taken as a dict.","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":6463,"name":"Jackie","email":"guanqiang85@gmail.com","username":"hzguanqiang"},"change_message_id":"8df80c8ca3e12f5b435cdb2c46bce9c320496f12","unresolved":false,"context_lines":[{"line_number":1430,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1431,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1432,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1433,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1434,"context_line":"            snapshot_backend.cleanup_direct_snapshot(failed_snap, destroy\u003dTrue,"},{"line_number":1435,"context_line":"                                                     ignore_errors\u003dTrue)"},{"line_number":1436,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba8a016a_745b472f","line":1433,"in_reply_to":"ba8a016a_0e1ef95e","updated":"2015-11-23 02:48:05.000000000","message":"if failed_snap:\n    failed_snap \u003d {\u0027url\u0027: str(failed_snap)}\n\nSomething like this need to be done here, or cleanup_direct_snapshot will raise a TypeError","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"95ef953892d27c777e7932962b9ee3f4f5642778","unresolved":false,"context_lines":[{"line_number":1430,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1431,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1432,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1433,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1434,"context_line":"            snapshot_backend.cleanup_direct_snapshot(failed_snap, destroy\u003dTrue,"},{"line_number":1435,"context_line":"                                                     ignore_errors\u003dTrue)"},{"line_number":1436,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa69d971_c889eecb","line":1433,"in_reply_to":"ba8a016a_745b472f","updated":"2016-01-09 00:55:36.000000000","message":"Done","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":6463,"name":"Jackie","email":"guanqiang85@gmail.com","username":"hzguanqiang"},"change_message_id":"d51804e27837e9ebf5803f1b87a7b7f2581aec47","unresolved":false,"context_lines":[{"line_number":1464,"context_line":"                                           image_id,"},{"line_number":1465,"context_line":"                                           metadata,"},{"line_number":1466,"context_line":"                                           image_file)"},{"line_number":1467,"context_line":"        except Exception:"},{"line_number":1468,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1469,"context_line":"                LOG.exception(_LE(\"Failed to snapshot image\"))"},{"line_number":1470,"context_line":"                failed_snap \u003d metadata.pop(\u0027location\u0027, None)"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba8a016a_c96bcbef","line":1467,"updated":"2015-11-20 02:19:53.000000000","message":"If clone in direct_snapshot() failed, exception rather than(exception.ImageUnacceptable, exception.Forbidden) will be thrown, and instance will never be resumed.","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"95ef953892d27c777e7932962b9ee3f4f5642778","unresolved":false,"context_lines":[{"line_number":1464,"context_line":"                                           image_id,"},{"line_number":1465,"context_line":"                                           metadata,"},{"line_number":1466,"context_line":"                                           image_file)"},{"line_number":1467,"context_line":"        except Exception:"},{"line_number":1468,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1469,"context_line":"                LOG.exception(_LE(\"Failed to snapshot image\"))"},{"line_number":1470,"context_line":"                failed_snap \u003d metadata.pop(\u0027location\u0027, None)"}],"source_content_type":"text/x-python","patch_set":23,"id":"fa69d971_28a20a4c","line":1467,"in_reply_to":"ba8a016a_c96bcbef","updated":"2016-01-09 00:55:36.000000000","message":"Correct.  If the prior snapshot operations succeed and the clone step fails, that is a serious/fatal error, and the \"fallback\" behavior would be harmful, and should not be attempted.","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":1464,"context_line":"        update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"        try:"},{"line_number":1467,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_UPLOADING,"},{"line_number":1468,"context_line":"                              expected_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1469,"context_line":"            metadata[\u0027location\u0027] \u003d snapshot_backend.direct_snapshot("},{"line_number":1470,"context_line":"                context, snapshot_name, image_format, image_id,"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_82cb92a3","line":1467,"updated":"2016-01-11 20:47:43.000000000","message":"Why do we need the task_state update on L1464 if we\u0027re just going to change it right here anyway?","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"3b14c3bcd7bfc1a18e69a8d12d02663ce34c8d2c","unresolved":false,"context_lines":[{"line_number":1464,"context_line":"        update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"        try:"},{"line_number":1467,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_UPLOADING,"},{"line_number":1468,"context_line":"                              expected_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1469,"context_line":"            metadata[\u0027location\u0027] \u003d snapshot_backend.direct_snapshot("},{"line_number":1470,"context_line":"                context, snapshot_name, image_format, image_id,"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_72f8697d","line":1467,"in_reply_to":"da6ed579_9e25e95d","updated":"2016-01-12 02:51:55.000000000","message":"...to how the \"fallback\" version does things.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"bd984f3a91b7014235064ee2803406232ca9c743","unresolved":false,"context_lines":[{"line_number":1464,"context_line":"        update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"        try:"},{"line_number":1467,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_UPLOADING,"},{"line_number":1468,"context_line":"                              expected_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1469,"context_line":"            metadata[\u0027location\u0027] \u003d snapshot_backend.direct_snapshot("},{"line_number":1470,"context_line":"                context, snapshot_name, image_format, image_id,"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_9e25e95d","line":1467,"in_reply_to":"fa69d971_82cb92a3","updated":"2016-01-12 02:51:01.000000000","message":"This was done so that the task_states end up being indentical","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9303455bf4209290106e7adae87237106c7afe99","unresolved":false,"context_lines":[{"line_number":1484,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"},{"line_number":1485,"context_line":"                              expected_state\u003dtask_states.IMAGE_UPLOADING)"},{"line_number":1486,"context_line":""},{"line_number":1487,"context_line":"            snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1488,"context_line":"            fileutils.ensure_tree(snapshot_directory)"},{"line_number":1489,"context_line":"            with utils.tempdir(dir\u003dsnapshot_directory) as tmpdir:"},{"line_number":1490,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_9ad220ff","line":1487,"updated":"2016-01-11 19:19:54.000000000","message":"The size of this overall snapshot method is getting huge, can we pull the existing code that\u0027s being re-used here (the standard snapshot method) into a separate private method so we just don\u0027t have so much going on here? Right now the snapshot method is 166 LOC, breaking this standard snapshot code out into a private method would be easier to readability and unit testing. Similar to what you did with _snapshot_domain.\n\nI\u0027d even probably break that out in a separate dependent patch just so that\u0027s an easy change to build on.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"7da4db666c476a166e611478f582d1e050ee9ff6","unresolved":false,"context_lines":[{"line_number":1484,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"},{"line_number":1485,"context_line":"                              expected_state\u003dtask_states.IMAGE_UPLOADING)"},{"line_number":1486,"context_line":""},{"line_number":1487,"context_line":"            snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1488,"context_line":"            fileutils.ensure_tree(snapshot_directory)"},{"line_number":1489,"context_line":"            with utils.tempdir(dir\u003dsnapshot_directory) as tmpdir:"},{"line_number":1490,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_a8d0aeca","line":1487,"in_reply_to":"da6ed579_32aaf11a","updated":"2016-01-12 04:07:38.000000000","message":"What nic said. This being inline is so much easier to read, please don\u0027t break it out.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0fb2a318ee864107cd24a1ea49cc660bd3d675f8","unresolved":false,"context_lines":[{"line_number":1484,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"},{"line_number":1485,"context_line":"                              expected_state\u003dtask_states.IMAGE_UPLOADING)"},{"line_number":1486,"context_line":""},{"line_number":1487,"context_line":"            snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1488,"context_line":"            fileutils.ensure_tree(snapshot_directory)"},{"line_number":1489,"context_line":"            with utils.tempdir(dir\u003dsnapshot_directory) as tmpdir:"},{"line_number":1490,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_fbb80077","line":1487,"in_reply_to":"da6ed579_a8d0aeca","updated":"2016-01-14 18:42:25.000000000","message":"As noted in IRC, arguably easier to read, much more complicated and gross to unit test, but I won\u0027t block on this.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"bc88cb7367634ee281e04fbd496884cdad303ffa","unresolved":false,"context_lines":[{"line_number":1484,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"},{"line_number":1485,"context_line":"                              expected_state\u003dtask_states.IMAGE_UPLOADING)"},{"line_number":1486,"context_line":""},{"line_number":1487,"context_line":"            snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1488,"context_line":"            fileutils.ensure_tree(snapshot_directory)"},{"line_number":1489,"context_line":"            with utils.tempdir(dir\u003dsnapshot_directory) as tmpdir:"},{"line_number":1490,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_32aaf11a","line":1487,"in_reply_to":"fa69d971_9ad220ff","updated":"2016-01-12 03:01:11.000000000","message":"Our internal version of this feature does have this broken out into a separate method, and it is for that very reason that the one I submitted upstream is inline.  It\u0027s just better this way.\n\nIf you really, really, REALLY want it broken out into a method with 16 positional arguments that only ever has one caller, then I\u0027ll acquiesce, but having worked with both, I don\u0027t recommend it.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50744e813abd9d1967cbc0668fdff03b830c9834","unresolved":false,"context_lines":[{"line_number":1506,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1507,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1508,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1509,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1510,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1511,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1512,"context_line":"            if failed_snap:"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_0f557cc4","line":1509,"range":{"start_line":1509,"start_character":16,"end_line":1509,"end_character":20},"updated":"2016-01-15 02:31:36.000000000","message":"Should probably be warning.","commit_id":"8e580d3c52217ae42401c5012b724308d301934d"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"2148b50ff33f2363914bb3355c3db005db60ba33","unresolved":false,"context_lines":[{"line_number":1506,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1507,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1508,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1509,"context_line":"            LOG.warn(_LW(\"Performing standard snapshot because \""},{"line_number":1510,"context_line":"                         \"direct snapshot failed: %(error)s\"), {\u0027error\u0027: e})"},{"line_number":1511,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1512,"context_line":"            if failed_snap:"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_c845f06e","line":1509,"range":{"start_line":1509,"start_character":16,"end_line":1509,"end_character":20},"in_reply_to":"da6ed579_0f557cc4","updated":"2016-01-15 18:27:24.000000000","message":"Done","commit_id":"8e580d3c52217ae42401c5012b724308d301934d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5deb656a187a8aedee0f9b67c46d186706f5afb9","unresolved":false,"context_lines":[{"line_number":1506,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1507,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1508,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1509,"context_line":"            LOG.warning(_LW(\u0027Performing standard snapshot because direct\u0027"},{"line_number":1510,"context_line":"                            \u0027snapshot failed: %(error)s\u0027), {\u0027error\u0027: e})"},{"line_number":1511,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1512,"context_line":"            if failed_snap:"}],"source_content_type":"text/x-python","patch_set":32,"id":"da6ed579_238f0f76","line":1509,"range":{"start_line":1509,"start_character":66,"end_line":1509,"end_character":72},"updated":"2016-01-15 18:32:23.000000000","message":"Need a space between \u0027direct\u0027 and \u0027snapshot\u0027.","commit_id":"3539276125ef1056667a16b6ccbd7414f4829cd7"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"a3d9eeb205c577392bd58b4ac008b400d72995b8","unresolved":false,"context_lines":[{"line_number":1506,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1507,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1508,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1509,"context_line":"            LOG.warning(_LW(\u0027Performing standard snapshot because direct\u0027"},{"line_number":1510,"context_line":"                            \u0027snapshot failed: %(error)s\u0027), {\u0027error\u0027: e})"},{"line_number":1511,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1512,"context_line":"            if failed_snap:"}],"source_content_type":"text/x-python","patch_set":32,"id":"da6ed579_43bec3ae","line":1509,"range":{"start_line":1509,"start_character":66,"end_line":1509,"end_character":72},"in_reply_to":"da6ed579_238f0f76","updated":"2016-01-15 18:37:16.000000000","message":"Oh FFS","commit_id":"3539276125ef1056667a16b6ccbd7414f4829cd7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":1505,"context_line":"                                  instance)"},{"line_number":1506,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1507,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1508,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1509,"context_line":"            LOG.warning(_LW(\u0027Performing standard snapshot because direct \u0027"},{"line_number":1510,"context_line":"                            \u0027snapshot failed: %(error)s\u0027), {\u0027error\u0027: e})"},{"line_number":1511,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_d9014406","line":1508,"range":{"start_line":1508,"start_character":45,"end_line":1508,"end_character":64},"updated":"2016-01-15 19:06:53.000000000","message":"I don\u0027t see any test case for this.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":1505,"context_line":"                                  instance)"},{"line_number":1506,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1507,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1508,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1509,"context_line":"            LOG.warning(_LW(\u0027Performing standard snapshot because direct \u0027"},{"line_number":1510,"context_line":"                            \u0027snapshot failed: %(error)s\u0027), {\u0027error\u0027: e})"},{"line_number":1511,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_585d4947","line":1508,"range":{"start_line":1508,"start_character":45,"end_line":1508,"end_character":64},"in_reply_to":"da6ed579_d9014406","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":1511,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1512,"context_line":"            if failed_snap:"},{"line_number":1513,"context_line":"                failed_snap \u003d {\u0027url\u0027: str(failed_snap)}"},{"line_number":1514,"context_line":"            snapshot_backend.cleanup_direct_snapshot(failed_snap,"},{"line_number":1515,"context_line":"                                                     also_destroy_volume\u003dTrue,"},{"line_number":1516,"context_line":"                                                     ignore_errors\u003dTrue)"},{"line_number":1517,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD,"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_991e6c5d","line":1514,"range":{"start_line":1514,"start_character":12,"end_line":1514,"end_character":52},"updated":"2016-01-15 19:06:53.000000000","message":"Where do you unit test the call to this?","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":1545,"context_line":"                                           image_id,"},{"line_number":1546,"context_line":"                                           metadata,"},{"line_number":1547,"context_line":"                                           image_file)"},{"line_number":1548,"context_line":"        except Exception:"},{"line_number":1549,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1550,"context_line":"                LOG.exception(_LE(\"Failed to snapshot image\"))"},{"line_number":1551,"context_line":"                failed_snap \u003d metadata.pop(\u0027location\u0027, None)"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_593d54c2","line":1548,"range":{"start_line":1548,"start_character":8,"end_line":1548,"end_character":24},"updated":"2016-01-15 19:06:53.000000000","message":"I don\u0027t see this case tested anywhere.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":1545,"context_line":"                                           image_id,"},{"line_number":1546,"context_line":"                                           metadata,"},{"line_number":1547,"context_line":"                                           image_file)"},{"line_number":1548,"context_line":"        except Exception:"},{"line_number":1549,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1550,"context_line":"                LOG.exception(_LE(\"Failed to snapshot image\"))"},{"line_number":1551,"context_line":"                failed_snap \u003d metadata.pop(\u0027location\u0027, None)"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_5e1f7109","line":1548,"range":{"start_line":1548,"start_character":8,"end_line":1548,"end_character":24},"in_reply_to":"da6ed579_593d54c2","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a5575d4039ff5db0f0d419b5c2158388a7f1984d","unresolved":false,"context_lines":[{"line_number":1522,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1523,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1524,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1525,"context_line":"            LOG.warning(_LW(\u0027Performing standard snapshot because direct \u0027"},{"line_number":1526,"context_line":"                            \u0027snapshot failed: %(error)s\u0027), {\u0027error\u0027: e})"},{"line_number":1527,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1528,"context_line":"            if failed_snap:"},{"line_number":1529,"context_line":"                failed_snap \u003d {\u0027url\u0027: str(failed_snap)}"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_d1925d57","line":1526,"range":{"start_line":1525,"start_character":12,"end_line":1526,"end_character":72},"updated":"2016-01-20 20:21:10.000000000","message":"I just thought about this, we\u0027re going to log this every time for non-rbd backends because the base image class method raises ImageUnacceptable. Maybe that should be raising NotImplementedError instead and then we don\u0027t log it but instead just do the standard snapshot.\n\nLike in this job:\n\nhttp://logs.openstack.org/82/205282/39/check/gate-tempest-dsvm-full/06003c5/logs/screen-n-cpu.txt.gz?level\u003dTRACE#_2016-01-20_16_37_12_966\n\nWe show that warning 16 times. We shouldn\u0027t be logging a warning for this in the non-rbd case.","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"4cf4fdab388cba0e55e96c4319cab4d83b2f1468","unresolved":false,"context_lines":[{"line_number":1522,"context_line":"            self._image_api.update(context, image_id, metadata,"},{"line_number":1523,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1524,"context_line":"        except (exception.ImageUnacceptable, exception.Forbidden) as e:"},{"line_number":1525,"context_line":"            LOG.warning(_LW(\u0027Performing standard snapshot because direct \u0027"},{"line_number":1526,"context_line":"                            \u0027snapshot failed: %(error)s\u0027), {\u0027error\u0027: e})"},{"line_number":1527,"context_line":"            failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1528,"context_line":"            if failed_snap:"},{"line_number":1529,"context_line":"                failed_snap \u003d {\u0027url\u0027: str(failed_snap)}"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_86090898","line":1526,"range":{"start_line":1525,"start_character":12,"end_line":1526,"end_character":72},"in_reply_to":"9a68dd71_d1925d57","updated":"2016-01-20 21:49:18.000000000","message":"Done","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a5575d4039ff5db0f0d419b5c2158388a7f1984d","unresolved":false,"context_lines":[{"line_number":1561,"context_line":"                                           image_id,"},{"line_number":1562,"context_line":"                                           metadata,"},{"line_number":1563,"context_line":"                                           image_file)"},{"line_number":1564,"context_line":"        except Exception:"},{"line_number":1565,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1566,"context_line":"                LOG.exception(_LE(\"Failed to snapshot image\"))"},{"line_number":1567,"context_line":"                failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1568,"context_line":"                if failed_snap:"},{"line_number":1569,"context_line":"                    failed_snap \u003d {\u0027url\u0027: str(failed_snap)}"},{"line_number":1570,"context_line":"                snapshot_backend.cleanup_direct_snapshot("},{"line_number":1571,"context_line":"                        failed_snap, also_destroy_volume\u003dTrue,"},{"line_number":1572,"context_line":"                        ignore_errors\u003dTrue)"},{"line_number":1573,"context_line":""},{"line_number":1574,"context_line":"        LOG.info(_LI(\"Snapshot image upload complete\"), instance\u003dinstance)"},{"line_number":1575,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_a7720b7b","line":1572,"range":{"start_line":1564,"start_character":8,"end_line":1572,"end_character":43},"updated":"2016-01-20 20:21:10.000000000","message":"I\u0027m pretty sure that this is still not tested.","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40d13cbfeabfc44c87bfbd00259626867200fc2a","unresolved":false,"context_lines":[{"line_number":1561,"context_line":"                                           image_id,"},{"line_number":1562,"context_line":"                                           metadata,"},{"line_number":1563,"context_line":"                                           image_file)"},{"line_number":1564,"context_line":"        except Exception:"},{"line_number":1565,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1566,"context_line":"                LOG.exception(_LE(\"Failed to snapshot image\"))"},{"line_number":1567,"context_line":"                failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1568,"context_line":"                if failed_snap:"},{"line_number":1569,"context_line":"                    failed_snap \u003d {\u0027url\u0027: str(failed_snap)}"},{"line_number":1570,"context_line":"                snapshot_backend.cleanup_direct_snapshot("},{"line_number":1571,"context_line":"                        failed_snap, also_destroy_volume\u003dTrue,"},{"line_number":1572,"context_line":"                        ignore_errors\u003dTrue)"},{"line_number":1573,"context_line":""},{"line_number":1574,"context_line":"        LOG.info(_LI(\"Snapshot image upload complete\"), instance\u003dinstance)"},{"line_number":1575,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_d79008ef","line":1572,"range":{"start_line":1564,"start_character":8,"end_line":1572,"end_character":43},"in_reply_to":"9a68dd71_6612d4b6","updated":"2016-01-20 22:07:42.000000000","message":"I did, I don\u0027t think that tests this block. I think that tests that we get a Forbidden from direct_snapshot which gets us into the fallback block, and then when we\u0027re doing the old style snapshot that fails, but that\u0027s not this block.","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"4cf4fdab388cba0e55e96c4319cab4d83b2f1468","unresolved":false,"context_lines":[{"line_number":1561,"context_line":"                                           image_id,"},{"line_number":1562,"context_line":"                                           metadata,"},{"line_number":1563,"context_line":"                                           image_file)"},{"line_number":1564,"context_line":"        except Exception:"},{"line_number":1565,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1566,"context_line":"                LOG.exception(_LE(\"Failed to snapshot image\"))"},{"line_number":1567,"context_line":"                failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1568,"context_line":"                if failed_snap:"},{"line_number":1569,"context_line":"                    failed_snap \u003d {\u0027url\u0027: str(failed_snap)}"},{"line_number":1570,"context_line":"                snapshot_backend.cleanup_direct_snapshot("},{"line_number":1571,"context_line":"                        failed_snap, also_destroy_volume\u003dTrue,"},{"line_number":1572,"context_line":"                        ignore_errors\u003dTrue)"},{"line_number":1573,"context_line":""},{"line_number":1574,"context_line":"        LOG.info(_LI(\"Snapshot image upload complete\"), instance\u003dinstance)"},{"line_number":1575,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_6612d4b6","line":1572,"range":{"start_line":1564,"start_character":8,"end_line":1572,"end_character":43},"in_reply_to":"9a68dd71_a7720b7b","updated":"2016-01-20 21:49:18.000000000","message":"See the test \"test_raw_with_rbd_clone_everything_fails\" in test_driver","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8813a354082c5f9265ea0a48ff28441ddfa8e8a7","unresolved":false,"context_lines":[{"line_number":1563,"context_line":"                                           image_id,"},{"line_number":1564,"context_line":"                                           metadata,"},{"line_number":1565,"context_line":"                                           image_file)"},{"line_number":1566,"context_line":"        except Exception:"},{"line_number":1567,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1568,"context_line":"                LOG.exception(_LE(\"Failed to snapshot image\"))"},{"line_number":1569,"context_line":"                failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1570,"context_line":"                if failed_snap:"},{"line_number":1571,"context_line":"                    failed_snap \u003d {\u0027url\u0027: str(failed_snap)}"},{"line_number":1572,"context_line":"                snapshot_backend.cleanup_direct_snapshot("},{"line_number":1573,"context_line":"                        failed_snap, also_destroy_volume\u003dTrue,"},{"line_number":1574,"context_line":"                        ignore_errors\u003dTrue)"},{"line_number":1575,"context_line":""},{"line_number":1576,"context_line":"        LOG.info(_LI(\"Snapshot image upload complete\"), instance\u003dinstance)"},{"line_number":1577,"context_line":""}],"source_content_type":"text/x-python","patch_set":41,"id":"9a68dd71_121eae0c","line":1574,"range":{"start_line":1566,"start_character":8,"end_line":1574,"end_character":43},"updated":"2016-01-20 22:16:01.000000000","message":"Yeah this isn\u0027t tested.","commit_id":"a1901ef32c2034f29d8a3d113f3f3e7728cb7c5b"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"9f76cf05958fe0ad9d03b7a412855207e0f7bd2c","unresolved":false,"context_lines":[{"line_number":1563,"context_line":"                                           image_id,"},{"line_number":1564,"context_line":"                                           metadata,"},{"line_number":1565,"context_line":"                                           image_file)"},{"line_number":1566,"context_line":"        except Exception:"},{"line_number":1567,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1568,"context_line":"                LOG.exception(_LE(\"Failed to snapshot image\"))"},{"line_number":1569,"context_line":"                failed_snap \u003d metadata.pop(\u0027location\u0027, None)"},{"line_number":1570,"context_line":"                if failed_snap:"},{"line_number":1571,"context_line":"                    failed_snap \u003d {\u0027url\u0027: str(failed_snap)}"},{"line_number":1572,"context_line":"                snapshot_backend.cleanup_direct_snapshot("},{"line_number":1573,"context_line":"                        failed_snap, also_destroy_volume\u003dTrue,"},{"line_number":1574,"context_line":"                        ignore_errors\u003dTrue)"},{"line_number":1575,"context_line":""},{"line_number":1576,"context_line":"        LOG.info(_LI(\"Snapshot image upload complete\"), instance\u003dinstance)"},{"line_number":1577,"context_line":""}],"source_content_type":"text/x-python","patch_set":41,"id":"9a68dd71_72b60ade","line":1574,"range":{"start_line":1566,"start_character":8,"end_line":1574,"end_character":43},"in_reply_to":"9a68dd71_121eae0c","updated":"2016-01-20 23:40:53.000000000","message":"Done.  See test_raw_with_rbd_clone_post_process_fails in PS42","commit_id":"a1901ef32c2034f29d8a3d113f3f3e7728cb7c5b"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":6984,"name":"Sébastien Han","email":"seb@redhat.com","username":"leseb"},"change_message_id":"dbc77e3bd697b919878e5ba24439e182ff064cdf","unresolved":false,"context_lines":[{"line_number":830,"context_line":"        # is configured, but we can infer what storage pool Glance is using"},{"line_number":831,"context_line":"        # by looking at the RBD parent.  If using authx, write access should"},{"line_number":832,"context_line":"        # be enabled on that pool for the Nova user"},{"line_number":833,"context_line":"        parent_pool, parent_im, parent_snap \u003d self.driver.parent_info("},{"line_number":834,"context_line":"            self.rbd_name)"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"        # Snapshot the disk and clone it into Glance\u0027s storage pool.  librbd"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_e49f635d","line":833,"updated":"2015-07-27 13:30:46.000000000","message":"This will return \"no usable parent snapshot\" if the image parent image has a qcow2 format. Can we try again and not give up on finding where glance is storing the images. For example we could use the base image used to boot the instance and get the location exposed by glance. This will allow us to find the destination pool.\n\nLet me know if I\u0027m missing something.","commit_id":"f0e3f80f069119d9ad5da07447b8b123c55880a2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"d943cb5f6112d188372bf424fc5746c8df065b77","unresolved":false,"context_lines":[{"line_number":830,"context_line":"        # is configured, but we can infer what storage pool Glance is using"},{"line_number":831,"context_line":"        # by looking at the RBD parent.  If using authx, write access should"},{"line_number":832,"context_line":"        # be enabled on that pool for the Nova user"},{"line_number":833,"context_line":"        parent_pool, parent_im, parent_snap \u003d self.driver.parent_info("},{"line_number":834,"context_line":"            self.rbd_name)"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"        # Snapshot the disk and clone it into Glance\u0027s storage pool.  librbd"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_609c8ac0","line":833,"in_reply_to":"3a50d1a3_1506261c","updated":"2015-07-27 20:02:31.000000000","message":"It does, and any calls for \"purity\" in the operation are blown out of the water by the fact that the \"inverse\" operation (clone-on-boot) talks to Glance as a matter of course.\n\nI don\u0027t think there\u0027s any explicit reason to make two calls for the same info, since the Glance call ostensibly handles it all, but at the risk of premature optimization, I think I\u0027ll keep the parent_info() check, since it\u0027s by all rights a highly inexpensive call compared to asking Glance.","commit_id":"f0e3f80f069119d9ad5da07447b8b123c55880a2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"a08e43e01f75a6be4c0987fea3a4405f4a711900","unresolved":false,"context_lines":[{"line_number":830,"context_line":"        # is configured, but we can infer what storage pool Glance is using"},{"line_number":831,"context_line":"        # by looking at the RBD parent.  If using authx, write access should"},{"line_number":832,"context_line":"        # be enabled on that pool for the Nova user"},{"line_number":833,"context_line":"        parent_pool, parent_im, parent_snap \u003d self.driver.parent_info("},{"line_number":834,"context_line":"            self.rbd_name)"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"        # Snapshot the disk and clone it into Glance\u0027s storage pool.  librbd"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_1be417de","line":833,"in_reply_to":"3a50d1a3_40b62efa","updated":"2015-07-27 20:37:24.000000000","message":"Correct.  Expect an update to this patch set soon.","commit_id":"f0e3f80f069119d9ad5da07447b8b123c55880a2"},{"author":{"_account_id":6984,"name":"Sébastien Han","email":"seb@redhat.com","username":"leseb"},"change_message_id":"9a23f8603d1be472d06b59afcad1ee11250408c7","unresolved":false,"context_lines":[{"line_number":830,"context_line":"        # is configured, but we can infer what storage pool Glance is using"},{"line_number":831,"context_line":"        # by looking at the RBD parent.  If using authx, write access should"},{"line_number":832,"context_line":"        # be enabled on that pool for the Nova user"},{"line_number":833,"context_line":"        parent_pool, parent_im, parent_snap \u003d self.driver.parent_info("},{"line_number":834,"context_line":"            self.rbd_name)"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"        # Snapshot the disk and clone it into Glance\u0027s storage pool.  librbd"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_40b62efa","line":833,"in_reply_to":"3a50d1a3_609c8ac0","updated":"2015-07-27 20:11:34.000000000","message":"does that mean that you\u0027re willing to implement the second check in case the first one fails? Just to make sure that we can even snapshot instances that are not cow clones :).","commit_id":"f0e3f80f069119d9ad5da07447b8b123c55880a2"},{"author":{"_account_id":6984,"name":"Sébastien Han","email":"seb@redhat.com","username":"leseb"},"change_message_id":"4b1f409f9887607656dda6a76ade15a41d9d3bf1","unresolved":false,"context_lines":[{"line_number":830,"context_line":"        # is configured, but we can infer what storage pool Glance is using"},{"line_number":831,"context_line":"        # by looking at the RBD parent.  If using authx, write access should"},{"line_number":832,"context_line":"        # be enabled on that pool for the Nova user"},{"line_number":833,"context_line":"        parent_pool, parent_im, parent_snap \u003d self.driver.parent_info("},{"line_number":834,"context_line":"            self.rbd_name)"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"        # Snapshot the disk and clone it into Glance\u0027s storage pool.  librbd"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_1506261c","line":833,"in_reply_to":"3a50d1a3_80075d80","updated":"2015-07-27 19:37:08.000000000","message":"Sorry for not being clear but yes that\u0027s what I meant here. Using qcow2 image to boot instance will result in falling back to the default fetch and thus won\u0027t give a you a clone but a default RBD image with layering.\nGiven that there is currently no mechanism (yet) to convert images during the upload we can not really conclude that \"they don\u0027t want to use cow clones\". We can\u0027t really force users to upload RAW images all the time so we will always end up in the mixed env with raw and qcow2 (till the glance patch for task triggers lands). Moreover users have no knowledge on what\u0027s running under the hood, so they just to care and simply do what\u0027s easier for them.\nTo me, it\u0027s not because the image is not a clone that we shouldn\u0027t attempt to create a RBD snapshot out of it.\nIf the operator sets \u0027image_type\u003drbd\u0027, I don\u0027t see any reasons why he/she will be reluctant to use RBD snapshots. I think it\u0027s worth double-checking. IMHO if we can get a location (just like this: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/imagebackend.py#L775) then we should use the pool and do the clone.\n\nDoes this make sense?","commit_id":"f0e3f80f069119d9ad5da07447b8b123c55880a2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"00bb256a8237dc6ae594ca60519aaf0c69108045","unresolved":false,"context_lines":[{"line_number":830,"context_line":"        # is configured, but we can infer what storage pool Glance is using"},{"line_number":831,"context_line":"        # by looking at the RBD parent.  If using authx, write access should"},{"line_number":832,"context_line":"        # be enabled on that pool for the Nova user"},{"line_number":833,"context_line":"        parent_pool, parent_im, parent_snap \u003d self.driver.parent_info("},{"line_number":834,"context_line":"            self.rbd_name)"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"        # Snapshot the disk and clone it into Glance\u0027s storage pool.  librbd"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_80075d80","line":833,"in_reply_to":"3a50d1a3_e49f635d","updated":"2015-07-27 18:45:28.000000000","message":"I\u0027m not 100% sure I follow you, but I\u0027ll assume you mean \"if the VM image is not itself an RBD clone\", i.e., it was made by downloading an image to local storage and then uploading it back into Ceph.\n\nI tend to view that kind of deployment as a matter of symmetry: if they don\u0027t want to use copy-on-write clones to boot images, they probably don\u0027t want to use copy-on-write clones to snapshot them, either; and falling back to \"legacy\" behaviors is desirable.\n\nBut it might be worth it to clone the images anyways for the performance increase (and it borders on unsettling just how much faster it is to do it this way) which raises the question of what to do if the storage pool lookup fails twice: should it make the snapshot in-place, or give up and fall back?  The former is akin to \"namespace pollution\", and the latter is slow.","commit_id":"f0e3f80f069119d9ad5da07447b8b123c55880a2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"45e57097a7b2c46e9994b618d8665d7b125bd1d5","unresolved":false,"context_lines":[{"line_number":840,"context_line":"                    parent_fsid, parent_pool, _im, _snap \u003d \\"},{"line_number":841,"context_line":"                        self.driver.parse_url(location[\u0027url\u0027])"},{"line_number":842,"context_line":"                    if parent_fsid \u003d\u003d fsid:"},{"line_number":843,"context_line":"                        break"},{"line_number":844,"context_line":"                except exception.ImageUnacceptable:"},{"line_number":845,"context_line":"                    continue"},{"line_number":846,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a50d1a3_f48311c9","line":843,"updated":"2015-07-29 16:58:47.000000000","message":"The fact that it doesn\u0027t reset parent_pool to None on mismatches means this could return a bad answer.  Will fix.","commit_id":"03af6adfb23a25079610ce82b2285163d6bcbda9"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"f2c961ffacf73ee9f6ae0986ebd52da3cac42712","unresolved":false,"context_lines":[{"line_number":879,"context_line":"                                pool\u003dself.pool,"},{"line_number":880,"context_line":"                                image\u003dself.rbd_name,"},{"line_number":881,"context_line":"                                snap\u003dsnapshot_name)}"},{"line_number":882,"context_line":"        self.driver.clone(location, image_id, dest_pool\u003dparent_pool)"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"        # Flatten the image, which detaches it from the source snapshot"},{"line_number":885,"context_line":"        self.driver.flatten(image_id, pool\u003dparent_pool)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a50d1a3_3fc5a609","line":882,"updated":"2015-07-30 13:40:17.000000000","message":"This is the first place where a write is attempted against Glance\u0027s pool.  If the user has not given the Nova ceph user write access to the pool this will fail and it will not clean up the protected snapshot of the instance\u0027s disk.  This will likely prevent the instance from being properly deleted later.  The clone and flatten calls should be placed in a try-except or try-finally and have the unprotect_snap and remove_snap methods called before re-raising the exception.","commit_id":"5ad9547d716e172760f4320d1ffd6c8f48b5990e"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"dcc2261fdc5221950f0b4e142ceb9fb1fce9028c","unresolved":false,"context_lines":[{"line_number":879,"context_line":"                                pool\u003dself.pool,"},{"line_number":880,"context_line":"                                image\u003dself.rbd_name,"},{"line_number":881,"context_line":"                                snap\u003dsnapshot_name)}"},{"line_number":882,"context_line":"        self.driver.clone(location, image_id, dest_pool\u003dparent_pool)"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"        # Flatten the image, which detaches it from the source snapshot"},{"line_number":885,"context_line":"        self.driver.flatten(image_id, pool\u003dparent_pool)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a50d1a3_0785f9ab","line":882,"in_reply_to":"3a50d1a3_3fc5a609","updated":"2015-07-30 16:56:41.000000000","message":"Done","commit_id":"5ad9547d716e172760f4320d1ffd6c8f48b5990e"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"d468b9dab8154a64a4550d851efe038c2238b2e0","unresolved":false,"context_lines":[{"line_number":894,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":895,"context_line":"        finally:"},{"line_number":896,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":897,"context_line":"            self.cleanup_snapshot(snapshot_name)"},{"line_number":898,"context_line":""},{"line_number":899,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":900,"context_line":"        # images and hands it out, so we\u0027ll do that too"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_1b1c7012","line":897,"updated":"2015-07-30 17:52:18.000000000","message":"should be self.cleanup_direct_snapshot(snapshot_name) as that method currently does unprotect and remove snap.  This may change depending on my comment on driver.py","commit_id":"52b2357c136e1948c0d6eb9d932e2c188a6dfa6a"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"1476908a94a0c6b692d5f7ff11289993d2f72693","unresolved":false,"context_lines":[{"line_number":894,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":895,"context_line":"        finally:"},{"line_number":896,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":897,"context_line":"            self.cleanup_snapshot(snapshot_name)"},{"line_number":898,"context_line":""},{"line_number":899,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":900,"context_line":"        # images and hands it out, so we\u0027ll do that too"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_16f525ca","line":897,"in_reply_to":"3a50d1a3_1b1c7012","updated":"2015-07-30 18:10:11.000000000","message":"Oh god damn it.","commit_id":"52b2357c136e1948c0d6eb9d932e2c188a6dfa6a"},{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"8e5c3de20a924dfa9f047dbebdd1a99c747ae8bd","unresolved":false,"context_lines":[{"line_number":869,"context_line":"        if image_format !\u003d \u0027raw\u0027:"},{"line_number":870,"context_line":"            reason \u003d _LE(\u0027only raw format is supported\u0027)"},{"line_number":871,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dreason)"},{"line_number":872,"context_line":"        if not self.driver.supports_layering():"},{"line_number":873,"context_line":"            reason \u003d _LE(\u0027librbd is too old\u0027)"},{"line_number":874,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dreason)"},{"line_number":875,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_39128a59","line":872,"updated":"2015-07-31 00:18:39.000000000","message":"you can get rid of this check - no one should be using a librbd that old anymore (last version that didn\u0027t have it was argonaut). \n\nThis change removes supports_layering(): https://review.openstack.org/#/c/186537/","commit_id":"151589e86cb16ab8d4aeb7477c353550296b21d1"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"8bbfdf3f7f391ab78d78358c822548af82d3debb","unresolved":false,"context_lines":[{"line_number":869,"context_line":"        if image_format !\u003d \u0027raw\u0027:"},{"line_number":870,"context_line":"            reason \u003d _LE(\u0027only raw format is supported\u0027)"},{"line_number":871,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dreason)"},{"line_number":872,"context_line":"        if not self.driver.supports_layering():"},{"line_number":873,"context_line":"            reason \u003d _LE(\u0027librbd is too old\u0027)"},{"line_number":874,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dreason)"},{"line_number":875,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_e14c409f","line":872,"in_reply_to":"3a50d1a3_39128a59","updated":"2015-07-31 17:55:01.000000000","message":"Done","commit_id":"151589e86cb16ab8d4aeb7477c353550296b21d1"},{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"8e5c3de20a924dfa9f047dbebdd1a99c747ae8bd","unresolved":false,"context_lines":[{"line_number":882,"context_line":""},{"line_number":883,"context_line":"        # Snapshot the disk and clone it into Glance\u0027s storage pool.  librbd"},{"line_number":884,"context_line":"        # requires that snapshots be set to \"protected\" in order to clone them"},{"line_number":885,"context_line":"        self.driver.create_snap(self.rbd_name, snapshot_name)"},{"line_number":886,"context_line":"        self.driver.protect_snap(self.rbd_name, snapshot_name)"},{"line_number":887,"context_line":"        location \u003d {\u0027url\u0027: \u0027rbd://%(fsid)s/%(pool)s/%(image)s/%(snap)s\u0027 %"},{"line_number":888,"context_line":"                           dict(fsid\u003dfsid,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_19546e1f","line":885,"updated":"2015-07-31 00:18:39.000000000","message":"You can fold protect/unprotect into the snap create and remove methods to avoid opening the image a few extra times.\n\nThere\u0027s not much overhead to doing the protect/unprotect, so that aspect can be hidden in the rbd_utils and not exposed to other layers.","commit_id":"151589e86cb16ab8d4aeb7477c353550296b21d1"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"8bbfdf3f7f391ab78d78358c822548af82d3debb","unresolved":false,"context_lines":[{"line_number":882,"context_line":""},{"line_number":883,"context_line":"        # Snapshot the disk and clone it into Glance\u0027s storage pool.  librbd"},{"line_number":884,"context_line":"        # requires that snapshots be set to \"protected\" in order to clone them"},{"line_number":885,"context_line":"        self.driver.create_snap(self.rbd_name, snapshot_name)"},{"line_number":886,"context_line":"        self.driver.protect_snap(self.rbd_name, snapshot_name)"},{"line_number":887,"context_line":"        location \u003d {\u0027url\u0027: \u0027rbd://%(fsid)s/%(pool)s/%(image)s/%(snap)s\u0027 %"},{"line_number":888,"context_line":"                           dict(fsid\u003dfsid,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_a13dd8de","line":885,"in_reply_to":"3a50d1a3_19546e1f","updated":"2015-07-31 17:55:01.000000000","message":"There\u0027s the whole angle of \"I might want to protect/unprotect a snapshot without necessarily creating/destroying it at the same time\", but I can\u0027t really think of any use cases for that.\n\nI\u0027ll have to throw out a bunch of unit tests though, so win-win?","commit_id":"151589e86cb16ab8d4aeb7477c353550296b21d1"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"4d22cfb0c554d3742eeb05c21144166ad8d6845b","unresolved":false,"context_lines":[{"line_number":885,"context_line":"        \"\"\"Creates an RBD snapshot directly."},{"line_number":886,"context_line":"        \"\"\""},{"line_number":887,"context_line":"        if image_format !\u003d \u0027raw\u0027:"},{"line_number":888,"context_line":"            reason \u003d _LE(\u0027only raw format is supported\u0027)"},{"line_number":889,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dreason)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"        # NOTE(nic): Nova has zero comprehension of how Glance\u0027s image store"}],"source_content_type":"text/x-python","patch_set":16,"id":"1a4dcd0f_50da63e9","line":888,"updated":"2015-08-12 00:13:12.000000000","message":"Seeing as this approach flattens the clone right away, we can\u0027t think of a terribly compelling reason for this restriction.  This should work fine with e.g. QCOW2 images, and would be more performant and efficient than a QEMU snapshot on the same data.\n\nI\u0027ll redact the restriction.","commit_id":"0a3df5e56e966cb32c58840b7737b53e7a028d7c"},{"author":{"_account_id":6463,"name":"Jackie","email":"guanqiang85@gmail.com","username":"hzguanqiang"},"change_message_id":"e06daf9ca3fbf3f226584af3fb371e4486c16fc0","unresolved":false,"context_lines":[{"line_number":937,"context_line":"                                image\u003dself.rbd_name,"},{"line_number":938,"context_line":"                                snap\u003dsnapshot_name)}"},{"line_number":939,"context_line":"        try:"},{"line_number":940,"context_line":"            self.driver.clone(location, image_id, dest_pool\u003dparent_pool)"},{"line_number":941,"context_line":"            # Flatten the image, which detaches it from the source snapshot"},{"line_number":942,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":943,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba8a016a_10ac0eec","line":940,"updated":"2015-11-18 05:01:06.000000000","message":"I\u0027ve tried this patch.  It seems that exception with message \"ImageExists: error creating clone\" will be raised here. \n\nShould the image_id be different from the instance image id? I guess this should be the instance snapshot image id, so maybe snapshot_name can be used here.","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":6463,"name":"Jackie","email":"guanqiang85@gmail.com","username":"hzguanqiang"},"change_message_id":"ab06aa3e3d61653bb8481487318e666e9de5ddc2","unresolved":false,"context_lines":[{"line_number":937,"context_line":"                                image\u003dself.rbd_name,"},{"line_number":938,"context_line":"                                snap\u003dsnapshot_name)}"},{"line_number":939,"context_line":"        try:"},{"line_number":940,"context_line":"            self.driver.clone(location, image_id, dest_pool\u003dparent_pool)"},{"line_number":941,"context_line":"            # Flatten the image, which detaches it from the source snapshot"},{"line_number":942,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":943,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba8a016a_2bb76787","line":940,"in_reply_to":"ba8a016a_10ac0eec","updated":"2015-11-18 06:12:35.000000000","message":"Sorry, Ignore the above comment. It seems it\u0027s a problem caused by the compability of this patch with my havana branch codes.\n\nThe image_id is already different from the instance image id, which I misunderstand.","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":6463,"name":"Jackie","email":"guanqiang85@gmail.com","username":"hzguanqiang"},"change_message_id":"8e63a5a9a281bb4782273008dbdb681d4ac006a2","unresolved":false,"context_lines":[{"line_number":939,"context_line":"        try:"},{"line_number":940,"context_line":"            self.driver.clone(location, image_id, dest_pool\u003dparent_pool)"},{"line_number":941,"context_line":"            # Flatten the image, which detaches it from the source snapshot"},{"line_number":942,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":943,"context_line":"        finally:"},{"line_number":944,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":945,"context_line":"            self.cleanup_direct_snapshot(location)"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba8a016a_73e1e1cf","line":942,"updated":"2015-11-18 07:47:58.000000000","message":"I tried this patch, It works. But flattening operation takes a long time. \n\nFor ceph, during flattening a rbd image, it\u0027s snapshot can be used meanwhile.\nSo Can the flatten and cleanup_direct_snapshot be done after making snapshot available?","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"82c861ebfb87dafd9c7f722a95a82f1408ce2c34","unresolved":false,"context_lines":[{"line_number":939,"context_line":"        try:"},{"line_number":940,"context_line":"            self.driver.clone(location, image_id, dest_pool\u003dparent_pool)"},{"line_number":941,"context_line":"            # Flatten the image, which detaches it from the source snapshot"},{"line_number":942,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":943,"context_line":"        finally:"},{"line_number":944,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":945,"context_line":"            self.cleanup_direct_snapshot(location)"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba8a016a_5f78c28f","line":942,"in_reply_to":"ba8a016a_578f5bcc","updated":"2015-11-20 18:01:22.000000000","message":"Basically this.  The flattening operation is still orders of magnitude faster than a \"traditional\" snapshot, and is far simpler.\n\nThere are undoubtedly dozens of races, corner-cases, circular dependency problems, and bugs waiting around in an asynchronous flatten operation; better to start with something simple and see if it\u0027s worth building upon, IMO.","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":14119,"name":"Sam Yaple","email":"sam+git@yaple.net","username":"SamYaple"},"change_message_id":"b5610281a33466320f9084d5ab6b39389e14c5f7","unresolved":false,"context_lines":[{"line_number":939,"context_line":"        try:"},{"line_number":940,"context_line":"            self.driver.clone(location, image_id, dest_pool\u003dparent_pool)"},{"line_number":941,"context_line":"            # Flatten the image, which detaches it from the source snapshot"},{"line_number":942,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":943,"context_line":"        finally:"},{"line_number":944,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":945,"context_line":"            self.cleanup_direct_snapshot(location)"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba8a016a_578f5bcc","line":942,"in_reply_to":"ba8a016a_73e1e1cf","updated":"2015-11-20 13:06:07.000000000","message":"What would the flow look like if the flattening failed? Right now if the flatten fails it fails back to regular qemu snapshotting. If we do the flattening in the background there would be no way to fall back","commit_id":"b5f350dcd0f5a4e8d1833dd8c948f080dd28ac8f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":901,"context_line":"        except exception.ImageUnacceptable:"},{"line_number":902,"context_line":"            # The hard way -- the image is itself a parent, so ask Glance"},{"line_number":903,"context_line":"            # where it came from"},{"line_number":904,"context_line":"            LOG.debug(\u0027No parent info; asking Glance where its store is\u0027)"},{"line_number":905,"context_line":"            try:"},{"line_number":906,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":907,"context_line":"                                           include_locations\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_c2174aa1","line":904,"updated":"2016-01-11 20:47:43.000000000","message":"Put the base_image_id in here for context.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"bd984f3a91b7014235064ee2803406232ca9c743","unresolved":false,"context_lines":[{"line_number":901,"context_line":"        except exception.ImageUnacceptable:"},{"line_number":902,"context_line":"            # The hard way -- the image is itself a parent, so ask Glance"},{"line_number":903,"context_line":"            # where it came from"},{"line_number":904,"context_line":"            LOG.debug(\u0027No parent info; asking Glance where its store is\u0027)"},{"line_number":905,"context_line":"            try:"},{"line_number":906,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":907,"context_line":"                                           include_locations\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_72962996","line":904,"in_reply_to":"fa69d971_c2174aa1","updated":"2016-01-12 02:51:01.000000000","message":"Done","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":904,"context_line":"            LOG.debug(\u0027No parent info; asking Glance where its store is\u0027)"},{"line_number":905,"context_line":"            try:"},{"line_number":906,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":907,"context_line":"                                           include_locations\u003dTrue)"},{"line_number":908,"context_line":"            except Exception:"},{"line_number":909,"context_line":"                image_meta \u003d {}"},{"line_number":910,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_253d10c4","line":907,"range":{"start_line":907,"start_character":43,"end_line":907,"end_character":60},"updated":"2016-01-11 20:47:43.000000000","message":"Ah I see, this is what forces us into glance v2:\n\nhttps://github.com/openstack/nova/blob/master/nova/image/glance.py#L313","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":905,"context_line":"            try:"},{"line_number":906,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":907,"context_line":"                                           include_locations\u003dTrue)"},{"line_number":908,"context_line":"            except Exception:"},{"line_number":909,"context_line":"                image_meta \u003d {}"},{"line_number":910,"context_line":""},{"line_number":911,"context_line":"            # Find the first location that is in the same RBD cluster"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_c552f412","line":908,"range":{"start_line":908,"start_character":19,"end_line":908,"end_character":28},"updated":"2016-01-11 20:47:43.000000000","message":"This is pretty generic, you don\u0027t even want to log at debug level?","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0fb2a318ee864107cd24a1ea49cc660bd3d675f8","unresolved":false,"context_lines":[{"line_number":905,"context_line":"            try:"},{"line_number":906,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":907,"context_line":"                                           include_locations\u003dTrue)"},{"line_number":908,"context_line":"            except Exception:"},{"line_number":909,"context_line":"                image_meta \u003d {}"},{"line_number":910,"context_line":""},{"line_number":911,"context_line":"            # Find the first location that is in the same RBD cluster"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_3606af1d","line":908,"range":{"start_line":908,"start_character":19,"end_line":908,"end_character":28},"in_reply_to":"da6ed579_b27b417b","updated":"2016-01-14 18:42:25.000000000","message":"Sure but \"cannot determine where to store images\" is, well, not deterministic, where as if we logged, at least at debug, why glance failed, like ImageNotFound or something, that\u0027s more clear. i.e. less work to debug.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"bd984f3a91b7014235064ee2803406232ca9c743","unresolved":false,"context_lines":[{"line_number":905,"context_line":"            try:"},{"line_number":906,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":907,"context_line":"                                           include_locations\u003dTrue)"},{"line_number":908,"context_line":"            except Exception:"},{"line_number":909,"context_line":"                image_meta \u003d {}"},{"line_number":910,"context_line":""},{"line_number":911,"context_line":"            # Find the first location that is in the same RBD cluster"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_b27b417b","line":908,"range":{"start_line":908,"start_character":19,"end_line":908,"end_character":28},"in_reply_to":"fa69d971_c552f412","updated":"2016-01-12 02:51:01.000000000","message":"It all comes out in the wash; the Glance call logs a bunch on its own, and the failure is logged when parent_pool fails to populate properly.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":921,"context_line":"                    continue"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"        if not parent_pool:"},{"line_number":924,"context_line":"            reason \u003d _LE(\u0027cannot determine where to store images\u0027)"},{"line_number":925,"context_line":"            raise exception.ImageUnacceptable(reason)"},{"line_number":926,"context_line":""},{"line_number":927,"context_line":"        return parent_pool"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_45db647c","line":924,"updated":"2016-01-11 20:47:43.000000000","message":"nit: throw the base_image_id in here also for context.\n\nAlso, shouldn\u0027t be using _LE() here.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"bd984f3a91b7014235064ee2803406232ca9c743","unresolved":false,"context_lines":[{"line_number":921,"context_line":"                    continue"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"        if not parent_pool:"},{"line_number":924,"context_line":"            reason \u003d _LE(\u0027cannot determine where to store images\u0027)"},{"line_number":925,"context_line":"            raise exception.ImageUnacceptable(reason)"},{"line_number":926,"context_line":""},{"line_number":927,"context_line":"        return parent_pool"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_f2e8d918","line":924,"in_reply_to":"fa69d971_45db647c","updated":"2016-01-12 02:51:01.000000000","message":"Done","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0ca29689f5e7d53b2b6d106732e7b1e9a5e5500e","unresolved":false,"context_lines":[{"line_number":951,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":952,"context_line":"        finally:"},{"line_number":953,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":954,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":957,"context_line":"        # images and hands it out, so we\u0027ll do that too"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_68dbdf4f","line":954,"range":{"start_line":954,"start_character":17,"end_line":954,"end_character":40},"updated":"2016-01-11 21:00:21.000000000","message":"Why not pass destroy\u003dTrue here?","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0fb2a318ee864107cd24a1ea49cc660bd3d675f8","unresolved":false,"context_lines":[{"line_number":951,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":952,"context_line":"        finally:"},{"line_number":953,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":954,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":957,"context_line":"        # images and hands it out, so we\u0027ll do that too"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_b6393fd9","line":954,"range":{"start_line":954,"start_character":17,"end_line":954,"end_character":40},"in_reply_to":"da6ed579_723aa9cf","updated":"2016-01-14 18:42:25.000000000","message":"I don\u0027t have better names, but if I have to ask and you think it\u0027s confusing, you should add a code comment for this - which I think your reply would be sufficient for explaining it.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"bd984f3a91b7014235064ee2803406232ca9c743","unresolved":false,"context_lines":[{"line_number":951,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":952,"context_line":"        finally:"},{"line_number":953,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":954,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":957,"context_line":"        # images and hands it out, so we\u0027ll do that too"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_723aa9cf","line":954,"range":{"start_line":954,"start_character":17,"end_line":954,"end_character":40},"in_reply_to":"fa69d971_68dbdf4f","updated":"2016-01-12 02:51:01.000000000","message":"destroy\u003dTrue destroys the entire volume, which would be bad here.\n\nThat flag is intended for cleaning up in preparation for the \"fallback\" behavior.\n\nI\u0027m going to guess that the conversation is going to rapidly move to how the names of the kwargs for this method are terrible and confusing.  Got better names?","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":953,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":954,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":957,"context_line":"        # images and hands it out, so we\u0027ll do that too"},{"line_number":958,"context_line":"        self.driver.create_snap(image_id, \u0027snap\u0027, pool\u003dparent_pool,"},{"line_number":959,"context_line":"                                protect\u003dTrue)"},{"line_number":960,"context_line":"        return (\u0027rbd://%(fsid)s/%(pool)s/%(image)s/snap\u0027 %"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_25fbd07a","line":957,"range":{"start_line":956,"start_character":8,"end_line":957,"end_character":55},"updated":"2016-01-11 20:47:43.000000000","message":"Does it matter what we call the snapshot? This just seems like an implementation detail within Glance that shouldn\u0027t be necessary, and if it is, then we\u0027re getting pretty tightly coupled here.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0fb2a318ee864107cd24a1ea49cc660bd3d675f8","unresolved":false,"context_lines":[{"line_number":953,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":954,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":957,"context_line":"        # images and hands it out, so we\u0027ll do that too"},{"line_number":958,"context_line":"        self.driver.create_snap(image_id, \u0027snap\u0027, pool\u003dparent_pool,"},{"line_number":959,"context_line":"                                protect\u003dTrue)"},{"line_number":960,"context_line":"        return (\u0027rbd://%(fsid)s/%(pool)s/%(image)s/snap\u0027 %"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_b6ae1fe4","line":957,"range":{"start_line":956,"start_character":8,"end_line":957,"end_character":55},"in_reply_to":"da6ed579_5edbf1ed","updated":"2016-01-14 18:42:25.000000000","message":"OK, yeah, I see it here:\n\nhttps://github.com/openstack/glance_store/blob/5b2eec73f2822e42f29d2151728ce03676cbc8cb/glance_store/_drivers/rbd.py#L49\n\nIt might be useful to note that the name doesn\u0027t actually matter in a code comment.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"bd984f3a91b7014235064ee2803406232ca9c743","unresolved":false,"context_lines":[{"line_number":953,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":954,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":957,"context_line":"        # images and hands it out, so we\u0027ll do that too"},{"line_number":958,"context_line":"        self.driver.create_snap(image_id, \u0027snap\u0027, pool\u003dparent_pool,"},{"line_number":959,"context_line":"                                protect\u003dTrue)"},{"line_number":960,"context_line":"        return (\u0027rbd://%(fsid)s/%(pool)s/%(image)s/snap\u0027 %"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_5edbf1ed","line":957,"range":{"start_line":956,"start_character":8,"end_line":957,"end_character":55},"in_reply_to":"fa69d971_25fbd07a","updated":"2016-01-12 02:51:01.000000000","message":"According to the glance_store code, it\u0027s a \"default\" value, but there is no facility to alter it via configuration (i.e., you can call it anything you like, so long as it\u0027s \"snap\")\n\nBut no, it does not appear to matter what the snapshot is called; all the code to parse RBD URIs appears to just break it apart and use the snapshot name in the URI.\n\nBut if I were to give these a name, I\u0027d likely just call them \"snap\" anyhow.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a28df1c38e9a01186682cdac4ec03b73b770cb50","unresolved":false,"context_lines":[{"line_number":917,"context_line":"            try:"},{"line_number":918,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":919,"context_line":"                                           include_locations\u003dTrue)"},{"line_number":920,"context_line":"            except Exception:"},{"line_number":921,"context_line":"                image_meta \u003d {}"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"            # Find the first location that is in the same RBD cluster"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_76a8a777","line":920,"range":{"start_line":920,"start_character":12,"end_line":920,"end_character":28},"updated":"2016-01-14 18:57:24.000000000","message":"See reply in ps24, but I think at least logging this at debug level would be helpful, I don\u0027t want to rely on what the image API is logging and the exception on line 937 is not deterministic, so having the exception logged here at debug would help IMO.","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0c6a45b9fdc38f124b104bad0cbcca64063758f6","unresolved":false,"context_lines":[{"line_number":917,"context_line":"            try:"},{"line_number":918,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":919,"context_line":"                                           include_locations\u003dTrue)"},{"line_number":920,"context_line":"            except Exception:"},{"line_number":921,"context_line":"                image_meta \u003d {}"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"            # Find the first location that is in the same RBD cluster"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_b14aa924","line":920,"range":{"start_line":920,"start_character":12,"end_line":920,"end_character":28},"in_reply_to":"da6ed579_76a8a777","updated":"2016-01-14 19:53:36.000000000","message":"Done","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a28df1c38e9a01186682cdac4ec03b73b770cb50","unresolved":false,"context_lines":[{"line_number":964,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":965,"context_line":"        finally:"},{"line_number":966,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":967,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":968,"context_line":""},{"line_number":969,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":970,"context_line":"        # images and hands it out, so we\u0027ll do that too"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_d6add384","line":967,"range":{"start_line":967,"start_character":12,"end_line":967,"end_character":50},"updated":"2016-01-14 18:57:24.000000000","message":"See reply in PS24, basically I think you should add a code comment here which was your reply to me explaining why we don\u0027t pass destroy\u003dTrue here, because \u0027cleanup\u0027 and \u0027destroy\u0027 are confusing, so we need code comments to explain the flow. (btw, you have already done a very nice job of documenting a lot of this code already, I applaud you!)","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0c6a45b9fdc38f124b104bad0cbcca64063758f6","unresolved":false,"context_lines":[{"line_number":964,"context_line":"            self.driver.flatten(image_id, pool\u003dparent_pool)"},{"line_number":965,"context_line":"        finally:"},{"line_number":966,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":967,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":968,"context_line":""},{"line_number":969,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":970,"context_line":"        # images and hands it out, so we\u0027ll do that too"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_a19ed6b6","line":967,"range":{"start_line":967,"start_character":12,"end_line":967,"end_character":50},"in_reply_to":"da6ed579_d6add384","updated":"2016-01-14 19:53:36.000000000","message":"Done, after a fashion.","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a28df1c38e9a01186682cdac4ec03b73b770cb50","unresolved":false,"context_lines":[{"line_number":966,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":967,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":968,"context_line":""},{"line_number":969,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":970,"context_line":"        # images and hands it out, so we\u0027ll do that too"},{"line_number":971,"context_line":"        self.driver.create_snap(image_id, \u0027snap\u0027, pool\u003dparent_pool,"},{"line_number":972,"context_line":"                                protect\u003dTrue)"},{"line_number":973,"context_line":"        return (\u0027rbd://%(fsid)s/%(pool)s/%(image)s/snap\u0027 %"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_b6c93f5b","line":970,"range":{"start_line":969,"start_character":8,"end_line":970,"end_character":55},"updated":"2016-01-14 18:57:24.000000000","message":"As noted in ps24, it might be worth noting that the name of the snapshot doesn\u0027t really matter, this just uses what the glance-store rbd backend sets (which is not configurable).","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0c6a45b9fdc38f124b104bad0cbcca64063758f6","unresolved":false,"context_lines":[{"line_number":966,"context_line":"            # all done with the source snapshot, clean it up"},{"line_number":967,"context_line":"            self.cleanup_direct_snapshot(location)"},{"line_number":968,"context_line":""},{"line_number":969,"context_line":"        # Glance makes a protected snapshot called \u0027snap\u0027 on uploaded"},{"line_number":970,"context_line":"        # images and hands it out, so we\u0027ll do that too"},{"line_number":971,"context_line":"        self.driver.create_snap(image_id, \u0027snap\u0027, pool\u003dparent_pool,"},{"line_number":972,"context_line":"                                protect\u003dTrue)"},{"line_number":973,"context_line":"        return (\u0027rbd://%(fsid)s/%(pool)s/%(image)s/snap\u0027 %"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_b153896a","line":970,"range":{"start_line":969,"start_character":8,"end_line":970,"end_character":55},"in_reply_to":"da6ed579_b6c93f5b","updated":"2016-01-14 19:53:36.000000000","message":"Done","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50744e813abd9d1967cbc0668fdff03b830c9834","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":919,"context_line":"                                           include_locations\u003dTrue)"},{"line_number":920,"context_line":"            except Exception:"},{"line_number":921,"context_line":"                LOG.debug(\u0027Nothing useful in the Image API for image %s; is \u0027"},{"line_number":922,"context_line":"                          \u0027Glance configured properly?\u0027, base_image_id)"},{"line_number":923,"context_line":"                image_meta \u003d {}"},{"line_number":924,"context_line":""},{"line_number":925,"context_line":"            # Find the first location that is in the same RBD cluster"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_cfd3e45e","line":922,"range":{"start_line":921,"start_character":27,"end_line":922,"end_character":54},"updated":"2016-01-15 02:31:36.000000000","message":"We should probably have the actual error in here, so I\u0027d probably re-word as:\n\nLOG.debug(\u0027Unable to get image %(base_image_id)s; error: %(error)s\u0027, {\u0027base_image_id\u0027: base_image_id, \u0027error\u0027: e})","commit_id":"8e580d3c52217ae42401c5012b724308d301934d"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"2148b50ff33f2363914bb3355c3db005db60ba33","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                image_meta \u003d IMAGE_API.get(context, base_image_id,"},{"line_number":919,"context_line":"                                           include_locations\u003dTrue)"},{"line_number":920,"context_line":"            except Exception:"},{"line_number":921,"context_line":"                LOG.debug(\u0027Nothing useful in the Image API for image %s; is \u0027"},{"line_number":922,"context_line":"                          \u0027Glance configured properly?\u0027, base_image_id)"},{"line_number":923,"context_line":"                image_meta \u003d {}"},{"line_number":924,"context_line":""},{"line_number":925,"context_line":"            # Find the first location that is in the same RBD cluster"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_c8a09044","line":922,"range":{"start_line":921,"start_character":27,"end_line":922,"end_character":54},"in_reply_to":"da6ed579_cfd3e45e","updated":"2016-01-15 18:27:24.000000000","message":"Done","commit_id":"8e580d3c52217ae42401c5012b724308d301934d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50744e813abd9d1967cbc0668fdff03b830c9834","unresolved":false,"context_lines":[{"line_number":936,"context_line":""},{"line_number":937,"context_line":"        if not parent_pool:"},{"line_number":938,"context_line":"            raise exception.ImageUnacceptable("},{"line_number":939,"context_line":"                    \u0027Cannot determine the parent storage pool for %s; cannot \u0027"},{"line_number":940,"context_line":"                    \u0027determine where to store images\u0027 % base_image_id)"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"        return parent_pool"},{"line_number":943,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_4fc854a6","line":940,"range":{"start_line":939,"start_character":20,"end_line":940,"end_character":53},"updated":"2016-01-15 02:31:36.000000000","message":"We should ask in IRC if this should be translatable, I feel like it wouldn\u0027t hurt if it gets into the logs later.","commit_id":"8e580d3c52217ae42401c5012b724308d301934d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":903,"context_line":"    def rollback_to_snap(self, name):"},{"line_number":904,"context_line":"        return self.driver.rollback_to_snap(self.rbd_name, name)"},{"line_number":905,"context_line":""},{"line_number":906,"context_line":"    def _get_parent_pool(self, context, base_image_id, fsid):"},{"line_number":907,"context_line":"        parent_pool \u003d None"},{"line_number":908,"context_line":"        try:"},{"line_number":909,"context_line":"            # The easy way -- the image is an RBD clone, so use the parent"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_7e87deb4","line":906,"range":{"start_line":906,"start_character":8,"end_line":906,"end_character":24},"updated":"2016-01-15 19:06:53.000000000","message":"Should have unit tests for this in https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_imagebackend.py which mock out the rbd_utils driver calls.\n\nI realize some of this might be covered in test_driver, but that\u0027s not a great place to test this given how big the snapshot method is already in there and how many conditions it has, we really need to unit test the parts.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":903,"context_line":"    def rollback_to_snap(self, name):"},{"line_number":904,"context_line":"        return self.driver.rollback_to_snap(self.rbd_name, name)"},{"line_number":905,"context_line":""},{"line_number":906,"context_line":"    def _get_parent_pool(self, context, base_image_id, fsid):"},{"line_number":907,"context_line":"        parent_pool \u003d None"},{"line_number":908,"context_line":"        try:"},{"line_number":909,"context_line":"            # The easy way -- the image is an RBD clone, so use the parent"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_d5d49fbe","line":906,"range":{"start_line":906,"start_character":8,"end_line":906,"end_character":24},"in_reply_to":"da6ed579_7e87deb4","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":942,"context_line":""},{"line_number":943,"context_line":"        return parent_pool"},{"line_number":944,"context_line":""},{"line_number":945,"context_line":"    def direct_snapshot(self, context, snapshot_name, image_format,"},{"line_number":946,"context_line":"                        image_id, base_image_id):"},{"line_number":947,"context_line":"        \"\"\"Creates an RBD snapshot directly."},{"line_number":948,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_7e003e34","line":945,"range":{"start_line":945,"start_character":8,"end_line":945,"end_character":23},"updated":"2016-01-15 19:06:53.000000000","message":"Should have a unit test for this in https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_imagebackend.py which mocks out _get_parent_pool.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":942,"context_line":""},{"line_number":943,"context_line":"        return parent_pool"},{"line_number":944,"context_line":""},{"line_number":945,"context_line":"    def direct_snapshot(self, context, snapshot_name, image_format,"},{"line_number":946,"context_line":"                        image_id, base_image_id):"},{"line_number":947,"context_line":"        \"\"\"Creates an RBD snapshot directly."},{"line_number":948,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_0b51b40e","line":945,"range":{"start_line":945,"start_character":8,"end_line":945,"end_character":23},"in_reply_to":"da6ed579_7e003e34","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":978,"context_line":"        return (\u0027rbd://%(fsid)s/%(pool)s/%(image)s/snap\u0027 %"},{"line_number":979,"context_line":"                dict(fsid\u003dfsid, pool\u003dparent_pool, image\u003dimage_id))"},{"line_number":980,"context_line":""},{"line_number":981,"context_line":"    def cleanup_direct_snapshot(self, location, also_destroy_volume\u003dFalse,"},{"line_number":982,"context_line":"                                ignore_errors\u003dFalse):"},{"line_number":983,"context_line":"        \"\"\"Unprotects and destroys the name snapshot."},{"line_number":984,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_1efb4248","line":981,"range":{"start_line":981,"start_character":8,"end_line":981,"end_character":31},"updated":"2016-01-15 19:06:53.000000000","message":"Should have a unit test for this in https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_imagebackend.py.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":978,"context_line":"        return (\u0027rbd://%(fsid)s/%(pool)s/%(image)s/snap\u0027 %"},{"line_number":979,"context_line":"                dict(fsid\u003dfsid, pool\u003dparent_pool, image\u003dimage_id))"},{"line_number":980,"context_line":""},{"line_number":981,"context_line":"    def cleanup_direct_snapshot(self, location, also_destroy_volume\u003dFalse,"},{"line_number":982,"context_line":"                                ignore_errors\u003dFalse):"},{"line_number":983,"context_line":"        \"\"\"Unprotects and destroys the name snapshot."},{"line_number":984,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_ab5508fb","line":981,"range":{"start_line":981,"start_character":8,"end_line":981,"end_character":31},"in_reply_to":"da6ed579_1efb4248","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a5575d4039ff5db0f0d419b5c2158388a7f1984d","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        :returns: URL to be given to glance"},{"line_number":403,"context_line":"        \"\"\""},{"line_number":404,"context_line":"        reason \u003d _(\u0027direct_snapshot() is not implemented\u0027)"},{"line_number":405,"context_line":"        raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dreason)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    def cleanup_direct_snapshot(self, location, also_destroy_volume\u003dFalse,"},{"line_number":408,"context_line":"                                ignore_errors\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_518d0d30","line":405,"range":{"start_line":405,"start_character":24,"end_line":405,"end_character":41},"updated":"2016-01-20 20:21:10.000000000","message":"Seems this should be NotImplementedError instead.","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"4cf4fdab388cba0e55e96c4319cab4d83b2f1468","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        :returns: URL to be given to glance"},{"line_number":403,"context_line":"        \"\"\""},{"line_number":404,"context_line":"        reason \u003d _(\u0027direct_snapshot() is not implemented\u0027)"},{"line_number":405,"context_line":"        raise exception.ImageUnacceptable(image_id\u003dimage_id, reason\u003dreason)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    def cleanup_direct_snapshot(self, location, also_destroy_volume\u003dFalse,"},{"line_number":408,"context_line":"                                ignore_errors\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":40,"id":"9a68dd71_096689ed","line":405,"range":{"start_line":405,"start_character":24,"end_line":405,"end_character":41},"in_reply_to":"9a68dd71_518d0d30","updated":"2016-01-20 21:49:18.000000000","message":"Done","commit_id":"7a84b14402233a8f815fe8c2859e0de092aec95d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8813a354082c5f9265ea0a48ff28441ddfa8e8a7","unresolved":false,"context_lines":[{"line_number":401,"context_line":"                 referenced directly in the specified image format"},{"line_number":402,"context_line":"        :returns: URL to be given to glance"},{"line_number":403,"context_line":"        \"\"\""},{"line_number":404,"context_line":"        raise NotImplementedError()"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def cleanup_direct_snapshot(self, location, also_destroy_volume\u003dFalse,"},{"line_number":407,"context_line":"                                ignore_errors\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":41,"id":"9a68dd71_72260abc","line":404,"updated":"2016-01-20 22:16:01.000000000","message":"nit: we could put a message in there:\n\n_(\u0027direct_snapshot() is not implemented\u0027)\n\nBut just add that if you respin.","commit_id":"a1901ef32c2034f29d8a3d113f3f3e7728cb7c5b"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"9f76cf05958fe0ad9d03b7a412855207e0f7bd2c","unresolved":false,"context_lines":[{"line_number":401,"context_line":"                 referenced directly in the specified image format"},{"line_number":402,"context_line":"        :returns: URL to be given to glance"},{"line_number":403,"context_line":"        \"\"\""},{"line_number":404,"context_line":"        raise NotImplementedError()"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def cleanup_direct_snapshot(self, location, also_destroy_volume\u003dFalse,"},{"line_number":407,"context_line":"                                ignore_errors\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":41,"id":"9a68dd71_d2aa5670","line":404,"in_reply_to":"9a68dd71_72260abc","updated":"2016-01-20 23:40:53.000000000","message":"Done","commit_id":"a1901ef32c2034f29d8a3d113f3f3e7728cb7c5b"}],"nova/virt/libvirt/storage/rbd_utils.py":[{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"13f95c383162f5045f5e07daafeca5434b8e5197","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            raise exception.ImageUnacceptable(image_id\u003durl, reason\u003dreason)"},{"line_number":170,"context_line":"        return pieces"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def get_fsid(self):"},{"line_number":173,"context_line":"        with RADOSClient(self) as client:"},{"line_number":174,"context_line":"            return client.cluster.get_fsid()"},{"line_number":175,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a50d1a3_dca14248","line":172,"updated":"2015-07-30 06:48:23.000000000","message":"nit: although I guess you change this for consistency in the code the change really is unrelated to the task at hand?","commit_id":"5ad9547d716e172760f4320d1ffd6c8f48b5990e"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"dcc2261fdc5221950f0b4e142ceb9fb1fce9028c","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            raise exception.ImageUnacceptable(image_id\u003durl, reason\u003dreason)"},{"line_number":170,"context_line":"        return pieces"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def get_fsid(self):"},{"line_number":173,"context_line":"        with RADOSClient(self) as client:"},{"line_number":174,"context_line":"            return client.cluster.get_fsid()"},{"line_number":175,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a50d1a3_07f41985","line":172,"in_reply_to":"3a50d1a3_dca14248","updated":"2015-07-30 16:56:41.000000000","message":"It\u0027s very much related to the task at hand: it is used in this change by the imagebackend to help put together the RBD URIs, which means it need to be made a \"public\" method.","commit_id":"5ad9547d716e172760f4320d1ffd6c8f48b5990e"},{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"0f9c2a2272000c97e0009c1275f6aeaea2e2aef7","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        \"\"\""},{"line_number":322,"context_line":"        LOG.debug(\u0027flattening %(pool)s/%(vol)s\u0027 % dict(pool\u003dpool, vol\u003dvolume))"},{"line_number":323,"context_line":"        with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":324,"context_line":"            vol.flatten()"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    def exists(self, name, pool\u003dNone, snapshot\u003dNone):"},{"line_number":327,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a50d1a3_db48418d","line":324,"updated":"2015-07-30 20:54:00.000000000","message":"you can wrap long-running librbd calls like this in eventlet.tpool.execute so they don\u0027t block nova. This is safe because the librbd python bindings are a very thin wrapper around the C calls.","commit_id":"3a1f26e7c93639cd0b89b1c7a330f5533b7ecba4"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"a9377e5c9d091c1ff2dd2df8508e19ffcc601241","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        \"\"\""},{"line_number":322,"context_line":"        LOG.debug(\u0027flattening %(pool)s/%(vol)s\u0027 % dict(pool\u003dpool, vol\u003dvolume))"},{"line_number":323,"context_line":"        with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":324,"context_line":"            vol.flatten()"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    def exists(self, name, pool\u003dNone, snapshot\u003dNone):"},{"line_number":327,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a50d1a3_ccce49b7","line":324,"in_reply_to":"3a50d1a3_db48418d","updated":"2015-07-30 21:53:34.000000000","message":"Done","commit_id":"3a1f26e7c93639cd0b89b1c7a330f5533b7ecba4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5a676b1963a562746720ae3920d1fc37e4fbedc9","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        _fsid, pool, image, snapshot \u003d self.parse_url("},{"line_number":212,"context_line":"                image_location[\u0027url\u0027])"},{"line_number":213,"context_line":"        LOG.debug(\u0027cloning %(pool)s/%(img)s@%(snap)s to \u0027"},{"line_number":214,"context_line":"                  \u0027%(dest_pool)s/%(dest_name)s\u0027 %"},{"line_number":215,"context_line":"                  dict(pool\u003dpool, img\u003dimage, snap\u003dsnapshot,"},{"line_number":216,"context_line":"                       dest_pool\u003ddest_pool, dest_name\u003ddest_name))"},{"line_number":217,"context_line":"        with RADOSClient(self, str(pool)) as src_client:"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_55a7994f","line":214,"range":{"start_line":214,"start_character":48,"end_line":214,"end_character":49},"updated":"2016-01-11 19:21:22.000000000","message":"nit: we could change this to a comma to pass the dict as an arg.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"d564596708484f7971f37b31c5054b053089cd0e","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        _fsid, pool, image, snapshot \u003d self.parse_url("},{"line_number":212,"context_line":"                image_location[\u0027url\u0027])"},{"line_number":213,"context_line":"        LOG.debug(\u0027cloning %(pool)s/%(img)s@%(snap)s to \u0027"},{"line_number":214,"context_line":"                  \u0027%(dest_pool)s/%(dest_name)s\u0027 %"},{"line_number":215,"context_line":"                  dict(pool\u003dpool, img\u003dimage, snap\u003dsnapshot,"},{"line_number":216,"context_line":"                       dest_pool\u003ddest_pool, dest_name\u003ddest_name))"},{"line_number":217,"context_line":"        with RADOSClient(self, str(pool)) as src_client:"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_55174fed","line":214,"range":{"start_line":214,"start_character":48,"end_line":214,"end_character":49},"in_reply_to":"fa69d971_55a7994f","updated":"2016-01-12 03:16:48.000000000","message":"Done","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":14119,"name":"Sam Yaple","email":"sam+git@yaple.net","username":"SamYaple"},"change_message_id":"5ec63199c80f6bb32b1095b402e524426db8818f","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        _fsid, pool, image, snapshot \u003d self.parse_url("},{"line_number":212,"context_line":"                image_location[\u0027url\u0027])"},{"line_number":213,"context_line":"        LOG.debug(\u0027cloning %(pool)s/%(img)s@%(snap)s to \u0027"},{"line_number":214,"context_line":"                  \u0027%(dest_pool)s/%(dest_name)s\u0027 %"},{"line_number":215,"context_line":"                  dict(pool\u003dpool, img\u003dimage, snap\u003dsnapshot,"},{"line_number":216,"context_line":"                       dest_pool\u003ddest_pool, dest_name\u003ddest_name))"},{"line_number":217,"context_line":"        with RADOSClient(self, str(pool)) as src_client:"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_5069e78a","line":214,"range":{"start_line":214,"start_character":48,"end_line":214,"end_character":49},"in_reply_to":"fa69d971_55a7994f","updated":"2016-01-11 19:35:45.000000000","message":"there are bugs associated with not using a \u0027,\u0027 and letting the logging module insert variables. The recommended, documented way to use the logging module is with a \u0027,\u0027\n\nWe just hit one of these bugs in Kolla from not using a \u0027,\u0027 in the log message\n\nI would say this is a bit more than a nit","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9303455bf4209290106e7adae87237106c7afe99","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                                    str(dest_name),"},{"line_number":225,"context_line":"                                    features\u003dsrc_client.features)"},{"line_number":226,"context_line":"                except rbd.PermissionError:"},{"line_number":227,"context_line":"                    reason \u003d _LE(\u0027no write permission on storage pool(%s)\u0027)"},{"line_number":228,"context_line":"                    raise exception.Forbidden(reason % dest_pool)"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"    def size(self, name):"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_157a81f1","line":227,"updated":"2016-01-11 19:19:54.000000000","message":"I don\u0027t think you\u0027re supposed to use translation domains (_LE) for exception messages, only for logging.  See:\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#choosing-a-marker-function\n\nAlso, isn\u0027t this the case where nova\u0027s ceph user isn\u0027t configured in authx to the images pool?  So maybe putting that information in here would be helpful as to the actual error, i.e. straight from the spec, \"To use this in an existing installation with authx, adding ‘allow rwx pool\u003dimages’ to nova’s ceph user capabilities is necessary.\"","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        with RBDVolumeProxy(self, name) as vol:"},{"line_number":242,"context_line":"            vol.resize(size)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def create_snap(self, volume, name, pool\u003dNone, protect\u003dFalse):"},{"line_number":245,"context_line":"        \"\"\"Create a snapshot of an RBD volume."},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        :volume: Name of RBD object"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_850ddc90","line":244,"range":{"start_line":244,"start_character":8,"end_line":244,"end_character":19},"updated":"2016-01-11 20:47:43.000000000","message":"nit: I\u0027d call this create_snapshot, I don\u0027t see the need in truncating that.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"bd984f3a91b7014235064ee2803406232ca9c743","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        with RBDVolumeProxy(self, name) as vol:"},{"line_number":242,"context_line":"            vol.resize(size)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def create_snap(self, volume, name, pool\u003dNone, protect\u003dFalse):"},{"line_number":245,"context_line":"        \"\"\"Create a snapshot of an RBD volume."},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        :volume: Name of RBD object"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_d25cb5ef","line":244,"range":{"start_line":244,"start_character":8,"end_line":244,"end_character":19},"in_reply_to":"fa69d971_850ddc90","updated":"2016-01-12 02:51:01.000000000","message":"I named it after the underlying librbd method, but clarity is always better","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            if protect and not vol.is_protected_snap(name):"},{"line_number":256,"context_line":"                vol.protect_snap(name)"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    def remove_snap(self, volume, name, ignore_errors\u003dFalse, pool\u003dNone,"},{"line_number":259,"context_line":"                    force\u003dFalse):"},{"line_number":260,"context_line":"        \"\"\"Removes a snapshot from an RBD volume."},{"line_number":261,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_051a4cc6","line":258,"range":{"start_line":258,"start_character":8,"end_line":258,"end_character":19},"updated":"2016-01-11 20:47:43.000000000","message":"I\u0027d name this remove_snapshot to be clear.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":271,"context_line":"                    if force:"},{"line_number":272,"context_line":"                        vol.unprotect_snap(name)"},{"line_number":273,"context_line":"                    elif not ignore_errors:"},{"line_number":274,"context_line":"                        LOG.warn(_LW(\u0027snapshot(%(name)s) on rbd image(%(img)s)\u0027"},{"line_number":275,"context_line":"                                     \u0027 is protected, skipping\u0027),"},{"line_number":276,"context_line":"                         {\u0027name\u0027: name, \u0027img\u0027: volume})"},{"line_number":277,"context_line":"                        return"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_85b71c84","line":274,"range":{"start_line":274,"start_character":28,"end_line":274,"end_character":32},"updated":"2016-01-11 20:47:43.000000000","message":"nit: should probably using LOG.warning since warning is the method in python 3. I\u0027ve just seen other projects doing mass cleanups for py3 compat with this type of change.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"d564596708484f7971f37b31c5054b053089cd0e","unresolved":false,"context_lines":[{"line_number":271,"context_line":"                    if force:"},{"line_number":272,"context_line":"                        vol.unprotect_snap(name)"},{"line_number":273,"context_line":"                    elif not ignore_errors:"},{"line_number":274,"context_line":"                        LOG.warn(_LW(\u0027snapshot(%(name)s) on rbd image(%(img)s)\u0027"},{"line_number":275,"context_line":"                                     \u0027 is protected, skipping\u0027),"},{"line_number":276,"context_line":"                         {\u0027name\u0027: name, \u0027img\u0027: volume})"},{"line_number":277,"context_line":"                        return"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_b5256b5d","line":274,"range":{"start_line":274,"start_character":28,"end_line":274,"end_character":32},"in_reply_to":"fa69d971_85b71c84","updated":"2016-01-12 03:16:48.000000000","message":"Done","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                          {\u0027name\u0027: name, \u0027img\u0027: volume})"},{"line_number":280,"context_line":"                vol.remove_snap(name)"},{"line_number":281,"context_line":"            elif not ignore_errors:"},{"line_number":282,"context_line":"                LOG.warn(_LW(\u0027no snapshot(%(name)s) found on image(%(img)s)\u0027),"},{"line_number":283,"context_line":"                         {\u0027name\u0027: name, \u0027img\u0027: volume})"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def parent_info(self, volume, pool\u003dNone):"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_a597c0ea","line":282,"range":{"start_line":282,"start_character":61,"end_line":282,"end_character":66},"updated":"2016-01-11 20:47:43.000000000","message":"s/on image/on rbd image/ to be consistent with the above and for context.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"d564596708484f7971f37b31c5054b053089cd0e","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                          {\u0027name\u0027: name, \u0027img\u0027: volume})"},{"line_number":280,"context_line":"                vol.remove_snap(name)"},{"line_number":281,"context_line":"            elif not ignore_errors:"},{"line_number":282,"context_line":"                LOG.warn(_LW(\u0027no snapshot(%(name)s) found on image(%(img)s)\u0027),"},{"line_number":283,"context_line":"                         {\u0027name\u0027: name, \u0027img\u0027: volume})"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def parent_info(self, volume, pool\u003dNone):"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_558d0f7a","line":282,"range":{"start_line":282,"start_character":61,"end_line":282,"end_character":66},"in_reply_to":"fa69d971_a597c0ea","updated":"2016-01-12 03:16:48.000000000","message":"Done","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9303455bf4209290106e7adae87237106c7afe99","unresolved":false,"context_lines":[{"line_number":293,"context_line":"            with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":294,"context_line":"                return vol.parent_info()"},{"line_number":295,"context_line":"        except rbd.ImageNotFound:"},{"line_number":296,"context_line":"            raise exception.ImageUnacceptable(\"no usable parent snapshot\")"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    def flatten(self, volume, pool\u003dNone):"},{"line_number":299,"context_line":"        \"\"\"\"Flattens\" a snapshotted image with the parents\u0027 data,"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_f5d24dd0","line":296,"range":{"start_line":296,"start_character":28,"end_line":296,"end_character":45},"updated":"2016-01-11 19:19:54.000000000","message":"Why not use ImageNotFound here?","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9303455bf4209290106e7adae87237106c7afe99","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        :volume: Name of RBD object"},{"line_number":303,"context_line":"        :pool: Name of pool"},{"line_number":304,"context_line":"        \"\"\""},{"line_number":305,"context_line":"        LOG.debug(\u0027flattening %(pool)s/%(vol)s\u0027 % dict(pool\u003dpool, vol\u003dvolume))"},{"line_number":306,"context_line":"        with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":307,"context_line":"            tpool.execute(vol.flatten)"},{"line_number":308,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_35f46559","line":305,"range":{"start_line":305,"start_character":48,"end_line":305,"end_character":49},"updated":"2016-01-11 19:19:54.000000000","message":"Pass the dict as an arg, so:\n\nLOG.debug(\u0027flattening %(pool)s/%(vol)s\u0027, dict(pool\u003dpool, vol\u003dvolume))","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"d564596708484f7971f37b31c5054b053089cd0e","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        :volume: Name of RBD object"},{"line_number":303,"context_line":"        :pool: Name of pool"},{"line_number":304,"context_line":"        \"\"\""},{"line_number":305,"context_line":"        LOG.debug(\u0027flattening %(pool)s/%(vol)s\u0027 % dict(pool\u003dpool, vol\u003dvolume))"},{"line_number":306,"context_line":"        with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":307,"context_line":"            tpool.execute(vol.flatten)"},{"line_number":308,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_35909b95","line":305,"range":{"start_line":305,"start_character":48,"end_line":305,"end_character":49},"in_reply_to":"fa69d971_35f46559","updated":"2016-01-12 03:16:48.000000000","message":"Done","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9303455bf4209290106e7adae87237106c7afe99","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        \"\"\""},{"line_number":305,"context_line":"        LOG.debug(\u0027flattening %(pool)s/%(vol)s\u0027 % dict(pool\u003dpool, vol\u003dvolume))"},{"line_number":306,"context_line":"        with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":307,"context_line":"            tpool.execute(vol.flatten)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def exists(self, name, pool\u003dNone, snapshot\u003dNone):"},{"line_number":310,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_75f65d5e","line":307,"range":{"start_line":307,"start_character":30,"end_line":307,"end_character":37},"updated":"2016-01-11 19:19:54.000000000","message":"Is this asynchronous? If so, how can we tell when it\u0027s done - or do we not care?","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":14119,"name":"Sam Yaple","email":"sam+git@yaple.net","username":"SamYaple"},"change_message_id":"5ec63199c80f6bb32b1095b402e524426db8818f","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        \"\"\""},{"line_number":305,"context_line":"        LOG.debug(\u0027flattening %(pool)s/%(vol)s\u0027 % dict(pool\u003dpool, vol\u003dvolume))"},{"line_number":306,"context_line":"        with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":307,"context_line":"            tpool.execute(vol.flatten)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def exists(self, name, pool\u003dNone, snapshot\u003dNone):"},{"line_number":310,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_30d6b34f","line":307,"range":{"start_line":307,"start_character":30,"end_line":307,"end_character":37},"in_reply_to":"fa69d971_75f65d5e","updated":"2016-01-11 19:35:45.000000000","message":"It is not asynchronous and can take some time to return. This provides the ability to (if this fails) fall back to normal nova snapshot","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3040ec1bcb3d838f6b9a3b27c8dc7d8db45f99e","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":351,"context_line":"        utils.execute(\u0027rbd\u0027, \u0027import\u0027, *args)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def _cleanup_vol(self, ioctx, volume, retryctx):"},{"line_number":354,"context_line":"        try:"},{"line_number":355,"context_line":"            rbd.RBD().remove(ioctx, volume)"},{"line_number":356,"context_line":"            raise loopingcall.LoopingCallDone(retvalue\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_88f413f1","line":353,"range":{"start_line":353,"start_character":8,"end_line":353,"end_character":20},"updated":"2016-01-11 20:47:43.000000000","message":"Seems you could keep this as an inner method in _destroy_volume, but it\u0027s not that big a deal to pull it out, unless I\u0027m missing something.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0fb2a318ee864107cd24a1ea49cc660bd3d675f8","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":351,"context_line":"        utils.execute(\u0027rbd\u0027, \u0027import\u0027, *args)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def _cleanup_vol(self, ioctx, volume, retryctx):"},{"line_number":354,"context_line":"        try:"},{"line_number":355,"context_line":"            rbd.RBD().remove(ioctx, volume)"},{"line_number":356,"context_line":"            raise loopingcall.LoopingCallDone(retvalue\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_f6433711","line":353,"range":{"start_line":353,"start_character":8,"end_line":353,"end_character":20},"in_reply_to":"da6ed579_72ce2985","updated":"2016-01-14 18:42:25.000000000","message":"It\u0027s an unnecessary change though, which makes the review bigger and more complicated, so if it\u0027s all the same I\u0027d like it not broken out if that\u0027s OK.","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"bd984f3a91b7014235064ee2803406232ca9c743","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":351,"context_line":"        utils.execute(\u0027rbd\u0027, \u0027import\u0027, *args)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def _cleanup_vol(self, ioctx, volume, retryctx):"},{"line_number":354,"context_line":"        try:"},{"line_number":355,"context_line":"            rbd.RBD().remove(ioctx, volume)"},{"line_number":356,"context_line":"            raise loopingcall.LoopingCallDone(retvalue\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_72ce2985","line":353,"range":{"start_line":353,"start_character":8,"end_line":353,"end_character":20},"in_reply_to":"fa69d971_88f413f1","updated":"2016-01-12 02:51:01.000000000","message":"Honestly, I\u0027ve forgotten why this needed to get pulled out.  My suspicion is that once upon a time, cleanup_volumes() and destroy_volume() below both wanted to use this method, but as you can see, the latter has been refactored down to not need it anymore.\n\nConsider it future-proofing?","commit_id":"b7ab750b9690c5c39f957a083557db451fc87ed2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a28df1c38e9a01186682cdac4ec03b73b770cb50","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                                    str(dest_name),"},{"line_number":226,"context_line":"                                    features\u003dsrc_client.features)"},{"line_number":227,"context_line":"                except rbd.PermissionError:"},{"line_number":228,"context_line":"                    raise exception.Forbidden(\u0027no write permission on storage \u0027"},{"line_number":229,"context_line":"                                              \u0027pool(%s)\u0027, dest_pool)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def size(self, name):"},{"line_number":232,"context_line":"        with RBDVolumeProxy(self, name) as vol:"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_96d3db03","line":229,"range":{"start_line":228,"start_character":46,"end_line":229,"end_character":67},"updated":"2016-01-14 18:57:24.000000000","message":"This is not a logging statement, using the comma won\u0027t work, you have to use %. Which leads me to believe we don\u0027t have unit test coverage for this case...","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0c6a45b9fdc38f124b104bad0cbcca64063758f6","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                                    str(dest_name),"},{"line_number":226,"context_line":"                                    features\u003dsrc_client.features)"},{"line_number":227,"context_line":"                except rbd.PermissionError:"},{"line_number":228,"context_line":"                    raise exception.Forbidden(\u0027no write permission on storage \u0027"},{"line_number":229,"context_line":"                                              \u0027pool(%s)\u0027, dest_pool)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def size(self, name):"},{"line_number":232,"context_line":"        with RBDVolumeProxy(self, name) as vol:"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_81f5d2ed","line":229,"range":{"start_line":228,"start_character":46,"end_line":229,"end_character":67},"in_reply_to":"da6ed579_96d3db03","updated":"2016-01-14 19:53:36.000000000","message":"Done","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a28df1c38e9a01186682cdac4ec03b73b770cb50","unresolved":false,"context_lines":[{"line_number":296,"context_line":"            with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":297,"context_line":"                return vol.parent_info()"},{"line_number":298,"context_line":"        except rbd.ImageNotFound:"},{"line_number":299,"context_line":"            raise exception.ImageUnacceptable(\"no usable parent snapshot\")"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def flatten(self, volume, pool\u003dNone):"},{"line_number":302,"context_line":"        \"\"\"\"Flattens\" a snapshotted image with the parents\u0027 data,"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_7672e712","line":299,"range":{"start_line":299,"start_character":47,"end_line":299,"end_character":72},"updated":"2016-01-14 18:57:24.000000000","message":"I tend to think that exception messages are supposed to be translated, so using _().","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a28df1c38e9a01186682cdac4ec03b73b770cb50","unresolved":false,"context_lines":[{"line_number":296,"context_line":"            with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":297,"context_line":"                return vol.parent_info()"},{"line_number":298,"context_line":"        except rbd.ImageNotFound:"},{"line_number":299,"context_line":"            raise exception.ImageUnacceptable(\"no usable parent snapshot\")"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def flatten(self, volume, pool\u003dNone):"},{"line_number":302,"context_line":"        \"\"\"\"Flattens\" a snapshotted image with the parents\u0027 data,"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_761bc7ca","line":299,"range":{"start_line":299,"start_character":28,"end_line":299,"end_character":45},"updated":"2016-01-14 18:57:24.000000000","message":"You didn\u0027t answer this in PS24, so I\u0027ll ask again, why not raise ImageNotFound here? I get that we are happy with being lazy and raising ImageUnacceptable for everything out of this module, and maybe that\u0027s OK since that\u0027s what we are handling at the top level in the libvirt driver module, idk, it just seems not great to me, but maybe not worth changing right now.","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe380b285be2be24bd6e87fdb7bed84d478116c7","unresolved":false,"context_lines":[{"line_number":296,"context_line":"            with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":297,"context_line":"                return vol.parent_info()"},{"line_number":298,"context_line":"        except rbd.ImageNotFound:"},{"line_number":299,"context_line":"            raise exception.ImageUnacceptable(\"no usable parent snapshot\")"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def flatten(self, volume, pool\u003dNone):"},{"line_number":302,"context_line":"        \"\"\"\"Flattens\" a snapshotted image with the parents\u0027 data,"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_cf9e84b8","line":299,"range":{"start_line":299,"start_character":28,"end_line":299,"end_character":45},"in_reply_to":"da6ed579_67e78eac","updated":"2016-01-15 02:09:18.000000000","message":"Like I said, ImageUnacceptable is OK, it just seems to be over-used in this module for all reasons that anything can go wrong with an image. In this case it\u0027s unacceptable because we can\u0027t find the parent. We don\u0027t want to leak rbd.ImageNotFound out of there because the calling code is handling nova exceptions (or at least should be). So leave it or change it, it\u0027s up to you. I\u0027m thinking we probably want the error message translatable though, but not 100% sure on that, it might be worth bringing up in IRC.","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"826e1715de929d0eb91faf790da801ab28aa762b","unresolved":false,"context_lines":[{"line_number":296,"context_line":"            with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":297,"context_line":"                return vol.parent_info()"},{"line_number":298,"context_line":"        except rbd.ImageNotFound:"},{"line_number":299,"context_line":"            raise exception.ImageUnacceptable(\"no usable parent snapshot\")"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def flatten(self, volume, pool\u003dNone):"},{"line_number":302,"context_line":"        \"\"\"\"Flattens\" a snapshotted image with the parents\u0027 data,"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_67e78eac","line":299,"range":{"start_line":299,"start_character":28,"end_line":299,"end_character":45},"in_reply_to":"da6ed579_761bc7ca","updated":"2016-01-14 20:27:17.000000000","message":"It\u0027s not laziness; it would be far easier to just catch the rbd.ImageNotFound exception in the caller.  The rationale is that if there is a problem getting the volume\u0027s parent_info, then the image cannot be used, so ImageUnacceptable is the \"more correct\" exception to throw.\n\nReasonable people can disagree on this, however.","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0c6a45b9fdc38f124b104bad0cbcca64063758f6","unresolved":false,"context_lines":[{"line_number":296,"context_line":"            with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":297,"context_line":"                return vol.parent_info()"},{"line_number":298,"context_line":"        except rbd.ImageNotFound:"},{"line_number":299,"context_line":"            raise exception.ImageUnacceptable(\"no usable parent snapshot\")"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def flatten(self, volume, pool\u003dNone):"},{"line_number":302,"context_line":"        \"\"\"\"Flattens\" a snapshotted image with the parents\u0027 data,"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_c1ef4a1c","line":299,"range":{"start_line":299,"start_character":47,"end_line":299,"end_character":72},"in_reply_to":"da6ed579_7672e712","updated":"2016-01-14 19:53:36.000000000","message":"Done","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a28df1c38e9a01186682cdac4ec03b73b770cb50","unresolved":false,"context_lines":[{"line_number":328,"context_line":"            try:"},{"line_number":329,"context_line":"                rbd.RBD().remove(client.ioctx, name)"},{"line_number":330,"context_line":"            except rbd.ImageNotFound:"},{"line_number":331,"context_line":"                LOG.warning(_LW(\u0027image %(volume)s in pool %(pool)s cannot be \u0027"},{"line_number":332,"context_line":"                                \u0027found, failed to remove\u0027),"},{"line_number":333,"context_line":"                            {\u0027volume\u0027: name, \u0027pool\u0027: self.pool})"},{"line_number":334,"context_line":"            except rbd.ImageHasSnapshots:"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_56e323a4","line":331,"updated":"2016-01-14 18:57:24.000000000","message":"This is an unrelated change, should probably undo this.","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0c6a45b9fdc38f124b104bad0cbcca64063758f6","unresolved":false,"context_lines":[{"line_number":328,"context_line":"            try:"},{"line_number":329,"context_line":"                rbd.RBD().remove(client.ioctx, name)"},{"line_number":330,"context_line":"            except rbd.ImageNotFound:"},{"line_number":331,"context_line":"                LOG.warning(_LW(\u0027image %(volume)s in pool %(pool)s cannot be \u0027"},{"line_number":332,"context_line":"                                \u0027found, failed to remove\u0027),"},{"line_number":333,"context_line":"                            {\u0027volume\u0027: name, \u0027pool\u0027: self.pool})"},{"line_number":334,"context_line":"            except rbd.ImageHasSnapshots:"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_01b48232","line":331,"in_reply_to":"da6ed579_56e323a4","updated":"2016-01-14 19:53:36.000000000","message":"Done","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a28df1c38e9a01186682cdac4ec03b73b770cb50","unresolved":false,"context_lines":[{"line_number":353,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":354,"context_line":"        utils.execute(\u0027rbd\u0027, \u0027import\u0027, *args)"},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"    def _cleanup_vol(self, ioctx, volume, retryctx):"},{"line_number":357,"context_line":"        try:"},{"line_number":358,"context_line":"            rbd.RBD().remove(ioctx, volume)"},{"line_number":359,"context_line":"            raise loopingcall.LoopingCallDone(retvalue\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_369c6f0e","line":356,"updated":"2016-01-14 18:57:24.000000000","message":"As noted in PS24, I don\u0027t think there is a reason that this needs to be broken out into a separate method, so if it\u0027s all the same, I\u0027d just leave it the way it was (makes the diff smaller).","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0c6a45b9fdc38f124b104bad0cbcca64063758f6","unresolved":false,"context_lines":[{"line_number":353,"context_line":"        args +\u003d self.ceph_args()"},{"line_number":354,"context_line":"        utils.execute(\u0027rbd\u0027, \u0027import\u0027, *args)"},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"    def _cleanup_vol(self, ioctx, volume, retryctx):"},{"line_number":357,"context_line":"        try:"},{"line_number":358,"context_line":"            rbd.RBD().remove(ioctx, volume)"},{"line_number":359,"context_line":"            raise loopingcall.LoopingCallDone(retvalue\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6ed579_817e3268","line":356,"in_reply_to":"da6ed579_369c6f0e","updated":"2016-01-14 19:53:36.000000000","message":"Done","commit_id":"9f520157538a024fcb20ddd205dda967c5df0678"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50744e813abd9d1967cbc0668fdff03b830c9834","unresolved":false,"context_lines":[{"line_number":283,"context_line":"    def cleanup_volumes(self, instance):"},{"line_number":284,"context_line":"        def _cleanup_vol(ioctx, volume, retryctx):"},{"line_number":285,"context_line":"            try:"},{"line_number":286,"context_line":"                rbd.RBD().remove(client.ioctx, volume)"},{"line_number":287,"context_line":"                raise loopingcall.LoopingCallDone(retvalue\u003dFalse)"},{"line_number":288,"context_line":"            except rbd.ImageHasSnapshots:"},{"line_number":289,"context_line":"                self.remove_snap(volume, \u0027nova-resize\u0027, ignore_errors\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_2ffe40ae","side":"PARENT","line":286,"range":{"start_line":286,"start_character":33,"end_line":286,"end_character":45},"updated":"2016-01-15 02:31:36.000000000","message":"not really needed to change, but i don\u0027t care too much","commit_id":"0333d73c4004a68dbde3e059e6ce4431bd4c08d9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50744e813abd9d1967cbc0668fdff03b830c9834","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                                    str(dest_name),"},{"line_number":226,"context_line":"                                    features\u003dsrc_client.features)"},{"line_number":227,"context_line":"                except rbd.PermissionError:"},{"line_number":228,"context_line":"                    raise exception.Forbidden(\u0027no write permission on storage \u0027"},{"line_number":229,"context_line":"                                              \u0027pool(%s)\u0027 % dest_pool)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def size(self, name):"},{"line_number":232,"context_line":"        with RBDVolumeProxy(self, name) as vol:"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_4f761471","line":229,"range":{"start_line":228,"start_character":47,"end_line":229,"end_character":55},"updated":"2016-01-15 02:31:36.000000000","message":"Should probably be translatable with _().","commit_id":"8e580d3c52217ae42401c5012b724308d301934d"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"2148b50ff33f2363914bb3355c3db005db60ba33","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                                    str(dest_name),"},{"line_number":226,"context_line":"                                    features\u003dsrc_client.features)"},{"line_number":227,"context_line":"                except rbd.PermissionError:"},{"line_number":228,"context_line":"                    raise exception.Forbidden(\u0027no write permission on storage \u0027"},{"line_number":229,"context_line":"                                              \u0027pool(%s)\u0027 % dest_pool)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def size(self, name):"},{"line_number":232,"context_line":"        with RBDVolumeProxy(self, name) as vol:"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_48d580e2","line":229,"range":{"start_line":228,"start_character":47,"end_line":229,"end_character":55},"in_reply_to":"da6ed579_4f761471","updated":"2016-01-15 18:27:24.000000000","message":"Done","commit_id":"8e580d3c52217ae42401c5012b724308d301934d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50744e813abd9d1967cbc0668fdff03b830c9834","unresolved":false,"context_lines":[{"line_number":363,"context_line":"            except rbd.ImageHasSnapshots:"},{"line_number":364,"context_line":"                self.remove_snap(volume, \u0027nova-resize\u0027, ignore_errors\u003dTrue)"},{"line_number":365,"context_line":"            except (rbd.ImageBusy, rbd.ImageHasSnapshots):"},{"line_number":366,"context_line":"                LOG.warning(_LW(\u0027rbd remove %(volume)s in pool %(pool)s \u0027"},{"line_number":367,"context_line":"                                \u0027failed\u0027),"},{"line_number":368,"context_line":"                            {\u0027volume\u0027: volume, \u0027pool\u0027: self.pool})"},{"line_number":369,"context_line":"            retryctx[\u0027retries\u0027] -\u003d 1"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_ef3268a6","line":366,"range":{"start_line":366,"start_character":16,"end_line":366,"end_character":27},"updated":"2016-01-15 02:31:36.000000000","message":"ditto, didn\u0027t really have to change as part of this change.","commit_id":"8e580d3c52217ae42401c5012b724308d301934d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a97d53dac9e8828b8ec4877283cf216085e7bacc","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                    \u0027free\u0027: stats[\u0027kb_avail\u0027] * units.Ki,"},{"line_number":331,"context_line":"                    \u0027used\u0027: stats[\u0027kb_used\u0027] * units.Ki}"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    def create_snap(self, volume, name):"},{"line_number":334,"context_line":"        \"\"\"Create a snapshot on an RBD object."},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"        :volume: Name of RBD object"}],"source_content_type":"text/x-python","patch_set":32,"id":"da6ed579_e306d784","side":"PARENT","line":333,"range":{"start_line":333,"start_character":8,"end_line":333,"end_character":19},"updated":"2016-01-15 18:37:51.000000000","message":"Why did you move where create_snap and remove_snap are defined in the module? That really throws off the diff unnecessarily.","commit_id":"a2b0143207913dd6ca0657d24990a0b3f2c3c5ed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a97d53dac9e8828b8ec4877283cf216085e7bacc","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                                    dest_client.ioctx,"},{"line_number":225,"context_line":"                                    str(dest_name),"},{"line_number":226,"context_line":"                                    features\u003dsrc_client.features)"},{"line_number":227,"context_line":"                except rbd.PermissionError:"},{"line_number":228,"context_line":"                    raise exception.Forbidden(_(\u0027no write permission on\u0027"},{"line_number":229,"context_line":"                                                \u0027storage pool %s\u0027) % dest_pool)"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"da6ed579_e36017a7","line":227,"range":{"start_line":227,"start_character":16,"end_line":227,"end_character":42},"updated":"2016-01-15 18:37:51.000000000","message":"There isn\u0027t any unit test for this case in test_rbd.","commit_id":"3539276125ef1056667a16b6ccbd7414f4829cd7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5deb656a187a8aedee0f9b67c46d186706f5afb9","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                                    str(dest_name),"},{"line_number":226,"context_line":"                                    features\u003dsrc_client.features)"},{"line_number":227,"context_line":"                except rbd.PermissionError:"},{"line_number":228,"context_line":"                    raise exception.Forbidden(_(\u0027no write permission on\u0027"},{"line_number":229,"context_line":"                                                \u0027storage pool %s\u0027) % dest_pool)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def size(self, name):"}],"source_content_type":"text/x-python","patch_set":32,"id":"da6ed579_03d68b4e","line":228,"range":{"start_line":228,"start_character":69,"end_line":228,"end_character":71},"updated":"2016-01-15 18:32:23.000000000","message":"Need a space between \u0027on\u0027 and \u0027storage\u0027.","commit_id":"3539276125ef1056667a16b6ccbd7414f4829cd7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e04410b1a246fed74c7f2cff5aae08a1252e59ac","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                                    dest_client.ioctx,"},{"line_number":225,"context_line":"                                    str(dest_name),"},{"line_number":226,"context_line":"                                    features\u003dsrc_client.features)"},{"line_number":227,"context_line":"                except rbd.PermissionError:"},{"line_number":228,"context_line":"                    raise exception.Forbidden(_(\u0027no write permission on \u0027"},{"line_number":229,"context_line":"                                                \u0027storage pool %s\u0027) % dest_pool)"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_a39d3f0b","line":227,"range":{"start_line":227,"start_character":16,"end_line":227,"end_character":42},"updated":"2016-01-15 18:42:50.000000000","message":"There is no unit test for this case in rbd_utils.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                                    dest_client.ioctx,"},{"line_number":225,"context_line":"                                    str(dest_name),"},{"line_number":226,"context_line":"                                    features\u003dsrc_client.features)"},{"line_number":227,"context_line":"                except rbd.PermissionError:"},{"line_number":228,"context_line":"                    raise exception.Forbidden(_(\u0027no write permission on \u0027"},{"line_number":229,"context_line":"                                                \u0027storage pool %s\u0027) % dest_pool)"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_3e1ac6c6","line":227,"range":{"start_line":227,"start_character":16,"end_line":227,"end_character":42},"in_reply_to":"da6ed579_a39d3f0b","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e04410b1a246fed74c7f2cff5aae08a1252e59ac","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        with RBDVolumeProxy(self, name) as vol:"},{"line_number":243,"context_line":"            vol.resize(size)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"    def create_snap(self, volume, name, pool\u003dNone, protect\u003dFalse):"},{"line_number":246,"context_line":"        \"\"\"Create a snapshot of an RBD volume."},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"        :volume: Name of RBD object"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_43a48349","line":245,"range":{"start_line":245,"start_character":8,"end_line":245,"end_character":19},"updated":"2016-01-15 18:42:50.000000000","message":"Please don\u0027t move the definition of the create_snap and remove_snap methods in this change, I actually thought these were brand new methods, but just realized you were moving them for some reason. That explodes the size of the diff here, and makes this harder to review what you\u0027re actually changing, so please move that back and just apply the changes you need for this.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        with RBDVolumeProxy(self, name) as vol:"},{"line_number":243,"context_line":"            vol.resize(size)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"    def create_snap(self, volume, name, pool\u003dNone, protect\u003dFalse):"},{"line_number":246,"context_line":"        \"\"\"Create a snapshot of an RBD volume."},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"        :volume: Name of RBD object"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_1e1fc2b7","line":245,"range":{"start_line":245,"start_character":8,"end_line":245,"end_character":19},"in_reply_to":"da6ed579_43a48349","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            if protect and not vol.is_protected_snap(name):"},{"line_number":257,"context_line":"                tpool.execute(vol.protect_snap, name)"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    def remove_snap(self, volume, name, ignore_errors\u003dFalse, pool\u003dNone,"},{"line_number":260,"context_line":"                    force\u003dFalse):"},{"line_number":261,"context_line":"        \"\"\"Removes a snapshot from an RBD volume."},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_de284a88","line":259,"range":{"start_line":259,"start_character":40,"end_line":259,"end_character":53},"updated":"2016-01-15 19:06:53.000000000","message":"Is it just me or do we not have any unit test coverage for this kwarg?","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            if protect and not vol.is_protected_snap(name):"},{"line_number":257,"context_line":"                tpool.execute(vol.protect_snap, name)"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    def remove_snap(self, volume, name, ignore_errors\u003dFalse, pool\u003dNone,"},{"line_number":260,"context_line":"                    force\u003dFalse):"},{"line_number":261,"context_line":"        \"\"\"Removes a snapshot from an RBD volume."},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_b25a6629","line":259,"range":{"start_line":259,"start_character":40,"end_line":259,"end_character":53},"in_reply_to":"da6ed579_de284a88","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":271,"context_line":"                if vol.is_protected_snap(name):"},{"line_number":272,"context_line":"                    if force:"},{"line_number":273,"context_line":"                        tpool.execute(vol.unprotect_snap, name)"},{"line_number":274,"context_line":"                    elif not ignore_errors:"},{"line_number":275,"context_line":"                        LOG.warning(_LW(\u0027snapshot(%(name)s) on rbd \u0027"},{"line_number":276,"context_line":"                                        \u0027image(%(img)s) is protected, \u0027"},{"line_number":277,"context_line":"                                        \u0027skipping\u0027),"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_be6f96c2","line":274,"range":{"start_line":274,"start_character":20,"end_line":274,"end_character":42},"updated":"2016-01-15 19:06:53.000000000","message":"Should probably test this case to assert that vol.remove_snap is not called if it\u0027s protected and force is False and ignore_errors is False (and does call vol.remove_snap if it\u0027s protected and force\u003dFalse and ignore_errors\u003dTrue).","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":271,"context_line":"                if vol.is_protected_snap(name):"},{"line_number":272,"context_line":"                    if force:"},{"line_number":273,"context_line":"                        tpool.execute(vol.unprotect_snap, name)"},{"line_number":274,"context_line":"                    elif not ignore_errors:"},{"line_number":275,"context_line":"                        LOG.warning(_LW(\u0027snapshot(%(name)s) on rbd \u0027"},{"line_number":276,"context_line":"                                        \u0027image(%(img)s) is protected, \u0027"},{"line_number":277,"context_line":"                                        \u0027skipping\u0027),"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_8910595b","line":274,"range":{"start_line":274,"start_character":20,"end_line":274,"end_character":42},"in_reply_to":"da6ed579_be6f96c2","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":296,"context_line":"            with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":297,"context_line":"                return vol.parent_info()"},{"line_number":298,"context_line":"        except rbd.ImageNotFound:"},{"line_number":299,"context_line":"            raise exception.ImageUnacceptable(_(\"no usable parent snapshot\"))"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def flatten(self, volume, pool\u003dNone):"},{"line_number":302,"context_line":"        \"\"\"\"Flattens\" a snapshotted image with the parents\u0027 data,"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_de060a85","line":299,"range":{"start_line":299,"start_character":49,"end_line":299,"end_character":74},"updated":"2016-01-15 19:06:53.000000000","message":"I suppose we should put the volume (id) in here for context.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":296,"context_line":"            with RBDVolumeProxy(self, str(volume), pool\u003dpool) as vol:"},{"line_number":297,"context_line":"                return vol.parent_info()"},{"line_number":298,"context_line":"        except rbd.ImageNotFound:"},{"line_number":299,"context_line":"            raise exception.ImageUnacceptable(_(\"no usable parent snapshot\"))"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def flatten(self, volume, pool\u003dNone):"},{"line_number":302,"context_line":"        \"\"\"\"Flattens\" a snapshotted image with the parents\u0027 data,"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_0c1c7769","line":299,"range":{"start_line":299,"start_character":49,"end_line":299,"end_character":74},"in_reply_to":"da6ed579_de060a85","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":328,"context_line":"            try:"},{"line_number":329,"context_line":"                rbd.RBD().remove(client.ioctx, name)"},{"line_number":330,"context_line":"            except rbd.ImageNotFound:"},{"line_number":331,"context_line":"                LOG.warn(_LW(\u0027image %(volume)s in pool %(pool)s cannot be \u0027"},{"line_number":332,"context_line":"                             \u0027found, failed to remove\u0027),"},{"line_number":333,"context_line":"                         {\u0027volume\u0027: name, \u0027pool\u0027: self.pool})"},{"line_number":334,"context_line":"            except rbd.ImageHasSnapshots:"},{"line_number":335,"context_line":"                LOG.error(_LE(\u0027image %(volume)s in pool %(pool)s has \u0027"},{"line_number":336,"context_line":"                              \u0027snapshots, failed to remove\u0027),"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_be36b64e","line":333,"range":{"start_line":331,"start_character":16,"end_line":333,"end_character":61},"updated":"2016-01-15 19:06:53.000000000","message":"These are unrelated changes that you should remove, they just clutter up the diff.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":328,"context_line":"            try:"},{"line_number":329,"context_line":"                rbd.RBD().remove(client.ioctx, name)"},{"line_number":330,"context_line":"            except rbd.ImageNotFound:"},{"line_number":331,"context_line":"                LOG.warn(_LW(\u0027image %(volume)s in pool %(pool)s cannot be \u0027"},{"line_number":332,"context_line":"                             \u0027found, failed to remove\u0027),"},{"line_number":333,"context_line":"                         {\u0027volume\u0027: name, \u0027pool\u0027: self.pool})"},{"line_number":334,"context_line":"            except rbd.ImageHasSnapshots:"},{"line_number":335,"context_line":"                LOG.error(_LE(\u0027image %(volume)s in pool %(pool)s has \u0027"},{"line_number":336,"context_line":"                              \u0027snapshots, failed to remove\u0027),"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_723dceba","line":333,"range":{"start_line":331,"start_character":16,"end_line":333,"end_character":61},"in_reply_to":"da6ed579_be36b64e","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":363,"context_line":"            except rbd.ImageHasSnapshots:"},{"line_number":364,"context_line":"                self.remove_snap(volume, \u0027nova-resize\u0027, ignore_errors\u003dTrue)"},{"line_number":365,"context_line":"            except (rbd.ImageBusy, rbd.ImageHasSnapshots):"},{"line_number":366,"context_line":"                LOG.warning(_LW(\u0027rbd remove %(volume)s in pool %(pool)s \u0027"},{"line_number":367,"context_line":"                                \u0027failed\u0027),"},{"line_number":368,"context_line":"                            {\u0027volume\u0027: volume, \u0027pool\u0027: self.pool})"},{"line_number":369,"context_line":"            retryctx[\u0027retries\u0027] -\u003d 1"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_3e43a6b0","line":366,"range":{"start_line":366,"start_character":20,"end_line":366,"end_character":27},"updated":"2016-01-15 19:06:53.000000000","message":"Undo this as it\u0027s an unrelated change and goofs up the diff.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":363,"context_line":"            except rbd.ImageHasSnapshots:"},{"line_number":364,"context_line":"                self.remove_snap(volume, \u0027nova-resize\u0027, ignore_errors\u003dTrue)"},{"line_number":365,"context_line":"            except (rbd.ImageBusy, rbd.ImageHasSnapshots):"},{"line_number":366,"context_line":"                LOG.warning(_LW(\u0027rbd remove %(volume)s in pool %(pool)s \u0027"},{"line_number":367,"context_line":"                                \u0027failed\u0027),"},{"line_number":368,"context_line":"                            {\u0027volume\u0027: volume, \u0027pool\u0027: self.pool})"},{"line_number":369,"context_line":"            retryctx[\u0027retries\u0027] -\u003d 1"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_124a5256","line":366,"range":{"start_line":366,"start_character":20,"end_line":366,"end_character":27},"in_reply_to":"da6ed579_3e43a6b0","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ba6737f765efe067a920041d64f7fbfd6b13beb","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                             \u0027cannot rollback\u0027) %"},{"line_number":426,"context_line":"                           {\u0027snap\u0027: name, \u0027img\u0027: volume})"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"    def destroy_volume(self, volume, pool\u003dNone):"},{"line_number":429,"context_line":"        \"\"\"A one-shot version of cleanup_volumes()"},{"line_number":430,"context_line":"        \"\"\""},{"line_number":431,"context_line":"        with RADOSClient(self, pool) as client:"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_fe7e6ee2","line":428,"range":{"start_line":428,"start_character":8,"end_line":428,"end_character":22},"updated":"2016-01-15 19:06:53.000000000","message":"Should probably have a simple unit test for this in test_rbd.","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0a69e4bc8cfdcc2d942da8ffb24739d9d8bced1a","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                             \u0027cannot rollback\u0027) %"},{"line_number":426,"context_line":"                           {\u0027snap\u0027: name, \u0027img\u0027: volume})"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"    def destroy_volume(self, volume, pool\u003dNone):"},{"line_number":429,"context_line":"        \"\"\"A one-shot version of cleanup_volumes()"},{"line_number":430,"context_line":"        \"\"\""},{"line_number":431,"context_line":"        with RADOSClient(self, pool) as client:"}],"source_content_type":"text/x-python","patch_set":34,"id":"da6ed579_d2727ab0","line":428,"range":{"start_line":428,"start_character":8,"end_line":428,"end_character":22},"in_reply_to":"da6ed579_fe7e6ee2","updated":"2016-01-16 00:40:39.000000000","message":"Done","commit_id":"edb4742ae24f7201216533a23ab07928ab025afe"}],"releasenotes/notes/bp-rbd-instance-snapshots-130e860b726ddc16.yaml":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50744e813abd9d1967cbc0668fdff03b830c9834","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - When RBD is used for ephemeral disks and image storage, make"},{"line_number":4,"context_line":"    snapshot use Ceph directly, and update Glance with the new location."},{"line_number":5,"context_line":"    In case of failure, it will gracefully fallback to the \"generic\""},{"line_number":6,"context_line":"    snapshot method.  This requires changing the typical permissions"},{"line_number":7,"context_line":"    for the Nova Ceph user (if using authx) to allow writing to"},{"line_number":8,"context_line":"    the pool where vm images are stored, and it also requires"},{"line_number":9,"context_line":"    configuring Glance to provide a v2 endpoint with direct_url"},{"line_number":10,"context_line":"    support enabled (there are security implications to doing this)."},{"line_number":11,"context_line":"    See http://docs.ceph.com/docs/master/rbd/rbd-openstack/ for more"},{"line_number":12,"context_line":"    information on configuring OpenStack with RBD."}],"source_content_type":"text/x-yaml","patch_set":31,"id":"da6ed579_4feb74df","line":12,"range":{"start_line":3,"start_character":4,"end_line":12,"end_character":50},"updated":"2016-01-15 02:31:36.000000000","message":"Very nice.","commit_id":"8e580d3c52217ae42401c5012b724308d301934d"}]}