)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"81e556f05fe6ce383c1856f7ed02dcf32ee7956d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"libvirt:when deleting snapshot use qemu-img to blockRebase if domain is stopped"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Libvirt won\u0027t do a blockRebase on a domain that is not running. So, as a"},{"line_number":10,"context_line":"workaround, use \"qemu-img rebase\" instead."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I0e6819a6c8dc70b9bd7d1a9c18dc185b4537a3e4"},{"line_number":13,"context_line":"Closes-Bug: #1444806"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fa32b979_7c9eab55","line":10,"updated":"2015-06-22 16:18:39.000000000","message":"\"workaround\" is incorrect term here, you might want to rephrase it. Some notes on why (and feel free to reuse the below text in commit message if it helps):\n\nlibvirt\u0027s \u0027blockRebase\u0027 API works only (that\u0027s not a bug) when the guest is online -- the main functionality of blockRebase is that it allows the guest to concurrently read/write while the copy is taking place. So the behavior of libvirt (blockRebase API, to be precise) throwing an error when it doesn\u0027t see a running domain (guest) is expected.\n\nTo put that in an example, on a live guest \u0027vm1`\u0027, for a disk \u0027vda\u0027, you can do a live copy, while concurrent writes\n\n    $ virsh blockcopy --domain vm1 vda \\\n      /export/vm1-copy.qcow2 --wait --finish --verbose\n\nThe above command will continue to copy/mirror indefinitely as writes happen inside the guest, until you issue an explicit call to abort:\n\n    $ virsh blockjob --abort vm1 vda\n    # Or the equivalent Python API domain.blockJobAbort()\n\nto end the copy/mirroring operation.\n\nAs danpb noted on IRC once, about why it is not desirable to have offline copy for blockRebase: \"It is pointless to allow blockRebase to work when a guest is offline, because one can use more efficient methods [like `dd` or similar] to perform a copy.\"","commit_id":"7fbc18a80def636e274988e67652e3237934532f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"92d0ea22c1bda0cb9fb0f979adc5d7380f3eac8e","unresolved":false,"context_lines":[{"line_number":10,"context_line":"in that case, use \"qemu-img rebase\" instead."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Note: For now, trying to rebase a network disk using qemu-img raises"},{"line_number":13,"context_line":"a NovaException error because I can\u0027t test that it successfully works"},{"line_number":14,"context_line":"for every protocol (gluster, sheepdog, etc) that executes this code"},{"line_number":15,"context_line":"path. I successfully tested this with file-based disk."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"1a4dcd0f_b4013b3a","line":13,"updated":"2015-08-06 14:41:45.000000000","message":"Don\u0027t the cinder third party CI for file system type drivers run the test_volume_boot_pattern scenario which does volume-backed snapshots?","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"24e41b4043f24e744d0e35cecc812f1cb5587b04","unresolved":false,"context_lines":[{"line_number":10,"context_line":"in that case, use \"qemu-img rebase\" instead."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Note: For now, trying to rebase a network disk using qemu-img raises"},{"line_number":13,"context_line":"a NovaException error because I can\u0027t test that it successfully works"},{"line_number":14,"context_line":"for every protocol (gluster, sheepdog, etc) that executes this code"},{"line_number":15,"context_line":"path. I successfully tested this with file-based disk."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"1a4dcd0f_c0d67727","line":13,"in_reply_to":"1a4dcd0f_2ff076fd","updated":"2015-08-06 18:57:33.000000000","message":"Here are the results from parallels / virtuozzo CI:\n\nhttp://openstack-3rd-party-ci-logs.odin.net/42/203642/2/check/dsvm-tempest-cinder/a5a1c5d/logs/screen-n-cpu.txt.gz\n\nI see the debug log statement and test_volume_boot_pattern passes.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"26d5b0265504816bed8411ab13a35f387c42a7be","unresolved":false,"context_lines":[{"line_number":10,"context_line":"in that case, use \"qemu-img rebase\" instead."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Note: For now, trying to rebase a network disk using qemu-img raises"},{"line_number":13,"context_line":"a NovaException error because I can\u0027t test that it successfully works"},{"line_number":14,"context_line":"for every protocol (gluster, sheepdog, etc) that executes this code"},{"line_number":15,"context_line":"path. I successfully tested this with file-based disk."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"1a4dcd0f_2ff076fd","line":13,"in_reply_to":"1a4dcd0f_b4013b3a","updated":"2015-08-06 14:48:00.000000000","message":"What I\u0027d really like to see is cinder third party CI people testing this out in their CI via some dependent change, or specifying this nova patch like you can do in devstack or devstack-gate, to post results of the CI test run with this change so I can verify the error path is being hit and handled properly, which should be possible by looking in the n-cpu log for the debug logging you added.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"bc82d85b06f7e5eca4c8ffb38f8c332f27b953ca","unresolved":false,"context_lines":[{"line_number":13287,"context_line":"        should trigger a blockRebase using qemu-img not libvirt."},{"line_number":13288,"context_line":"        \"\"\""},{"line_number":13289,"context_line":""},{"line_number":13290,"context_line":"        mock_guest \u003d mock.Mock()"},{"line_number":13291,"context_line":"        mock_guest.get_xml_desc \u003d mock.Mock(return_value\u003dself.dom_xml)"},{"line_number":13292,"context_line":""},{"line_number":13293,"context_line":"        exc \u003d fakelibvirt.make_libvirtError("}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_8c0d9639","line":13290,"updated":"2015-06-22 08:06:36.000000000","message":"s/mock.Mock()/mock.Mock(spec\u003dlibvirt_guest.Guest)","commit_id":"7fbc18a80def636e274988e67652e3237934532f"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"4b5e3ae81f34a09e90a7945c01fd35fc4b7aaba6","unresolved":false,"context_lines":[{"line_number":13287,"context_line":"        should trigger a blockRebase using qemu-img not libvirt."},{"line_number":13288,"context_line":"        \"\"\""},{"line_number":13289,"context_line":""},{"line_number":13290,"context_line":"        mock_guest \u003d mock.Mock()"},{"line_number":13291,"context_line":"        mock_guest.get_xml_desc \u003d mock.Mock(return_value\u003dself.dom_xml)"},{"line_number":13292,"context_line":""},{"line_number":13293,"context_line":"        exc \u003d fakelibvirt.make_libvirtError("}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_da60be37","line":13290,"in_reply_to":"fa32b979_8c0d9639","updated":"2015-06-24 10:17:33.000000000","message":"I tried but then the test failed with \"AttributeError: Mock object has no attribute \u0027_domain\u0027\"\n\nI guess that\u0027s because I need to mock _domain.blockRebase() but _domain is an attribute set at run time in the __init__ method of libvirt/guest.py. But it is accessed directly in libvirt/driver.py. So when I use spec\u003dlibvirt_guest.Guest, my mock doesn\u0027t have this _domain attribute.","commit_id":"7fbc18a80def636e274988e67652e3237934532f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"b47783bb302dfa126b8c7f7d458d4f4638b0b4b0","unresolved":false,"context_lines":[{"line_number":13287,"context_line":"        should trigger a blockRebase using qemu-img not libvirt."},{"line_number":13288,"context_line":"        \"\"\""},{"line_number":13289,"context_line":""},{"line_number":13290,"context_line":"        mock_guest \u003d mock.Mock()"},{"line_number":13291,"context_line":"        mock_guest.get_xml_desc \u003d mock.Mock(return_value\u003dself.dom_xml)"},{"line_number":13292,"context_line":""},{"line_number":13293,"context_line":"        exc \u003d fakelibvirt.make_libvirtError("}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_5f057d0e","line":13290,"in_reply_to":"fa32b979_da60be37","updated":"2015-06-24 10:33:17.000000000","message":"So what you need to do is:\n\n  mock_domain \u003d mocj.Mock(spec\u003dfakelibvirt.virDomain)\n  mock_guest \u003d libvirt_guest.Guest(mock_domain)\n\nGuest object encapsulate a domain, you want to assert methods in domain.","commit_id":"7fbc18a80def636e274988e67652e3237934532f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"bc82d85b06f7e5eca4c8ffb38f8c332f27b953ca","unresolved":false,"context_lines":[{"line_number":13288,"context_line":"        \"\"\""},{"line_number":13289,"context_line":""},{"line_number":13290,"context_line":"        mock_guest \u003d mock.Mock()"},{"line_number":13291,"context_line":"        mock_guest.get_xml_desc \u003d mock.Mock(return_value\u003dself.dom_xml)"},{"line_number":13292,"context_line":""},{"line_number":13293,"context_line":"        exc \u003d fakelibvirt.make_libvirtError("},{"line_number":13294,"context_line":"            fakelibvirt.libvirtError, \u0027virDomainBlockRebase() failed\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_0c1ae66e","line":13291,"updated":"2015-06-22 08:06:36.000000000","message":"mock_guest has all of his methods mocked it is not necessary to override them.\n\ns/.../mock_guest.get_xml_desc.return_value \u003d self.dom_xml","commit_id":"7fbc18a80def636e274988e67652e3237934532f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"ef9527f8f857682beeea73764f9b425f3dbedfc7","unresolved":false,"context_lines":[{"line_number":13288,"context_line":"        \"\"\""},{"line_number":13289,"context_line":""},{"line_number":13290,"context_line":"        mock_guest \u003d mock.Mock()"},{"line_number":13291,"context_line":"        mock_guest.get_xml_desc.return_value \u003d self.dom_xml"},{"line_number":13292,"context_line":""},{"line_number":13293,"context_line":"        exc \u003d fakelibvirt.make_libvirtError("},{"line_number":13294,"context_line":"            fakelibvirt.libvirtError, \u0027virDomainBlockRebase() failed\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_3fd19957","line":13291,"updated":"2015-06-24 10:36:58.000000000","message":"I replied on the previous domain.\n\nYou need to mock domain, I guess methods XMLDesc and blockRebase. Then pass it to libvirt_guest.Guest(mocked_domain)","commit_id":"b7e61767980232b670e32e5cc98e50182c294678"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"3e6e429a1efdad73c6d6c39176362e2092d3c23f","unresolved":false,"context_lines":[{"line_number":13288,"context_line":"        \"\"\""},{"line_number":13289,"context_line":""},{"line_number":13290,"context_line":"        mock_guest \u003d mock.Mock()"},{"line_number":13291,"context_line":"        mock_guest.get_xml_desc.return_value \u003d self.dom_xml"},{"line_number":13292,"context_line":""},{"line_number":13293,"context_line":"        exc \u003d fakelibvirt.make_libvirtError("},{"line_number":13294,"context_line":"            fakelibvirt.libvirtError, \u0027virDomainBlockRebase() failed\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_166fdafc","line":13291,"in_reply_to":"fa32b979_3fd19957","updated":"2015-06-24 12:41:55.000000000","message":"Done","commit_id":"b7e61767980232b670e32e5cc98e50182c294678"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3b2aef344a41245b5b68fde0ba5794967606f265","unresolved":false,"context_lines":[{"line_number":13294,"context_line":"        exc \u003d fakelibvirt.make_libvirtError("},{"line_number":13295,"context_line":"            fakelibvirt.libvirtError, \u0027virDomainBlockRebase() failed\u0027,"},{"line_number":13296,"context_line":"            error_code\u003dfakelibvirt.VIR_ERR_OPERATION_INVALID)"},{"line_number":13297,"context_line":"        mock_domain.blockRebase \u003d mock.Mock(side_effect\u003dexc)"},{"line_number":13298,"context_line":""},{"line_number":13299,"context_line":"        instance \u003d objects.Instance(**self.inst)"},{"line_number":13300,"context_line":"        snapshot_id \u003d \u0027snapshot-1234\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa32b979_d13dfc2b","line":13297,"updated":"2015-06-24 12:55:50.000000000","message":"So here you override method \u0027mock_domain.blockRebase\u0027 which is actually already mocked. Should be:\n\n  mock_domain.blockRebase.side_effect \u003d exc","commit_id":"3e71993cc659f51b62b31fdd59e4991d80567d4f"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"fb19d39185240ad4393f74a3f91576deda374658","unresolved":false,"context_lines":[{"line_number":13294,"context_line":"        exc \u003d fakelibvirt.make_libvirtError("},{"line_number":13295,"context_line":"            fakelibvirt.libvirtError, \u0027virDomainBlockRebase() failed\u0027,"},{"line_number":13296,"context_line":"            error_code\u003dfakelibvirt.VIR_ERR_OPERATION_INVALID)"},{"line_number":13297,"context_line":"        mock_domain.blockRebase \u003d mock.Mock(side_effect\u003dexc)"},{"line_number":13298,"context_line":""},{"line_number":13299,"context_line":"        instance \u003d objects.Instance(**self.inst)"},{"line_number":13300,"context_line":"        snapshot_id \u003d \u0027snapshot-1234\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa32b979_99deba6b","line":13297,"in_reply_to":"fa32b979_d13dfc2b","updated":"2015-06-24 13:39:51.000000000","message":"Yep. You are right. Thanks.","commit_id":"3e71993cc659f51b62b31fdd59e4991d80567d4f"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"06e61a7c773ca0c8a14ddf45c100bdd08b9ecb16","unresolved":false,"context_lines":[{"line_number":13310,"context_line":"        mock_get_disk_type.assert_called_once_with(\"snap.img\")"},{"line_number":13311,"context_line":"        mock_execute.assert_called_once_with(\u0027qemu-img\u0027, \u0027rebase\u0027,"},{"line_number":13312,"context_line":"                                             \u0027-b\u0027, \u0027snap.img\u0027, \u0027-F\u0027,"},{"line_number":13313,"context_line":"                                             \u0027fake_fmt\u0027, \u0027disk1_file\u0027)"},{"line_number":13314,"context_line":""},{"line_number":13315,"context_line":"    def test_volume_snapshot_delete_2(self):"},{"line_number":13316,"context_line":"        \"\"\"Deleting older snapshot -- blockCommit.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_4778d0c4","line":13313,"updated":"2015-07-13 09:40:03.000000000","message":"I think we need tests for the below 2 cases too:\n1) When disk is of network type\n2) When rebase_base is None and hence we don\u0027t need -F option","commit_id":"a45bb4f0228f95fb66d6c105591daf5fa0283f5f"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"b33c83ac352754ab65d70e43a024c8f5b25b2505","unresolved":false,"context_lines":[{"line_number":13310,"context_line":"        mock_get_disk_type.assert_called_once_with(\"snap.img\")"},{"line_number":13311,"context_line":"        mock_execute.assert_called_once_with(\u0027qemu-img\u0027, \u0027rebase\u0027,"},{"line_number":13312,"context_line":"                                             \u0027-b\u0027, \u0027snap.img\u0027, \u0027-F\u0027,"},{"line_number":13313,"context_line":"                                             \u0027fake_fmt\u0027, \u0027disk1_file\u0027)"},{"line_number":13314,"context_line":""},{"line_number":13315,"context_line":"    def test_volume_snapshot_delete_2(self):"},{"line_number":13316,"context_line":"        \"\"\"Deleting older snapshot -- blockCommit.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_09381a3c","line":13313,"in_reply_to":"9a41bdd9_4778d0c4","updated":"2015-07-15 11:45:27.000000000","message":"Agreed. Will do.","commit_id":"a45bb4f0228f95fb66d6c105591daf5fa0283f5f"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"9b54418201525b8d2ba7999e3f177b168d94c191","unresolved":false,"context_lines":[{"line_number":13415,"context_line":""},{"line_number":13416,"context_line":"        self.assertEqual(0, mock_get_disk_type.call_count)"},{"line_number":13417,"context_line":"        mock_execute.assert_called_once_with(\u0027qemu-img\u0027, \u0027rebase\u0027,"},{"line_number":13418,"context_line":"                                             \u0027-b\u0027, \u0027\u0027, \u0027disk1_file\u0027)"},{"line_number":13419,"context_line":""},{"line_number":13420,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\","},{"line_number":13421,"context_line":"                       mock.Mock(return_value\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a50d1a3_a8f989ca","line":13418,"updated":"2015-07-20 08:23:46.000000000","message":"This test checks for a not working call (see the driver.py comment on an empty -b parameter call).\n\nlibvirt.py returns a no such file message on this but is (correctly) mocked in here which makes this test pass.","commit_id":"53bf87fc69491b0f8df50fd28e15c41f2d1f6862"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"94d21d0941ef85e9003d44284e9fc83286aef4ae","unresolved":false,"context_lines":[{"line_number":13415,"context_line":""},{"line_number":13416,"context_line":"        self.assertEqual(0, mock_get_disk_type.call_count)"},{"line_number":13417,"context_line":"        mock_execute.assert_called_once_with(\u0027qemu-img\u0027, \u0027rebase\u0027,"},{"line_number":13418,"context_line":"                                             \u0027-b\u0027, \u0027\u0027, \u0027disk1_file\u0027)"},{"line_number":13419,"context_line":""},{"line_number":13420,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\","},{"line_number":13421,"context_line":"                       mock.Mock(return_value\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a50d1a3_03465e23","line":13418,"in_reply_to":"3a50d1a3_a8f989ca","updated":"2015-07-20 08:31:02.000000000","message":"My comment above is wrong, please ignore.","commit_id":"53bf87fc69491b0f8df50fd28e15c41f2d1f6862"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42cd17a22f411b4d848b020e89b0c103b7c5f7de","unresolved":false,"context_lines":[{"line_number":14442,"context_line":""},{"line_number":14443,"context_line":"        self.mox.VerifyAll()"},{"line_number":14444,"context_line":""},{"line_number":14445,"context_line":"    def _setup_domain_and_guest_mocks(self, dom_xml):"},{"line_number":14446,"context_line":"        mock_domain \u003d mock.Mock(spec\u003dfakelibvirt.virDomain)"},{"line_number":14447,"context_line":"        mock_domain.XMLDesc.return_value \u003d dom_xml"},{"line_number":14448,"context_line":"        guest \u003d libvirt_guest.Guest(mock_domain)"}],"source_content_type":"text/x-python","patch_set":24,"id":"ba15a1d1_6214354c","line":14445,"updated":"2015-09-18 01:41:44.000000000","message":"nit: this is a very generic helper method name, I would have worked \u0027block_rebase\u0027 into that somehow, like _setup_block_rebase_domain-and_guest_mocks.","commit_id":"ad251d4e24e3cc7d81df832c875573336b213685"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fcdb9a9284c6f44a0ca9d1f2bdc78261b5cd2bc2","unresolved":false,"context_lines":[{"line_number":14526,"context_line":"        snapshot_id \u003d \u0027snapshot-1234\u0027"},{"line_number":14527,"context_line":"        with mock.patch.object(self.drvr._host, \u0027get_guest\u0027,"},{"line_number":14528,"context_line":"                               return_value\u003dmock_guest):"},{"line_number":14529,"context_line":"            self.assertRaises(exception.NovaException,"},{"line_number":14530,"context_line":"                              self.drvr._volume_snapshot_delete,"},{"line_number":14531,"context_line":"                              self.c, instance, self.volume_uuid, snapshot_id,"},{"line_number":14532,"context_line":"                              self.delete_info_1)"}],"source_content_type":"text/x-python","patch_set":24,"id":"ba15a1d1_bdbd8e03","line":14529,"updated":"2015-09-18 01:43:55.000000000","message":"NovaException is super generic here.  This test would be better if you stored the result into a variable and then checked the exception message is what you expected, e.g.:\n\nex \u003d self.assertRaises(exception.NovaException,)\n...\n\nself.assertIn(\u0027has not been fully tested\u0027, six.text_type(ex))","commit_id":"ad251d4e24e3cc7d81df832c875573336b213685"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42cd17a22f411b4d848b020e89b0c103b7c5f7de","unresolved":false,"context_lines":[{"line_number":14529,"context_line":"            self.assertRaises(exception.NovaException,"},{"line_number":14530,"context_line":"                              self.drvr._volume_snapshot_delete,"},{"line_number":14531,"context_line":"                              self.c, instance, self.volume_uuid, snapshot_id,"},{"line_number":14532,"context_line":"                              self.delete_info_1)"},{"line_number":14533,"context_line":""},{"line_number":14534,"context_line":"    def test_volume_snapshot_delete_2(self):"},{"line_number":14535,"context_line":"        \"\"\"Deleting older snapshot -- blockCommit.\"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"ba15a1d1_1d67dae9","line":14532,"updated":"2015-09-18 01:41:44.000000000","message":"Shouldn\u0027t this be self.delete_info_netdisk instead of self.delete_info_1?","commit_id":"ad251d4e24e3cc7d81df832c875573336b213685"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8ed0f527dbc7378d4b6994f64265ab05f5f988b","unresolved":false,"context_lines":[{"line_number":14529,"context_line":"            self.assertRaises(exception.NovaException,"},{"line_number":14530,"context_line":"                              self.drvr._volume_snapshot_delete,"},{"line_number":14531,"context_line":"                              self.c, instance, self.volume_uuid, snapshot_id,"},{"line_number":14532,"context_line":"                              self.delete_info_1)"},{"line_number":14533,"context_line":""},{"line_number":14534,"context_line":"    def test_volume_snapshot_delete_2(self):"},{"line_number":14535,"context_line":"        \"\"\"Deleting older snapshot -- blockCommit.\"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"ba15a1d1_98c0d851","line":14532,"in_reply_to":"ba15a1d1_1d67dae9","updated":"2015-09-18 02:31:57.000000000","message":"I tested this myself locally and it was OK, so nevermind.","commit_id":"ad251d4e24e3cc7d81df832c875573336b213685"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"abbe3712025134a5c48b9c34a032d5d90a55ba7f","unresolved":false,"context_lines":[{"line_number":1945,"context_line":"                       \u0027bw\u0027: rebase_bw,"},{"line_number":1946,"context_line":"                       \u0027flags\u0027: rebase_flags})"},{"line_number":1947,"context_line":""},{"line_number":1948,"context_line":"            try:"},{"line_number":1949,"context_line":"                result \u003d virt_dom.blockRebase(rebase_disk, rebase_base,"},{"line_number":1950,"context_line":"                                              rebase_bw, rebase_flags)"},{"line_number":1951,"context_line":"            except libvirt.libvirtError as ex:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_ac29e19c","line":1948,"updated":"2015-06-18 11:59:24.000000000","message":"Insteadof waiting for an error returned by blockRebase we can early verify whether the domain is running to choice one of these two methods.\n\n  if virt_dom.isActive():\n    blockRebase..\n  else\n    qemuRebase...","commit_id":"fca3ec7ee1446791b52a8cbfcd16ef9f8fb161fa"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"c53eda571e71c9dbbb8419516f3b9cbf29c882d2","unresolved":false,"context_lines":[{"line_number":1945,"context_line":"                       \u0027bw\u0027: rebase_bw,"},{"line_number":1946,"context_line":"                       \u0027flags\u0027: rebase_flags})"},{"line_number":1947,"context_line":""},{"line_number":1948,"context_line":"            try:"},{"line_number":1949,"context_line":"                result \u003d virt_dom.blockRebase(rebase_disk, rebase_base,"},{"line_number":1950,"context_line":"                                              rebase_bw, rebase_flags)"},{"line_number":1951,"context_line":"            except libvirt.libvirtError as ex:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_6bae31e9","line":1948,"in_reply_to":"fa32b979_ac29e19c","updated":"2015-06-19 09:12:25.000000000","message":"I agree with you on general but I prefer the try/except form in this case because this way, as soon as Libvirt supports blockRebase on a stopped domain we can let libvirt handle that and not fallback on using qemu-img.","commit_id":"fca3ec7ee1446791b52a8cbfcd16ef9f8fb161fa"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"bc82d85b06f7e5eca4c8ffb38f8c332f27b953ca","unresolved":false,"context_lines":[{"line_number":1950,"context_line":"                                              rebase_bw, rebase_flags)"},{"line_number":1951,"context_line":"            except libvirt.libvirtError as ex:"},{"line_number":1952,"context_line":"                # If the domain is not running libvirt can\u0027t do a blockRebase,"},{"line_number":1953,"context_line":"                # let\u0027s ask qemu-img to do it. See LP #1444806"},{"line_number":1954,"context_line":"                if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_OPERATION_INVALID:"},{"line_number":1955,"context_line":"                    LOG.debug(\u0027Domain is not running so doing a block rebase \u0027"},{"line_number":1956,"context_line":"                              \u0027using \"qemu-img rebase\"\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_c97dd0e8","line":1953,"updated":"2015-06-22 08:06:36.000000000","message":"nit: I guest instead to ask developer to go to launchpad to know why we do that, you can add some details.\n\n  # Currently libvirt does not support block rebase on not running guest.\n  # This block catch the invalid operation in order to execute a manual\n  # rebase operation of the disk with qemu.","commit_id":"7fbc18a80def636e274988e67652e3237934532f"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"81e556f05fe6ce383c1856f7ed02dcf32ee7956d","unresolved":false,"context_lines":[{"line_number":1950,"context_line":"                                              rebase_bw, rebase_flags)"},{"line_number":1951,"context_line":"            except libvirt.libvirtError as ex:"},{"line_number":1952,"context_line":"                # If the domain is not running libvirt can\u0027t do a blockRebase,"},{"line_number":1953,"context_line":"                # let\u0027s ask qemu-img to do it. See LP #1444806"},{"line_number":1954,"context_line":"                if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_OPERATION_INVALID:"},{"line_number":1955,"context_line":"                    LOG.debug(\u0027Domain is not running so doing a block rebase \u0027"},{"line_number":1956,"context_line":"                              \u0027using \"qemu-img rebase\"\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_5c02c73f","line":1953,"in_reply_to":"fa32b979_c97dd0e8","updated":"2015-06-22 16:18:39.000000000","message":"Yep, @Jordan: feel free to reuse the text noted in the commit message, if that\u0027s useful.","commit_id":"7fbc18a80def636e274988e67652e3237934532f"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"a712af5181d2ef6192a23530d2b4cdef24301633","unresolved":false,"context_lines":[{"line_number":1959,"context_line":"                    # will exist independently of any backing file)."},{"line_number":1960,"context_line":"                    backing_file \u003d rebase_base if rebase_base else \"\""},{"line_number":1961,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1962,"context_line":"                                  active_disk_object.source_path)"},{"line_number":1963,"context_line":"                else:"},{"line_number":1964,"context_line":"                    raise"},{"line_number":1965,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_d9ce7e2c","line":1962,"updated":"2015-07-02 11:17:36.000000000","message":"I think we should use -F \u003cbacking_file_fmt\u003e in the qemu-img rebase cmd, since otherwise its considered a security issue (bcos libvirt does auto-probe to figure the format of the backing file and if it happens to be raw, someone can spoof the raw file to make it non-raw and cause it to read un-intended host files)\n\nEric Blake, would be the best to confirm this.","commit_id":"1fd3dbfd02a13327a1e7927cdd11167b375a2504"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"ce96902aec166512e31f781da7219480f6afa67a","unresolved":false,"context_lines":[{"line_number":1959,"context_line":"                    # will exist independently of any backing file)."},{"line_number":1960,"context_line":"                    backing_file \u003d rebase_base if rebase_base else \"\""},{"line_number":1961,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1962,"context_line":"                                  active_disk_object.source_path)"},{"line_number":1963,"context_line":"                else:"},{"line_number":1964,"context_line":"                    raise"},{"line_number":1965,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_74b97714","line":1962,"in_reply_to":"ba3cc151_d9ce7e2c","updated":"2015-07-03 10:21:27.000000000","message":"-F qcow2 or raw ? How could I guess ?\n\n-F will set the format of active_disk_object.source_path right ?","commit_id":"1fd3dbfd02a13327a1e7927cdd11167b375a2504"},{"author":{"_account_id":12661,"name":"Dmitry Guryanov","email":"dmitry.guryanov@gmail.com","username":"dguryanov"},"change_message_id":"9ed752fda85f083bfd394b44ccadc4f7a745efdd","unresolved":false,"context_lines":[{"line_number":2014,"context_line":"                         \u0027commit_base\u0027: commit_base,"},{"line_number":2015,"context_line":"                         \u0027commit_top\u0027: commit_top})"},{"line_number":2016,"context_line":""},{"line_number":2017,"context_line":"            result \u003d virt_dom.blockCommit(commit_disk, commit_base, commit_top,"},{"line_number":2018,"context_line":"                                          bandwidth, commit_flags)"},{"line_number":2019,"context_line":""},{"line_number":2020,"context_line":"            if result \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_94660d28","line":2017,"updated":"2015-06-26 16:03:31.000000000","message":"What about this blockCommit? It will fail on a stopped domain too.","commit_id":"1fd3dbfd02a13327a1e7927cdd11167b375a2504"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"a712af5181d2ef6192a23530d2b4cdef24301633","unresolved":false,"context_lines":[{"line_number":2014,"context_line":"                         \u0027commit_base\u0027: commit_base,"},{"line_number":2015,"context_line":"                         \u0027commit_top\u0027: commit_top})"},{"line_number":2016,"context_line":""},{"line_number":2017,"context_line":"            result \u003d virt_dom.blockCommit(commit_disk, commit_base, commit_top,"},{"line_number":2018,"context_line":"                                          bandwidth, commit_flags)"},{"line_number":2019,"context_line":""},{"line_number":2020,"context_line":"            if result \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_996f962a","line":2017,"in_reply_to":"ba3cc151_78e7d740","updated":"2015-07-02 11:17:36.000000000","message":"I think we should fix this for blockCommit too.\n\nIIUC tempest testcase tries to delete the active file after domain is shutoff, and presently libvirt does a blockRebase if the snap to be deleted happens to be the active file, bcos in the past libvirt did not support blockCommit for active files, which I am told is fixed now.\n\nSo in near future when blockCommit gets done for active file (since typically its better to commit than to rebase) we would break again for the same reason.\n\nI guess eblake (Eric Blake) would be a better person to comment on this.","commit_id":"1fd3dbfd02a13327a1e7927cdd11167b375a2504"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"81b2b9e8de826ad49ca7cb0428b74c3fb241ce22","unresolved":false,"context_lines":[{"line_number":2014,"context_line":"                         \u0027commit_base\u0027: commit_base,"},{"line_number":2015,"context_line":"                         \u0027commit_top\u0027: commit_top})"},{"line_number":2016,"context_line":""},{"line_number":2017,"context_line":"            result \u003d virt_dom.blockCommit(commit_disk, commit_base, commit_top,"},{"line_number":2018,"context_line":"                                          bandwidth, commit_flags)"},{"line_number":2019,"context_line":""},{"line_number":2020,"context_line":"            if result \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_c1aceeef","line":2017,"in_reply_to":"ba3cc151_78e7d740","updated":"2015-06-29 12:58:09.000000000","message":"I\u0027m not sure qemu-img command is able to handle a chain of disks so an extra work has to be done to rebase disks per \"couple\".","commit_id":"1fd3dbfd02a13327a1e7927cdd11167b375a2504"},{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"d7ced4e1db2af4f9e7a96e1a29eba7d54047f317","unresolved":false,"context_lines":[{"line_number":2014,"context_line":"                         \u0027commit_base\u0027: commit_base,"},{"line_number":2015,"context_line":"                         \u0027commit_top\u0027: commit_top})"},{"line_number":2016,"context_line":""},{"line_number":2017,"context_line":"            result \u003d virt_dom.blockCommit(commit_disk, commit_base, commit_top,"},{"line_number":2018,"context_line":"                                          bandwidth, commit_flags)"},{"line_number":2019,"context_line":""},{"line_number":2020,"context_line":"            if result \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_9542c7ac","line":2017,"in_reply_to":"ba3cc151_996f962a","updated":"2015-07-03 09:53:07.000000000","message":"@Deepak That sounds like another bug (that should be filed and fixed, for sure, but not necessarily in this patch)","commit_id":"1fd3dbfd02a13327a1e7927cdd11167b375a2504"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"ce96902aec166512e31f781da7219480f6afa67a","unresolved":false,"context_lines":[{"line_number":2014,"context_line":"                         \u0027commit_base\u0027: commit_base,"},{"line_number":2015,"context_line":"                         \u0027commit_top\u0027: commit_top})"},{"line_number":2016,"context_line":""},{"line_number":2017,"context_line":"            result \u003d virt_dom.blockCommit(commit_disk, commit_base, commit_top,"},{"line_number":2018,"context_line":"                                          bandwidth, commit_flags)"},{"line_number":2019,"context_line":""},{"line_number":2020,"context_line":"            if result \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_947e7b17","line":2017,"in_reply_to":"ba3cc151_996f962a","updated":"2015-07-03 10:21:27.000000000","message":"You said \u0027So in near future when blockCommit gets done\u0027. Do you know when this will be done ? \n\nI am trying to fix a real issue, today. I don\u0027t want to fix a potential issue that might arise in the future. If you are up to it, go ahead and propose a patch :)","commit_id":"1fd3dbfd02a13327a1e7927cdd11167b375a2504"},{"author":{"_account_id":12661,"name":"Dmitry Guryanov","email":"dmitry.guryanov@gmail.com","username":"dguryanov"},"change_message_id":"fb8b4c851045eb8d1247617a00b1d9bcce750780","unresolved":false,"context_lines":[{"line_number":2014,"context_line":"                         \u0027commit_base\u0027: commit_base,"},{"line_number":2015,"context_line":"                         \u0027commit_top\u0027: commit_top})"},{"line_number":2016,"context_line":""},{"line_number":2017,"context_line":"            result \u003d virt_dom.blockCommit(commit_disk, commit_base, commit_top,"},{"line_number":2018,"context_line":"                                          bandwidth, commit_flags)"},{"line_number":2019,"context_line":""},{"line_number":2020,"context_line":"            if result \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_2fd6e0c5","line":2017,"in_reply_to":"ba3cc151_c1aceeef","updated":"2015-06-29 14:41:23.000000000","message":"OK, I agree, that blockRebase should be fixed first.","commit_id":"1fd3dbfd02a13327a1e7927cdd11167b375a2504"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"44bd7d7dee636ca5fa750578330925d7c927fd9c","unresolved":false,"context_lines":[{"line_number":2014,"context_line":"                         \u0027commit_base\u0027: commit_base,"},{"line_number":2015,"context_line":"                         \u0027commit_top\u0027: commit_top})"},{"line_number":2016,"context_line":""},{"line_number":2017,"context_line":"            result \u003d virt_dom.blockCommit(commit_disk, commit_base, commit_top,"},{"line_number":2018,"context_line":"                                          bandwidth, commit_flags)"},{"line_number":2019,"context_line":""},{"line_number":2020,"context_line":"            if result \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_78e7d740","line":2017,"in_reply_to":"fa32b979_94660d28","updated":"2015-06-29 11:36:43.000000000","message":"I don\u0027t know, maybe. There\u0027s no Tempest that that exercises this code path when a domain is stopped.\n\nAnyway, changing this part of the code is not what I intend in this patch. Please let us know and submit a follow up patch if there\u0027s something to improve in this area.","commit_id":"1fd3dbfd02a13327a1e7927cdd11167b375a2504"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"0b5e7725197046fb2a517c89d67534cf1fdc94cc","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"                        b_file_fmt \u003d libvirt_utils.get_disk_type(backing_file)"},{"line_number":1972,"context_line":"                        qemu_img_extra_arg \u003d [\u0027-F\u0027, b_file_fmt]"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"                    qemu_img_extra_arg.append(active_disk_object.source_path)"},{"line_number":1975,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1976,"context_line":"                                  *qemu_img_extra_arg)"},{"line_number":1977,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_1cd39b05","line":1974,"updated":"2015-07-08 11:31:46.000000000","message":"I am not sure if active_disk_object.source_path would be a valid path if the disk is of \u0027network\u0027 type ?\n\nLooking at libvirt/config.py, it looks like source_path is valid only for file, block type disks, but I am not a libvirt expert, hence just flagging this here for someone better knowledgible to assert whether this works for network type disks.","commit_id":"660926c938f23670615c4273ce97ba9fa7f8408d"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"64c9940c60a0129496823d268f8e42f700ae7611","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"                        b_file_fmt \u003d libvirt_utils.get_disk_type(backing_file)"},{"line_number":1972,"context_line":"                        qemu_img_extra_arg \u003d [\u0027-F\u0027, b_file_fmt]"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"                    qemu_img_extra_arg.append(active_disk_object.source_path)"},{"line_number":1975,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1976,"context_line":"                                  *qemu_img_extra_arg)"},{"line_number":1977,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_8ea89521","line":1974,"in_reply_to":"ba3cc151_0f66f550","updated":"2015-07-09 09:38:57.000000000","message":"Agreed.\nSo i see 2 ways to go from here\n\n1) Find a way to fix this for network disks. I tried to pull in few nova folks on IRC, lets see, if anyone helps us in figuring out how to form the network disk path correctly\n\n-- or --\n\n2) As eharney mentioned, use active_disk_object.source_path if active_protocol si None (ie not a network disk) and if network disk (else part), raise excp","commit_id":"660926c938f23670615c4273ce97ba9fa7f8408d"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"6f8d6cf06fdfdd82f3beb9dc55485a0403214cf4","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"                        b_file_fmt \u003d libvirt_utils.get_disk_type(backing_file)"},{"line_number":1972,"context_line":"                        qemu_img_extra_arg \u003d [\u0027-F\u0027, b_file_fmt]"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"                    qemu_img_extra_arg.append(active_disk_object.source_path)"},{"line_number":1975,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1976,"context_line":"                                  *qemu_img_extra_arg)"},{"line_number":1977,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_5263d241","line":1974,"in_reply_to":"ba3cc151_1201ca6a","updated":"2015-07-10 09:18:56.000000000","message":"I know you would go for 2 ;-) Thats fine.\nNotImplementedError seems good for now.","commit_id":"660926c938f23670615c4273ce97ba9fa7f8408d"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e6cb2dbff3458b9e2231346e5d4c188ab348b468","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"                        b_file_fmt \u003d libvirt_utils.get_disk_type(backing_file)"},{"line_number":1972,"context_line":"                        qemu_img_extra_arg \u003d [\u0027-F\u0027, b_file_fmt]"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"                    qemu_img_extra_arg.append(active_disk_object.source_path)"},{"line_number":1975,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1976,"context_line":"                                  *qemu_img_extra_arg)"},{"line_number":1977,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_0f66f550","line":1974,"in_reply_to":"ba3cc151_1cd39b05","updated":"2015-07-08 14:28:59.000000000","message":"I think you are right that this will break for network disks (libgfapi etc).  Not sure about the best way to fix that... qemu-img supports libgfapi but I\u0027m not sure about other protocols.\n\nIt seems reasonable to me to fix this for files here, fail in a sane way for network disks, and follow that up in a separate patch.  (Because I\u0027m not sure how much work is required to fix that part.)","commit_id":"660926c938f23670615c4273ce97ba9fa7f8408d"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"6d3b5032a8aeb33ebed159c7c303bc660b334382","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"                        b_file_fmt \u003d libvirt_utils.get_disk_type(backing_file)"},{"line_number":1972,"context_line":"                        qemu_img_extra_arg \u003d [\u0027-F\u0027, b_file_fmt]"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"                    qemu_img_extra_arg.append(active_disk_object.source_path)"},{"line_number":1975,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1976,"context_line":"                                  *qemu_img_extra_arg)"},{"line_number":1977,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_3293ae02","line":1974,"in_reply_to":"ba3cc151_5263d241","updated":"2015-07-10 09:21:22.000000000","message":"Haha ! \nOkay. I\u0027ll submit something soon.","commit_id":"660926c938f23670615c4273ce97ba9fa7f8408d"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"67de3612487e78b0aa30d1ba02c805b8e0c7e3bb","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"                        b_file_fmt \u003d libvirt_utils.get_disk_type(backing_file)"},{"line_number":1972,"context_line":"                        qemu_img_extra_arg \u003d [\u0027-F\u0027, b_file_fmt]"},{"line_number":1973,"context_line":""},{"line_number":1974,"context_line":"                    qemu_img_extra_arg.append(active_disk_object.source_path)"},{"line_number":1975,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1976,"context_line":"                                  *qemu_img_extra_arg)"},{"line_number":1977,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_1201ca6a","line":1974,"in_reply_to":"ba3cc151_8ea89521","updated":"2015-07-10 09:16:57.000000000","message":"So I am going for solution number 2 :) \n\nI could raise either several type of exceptions: NovaException, Invalid or NotImplementedError. I could also just \"raise\" (it would raise a libvirtError).\n\nWhich do you prefer ?\nI like notImplementedError because it clearly says \"please do test with your protocol and implement this\".","commit_id":"660926c938f23670615c4273ce97ba9fa7f8408d"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"3c709f5d04ad72f6ea926026bf2c055c532bb3b2","unresolved":false,"context_lines":[{"line_number":1958,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"},{"line_number":1959,"context_line":"                    # disks for every protocol. So let\u0027s be safe:"},{"line_number":1960,"context_line":"                    if active_protocol is not None:"},{"line_number":1961,"context_line":"                        msg \u003d (\"Rebasing a network disk using qemu-img has \" +"},{"line_number":1962,"context_line":"                               \"not been fully tested\")"},{"line_number":1963,"context_line":"                        raise NotImplementedError(msg)"},{"line_number":1964,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_a788bce8","line":1961,"updated":"2015-07-13 09:37:17.000000000","message":"msg should be i18n formatted... \n_LE(..)","commit_id":"a45bb4f0228f95fb66d6c105591daf5fa0283f5f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"bb33c4d71cabecd47efdec7c11cd9de4b03b0595","unresolved":false,"context_lines":[{"line_number":1958,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"},{"line_number":1959,"context_line":"                    # disks for every protocol. So let\u0027s be safe:"},{"line_number":1960,"context_line":"                    if active_protocol is not None:"},{"line_number":1961,"context_line":"                        msg \u003d (\"Rebasing a network disk using qemu-img has \" +"},{"line_number":1962,"context_line":"                               \"not been fully tested\")"},{"line_number":1963,"context_line":"                        raise NotImplementedError(msg)"},{"line_number":1964,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_e6ce452c","line":1961,"in_reply_to":"9a41bdd9_a788bce8","updated":"2015-07-15 11:29:57.000000000","message":"no, the _LE is only required for log messages. It should be\nmsg \u003d _(...)","commit_id":"a45bb4f0228f95fb66d6c105591daf5fa0283f5f"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"b33c83ac352754ab65d70e43a024c8f5b25b2505","unresolved":false,"context_lines":[{"line_number":1958,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"},{"line_number":1959,"context_line":"                    # disks for every protocol. So let\u0027s be safe:"},{"line_number":1960,"context_line":"                    if active_protocol is not None:"},{"line_number":1961,"context_line":"                        msg \u003d (\"Rebasing a network disk using qemu-img has \" +"},{"line_number":1962,"context_line":"                               \"not been fully tested\")"},{"line_number":1963,"context_line":"                        raise NotImplementedError(msg)"},{"line_number":1964,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_e9da964a","line":1961,"in_reply_to":"9a41bdd9_e6ce452c","updated":"2015-07-15 11:45:27.000000000","message":"Yes. The correct form is _() see http://docs.openstack.org/developer/oslo.i18n/guidelines.html\n\nWill fix soon.","commit_id":"a45bb4f0228f95fb66d6c105591daf5fa0283f5f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"bb33c4d71cabecd47efdec7c11cd9de4b03b0595","unresolved":false,"context_lines":[{"line_number":1959,"context_line":"                    # disks for every protocol. So let\u0027s be safe:"},{"line_number":1960,"context_line":"                    if active_protocol is not None:"},{"line_number":1961,"context_line":"                        msg \u003d (\"Rebasing a network disk using qemu-img has \" +"},{"line_number":1962,"context_line":"                               \"not been fully tested\")"},{"line_number":1963,"context_line":"                        raise NotImplementedError(msg)"},{"line_number":1964,"context_line":""},{"line_number":1965,"context_line":"                    if not rebase_base:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_26650d1d","line":1962,"updated":"2015-07-15 11:29:57.000000000","message":"how will we ever know if this will have been tested? Could this be done according to the libvirt version?","commit_id":"a45bb4f0228f95fb66d6c105591daf5fa0283f5f"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"b33c83ac352754ab65d70e43a024c8f5b25b2505","unresolved":false,"context_lines":[{"line_number":1959,"context_line":"                    # disks for every protocol. So let\u0027s be safe:"},{"line_number":1960,"context_line":"                    if active_protocol is not None:"},{"line_number":1961,"context_line":"                        msg \u003d (\"Rebasing a network disk using qemu-img has \" +"},{"line_number":1962,"context_line":"                               \"not been fully tested\")"},{"line_number":1963,"context_line":"                        raise NotImplementedError(msg)"},{"line_number":1964,"context_line":""},{"line_number":1965,"context_line":"                    if not rebase_base:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_c9f79289","line":1962,"in_reply_to":"9a41bdd9_26650d1d","updated":"2015-07-15 11:45:27.000000000","message":"I think glusterfs people, sheepdog people, etc... should test this and update this part of the code once this is merged.\n\nMy intention now is to only support what I am able to test. I am not introducing any regression or risk since at the moment a libvirt.libvirtError exception is raised for everybody that hits this part of the code.","commit_id":"a45bb4f0228f95fb66d6c105591daf5fa0283f5f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3b7da88104d279b487fad70f3e4d4c61535f6763","unresolved":false,"context_lines":[{"line_number":1949,"context_line":"                    if active_protocol is not None:"},{"line_number":1950,"context_line":"                        msg \u003d _(\"Rebasing a network disk using qemu-img has \""},{"line_number":1951,"context_line":"                                \"not been fully tested\")"},{"line_number":1952,"context_line":"                        raise NotImplementedError(msg)"},{"line_number":1953,"context_line":""},{"line_number":1954,"context_line":"                    if not rebase_base:"},{"line_number":1955,"context_line":"                        # If backing_file is specified as \"\" (the empty"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a41bdd9_9b38b5d4","line":1952,"updated":"2015-07-16 10:16:49.000000000","message":"We probably want to have something that inherit NovaException here. You can create a new exception in nova/exception.py or just raise NovaException with your well defined message. (I guess it will be enough)","commit_id":"b27b2c283444089ffb44d873a8d8699055b5bac9"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"50c9fa9665fb42e79078ecf8ac382699f4b067a2","unresolved":false,"context_lines":[{"line_number":1949,"context_line":"                    if active_protocol is not None:"},{"line_number":1950,"context_line":"                        msg \u003d _(\"Rebasing a network disk using qemu-img has \""},{"line_number":1951,"context_line":"                                \"not been fully tested\")"},{"line_number":1952,"context_line":"                        raise NotImplementedError(msg)"},{"line_number":1953,"context_line":""},{"line_number":1954,"context_line":"                    if not rebase_base:"},{"line_number":1955,"context_line":"                        # If backing_file is specified as \"\" (the empty"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a41bdd9_de7dcb6d","line":1952,"in_reply_to":"9a41bdd9_9b38b5d4","updated":"2015-07-16 10:33:24.000000000","message":"Just so I understand, why should I raise a NovaException (or a child of) ? \n\ngit grep \"NotImplementedE\" nova/virt/libvirt/\nreturns several other occurrences of NotImplementedError being raised.","commit_id":"b27b2c283444089ffb44d873a8d8699055b5bac9"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"7db435380e8658a2e1717d507897424e8742f1ff","unresolved":false,"context_lines":[{"line_number":1949,"context_line":"                    if active_protocol is not None:"},{"line_number":1950,"context_line":"                        msg \u003d _(\"Rebasing a network disk using qemu-img has \""},{"line_number":1951,"context_line":"                                \"not been fully tested\")"},{"line_number":1952,"context_line":"                        raise NotImplementedError(msg)"},{"line_number":1953,"context_line":""},{"line_number":1954,"context_line":"                    if not rebase_base:"},{"line_number":1955,"context_line":"                        # If backing_file is specified as \"\" (the empty"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a41bdd9_0ad68d4e","line":1952,"in_reply_to":"9a41bdd9_de7dcb6d","updated":"2015-07-16 11:36:26.000000000","message":"We usually use NotImplemntedError for a feature not handled by driver. Here it\u0027s not the case, we are not able to handle the request because a) instance is not running b) we are not sure about the protocol used.","commit_id":"b27b2c283444089ffb44d873a8d8699055b5bac9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"3ad11b8b08736f781490c1e1b1db4fc440ff58bf","unresolved":false,"context_lines":[{"line_number":1957,"context_line":"                        # file (i.e. it will exist independently of any"},{"line_number":1958,"context_line":"                        # backing file)."},{"line_number":1959,"context_line":"                        backing_file \u003d \"\""},{"line_number":1960,"context_line":"                        qemu_img_extra_arg \u003d []"},{"line_number":1961,"context_line":"                    else:"},{"line_number":1962,"context_line":"                        # If the rebased image is going to have a backing file"},{"line_number":1963,"context_line":"                        # then explicitely set the backing file format to avoid"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a50d1a3_c893b5a2","line":1960,"range":{"start_line":1960,"start_character":45,"end_line":1960,"end_character":47},"updated":"2015-07-20 07:59:39.000000000","message":"Setting backing_file \u003d \"\" yields a runtime exception due to the fact that the resulting qemu-img call is something like:\n\nCommand: qemu-img rebase -b  /opt/stack/data/nova/mnt/38effaab97f9ee27ecb78121f511fa07/volume-9ed57a88-686f-4d22-b3e5-d2781c1fab5d.556afb73-9a54-40d5-a81e-6b78d4b0c150\n\nqemu-img is missing a parameter since -b is provided with \"\", the final path param is therefore interpreted as -b param and the main filename param is missing in my tests.\n\nSidenote: libvirt.py returns \u0027no such file or directory\u0027 on this but if runnig this manually on the commandline you get a missing parameter reaction from the qemu-img binary. Looks like the libvirt.py wrapper misinterprets the binaries feedback in this case.","commit_id":"53bf87fc69491b0f8df50fd28e15c41f2d1f6862"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"94d21d0941ef85e9003d44284e9fc83286aef4ae","unresolved":false,"context_lines":[{"line_number":1957,"context_line":"                        # file (i.e. it will exist independently of any"},{"line_number":1958,"context_line":"                        # backing file)."},{"line_number":1959,"context_line":"                        backing_file \u003d \"\""},{"line_number":1960,"context_line":"                        qemu_img_extra_arg \u003d []"},{"line_number":1961,"context_line":"                    else:"},{"line_number":1962,"context_line":"                        # If the rebased image is going to have a backing file"},{"line_number":1963,"context_line":"                        # then explicitely set the backing file format to avoid"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a50d1a3_433c6693","line":1960,"range":{"start_line":1960,"start_character":45,"end_line":1960,"end_character":47},"in_reply_to":"3a50d1a3_c893b5a2","updated":"2015-07-20 08:31:02.000000000","message":"My Comment above is wrong, please ignore.","commit_id":"53bf87fc69491b0f8df50fd28e15c41f2d1f6862"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ed9458861eaa84e073d18564166fd5bd12fda98b","unresolved":false,"context_lines":[{"line_number":713,"context_line":"                                 instance\u003dinstance)"},{"line_number":714,"context_line":"                        with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":715,"context_line":"                            # Try up to 3 times before giving up."},{"line_number":716,"context_line":"                            if attempt \u003c 3:"},{"line_number":717,"context_line":"                                ctxt.reraise \u003d False"},{"line_number":718,"context_line":"                                self._destroy(instance, attempt + 1)"},{"line_number":719,"context_line":"                                return"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_72841e0a","line":716,"updated":"2015-08-06 16:29:40.000000000","message":"See how the reraise is handled here.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ed9458861eaa84e073d18564166fd5bd12fda98b","unresolved":false,"context_lines":[{"line_number":858,"context_line":"                    if destroy_disks:"},{"line_number":859,"context_line":"                        # Don\u0027t block on Volume errors if we\u0027re trying to"},{"line_number":860,"context_line":"                        # delete the instance as we may be partially created"},{"line_number":861,"context_line":"                        # or deleted"},{"line_number":862,"context_line":"                        ctxt.reraise \u003d False"},{"line_number":863,"context_line":"                        LOG.warn(_LW(\"Ignoring Volume Error on vol %(vol_id)s \""},{"line_number":864,"context_line":"                                     \"during delete %(exc)s\"),"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_12803af4","line":861,"updated":"2015-08-06 16:29:40.000000000","message":"And here.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2a70510c0d0013c5ad75c1e8ade40ac5aa284cd6","unresolved":false,"context_lines":[{"line_number":1896,"context_line":"            except libvirt.libvirtError as ex:"},{"line_number":1897,"context_line":"                # If the domain is not running libvirt won\u0027t do a blockRebase."},{"line_number":1898,"context_line":"                # In that case, let\u0027s ask qemu-img to rebase the disk."},{"line_number":1899,"context_line":"                if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_OPERATION_INVALID:"},{"line_number":1900,"context_line":"                    LOG.debug(\u0027Domain is not running so doing a block rebase \u0027"},{"line_number":1901,"context_line":"                              \u0027using \"qemu-img rebase\"\u0027)"},{"line_number":1902,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_74ec13f3","line":1899,"updated":"2015-08-06 14:40:24.000000000","message":"How do you know that this is for the domain not running?  We should probably check the domain power state before we just assume this is the case for the failure, or parse the error message from the libvirt error, danpb or sahid might have other ideas.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"0e90644401dea1157edb7343d24b23fe8aab7153","unresolved":false,"context_lines":[{"line_number":1896,"context_line":"            except libvirt.libvirtError as ex:"},{"line_number":1897,"context_line":"                # If the domain is not running libvirt won\u0027t do a blockRebase."},{"line_number":1898,"context_line":"                # In that case, let\u0027s ask qemu-img to rebase the disk."},{"line_number":1899,"context_line":"                if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_OPERATION_INVALID:"},{"line_number":1900,"context_line":"                    LOG.debug(\u0027Domain is not running so doing a block rebase \u0027"},{"line_number":1901,"context_line":"                              \u0027using \"qemu-img rebase\"\u0027)"},{"line_number":1902,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_04c5e159","line":1899,"in_reply_to":"1a4dcd0f_74ec13f3","updated":"2015-08-10 12:48:34.000000000","message":"It seems that VIR_ERR_OPERATION_INVALID is used when domain is not running in this particular context [1], I guess doing a check is better to avoid any future pb yes.\n\n[1] https://github.com/libvirt/libvirt/blob/master/src/qemu/qemu_driver.c#L16211","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03fbf70d4ec5a6fab4de80d30649719fb00741d9","unresolved":false,"context_lines":[{"line_number":1896,"context_line":"            except libvirt.libvirtError as ex:"},{"line_number":1897,"context_line":"                # If the domain is not running libvirt won\u0027t do a blockRebase."},{"line_number":1898,"context_line":"                # In that case, let\u0027s ask qemu-img to rebase the disk."},{"line_number":1899,"context_line":"                if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_OPERATION_INVALID:"},{"line_number":1900,"context_line":"                    LOG.debug(\u0027Domain is not running so doing a block rebase \u0027"},{"line_number":1901,"context_line":"                              \u0027using \"qemu-img rebase\"\u0027)"},{"line_number":1902,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_af948642","line":1899,"in_reply_to":"1a4dcd0f_74ec13f3","updated":"2015-08-06 14:54:29.000000000","message":"Ok, will do.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2a70510c0d0013c5ad75c1e8ade40ac5aa284cd6","unresolved":false,"context_lines":[{"line_number":1897,"context_line":"                # If the domain is not running libvirt won\u0027t do a blockRebase."},{"line_number":1898,"context_line":"                # In that case, let\u0027s ask qemu-img to rebase the disk."},{"line_number":1899,"context_line":"                if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_OPERATION_INVALID:"},{"line_number":1900,"context_line":"                    LOG.debug(\u0027Domain is not running so doing a block rebase \u0027"},{"line_number":1901,"context_line":"                              \u0027using \"qemu-img rebase\"\u0027)"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_94419fab","line":1900,"updated":"2015-08-06 14:40:24.000000000","message":"With this addition this method is now ~250 LOC.  It\u0027d be nice if we could pull this new chunk of code out into it\u0027s own private method so we can just unit test this part and now have to stub out everything that happens before it to unit test this.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03fbf70d4ec5a6fab4de80d30649719fb00741d9","unresolved":false,"context_lines":[{"line_number":1897,"context_line":"                # If the domain is not running libvirt won\u0027t do a blockRebase."},{"line_number":1898,"context_line":"                # In that case, let\u0027s ask qemu-img to rebase the disk."},{"line_number":1899,"context_line":"                if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_OPERATION_INVALID:"},{"line_number":1900,"context_line":"                    LOG.debug(\u0027Domain is not running so doing a block rebase \u0027"},{"line_number":1901,"context_line":"                              \u0027using \"qemu-img rebase\"\u0027)"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_cfe472e1","line":1900,"in_reply_to":"1a4dcd0f_94419fab","updated":"2015-08-06 14:54:29.000000000","message":"Ok will do.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2a70510c0d0013c5ad75c1e8ade40ac5aa284cd6","unresolved":false,"context_lines":[{"line_number":1903,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"},{"line_number":1904,"context_line":"                    # disks for every protocol. So let\u0027s be safe:"},{"line_number":1905,"context_line":"                    if active_protocol is not None:"},{"line_number":1906,"context_line":"                        msg \u003d _(\"Rebasing a network disk using qemu-img has \""},{"line_number":1907,"context_line":"                                \"not been fully tested\")"},{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_b494db42","line":1906,"updated":"2015-08-06 14:40:24.000000000","message":"This seems like kind of an odd message, what should the user do in this case?  Also, probably good to throw the active_protocol in the exception message for context.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"b1ed6c3906cc5e538dec3a426382260cfae2f1fc","unresolved":false,"context_lines":[{"line_number":1903,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"},{"line_number":1904,"context_line":"                    # disks for every protocol. So let\u0027s be safe:"},{"line_number":1905,"context_line":"                    if active_protocol is not None:"},{"line_number":1906,"context_line":"                        msg \u003d _(\"Rebasing a network disk using qemu-img has \""},{"line_number":1907,"context_line":"                                \"not been fully tested\")"},{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_c44e1996","line":1906,"in_reply_to":"1a4dcd0f_446a091c","updated":"2015-08-10 12:50:01.000000000","message":"And you can set a log message to operator to explain that \"Rebasing a network disk using qemu-img has not been fully tested\"\"","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"0e90644401dea1157edb7343d24b23fe8aab7153","unresolved":false,"context_lines":[{"line_number":1903,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"},{"line_number":1904,"context_line":"                    # disks for every protocol. So let\u0027s be safe:"},{"line_number":1905,"context_line":"                    if active_protocol is not None:"},{"line_number":1906,"context_line":"                        msg \u003d _(\"Rebasing a network disk using qemu-img has \""},{"line_number":1907,"context_line":"                                \"not been fully tested\")"},{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_446a091c","line":1906,"in_reply_to":"1a4dcd0f_aff806c4","updated":"2015-08-10 12:48:34.000000000","message":"Well actually I think this message will be returned to user. Perhaps just say that \"the operation is not supported...\" could be enough.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03fbf70d4ec5a6fab4de80d30649719fb00741d9","unresolved":false,"context_lines":[{"line_number":1903,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"},{"line_number":1904,"context_line":"                    # disks for every protocol. So let\u0027s be safe:"},{"line_number":1905,"context_line":"                    if active_protocol is not None:"},{"line_number":1906,"context_line":"                        msg \u003d _(\"Rebasing a network disk using qemu-img has \""},{"line_number":1907,"context_line":"                                \"not been fully tested\")"},{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_aff806c4","line":1906,"in_reply_to":"1a4dcd0f_b494db42","updated":"2015-08-06 14:54:29.000000000","message":"This exception is not returned to the user. It\u0027s only printed in the log.\n\nOk to add the active_protocol. Will do.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2a70510c0d0013c5ad75c1e8ade40ac5aa284cd6","unresolved":false,"context_lines":[{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""},{"line_number":1910,"context_line":"                    if not rebase_base:"},{"line_number":1911,"context_line":"                        # If backing_file is specified as \"\" (the empty"},{"line_number":1912,"context_line":"                        # string), then the image is rebased onto no backing"},{"line_number":1913,"context_line":"                        # file (i.e. it will exist independently of any"},{"line_number":1914,"context_line":"                        # backing file)."}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_7472d312","line":1911,"updated":"2015-08-06 14:40:24.000000000","message":"Where is this documented?  Not seeing it mentioned as a special case in the man page:\n\nhttp://linux.die.net/man/1/qemu-img","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03fbf70d4ec5a6fab4de80d30649719fb00741d9","unresolved":false,"context_lines":[{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""},{"line_number":1910,"context_line":"                    if not rebase_base:"},{"line_number":1911,"context_line":"                        # If backing_file is specified as \"\" (the empty"},{"line_number":1912,"context_line":"                        # string), then the image is rebased onto no backing"},{"line_number":1913,"context_line":"                        # file (i.e. it will exist independently of any"},{"line_number":1914,"context_line":"                        # backing file)."}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_8fd3ca03","line":1911,"in_reply_to":"1a4dcd0f_7472d312","updated":"2015-08-06 14:54:29.000000000","message":"Your man page is outdated: http://www.tin.org/bin/man.cgi?section\u003d1\u0026topic\u003dqemu-img","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ed9458861eaa84e073d18564166fd5bd12fda98b","unresolved":false,"context_lines":[{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""},{"line_number":1910,"context_line":"                    if not rebase_base:"},{"line_number":1911,"context_line":"                        # If backing_file is specified as \"\" (the empty"},{"line_number":1912,"context_line":"                        # string), then the image is rebased onto no backing"},{"line_number":1913,"context_line":"                        # file (i.e. it will exist independently of any"},{"line_number":1914,"context_line":"                        # backing file)."}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_f2c26e52","line":1911,"in_reply_to":"1a4dcd0f_8fd3ca03","updated":"2015-08-06 16:29:40.000000000","message":"OK, yup - so is there a certain minimum version of QEMU required to assume -b \"\" is how this works?","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"953e259815e2947d19047bcb8f7976fbdfc93cd5","unresolved":false,"context_lines":[{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""},{"line_number":1910,"context_line":"                    if not rebase_base:"},{"line_number":1911,"context_line":"                        # If backing_file is specified as \"\" (the empty"},{"line_number":1912,"context_line":"                        # string), then the image is rebased onto no backing"},{"line_number":1913,"context_line":"                        # file (i.e. it will exist independently of any"},{"line_number":1914,"context_line":"                        # backing file)."}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_c8d86382","line":1911,"in_reply_to":"1a4dcd0f_ae364cb2","updated":"2015-08-10 14:56:31.000000000","message":"So I\u0027m thinking we need a QEMU version check in here then, we currently gate on what\u0027s in ubuntu trusty which is 1.2.2 I believe.  There is a fedora 21 or 22 job on the experimental queue which has a newer version of libvirt and qemu so we can test some things there.\n\nBut we should probably have the version check like we have for MIN_QEMU_LIVESNAPSHOT_VERSION above and similar feature checks before calling out to qemu-img.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"76b73b26b28421044553fef406869103e7d11e62","unresolved":false,"context_lines":[{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""},{"line_number":1910,"context_line":"                    if not rebase_base:"},{"line_number":1911,"context_line":"                        # If backing_file is specified as \"\" (the empty"},{"line_number":1912,"context_line":"                        # string), then the image is rebased onto no backing"},{"line_number":1913,"context_line":"                        # file (i.e. it will exist independently of any"},{"line_number":1914,"context_line":"                        # backing file)."}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_bfce6a81","line":1911,"in_reply_to":"1a4dcd0f_c8d86382","updated":"2015-08-10 15:29:33.000000000","message":"Ubuntu trusty ships qemu (and qemu-utils) 2.0. 1.2.2 is the version of python-libvirt.\n\nCentos 7 ships qemu 1.5.\n\nI am not sure Liberty will run on Centos 6 and/or Ubuntu 12.\n\nI can add a qemu version check, but the code is already \"complicated\" with a lot of conditionals..","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"2dcbe72b7b08689ced66068e686d7996438b6dd6","unresolved":false,"context_lines":[{"line_number":1908,"context_line":"                        raise exception.NovaException(msg)"},{"line_number":1909,"context_line":""},{"line_number":1910,"context_line":"                    if not rebase_base:"},{"line_number":1911,"context_line":"                        # If backing_file is specified as \"\" (the empty"},{"line_number":1912,"context_line":"                        # string), then the image is rebased onto no backing"},{"line_number":1913,"context_line":"                        # file (i.e. it will exist independently of any"},{"line_number":1914,"context_line":"                        # backing file)."}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_ae364cb2","line":1911,"in_reply_to":"1a4dcd0f_f2c26e52","updated":"2015-08-10 12:14:03.000000000","message":"Yes. You need qemu 1.3 (https://github.com/qemu/qemu/commit/a616673dd1c2e00db5e3458d2ba4b6619b78876a)\n\nI am not sure Liberty will support qemu before 1.3.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ed9458861eaa84e073d18564166fd5bd12fda98b","unresolved":false,"context_lines":[{"line_number":1927,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1928,"context_line":"                                  *qemu_img_extra_arg)"},{"line_number":1929,"context_line":"                else:"},{"line_number":1930,"context_line":"                    raise"},{"line_number":1931,"context_line":"            else:"},{"line_number":1932,"context_line":"                if result \u003d\u003d 0:"},{"line_number":1933,"context_line":"                    LOG.debug(\u0027blockRebase started successfully\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_928bea1e","line":1930,"updated":"2015-08-06 16:29:40.000000000","message":"I feel like we should be doing an excutils.save_and_reraise_exception with a ctxt.reraise\u003dTrue here.  I added some links to other examples of how we handle errors in this module using reraise.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"953e259815e2947d19047bcb8f7976fbdfc93cd5","unresolved":false,"context_lines":[{"line_number":1927,"context_line":"                    utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":1928,"context_line":"                                  *qemu_img_extra_arg)"},{"line_number":1929,"context_line":"                else:"},{"line_number":1930,"context_line":"                    raise"},{"line_number":1931,"context_line":"            else:"},{"line_number":1932,"context_line":"                if result \u003d\u003d 0:"},{"line_number":1933,"context_line":"                    LOG.debug(\u0027blockRebase started successfully\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_889a5b6d","line":1930,"in_reply_to":"1a4dcd0f_928bea1e","updated":"2015-08-10 14:56:31.000000000","message":"Sorry, to clarify:\n\nyou put that exception block in the save_and_reraise_exception block which defaults to reraise, which is your L1930, but if you get to L1927 and that passes (the qemu-img call), then you set ctxt.reraise\u003dFalse,\nalthough that might get goofy with the exceptions that come out of the error handling block itself, like the NovaException on line 1908.","commit_id":"bc698c60cd2139269f801371eff8934ca590db31"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"039635275c6bf39a63e9a8994efd78612c18b96a","unresolved":false,"context_lines":[{"line_number":1803,"context_line":"                        msg \u003d _LE(\"Rebasing a %s network disk using qemu-img \""},{"line_number":1804,"context_line":"                                  \"has not been fully tested\")"},{"line_number":1805,"context_line":"                        LOG.exception(msg, active_protocol)"},{"line_number":1806,"context_line":"                        raise exception.NovaException(\"Something went wrong \""},{"line_number":1807,"context_line":"                                                      \"when deleting a volume \""},{"line_number":1808,"context_line":"                                                      \"snapshot\")"},{"line_number":1809,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"1a4dcd0f_34733d82","line":1806,"updated":"2015-08-11 07:47:41.000000000","message":"So here probably want that the exception continue. Instead of raising new exception using a else could be enough.\n\n  if active_protocol is not None:\n  ...\n    LOG.exception(msg, ...)\n  else:\n    if not rebase_base:\n  ...","commit_id":"e1aa9d3073aca79af0c16c37675fd267d30dc02e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"606ceb682d50ecf8a9424df8bff4dccd9143b518","unresolved":false,"context_lines":[{"line_number":1772,"context_line":"        Rebase `device` with Libvirt unless the `guest` is not running and"},{"line_number":1773,"context_line":"        Libvirt doesn\u0027t support blockRebase on a unactive guest."},{"line_number":1774,"context_line":""},{"line_number":1775,"context_line":"        :type instance: nova.objects.instance.Instance"},{"line_number":1776,"context_line":"        :type guest: nova.virt.libvirt.guest.Guest"},{"line_number":1777,"context_line":"        :param device: the guest block device to rebase"},{"line_number":1778,"context_line":"        :type device: nova.virt.libvirt.guest.BlockDevice"}],"source_content_type":"text/x-python","patch_set":16,"id":"1a4dcd0f_efa89b79","line":1775,"updated":"2015-08-11 10:41:47.000000000","message":"We usually do not make difference between type/param and prefer to use \u0027param\u0027\n\n  :param instance: A nova.objects.instance.Instance\u0027s Object\n  ...\n\nPS: Hum instance can be removed i guess.","commit_id":"3989b31a1e1a00c97dae4d36cdec8208f46e0c42"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"606ceb682d50ecf8a9424df8bff4dccd9143b518","unresolved":false,"context_lines":[{"line_number":1786,"context_line":"        \"\"\""},{"line_number":1787,"context_line":""},{"line_number":1788,"context_line":"        try:"},{"line_number":1789,"context_line":"            result \u003d device.rebase(rebase_base, relative\u003drelative)"},{"line_number":1790,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":1791,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1792,"context_line":"                # If the guest is not running libvirt won\u0027t do a blockRebase."}],"source_content_type":"text/x-python","patch_set":16,"id":"1a4dcd0f_52829401","line":1789,"updated":"2015-08-11 10:41:47.000000000","message":"So I think we should keep this out of that new method. The new method you have implemented should only have code related to qemu.","commit_id":"3989b31a1e1a00c97dae4d36cdec8208f46e0c42"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"606ceb682d50ecf8a9424df8bff4dccd9143b518","unresolved":false,"context_lines":[{"line_number":1796,"context_line":"                    LOG.debug(\u0027Guest is not running so doing a block rebase \u0027"},{"line_number":1797,"context_line":"                              \u0027using \"qemu-img rebase\"\u0027)"},{"line_number":1798,"context_line":""},{"line_number":1799,"context_line":"                    # Unfortunately, not sure how well qemu-img handles network"},{"line_number":1800,"context_line":"                    # disks for every protocol. So let\u0027s be safe and let the"},{"line_number":1801,"context_line":"                    # exception bubble up:"},{"line_number":1802,"context_line":"                    active_protocol \u003d active_disk_object.source_protocol"}],"source_content_type":"text/x-python","patch_set":16,"id":"1a4dcd0f_0ff94780","line":1799,"updated":"2015-08-11 10:41:47.000000000","message":"I would recommend to move this interesting comment line 1804 (just after the \u0027if\u0027 condition) to make it clear which condition we are talking and so what we do.","commit_id":"3989b31a1e1a00c97dae4d36cdec8208f46e0c42"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"606ceb682d50ecf8a9424df8bff4dccd9143b518","unresolved":false,"context_lines":[{"line_number":1991,"context_line":"                 \u0027relative\u0027: str(relative)}, instance\u003dinstance)"},{"line_number":1992,"context_line":""},{"line_number":1993,"context_line":"            dev \u003d guest.get_block_device(rebase_disk)"},{"line_number":1994,"context_line":"            self._safe_rebase_device(instance, guest, dev, active_disk_object,"},{"line_number":1995,"context_line":"                                     rebase_base, relative)"},{"line_number":1996,"context_line":""},{"line_number":1997,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":16,"id":"1a4dcd0f_f24400b8","line":1994,"updated":"2015-08-11 10:41:47.000000000","message":"so you pass \u0027instance\u0027 just for two LOG.debug. Actually the problem comes from your new method, you should verify before whether domain is active.\n\n  if guest.is_active():\n    dev \u003d guest.get_block_device(rebase_disk)\n    result \u003d dev.rebase(rebase_base, relative\u003drelative)\n    if result \u003d\u003d 0:\n      LOG.debug(\u0027blockRebase started successfully\u0027,\n                      instance\u003dinstance)\n      while dev.wait_for_job(abort_on_error\u003dTrue):\n        LOG.debug(\u0027waiting for blockRebase job completion\u0027,\n                          instance\u003dinstance)\n        time.sleep(0.5)\n  else:\n    self._my_method_to_rebase_with_qemu(guest, rebase_base, active_disk_object)","commit_id":"3989b31a1e1a00c97dae4d36cdec8208f46e0c42"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c9b91a5419343ba18cdd5140c7535d12e1f0a3f6","unresolved":false,"context_lines":[{"line_number":1791,"context_line":"            msg \u003d _LE(\"Rebasing a %s network disk using qemu-img \""},{"line_number":1792,"context_line":"                      \"has not been fully tested\")"},{"line_number":1793,"context_line":"            LOG.error(msg, active_protocol)"},{"line_number":1794,"context_line":"            raise exception.NovaException(\"Something went wrong when deleting \""},{"line_number":1795,"context_line":"                                          \"a volume snapshot\")"},{"line_number":1796,"context_line":""},{"line_number":1797,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a4dcd0f_07c255e9","line":1794,"updated":"2015-08-12 09:16:10.000000000","message":"So this will be sent to cinder as a 500, right ?","commit_id":"503319a459918104d8da16bac98b9d9f0b905483"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"4ae74b133cbb625b8cf9d1bd8e6f4a6e748c965e","unresolved":false,"context_lines":[{"line_number":1791,"context_line":"            msg \u003d _LE(\"Rebasing a %s network disk using qemu-img \""},{"line_number":1792,"context_line":"                      \"has not been fully tested\")"},{"line_number":1793,"context_line":"            LOG.error(msg, active_protocol)"},{"line_number":1794,"context_line":"            raise exception.NovaException(\"Something went wrong when deleting \""},{"line_number":1795,"context_line":"                                          \"a volume snapshot\")"},{"line_number":1796,"context_line":""},{"line_number":1797,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a4dcd0f_fd09f695","line":1794,"in_reply_to":"1a4dcd0f_07c255e9","updated":"2015-08-12 10:01:09.000000000","message":"Not exactly (sorry I told you something wrong last time).\n\nRaising a NovaException here will make Nova updates Cinder snapshot status with the following curl command:\n\ncurl -g -i -X POST      http://192.168.200.107:8776/v2/$id/snapshots/$id/action -H   \"User-Agent: python-cinderclient\" -H \"Content-Type:   application/json\" -H \"Accept: application/json\" -H \"X-Auth-Token: {SHA1}$id\" -d \u0027{\"os-update_snapshot_status\":   {\"status\": \"error_deleting\", \"progress\": \"90%\"}}\n\n\nYou see that the status is set to \"error_deleting\".\n\n\nCinder just wait and poll the status of the snapshot (see https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/remotefs.py#L1367)","commit_id":"503319a459918104d8da16bac98b9d9f0b905483"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0777100ccdfcb1849a2fd343f8e3f9bfbf237069","unresolved":false,"context_lines":[{"line_number":1808,"context_line":"                              rebase_base, relative):"},{"line_number":1809,"context_line":"        \"\"\"Rebase a device tied to a guest using qemu-img."},{"line_number":1810,"context_line":""},{"line_number":1811,"context_line":"        :type guest: nova.virt.libvirt.guest.Guest"},{"line_number":1812,"context_line":"        :param device: the guest block device to rebase"},{"line_number":1813,"context_line":"        :type device: nova.virt.libvirt.guest.BlockDevice"},{"line_number":1814,"context_line":"        :param active_disk_object: the guest block device to rebase"}],"source_content_type":"text/x-python","patch_set":20,"id":"da20952f_79d52819","line":1811,"updated":"2015-09-10 21:34:41.000000000","message":"Did you forget :param guest: ?","commit_id":"6905df54942df29f67154ba1b461dcd5e0c68e58"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0777100ccdfcb1849a2fd343f8e3f9bfbf237069","unresolved":false,"context_lines":[{"line_number":1816,"context_line":"                                    LibvirtConfigGuestDisk"},{"line_number":1817,"context_line":"        :param rebase_base: the new parent in the backing chain"},{"line_number":1818,"context_line":"        :type rebase_base: string"},{"line_number":1819,"context_line":"        :param relative: keep backing chain referenced using relative names"},{"line_number":1820,"context_line":"        :type relative: boolean"},{"line_number":1821,"context_line":"        \"\"\""},{"line_number":1822,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"da20952f_19aaec85","line":1819,"updated":"2015-09-10 21:34:41.000000000","message":"This parameter is not used, so remove from the args list.","commit_id":"6905df54942df29f67154ba1b461dcd5e0c68e58"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0777100ccdfcb1849a2fd343f8e3f9bfbf237069","unresolved":false,"context_lines":[{"line_number":1820,"context_line":"        :type relative: boolean"},{"line_number":1821,"context_line":"        \"\"\""},{"line_number":1822,"context_line":""},{"line_number":1823,"context_line":"        LOG.debug(\u0027Guest is not running so doing a block rebase \u0027"},{"line_number":1824,"context_line":"                  \u0027using \"qemu-img rebase\"\u0027)"},{"line_number":1825,"context_line":""},{"line_number":1826,"context_line":"        # Unfortunately, not sure how well qemu-img handles network disks for"}],"source_content_type":"text/x-python","patch_set":20,"id":"da20952f_698d5530","line":1823,"updated":"2015-09-10 21:34:41.000000000","message":"We should log the instance / guest uuid in here for context, like using instance\u003dinstance kwarg, but maybe we could use instance_uuid\u003dguest.uuid?","commit_id":"6905df54942df29f67154ba1b461dcd5e0c68e58"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0777100ccdfcb1849a2fd343f8e3f9bfbf237069","unresolved":false,"context_lines":[{"line_number":1830,"context_line":"            msg \u003d _LE(\"Rebasing a %s network disk using qemu-img \""},{"line_number":1831,"context_line":"                      \"has not been fully tested\")"},{"line_number":1832,"context_line":"            LOG.error(msg, active_protocol)"},{"line_number":1833,"context_line":"            raise exception.NovaException(\"Something went wrong when deleting \""},{"line_number":1834,"context_line":"                                          \"a volume snapshot\")"},{"line_number":1835,"context_line":""},{"line_number":1836,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":20,"id":"da20952f_299afd62","line":1833,"updated":"2015-09-10 21:34:41.000000000","message":"This is really obscure - seems you know what went wrong right above this, just use that as your exception message.","commit_id":"6905df54942df29f67154ba1b461dcd5e0c68e58"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0777100ccdfcb1849a2fd343f8e3f9bfbf237069","unresolved":false,"context_lines":[{"line_number":1834,"context_line":"                                          \"a volume snapshot\")"},{"line_number":1835,"context_line":""},{"line_number":1836,"context_line":"        else:"},{"line_number":1837,"context_line":"            if not rebase_base:"},{"line_number":1838,"context_line":"                # If backing_file is specified as \"\" (the empty string), then"},{"line_number":1839,"context_line":"                # the image is rebased onto no backing file (i.e. it will exist"},{"line_number":1840,"context_line":"                # independently of any backing file)."}],"source_content_type":"text/x-python","patch_set":20,"id":"da20952f_29e8bdb2","line":1837,"updated":"2015-09-10 21:34:41.000000000","message":"Given how this is checked below, I\u0027d be explicit:\n\nif rebase_base is not None:","commit_id":"6905df54942df29f67154ba1b461dcd5e0c68e58"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0777100ccdfcb1849a2fd343f8e3f9bfbf237069","unresolved":false,"context_lines":[{"line_number":1842,"context_line":"                qemu_img_extra_arg \u003d []"},{"line_number":1843,"context_line":"            else:"},{"line_number":1844,"context_line":"                # If the rebased image is going to have a backing file then"},{"line_number":1845,"context_line":"                # explicitely set the backing file format to avoid any security"},{"line_number":1846,"context_line":"                # concerns related to file format auto detection."},{"line_number":1847,"context_line":"                backing_file \u003d rebase_base"},{"line_number":1848,"context_line":"                b_file_fmt \u003d libvirt_utils.get_disk_type(backing_file)"}],"source_content_type":"text/x-python","patch_set":20,"id":"da20952f_a9d4cde8","line":1845,"updated":"2015-09-10 21:34:41.000000000","message":"s/explicitely/explicitly/","commit_id":"6905df54942df29f67154ba1b461dcd5e0c68e58"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2f6819f9b1a9e9e6cc1a7e305d0d559f7228c143","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"        :type rebase_base: None or string"},{"line_number":1822,"context_line":"        \"\"\""},{"line_number":1823,"context_line":""},{"line_number":1824,"context_line":"        LOG.debug(\u0027Guest %s is not running so doing a block rebase \u0027"},{"line_number":1825,"context_line":"                  \u0027using \"qemu-img rebase\"\u0027, guest.uuid)"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":"        # Unfortunately, not sure how well qemu-img handles network disks for"}],"source_content_type":"text/x-python","patch_set":22,"id":"ba15a1d1_07b5ddec","line":1824,"updated":"2015-09-15 09:15:14.000000000","message":"I think Matt is talking about using special keyword:\n\n  LOG.debug(\u0027Guest is not running...\u0027, instance_uuid\u003dguest.uuid) - That say I believe using instance\u003dguest can work too.","commit_id":"4e640ca22ba199ff0aa5f61bd08ad5c7e8051290"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cf31e99cf35a392526496b2b2d4d0c7610bb82bb","unresolved":false,"context_lines":[{"line_number":1821,"context_line":"        :type rebase_base: None or string"},{"line_number":1822,"context_line":"        \"\"\""},{"line_number":1823,"context_line":""},{"line_number":1824,"context_line":"        LOG.debug(\u0027Guest is not running so doing a block rebase \u0027"},{"line_number":1825,"context_line":"                  \u0027using \"qemu-img rebase\"\u0027, instance_uuid\u003dguest.uuid)"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":"        # Unfortunately, not sure how well qemu-img handles network disks for"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba15a1d1_3673544f","line":1824,"updated":"2015-09-15 19:01:51.000000000","message":"Technically this method shouldn\u0027t care why it\u0027s being called, so I\u0027d actually have this logging call *before* calling this method, but that\u0027s a nit - just please address if you have to push another patch set.\n\nIf you move this up a level then you can just pass instance\u003dinstance rather than instance_uuid\u003dguest.uuid).","commit_id":"108f73a87a5fe59411cadc851d77cdcdb7fdcfe0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cf31e99cf35a392526496b2b2d4d0c7610bb82bb","unresolved":false,"context_lines":[{"line_number":1828,"context_line":"        # every protocol. So let\u0027s be safe."},{"line_number":1829,"context_line":"        active_protocol \u003d active_disk_object.source_protocol"},{"line_number":1830,"context_line":"        if active_protocol is not None:"},{"line_number":1831,"context_line":"            msg \u003d _(\"Something went wrong when deleting a volume snapshot :\""},{"line_number":1832,"context_line":"                    \"rebasing a %s network disk using qemu-img \""},{"line_number":1833,"context_line":"                    \"has not been fully tested\") % active_protocol"},{"line_number":1834,"context_line":"            LOG.error(msg)"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba15a1d1_969a6062","line":1831,"updated":"2015-09-15 19:01:51.000000000","message":"s/ :/: /","commit_id":"108f73a87a5fe59411cadc851d77cdcdb7fdcfe0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cf31e99cf35a392526496b2b2d4d0c7610bb82bb","unresolved":false,"context_lines":[{"line_number":1829,"context_line":"        active_protocol \u003d active_disk_object.source_protocol"},{"line_number":1830,"context_line":"        if active_protocol is not None:"},{"line_number":1831,"context_line":"            msg \u003d _(\"Something went wrong when deleting a volume snapshot :\""},{"line_number":1832,"context_line":"                    \"rebasing a %s network disk using qemu-img \""},{"line_number":1833,"context_line":"                    \"has not been fully tested\") % active_protocol"},{"line_number":1834,"context_line":"            LOG.error(msg)"},{"line_number":1835,"context_line":"            raise exception.NovaException(msg)"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba15a1d1_d67a18f1","line":1832,"updated":"2015-09-15 19:01:51.000000000","message":"nit: using kwargs for the message rather than %s would probably be more helpful for translators, i.e. %(protocol)s vs %s, or re-wording as \"rebasing a network disk with protocol %s using qemu-image...\"","commit_id":"108f73a87a5fe59411cadc851d77cdcdb7fdcfe0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cf31e99cf35a392526496b2b2d4d0c7610bb82bb","unresolved":false,"context_lines":[{"line_number":1834,"context_line":"            LOG.error(msg)"},{"line_number":1835,"context_line":"            raise exception.NovaException(msg)"},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"        else:"},{"line_number":1838,"context_line":"            if rebase_base is None:"},{"line_number":1839,"context_line":"                # If backing_file is specified as \"\" (the empty string), then"},{"line_number":1840,"context_line":"                # the image is rebased onto no backing file (i.e. it will exist"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba15a1d1_d64bf885","line":1837,"updated":"2015-09-15 19:01:51.000000000","message":"You can drop this else here since the conditional block above raises an Exception, then you can shift everything in this else block left a tab.","commit_id":"108f73a87a5fe59411cadc851d77cdcdb7fdcfe0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d02a66b63baaeea96d5b2a9cff989ca1fe0e5dfd","unresolved":false,"context_lines":[{"line_number":1834,"context_line":"            LOG.error(msg)"},{"line_number":1835,"context_line":"            raise exception.NovaException(msg)"},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"        else:"},{"line_number":1838,"context_line":"            if rebase_base is None:"},{"line_number":1839,"context_line":"                # If backing_file is specified as \"\" (the empty string), then"},{"line_number":1840,"context_line":"                # the image is rebased onto no backing file (i.e. it will exist"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba15a1d1_e27b65bc","line":1837,"in_reply_to":"ba15a1d1_06aa9f70","updated":"2015-09-18 01:33:43.000000000","message":"No, no elif, no else.  The if block raises an exception so there is no point in else/elif after it.","commit_id":"108f73a87a5fe59411cadc851d77cdcdb7fdcfe0"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"080ae58bcbd54fd166dfa2147471e68f665b8c30","unresolved":false,"context_lines":[{"line_number":1834,"context_line":"            LOG.error(msg)"},{"line_number":1835,"context_line":"            raise exception.NovaException(msg)"},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"        else:"},{"line_number":1838,"context_line":"            if rebase_base is None:"},{"line_number":1839,"context_line":"                # If backing_file is specified as \"\" (the empty string), then"},{"line_number":1840,"context_line":"                # the image is rebased onto no backing file (i.e. it will exist"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba15a1d1_06aa9f70","line":1837,"in_reply_to":"ba15a1d1_d64bf885","updated":"2015-09-15 20:16:18.000000000","message":"And use elif instead :)","commit_id":"108f73a87a5fe59411cadc851d77cdcdb7fdcfe0"}]}