)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"32321c1e64a3996881de3275318db257f426dfee","unresolved":false,"context_lines":[{"line_number":13,"context_line":"At this point glance is effectively the owner of the snapshot, and"},{"line_number":14,"context_line":"nova cannot delete the original disk, so a corresponding change to"},{"line_number":15,"context_line":"glance is needed to handle this cleanup later, when the snapshot is"},{"line_number":16,"context_line":"deleted."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"To accomplish this, create an rbd clone and store the clone\u0027s location"},{"line_number":19,"context_line":"in glance, and when nova deletes an rbd disk with snapshots, rename"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7a016987_f3f10996","line":16,"updated":"2015-05-22 13:58:31.000000000","message":"Has this change to glance been made? This is a really good reason for this to be a spec, IMHO, because it changes the interaction between nova and another service, and in a way I can\u0027t easily verify as valid.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"5ec197cb7c907f03127f2a77bbbb899d6956eade","unresolved":false,"context_lines":[{"line_number":13,"context_line":"At this point glance is effectively the owner of the snapshot, and"},{"line_number":14,"context_line":"nova cannot delete the original disk, so a corresponding change to"},{"line_number":15,"context_line":"glance is needed to handle this cleanup later, when the snapshot is"},{"line_number":16,"context_line":"deleted."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"To accomplish this, create an rbd clone and store the clone\u0027s location"},{"line_number":19,"context_line":"in glance, and when nova deletes an rbd disk with snapshots, rename"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"3afb71cf_1390f6eb","line":16,"in_reply_to":"7a016987_f3f10996","updated":"2015-06-04 21:59:36.000000000","message":"The glance change is here, blocked on the nova spec:\n\nhttps://review.openstack.org/#/c/127397/","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"32321c1e64a3996881de3275318db257f426dfee","unresolved":false,"context_lines":[{"line_number":21,"context_line":"deleted when glance deletes the snapshots."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"This requires changing the typical permissions for the glance ceph"},{"line_number":24,"context_line":"user to allow writing to the pool where vm images are stored."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: Idefa95f126f2922acf008038e2ac7430b1d26a33"},{"line_number":27,"context_line":"Implements: blueprint implement-rbd-snapshots-instead-of-qemu-snapshots"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7a016987_7334f948","line":24,"updated":"2015-05-22 13:58:31.000000000","message":"This probably means that you need a DocImpact here. Presumably the spec can describe the changes needed to a deployer\u0027s ceph/glance installation, as well as specify whether we need a specific version of glance installed to support the above new interactions.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"9ff8751f656fd3e20221534c3559cb88910ba61a","unresolved":false,"context_lines":[{"line_number":21,"context_line":"deleted when glance deletes the snapshots."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"This requires changing the typical permissions for the glance ceph"},{"line_number":24,"context_line":"user to allow writing to the pool where vm images are stored."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: Idefa95f126f2922acf008038e2ac7430b1d26a33"},{"line_number":27,"context_line":"Implements: blueprint implement-rbd-snapshots-instead-of-qemu-snapshots"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"3afb71cf_76bb830f","line":24,"in_reply_to":"7a016987_7334f948","updated":"2015-06-05 11:58:55.000000000","message":"Done","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"457fd541d1e00db586ac886bc996175b213c82a2","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"DocImpact: glance will need write access to rbd to delete snapshots"},{"line_number":27,"context_line":"Change-Id: Idefa95f126f2922acf008038e2ac7430b1d26a33"},{"line_number":28,"context_line":"Implements: blueprint implement-rbd-snapshots-instead-of-qemu-snapshots"},{"line_number":29,"context_line":"Closes-Bug: 1346525"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"3a50d1a3_0e757e96","line":28,"updated":"2015-07-28 20:47:30.000000000","message":"This doesn\u0027t link to anything, was it supposed to be this?\n\nhttps://blueprints.launchpad.net/nova/+spec/rbd-instance-snapshots\n\njdurgin has a spec here which didn\u0027t make the cutoff for liberty:\n\nhttps://review.openstack.org/#/c/188244/\n\nI suggest you guys work together on this.","commit_id":"55fd681d75abeaffd614ad4b45d3df7cbae38fd2"}],"nova/tests/unit/virt/libvirt/test_rbd.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"32321c1e64a3996881de3275318db257f426dfee","unresolved":false,"context_lines":[{"line_number":102,"context_line":"            self.assertFalse(self.driver.is_cloneable({\u0027url\u0027: loc},"},{"line_number":103,"context_line":"                                                      {\u0027disk_format\u0027: \u0027raw\u0027}))"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    @mock.patch.object(rbd_utils.RBDDriver, \u0027get_fsid\u0027)"},{"line_number":106,"context_line":"    @mock.patch.object(rbd_utils, \u0027rbd\u0027)"},{"line_number":107,"context_line":"    @mock.patch.object(rbd_utils, \u0027rados\u0027)"},{"line_number":108,"context_line":"    def test_cloneable(self, mock_rados, mock_rbd, mock_get_fsid):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_136ca533","line":105,"updated":"2015-05-22 13:58:31.000000000","message":"Mocking out a thing generally indicates a place where we should verify that a call is (or is not made). Please do that.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"5ec197cb7c907f03127f2a77bbbb899d6956eade","unresolved":false,"context_lines":[{"line_number":102,"context_line":"            self.assertFalse(self.driver.is_cloneable({\u0027url\u0027: loc},"},{"line_number":103,"context_line":"                                                      {\u0027disk_format\u0027: \u0027raw\u0027}))"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    @mock.patch.object(rbd_utils.RBDDriver, \u0027get_fsid\u0027)"},{"line_number":106,"context_line":"    @mock.patch.object(rbd_utils, \u0027rbd\u0027)"},{"line_number":107,"context_line":"    @mock.patch.object(rbd_utils, \u0027rados\u0027)"},{"line_number":108,"context_line":"    def test_cloneable(self, mock_rados, mock_rbd, mock_get_fsid):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_1355961c","line":105,"in_reply_to":"7a016987_136ca533","updated":"2015-06-04 21:59:36.000000000","message":"This is already done, the function name (but not the mock name) changed","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9569,"name":"Sergey Nikitin","email":"nikitinserv@gmail.com","username":"snikitin"},"change_message_id":"297e733c17410e81396fde452fb2e2e83c11f7bd","unresolved":false,"context_lines":[{"line_number":1643,"context_line":"        image_format \u003d CONF.libvirt.snapshot_image_format or source_format"},{"line_number":1644,"context_line":""},{"line_number":1645,"context_line":"        # NOTE(bfilippov): save lvm as raw"},{"line_number":1646,"context_line":"        if image_format \u003d\u003d \u0027lvm\u0027:"},{"line_number":1647,"context_line":"            image_format \u003d \u0027raw\u0027"},{"line_number":1648,"context_line":""},{"line_number":1649,"context_line":"        metadata \u003d self._create_snapshot_metadata(base,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_ce298638","line":1646,"updated":"2014-10-07 08:59:38.000000000","message":"nit: I think it would be more readable if you will use\n\n  if image_format in [\u0027lvm\u0027, \u0027rbd\u0027]:","commit_id":"1e8e62e40a0d33269a00daf3fd78fae2a7a03b2e"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"5ff94a00994e6365e85f9742809c3cb4f0bd6d37","unresolved":false,"context_lines":[{"line_number":1643,"context_line":"        image_format \u003d CONF.libvirt.snapshot_image_format or source_format"},{"line_number":1644,"context_line":""},{"line_number":1645,"context_line":"        # NOTE(bfilippov): save lvm as raw"},{"line_number":1646,"context_line":"        if image_format \u003d\u003d \u0027lvm\u0027:"},{"line_number":1647,"context_line":"            image_format \u003d \u0027raw\u0027"},{"line_number":1648,"context_line":""},{"line_number":1649,"context_line":"        metadata \u003d self._create_snapshot_metadata(base,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_4547de4b","line":1646,"in_reply_to":"baa201ad_ce298638","updated":"2014-10-09 20:48:57.000000000","message":"At this point it\u0027s complex enough to justify extracting it into an imagebackend method like is_file_in_instance_path() (see _is_shared_block_storage() around line 5032 here for an example of usage). E.g. get_preferred_snapshot_format().","commit_id":"1e8e62e40a0d33269a00daf3fd78fae2a7a03b2e"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"5ff94a00994e6365e85f9742809c3cb4f0bd6d37","unresolved":false,"context_lines":[{"line_number":1709,"context_line":"            LOG.info(_LI(\"Beginning cold snapshot process\"),"},{"line_number":1710,"context_line":"                     instance\u003dinstance)"},{"line_number":1711,"context_line":""},{"line_number":1712,"context_line":"        if image_format \u003d\u003d \u0027rbd\u0027 and source_format \u003d\u003d \u0027rbd\u0027:"},{"line_number":1713,"context_line":"            # if everything is stored in ceph, we can do the snapshotting"},{"line_number":1714,"context_line":"            # inside the backend"},{"line_number":1715,"context_line":"            snap_name \u003d \u0027image-\u0027 + image_id"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_e594528e","line":1712,"updated":"2014-10-09 20:48:57.000000000","message":"I don\u0027t like how this is rbd specific, what happens when other backends become able to share and snapshot volume images between Nova and Glance?","commit_id":"1e8e62e40a0d33269a00daf3fd78fae2a7a03b2e"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"5ff94a00994e6365e85f9742809c3cb4f0bd6d37","unresolved":false,"context_lines":[{"line_number":1713,"context_line":"            # if everything is stored in ceph, we can do the snapshotting"},{"line_number":1714,"context_line":"            # inside the backend"},{"line_number":1715,"context_line":"            snap_name \u003d \u0027image-\u0027 + image_id"},{"line_number":1716,"context_line":"            snapshot_backend.create_snapshot(snap_name)"},{"line_number":1717,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_UPLOADING)"},{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"            fsid \u003d LibvirtDriver._get_rbd_driver()._get_fsid()"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_c2166455","line":1716,"updated":"2014-10-09 20:48:57.000000000","message":"Why are you skipping the pending state here? What if create_snapshot() here fails and throws an exception? Also see my comment in rbd_utils.py.","commit_id":"1e8e62e40a0d33269a00daf3fd78fae2a7a03b2e"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"5ff94a00994e6365e85f9742809c3cb4f0bd6d37","unresolved":false,"context_lines":[{"line_number":1719,"context_line":"            fsid \u003d LibvirtDriver._get_rbd_driver()._get_fsid()"},{"line_number":1720,"context_line":"            pool, image \u003d disk_path[len(\u0027rbd:\u0027):].split(\u0027/\u0027)"},{"line_number":1721,"context_line":""},{"line_number":1722,"context_line":"            image_path \u003d \"rbd://%s/%s/%s/%s\" % (fsid, pool, image, snap_name)"},{"line_number":1723,"context_line":"            self._image_api.update(context, image_id, {\u0027location\u0027: image_path},"},{"line_number":1724,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1725,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_659a22c7","line":1722,"updated":"2014-10-09 20:48:57.000000000","message":"Lines 1719-1722 are imagebackend specific and should be moved to Rbd backend there. Why not just make create_snapshot return image_path? That way, you\u0027d also be able to generalize the conditional on line 1712.","commit_id":"1e8e62e40a0d33269a00daf3fd78fae2a7a03b2e"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"5ff94a00994e6365e85f9742809c3cb4f0bd6d37","unresolved":false,"context_lines":[{"line_number":1722,"context_line":"            image_path \u003d \"rbd://%s/%s/%s/%s\" % (fsid, pool, image, snap_name)"},{"line_number":1723,"context_line":"            self._image_api.update(context, image_id, {\u0027location\u0027: image_path},"},{"line_number":1724,"context_line":"                                   purge_props\u003dFalse)"},{"line_number":1725,"context_line":"        else:"},{"line_number":1726,"context_line":"            update_task_state(task_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1727,"context_line":"            snapshot_directory \u003d CONF.libvirt.snapshots_directory"},{"line_number":1728,"context_line":"            fileutils.ensure_tree(snapshot_directory)"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_422b5494","line":1725,"updated":"2014-10-09 20:48:57.000000000","message":"This method already was too long at 130 lines before you added 16 more lines to it. When you find yourself wrapping half of the method inside a conditional, it\u0027s a sign that this code block is asking to be extracted into its own method.","commit_id":"1e8e62e40a0d33269a00daf3fd78fae2a7a03b2e"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"18fd763471997fd7adf13b265c5c0f327af335f5","unresolved":false,"context_lines":[{"line_number":1412,"context_line":"        try:"},{"line_number":1413,"context_line":"            url \u003d snapshot_backend.direct_snapshot(snapshot_name,"},{"line_number":1414,"context_line":"                                                   image_format,"},{"line_number":1415,"context_line":"                                                   image_id)"},{"line_number":1416,"context_line":"            metadata[\u0027location\u0027] \u003d url"},{"line_number":1417,"context_line":"            try:"},{"line_number":1418,"context_line":"                image_service.update(context,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_5efffa65","line":1415,"updated":"2015-01-07 19:15:03.000000000","message":"If no direct_snapshot implementation is provided, the base class will raise an exception, which falls back to _generic_snapshot - jonly image_href has not been assigned.","commit_id":"14bc57f80c2e0063cc95824570347d5bd9ce5c46"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"18fd763471997fd7adf13b265c5c0f327af335f5","unresolved":false,"context_lines":[{"line_number":1417,"context_line":"            try:"},{"line_number":1418,"context_line":"                image_service.update(context,"},{"line_number":1419,"context_line":"                                     image_href,"},{"line_number":1420,"context_line":"                                     metadata)"},{"line_number":1421,"context_line":"            except Exception as e:"},{"line_number":1422,"context_line":"                LOG.debug(\u0027failed to update glance\u0027, exc_info\u003dTrue)"},{"line_number":1423,"context_line":"                reason \u003d _(\u0027failed to update glance: %s\u0027) % e"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_deea0a9f","line":1420,"updated":"2015-01-07 19:15:03.000000000","message":"The image_service symbol is not defined in this scope (yet, I assume)","commit_id":"14bc57f80c2e0063cc95824570347d5bd9ce5c46"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"343da3d82d910e325136654b5dd1d26a77bf4387","unresolved":false,"context_lines":[{"line_number":1378,"context_line":"                self._image_api.update(context,"},{"line_number":1379,"context_line":"                                       image_id,"},{"line_number":1380,"context_line":"                                       metadata)"},{"line_number":1381,"context_line":"            except Exception as e:"},{"line_number":1382,"context_line":"                LOG.debug(\u0027failed to update glance\u0027, exc_info\u003dTrue)"},{"line_number":1383,"context_line":"                reason \u003d _(\u0027failed to update glance: %s\u0027) % e"},{"line_number":1384,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"5afe65bd_12d184fa","line":1381,"updated":"2015-06-04 16:00:52.000000000","message":"If the image_api.update fails, metadata[\u0027location\u0027] needs to be popped.  For instance, if Glance\u0027s rbd_user does not have read access to the vms pool this update will fail.  The code then falls back to the old extract snapshot binary and upload to glance through the _generic_snapshot call below.  That upload fails because the image_api.update call contains both image data and the location put in for the direct snapshot.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"32321c1e64a3996881de3275318db257f426dfee","unresolved":false,"context_lines":[{"line_number":1378,"context_line":"                self._image_api.update(context,"},{"line_number":1379,"context_line":"                                       image_id,"},{"line_number":1380,"context_line":"                                       metadata)"},{"line_number":1381,"context_line":"            except Exception as e:"},{"line_number":1382,"context_line":"                LOG.debug(\u0027failed to update glance\u0027, exc_info\u003dTrue)"},{"line_number":1383,"context_line":"                reason \u003d _(\u0027failed to update glance: %s\u0027) % e"},{"line_number":1384,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_931895d4","line":1381,"updated":"2015-05-22 13:58:31.000000000","message":"Surely there are a richer set of exceptions we can catch here, right? AFAIK the image api is fairly well defined. If you need to catch Exception at the end, then that may be reasonable, but please be more specific here.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"9ff8751f656fd3e20221534c3559cb88910ba61a","unresolved":false,"context_lines":[{"line_number":1378,"context_line":"                self._image_api.update(context,"},{"line_number":1379,"context_line":"                                       image_id,"},{"line_number":1380,"context_line":"                                       metadata)"},{"line_number":1381,"context_line":"            except Exception as e:"},{"line_number":1382,"context_line":"                LOG.debug(\u0027failed to update glance\u0027, exc_info\u003dTrue)"},{"line_number":1383,"context_line":"                reason \u003d _(\u0027failed to update glance: %s\u0027) % e"},{"line_number":1384,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_13fea16b","line":1381,"in_reply_to":"7a016987_931895d4","updated":"2015-06-05 11:58:55.000000000","message":"Hmm, you can get at least:\n\n  exception.ImageNotAuthorized\n  exception.ImageNotFound\n  exception.Invalid\n  glanceclient.exc.CommunicationError\n  glanceclient.exc.InvalidEndpoint\n\nIt\u0027s probably best to not have glanceclient specific exceptions here,\nwhich would imply needing improved exception translation in the image API,\nbut that\u0027s probably best done as a separate patch?","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"343da3d82d910e325136654b5dd1d26a77bf4387","unresolved":false,"context_lines":[{"line_number":1379,"context_line":"                                       image_id,"},{"line_number":1380,"context_line":"                                       metadata)"},{"line_number":1381,"context_line":"            except Exception as e:"},{"line_number":1382,"context_line":"                LOG.debug(\u0027failed to update glance\u0027, exc_info\u003dTrue)"},{"line_number":1383,"context_line":"                reason \u003d _(\u0027failed to update glance: %s\u0027) % e"},{"line_number":1384,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":1385,"context_line":"                                                  reason\u003dreason)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5afe65bd_520ffc87","line":1382,"updated":"2015-06-04 16:00:52.000000000","message":"In this particular error case the direct snapshot has succeeded but the glance image failed to update.  This could be possible if the glance rbd_user permissions on the RBD pool nova is using.\n\nThis exception block should call some code to clean up the clone and snapshots created by direct_snapshot before proceeding to the extra binary and upload path.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"32321c1e64a3996881de3275318db257f426dfee","unresolved":false,"context_lines":[{"line_number":1379,"context_line":"                                       image_id,"},{"line_number":1380,"context_line":"                                       metadata)"},{"line_number":1381,"context_line":"            except Exception as e:"},{"line_number":1382,"context_line":"                LOG.debug(\u0027failed to update glance\u0027, exc_info\u003dTrue)"},{"line_number":1383,"context_line":"                reason \u003d _(\u0027failed to update glance: %s\u0027) % e"},{"line_number":1384,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":1385,"context_line":"                                                  reason\u003dreason)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_1325c507","line":1382,"updated":"2015-05-22 13:58:31.000000000","message":"This explicitly logs a traceback to our debug logs. That should be avoided at all costs, IMHO. If it\u0027s an error, then log it as such. It would be much better to catch the errors we know can happen, log a reasonable message, and then log.exception() when we catch something unknown.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"9ff8751f656fd3e20221534c3559cb88910ba61a","unresolved":false,"context_lines":[{"line_number":1379,"context_line":"                                       image_id,"},{"line_number":1380,"context_line":"                                       metadata)"},{"line_number":1381,"context_line":"            except Exception as e:"},{"line_number":1382,"context_line":"                LOG.debug(\u0027failed to update glance\u0027, exc_info\u003dTrue)"},{"line_number":1383,"context_line":"                reason \u003d _(\u0027failed to update glance: %s\u0027) % e"},{"line_number":1384,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":1385,"context_line":"                                                  reason\u003dreason)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_768dc37a","line":1382,"in_reply_to":"5afe65bd_520ffc87","updated":"2015-06-05 11:58:55.000000000","message":"Will glance clean these at some stage?","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"32321c1e64a3996881de3275318db257f426dfee","unresolved":false,"context_lines":[{"line_number":1389,"context_line":"                     instance\u003dinstance)"},{"line_number":1390,"context_line":"            self._snapshot_domain(live_snapshot, virt_dom, state, instance)"},{"line_number":1391,"context_line":"        except exception.ImageUnacceptable:"},{"line_number":1392,"context_line":"            LOG.debug(\u0027direct snapshot failed\u0027, exc_info\u003dTrue)"},{"line_number":1393,"context_line":"            self._generic_snapshot(context, snapshot_name,"},{"line_number":1394,"context_line":"                                   snapshot_backend,"},{"line_number":1395,"context_line":"                                   disk_path,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_f31fc9b4","line":1392,"updated":"2015-05-22 13:58:31.000000000","message":"Same here. Why do we need to explicitly throw a traceback into our logs? We\u0027re trying to *avoid* doing this not add new ones.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"32321c1e64a3996881de3275318db257f426dfee","unresolved":false,"context_lines":[{"line_number":1401,"context_line":"                                   instance,"},{"line_number":1402,"context_line":"                                   image_id,"},{"line_number":1403,"context_line":"                                   metadata,"},{"line_number":1404,"context_line":"                                   update_task_state)"},{"line_number":1405,"context_line":""},{"line_number":1406,"context_line":"    def _snapshot_domain(self, live_snapshot, virt_dom, state, instance):"},{"line_number":1407,"context_line":"        new_dom \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_7359998d","line":1404,"updated":"2015-05-22 13:58:31.000000000","message":"This whole chunk is a lot of new code. I don\u0027t see where it\u0027s tested at all. In fact, the only test adds in this patch are a few mocks to make some current tests still pass.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"9ff8751f656fd3e20221534c3559cb88910ba61a","unresolved":false,"context_lines":[{"line_number":1415,"context_line":"                    launch_flags\u003dlibvirt.VIR_DOMAIN_START_PAUSED)"},{"line_number":1416,"context_line":"            if new_dom is not None:"},{"line_number":1417,"context_line":"                self._attach_pci_devices(new_dom,"},{"line_number":1418,"context_line":"                    pci_manager.get_instance_pci_devs(instance))"},{"line_number":1419,"context_line":""},{"line_number":1420,"context_line":"    def _generic_snapshot(self, context, snapshot_name, snapshot_backend,"},{"line_number":1421,"context_line":"                          disk_path, live_snapshot, virt_dom, state,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_002be931","line":1418,"updated":"2015-06-05 11:58:55.000000000","message":"This needs to also call _attach_sriov_ports()","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"41255c5a2dd3f1ca6fa5ebcd38b448b8e03f85bd","unresolved":false,"context_lines":[{"line_number":1441,"context_line":"                self._image_api.update(context,"},{"line_number":1442,"context_line":"                                       image_id,"},{"line_number":1443,"context_line":"                                       metadata)"},{"line_number":1444,"context_line":"            except Exception as e:"},{"line_number":1445,"context_line":"                metadata.pop(\u0027location\u0027)"},{"line_number":1446,"context_line":"                reason \u003d _(\u0027failed to update glance: %s\u0027) % e"},{"line_number":1447,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_0c9fa204","line":1444,"updated":"2015-06-05 13:05:00.000000000","message":"Bringing forward the comment from the previous patch set.  Direct snapshot is successful but Glance update failed.  The snapshots and clone created by direct_snapshot need to be cleaned up.\n\nI do not believe Glance will clean these up at a later stage such as Glance snapshot deletion because the Glance created from this point onward is created by Glance allocating a new RBD image and uploading the image binary to it.  The Glance image\u0027s location will point at that new RBD image and Glance will have no pointers back to the snapshot created above, so it can\u0027t walk the parent tree backwards to remove them.","commit_id":"03e5959bb160d9772a5b896abbc56b1d95d6912a"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"6e087159321c0755324ee419c30341eb2e68f762","unresolved":false,"context_lines":[{"line_number":1420,"context_line":"                self._image_api.update(context,"},{"line_number":1421,"context_line":"                                       image_id,"},{"line_number":1422,"context_line":"                                       metadata)"},{"line_number":1423,"context_line":"            except Exception as e:"},{"line_number":1424,"context_line":"                metadata.pop(\u0027location\u0027)"},{"line_number":1425,"context_line":"                reason \u003d _(\u0027failed to update glance: %s\u0027) % e"},{"line_number":1426,"context_line":"                raise exception.ImageUnacceptable(image_id\u003dimage_id,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3afb71cf_ba6115d5","line":1423,"updated":"2015-06-10 23:47:48.000000000","message":"Bringing forward the comment from Samuel Matzek:\n\n[Here] Direct snapshot is successful but Glance update failed. The snapshots and clone created by direct_snapshot need to be cleaned up.\n\nI do not believe Glance will clean these up at a later stage such as Glance snapshot deletion because the Glance created from this point onward is created by Glance allocating a new RBD image and uploading the image binary to it. The Glance image\u0027s location will point at that new RBD image and Glance will have no pointers back to the snapshot created above, so it can\u0027t walk the parent tree backwards to remove them.","commit_id":"55fd681d75abeaffd614ad4b45d3df7cbae38fd2"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"5ff94a00994e6365e85f9742809c3cb4f0bd6d37","unresolved":false,"context_lines":[{"line_number":707,"context_line":"            self.driver.resize(self.rbd_name, size)"},{"line_number":708,"context_line":""},{"line_number":709,"context_line":"    def create_snapshot(self, snap_name):"},{"line_number":710,"context_line":"        self.driver.snapshot(self.pool, self.rbd_name, snap_name)"},{"line_number":711,"context_line":""},{"line_number":712,"context_line":"    def snapshot_extract(self, target, out_format):"},{"line_number":713,"context_line":"        images.convert_image(self.path, target, out_format)"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_a5f24a04","line":710,"updated":"2014-10-09 20:48:57.000000000","message":"If it\u0027s not a private method, you should expose (and document) it in the parent class and override here.","commit_id":"1e8e62e40a0d33269a00daf3fd78fae2a7a03b2e"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"18fd763471997fd7adf13b265c5c0f327af335f5","unresolved":false,"context_lines":[{"line_number":606,"context_line":"                             run_as_root\u003dTrue)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":""},{"line_number":609,"context_line":"class Rbd(Image):"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"    SUPPORTS_CLONE \u003d True"},{"line_number":612,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_7ecbd652","side":"PARENT","line":609,"updated":"2015-01-07 19:15:03.000000000","message":"We\u0027ll certainly need a direct_snapshot() implementation here in addition to LVM.","commit_id":"3ae2ddd4a2519b0af4130237cf4b3b2f0ca23dfc"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"011d5a2d0e0447e9051047c795d69369afd6fe4f","unresolved":false,"context_lines":[{"line_number":615,"context_line":"        images.convert_image(self.path, target, out_format,"},{"line_number":616,"context_line":"                             run_as_root\u003dTrue)"},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"    def direct_snapshot(self, snapshot_name, image_format, image_id):"},{"line_number":619,"context_line":"        deletion_marker \u003d \u0027_to_be_deleted_by_glance\u0027"},{"line_number":620,"context_line":"        if image_format !\u003d \u0027raw\u0027:"},{"line_number":621,"context_line":"            reason \u003d _(\u0027only raw format is supported\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_ad208f9a","line":618,"updated":"2015-01-08 21:51:34.000000000","message":"I think an implementation of this should be provided for the Rbd image backend as well, folks using Ceph are likely to enable this as well.","commit_id":"76a002603a33b87850ebd354e481d11965ec9372"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"80130f4def839d457fd8f3f19c80aaec6a4f6e48","unresolved":false,"context_lines":[{"line_number":615,"context_line":"        images.convert_image(self.path, target, out_format,"},{"line_number":616,"context_line":"                             run_as_root\u003dTrue)"},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"    def direct_snapshot(self, snapshot_name, image_format, image_id):"},{"line_number":619,"context_line":"        deletion_marker \u003d \u0027_to_be_deleted_by_glance\u0027"},{"line_number":620,"context_line":"        if image_format !\u003d \u0027raw\u0027:"},{"line_number":621,"context_line":"            reason \u003d _(\u0027only raw format is supported\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba7be1f8_b8d37002","line":618,"in_reply_to":"3a961159_ad208f9a","updated":"2015-02-26 16:53:13.000000000","message":"In fact I think this method does already belong to class Rbd below, doesn\u0027t make much sense to use librbd stuff in class Lvm, does it?","commit_id":"76a002603a33b87850ebd354e481d11965ec9372"},{"author":{"_account_id":15292,"name":"Scott Yoder","email":"syoder@imagestream.com"},"change_message_id":"64e3e15359c697a86ce74130d97983fb26123b8f","unresolved":false,"context_lines":[{"line_number":615,"context_line":"        images.convert_image(self.path, target, out_format,"},{"line_number":616,"context_line":"                             run_as_root\u003dTrue)"},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"    def direct_snapshot(self, snapshot_name, image_format, image_id):"},{"line_number":619,"context_line":"        deletion_marker \u003d \u0027_to_be_deleted_by_glance\u0027"},{"line_number":620,"context_line":"        if image_format !\u003d \u0027raw\u0027:"},{"line_number":621,"context_line":"            reason \u003d _(\u0027only raw format is supported\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba7be1f8_1c8d1732","line":618,"in_reply_to":"ba7be1f8_b8d37002","updated":"2015-02-27 17:53:28.000000000","message":"I agree this method should be moved to the class Rbd below. Once I moved it there the Ceph snapshots started working.","commit_id":"76a002603a33b87850ebd354e481d11965ec9372"}],"nova/virt/libvirt/rbd_utils.py":[{"author":{"_account_id":9569,"name":"Sergey Nikitin","email":"nikitinserv@gmail.com","username":"snikitin"},"change_message_id":"297e733c17410e81396fde452fb2e2e83c11f7bd","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                                     features\u003drbd.RBD_FEATURE_LAYERING)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def snapshot(self, pool, image, snap_name):"},{"line_number":218,"context_line":"        LOG.debug(\u0027snapshotting %(pool)s/%(img)s to snapshot %(snap)s\u0027 %"},{"line_number":219,"context_line":"                  dict(pool \u003d pool, img \u003d image, snap \u003d snap_name))"},{"line_number":220,"context_line":"        with RADOSClient(self, str(pool)) as client:"},{"line_number":221,"context_line":"            with rbd.Image(client.ioctx, image) as img:"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_ce7fc664","line":218,"updated":"2014-10-07 08:59:38.000000000","message":"Could you use here \u0027,\u0027 instead of \u0027%\u0027, please?\n\n LOG.debug(\u0027snapshotting %(pool)s/%(img)s to snapshot %(snap)s\u0027, dict....\n\n\n\n\n\nP.S. {} more faster than dict().\n\nhttp://doughellmann.com/2012/11/12/the-performance-impact-of-using-dict-instead-of-in-cpython-2-7-2.html","commit_id":"1e8e62e40a0d33269a00daf3fd78fae2a7a03b2e"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"5ff94a00994e6365e85f9742809c3cb4f0bd6d37","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                except rbd.ImageNotFound:"},{"line_number":226,"context_line":"                    LOG.warn(_LW(\u0027rbd creating snapshot of %(pools)/%(img)s \u0027"},{"line_number":227,"context_line":"                                 \u0027failed\u0027),"},{"line_number":228,"context_line":"                             dict(img \u003d image, pool \u003d pool))"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"    def size(self, name):"},{"line_number":231,"context_line":"        with RBDVolumeProxy(self, name) as vol:"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_65cf82b4","line":228,"updated":"2014-10-09 20:48:57.000000000","message":"Log a warning and then what? Shouldn\u0027t you throw a new exception that would tell libvirt driver to use the old non-cow way to create a snapshot?","commit_id":"1e8e62e40a0d33269a00daf3fd78fae2a7a03b2e"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"343da3d82d910e325136654b5dd1d26a77bf4387","unresolved":false,"context_lines":[{"line_number":289,"context_line":"                raise loopingcall.LoopingCallDone(retvalue\u003dFalse)"},{"line_number":290,"context_line":"            except rbd.ImageHasSnapshots:"},{"line_number":291,"context_line":"                new_name \u003d volume + deletion_marker"},{"line_number":292,"context_line":"                rbd.RBD().rename(client.ioctx, volume, new_name)"},{"line_number":293,"context_line":"            except rbd.ImageBusy:"},{"line_number":294,"context_line":"                LOG.warn(_LW(\u0027rbd remove %(volume)s in pool %(pool)s \u0027"},{"line_number":295,"context_line":"                             \u0027failed\u0027),"}],"source_content_type":"text/x-python","patch_set":8,"id":"5afe65bd_f28ee877","line":292,"updated":"2015-06-04 16:00:52.000000000","message":"I believe a raise loopincall.LoopingCallDone(??) is needed here.  When I tested this patch in my environment, did a VM snapshot followed by a VM delete, the VM delete failed with ImageNotFound: error removing image from this block of code.  I believe, but have not yet proven that the VM disk was renamed by this rename, and then it looped back around on another retry and couldn\u0027t find the original volume name on the RBD().remove and failed.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":13562,"name":"Samuel Matzek","email":"smatzek@us.ibm.com","username":"smatzek"},"change_message_id":"5b1be0ad71a4921ba07e38e1eb5b40181174de93","unresolved":false,"context_lines":[{"line_number":289,"context_line":"                raise loopingcall.LoopingCallDone(retvalue\u003dFalse)"},{"line_number":290,"context_line":"            except rbd.ImageHasSnapshots:"},{"line_number":291,"context_line":"                new_name \u003d volume + deletion_marker"},{"line_number":292,"context_line":"                rbd.RBD().rename(client.ioctx, volume, new_name)"},{"line_number":293,"context_line":"            except rbd.ImageBusy:"},{"line_number":294,"context_line":"                LOG.warn(_LW(\u0027rbd remove %(volume)s in pool %(pool)s \u0027"},{"line_number":295,"context_line":"                             \u0027failed\u0027),"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_d5e7dbaa","line":292,"updated":"2015-06-05 03:45:41.000000000","message":"Just following up. I had a chance to revisit this in my test environment. Adding \u0027raise loopingcall.LoopingCallDone(retvalue\u003dFalse)\u0027 here makes the VM delete succeed.  The VM\u0027s RBD disk image is renamed, and a later delete of the Glance image/VM snapshot correctly removes the former VM disk, its snapshots and clones.","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"9ff8751f656fd3e20221534c3559cb88910ba61a","unresolved":false,"context_lines":[{"line_number":289,"context_line":"                raise loopingcall.LoopingCallDone(retvalue\u003dFalse)"},{"line_number":290,"context_line":"            except rbd.ImageHasSnapshots:"},{"line_number":291,"context_line":"                new_name \u003d volume + deletion_marker"},{"line_number":292,"context_line":"                rbd.RBD().rename(client.ioctx, volume, new_name)"},{"line_number":293,"context_line":"            except rbd.ImageBusy:"},{"line_number":294,"context_line":"                LOG.warn(_LW(\u0027rbd remove %(volume)s in pool %(pool)s \u0027"},{"line_number":295,"context_line":"                             \u0027failed\u0027),"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_1658cff9","line":292,"in_reply_to":"3afb71cf_d5e7dbaa","updated":"2015-06-05 11:58:55.000000000","message":"Done","commit_id":"2e3fbab4e4e1ac7d14dd14b89e83326d793cbaa7"}]}