)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":29122,"name":"Raghavendra Tilay","email":"raghavendra-uddhav.tilay@hpe.com","username":"raghavendrat"},"change_message_id":"b83b7663fec8232edb59a2f23b8b6e7ece1f746e","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Sofia Enriquez \u003clsofia.enriquez@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-09-27 12:00:06 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[rbd] Fix creating encrypted volume from unencrypted image"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When creating an encrypted volume from an encrypted image"},{"line_number":10,"context_line":"the volume gets truncated and it\u0027s inaccessible after the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bdde9a0c_1c742583","line":7,"range":{"start_line":7,"start_character":36,"end_line":7,"end_character":58},"updated":"2021-09-27 12:52:01.000000000","message":"Should this be \"from an encrypted image\" like next line ?\nOR\nthe next line be updated to \"from unencrypted image\" ?","commit_id":"7065340e260f375c28872963e478059f3902e158"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"57a73c512bf0565b2cefea6a1b0782a9342df25a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sofia Enriquez \u003clsofia.enriquez@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-09-27 12:00:06 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[rbd] Fix creating encrypted volume from unencrypted image"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When creating an encrypted volume from an encrypted image"},{"line_number":10,"context_line":"the volume gets truncated and it\u0027s inaccessible after the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"c1a08b24_84f86e7c","line":7,"range":{"start_line":7,"start_character":36,"end_line":7,"end_character":58},"in_reply_to":"bdde9a0c_1c742583","updated":"2021-10-14 13:01:35.000000000","message":"You\u0027re right. Updated","commit_id":"7065340e260f375c28872963e478059f3902e158"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ea0411af0689bc25ea67b6a943c6d32488124029","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[rbd] Fix creating encrypted volume from unencrypted image"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When creating an encrypted volume from an encrypted image"},{"line_number":10,"context_line":"the volume gets truncated and it\u0027s inaccessible after the"},{"line_number":11,"context_line":"operation. This happens if the amount of data in the"},{"line_number":12,"context_line":"source image at the time the volume was created is very"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"54a2c14e_c79024ea","line":9,"range":{"start_line":9,"start_character":42,"end_line":9,"end_character":51},"updated":"2021-09-28 04:40:55.000000000","message":"unencrypted\n\nAlso, so that you don\u0027t freak people out, you might want to add \"Under some circumstances\" at the beginning of the sentence.","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"57a73c512bf0565b2cefea6a1b0782a9342df25a","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[rbd] Fix creating encrypted volume from unencrypted image"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When creating an encrypted volume from an encrypted image"},{"line_number":10,"context_line":"the volume gets truncated and it\u0027s inaccessible after the"},{"line_number":11,"context_line":"operation. This happens if the amount of data in the"},{"line_number":12,"context_line":"source image at the time the volume was created is very"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"a1608517_1b7a9f4e","line":9,"range":{"start_line":9,"start_character":42,"end_line":9,"end_character":51},"in_reply_to":"54a2c14e_c79024ea","updated":"2021-10-14 13:01:35.000000000","message":"Ack","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"16b87c35e76696e013bff915b67c36d808592329","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"916ad497_3dee5f45","updated":"2021-11-15 18:30:13.000000000","message":"recheck tempest-integrated-storage","commit_id":"0d71689a46e4971f17936de05d144054f2bb3345"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"ab4b50659b4c6365f646ad3db02a8855406a3ffe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"4fe68473_b72cd541","updated":"2021-11-15 18:30:41.000000000","message":"recheck tempest-ipv6-only","commit_id":"0d71689a46e4971f17936de05d144054f2bb3345"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"612830fc2c664b7cbd303b2c5cc345bff5e2e5f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"78db2139_a89639b0","updated":"2021-11-23 20:02:21.000000000","message":"recheck","commit_id":"de81d48db0fae2b5d81acf54c43d34a9f7b6a307"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"701ced5b1019b0d709730a618566e03a5153165e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b113ff09_eb808c03","updated":"2021-12-15 17:52:56.000000000","message":"recheck","commit_id":"de81d48db0fae2b5d81acf54c43d34a9f7b6a307"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"f4e80668df30bb5bbe8c15142e575648d5b1b9dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"9ea22877_ea10476a","updated":"2021-12-16 14:41:48.000000000","message":"recheck openstack-tox-py36 ","commit_id":"de81d48db0fae2b5d81acf54c43d34a9f7b6a307"}],"cinder/tests/unit/volume/drivers/test_rbd.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ea0411af0689bc25ea67b6a943c6d32488124029","unresolved":true,"context_lines":[{"line_number":1660,"context_line":""},{"line_number":1661,"context_line":"    @common_mocks"},{"line_number":1662,"context_line":"    def test_round_up_to_nearest_gb(self):"},{"line_number":1663,"context_line":"        image_size_bytes \u003d 1075810304"},{"line_number":1664,"context_line":"        size_gb \u003d self.driver._round_up_to_nearest_gb(image_size_bytes)"},{"line_number":1665,"context_line":"        self.assertEqual(size_gb, 2147483648)"},{"line_number":1666,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bc31df8e_96c8eeb3","line":1663,"range":{"start_line":1663,"start_character":27,"end_line":1663,"end_character":37},"updated":"2021-09-28 04:40:55.000000000","message":"I think this would be more clear if you expressed this as:\n\n  1 * units.Gi + 2068480","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"00ea5782d93f60c7d30ed7107e588ac43bd10a81","unresolved":false,"context_lines":[{"line_number":1660,"context_line":""},{"line_number":1661,"context_line":"    @common_mocks"},{"line_number":1662,"context_line":"    def test_round_up_to_nearest_gb(self):"},{"line_number":1663,"context_line":"        image_size_bytes \u003d 1075810304"},{"line_number":1664,"context_line":"        size_gb \u003d self.driver._round_up_to_nearest_gb(image_size_bytes)"},{"line_number":1665,"context_line":"        self.assertEqual(size_gb, 2147483648)"},{"line_number":1666,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"b2a9b42c_da5a1cf5","line":1663,"range":{"start_line":1663,"start_character":27,"end_line":1663,"end_character":37},"in_reply_to":"bc31df8e_96c8eeb3","updated":"2022-10-14 02:17:12.000000000","message":"Done","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ea0411af0689bc25ea67b6a943c6d32488124029","unresolved":true,"context_lines":[{"line_number":1662,"context_line":"    def test_round_up_to_nearest_gb(self):"},{"line_number":1663,"context_line":"        image_size_bytes \u003d 1075810304"},{"line_number":1664,"context_line":"        size_gb \u003d self.driver._round_up_to_nearest_gb(image_size_bytes)"},{"line_number":1665,"context_line":"        self.assertEqual(size_gb, 2147483648)"},{"line_number":1666,"context_line":""},{"line_number":1667,"context_line":"    @common_mocks"},{"line_number":1668,"context_line":"    def test_copy_image_no_volume_tmp(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"25332f01_05bce500","line":1665,"range":{"start_line":1665,"start_character":34,"end_line":1665,"end_character":44},"updated":"2021-09-28 04:40:55.000000000","message":"this would be more clear as:\n\n  2 * units.Gi","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"00ea5782d93f60c7d30ed7107e588ac43bd10a81","unresolved":false,"context_lines":[{"line_number":1662,"context_line":"    def test_round_up_to_nearest_gb(self):"},{"line_number":1663,"context_line":"        image_size_bytes \u003d 1075810304"},{"line_number":1664,"context_line":"        size_gb \u003d self.driver._round_up_to_nearest_gb(image_size_bytes)"},{"line_number":1665,"context_line":"        self.assertEqual(size_gb, 2147483648)"},{"line_number":1666,"context_line":""},{"line_number":1667,"context_line":"    @common_mocks"},{"line_number":1668,"context_line":"    def test_copy_image_no_volume_tmp(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"1e34b1d5_1c5ffc5c","line":1665,"range":{"start_line":1665,"start_character":34,"end_line":1665,"end_character":44},"in_reply_to":"25332f01_05bce500","updated":"2022-10-14 02:17:12.000000000","message":"Done","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ea0411af0689bc25ea67b6a943c6d32488124029","unresolved":true,"context_lines":[{"line_number":1663,"context_line":"        image_size_bytes \u003d 1075810304"},{"line_number":1664,"context_line":"        size_gb \u003d self.driver._round_up_to_nearest_gb(image_size_bytes)"},{"line_number":1665,"context_line":"        self.assertEqual(size_gb, 2147483648)"},{"line_number":1666,"context_line":""},{"line_number":1667,"context_line":"    @common_mocks"},{"line_number":1668,"context_line":"    def test_copy_image_no_volume_tmp(self):"},{"line_number":1669,"context_line":"        self.cfg.image_conversion_dir \u003d None"}],"source_content_type":"text/x-python","patch_set":4,"id":"d84b6cf7_996c1418","line":1666,"updated":"2021-09-28 04:40:55.000000000","message":"I think it would also be good to check _round_up_to_nearest_gb(1 * units.Gi) and make sure it gives you 1 * units.Gi back","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"00ea5782d93f60c7d30ed7107e588ac43bd10a81","unresolved":false,"context_lines":[{"line_number":1663,"context_line":"        image_size_bytes \u003d 1075810304"},{"line_number":1664,"context_line":"        size_gb \u003d self.driver._round_up_to_nearest_gb(image_size_bytes)"},{"line_number":1665,"context_line":"        self.assertEqual(size_gb, 2147483648)"},{"line_number":1666,"context_line":""},{"line_number":1667,"context_line":"    @common_mocks"},{"line_number":1668,"context_line":"    def test_copy_image_no_volume_tmp(self):"},{"line_number":1669,"context_line":"        self.cfg.image_conversion_dir \u003d None"}],"source_content_type":"text/x-python","patch_set":4,"id":"a8edc114_f4cace67","line":1666,"in_reply_to":"d84b6cf7_996c1418","updated":"2022-10-14 02:17:12.000000000","message":"Done","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"d03a27aac897d8879fd0ef7d80e0001efe78325a","unresolved":true,"context_lines":[{"line_number":1658,"context_line":"                                self.assertEqual(1,"},{"line_number":1659,"context_line":"                                                 mock_temp_delete.call_count)"},{"line_number":1660,"context_line":""},{"line_number":1661,"context_line":"    @common_mocks"},{"line_number":1662,"context_line":"    def test_copy_image_no_volume_tmp(self):"},{"line_number":1663,"context_line":"        self.cfg.image_conversion_dir \u003d None"},{"line_number":1664,"context_line":"        self._copy_image()"}],"source_content_type":"text/x-python","patch_set":7,"id":"ee4552e7_9fbb5e5a","line":1661,"updated":"2021-11-17 18:19:43.000000000","message":"There are only two lines that differ between these two functions: test_copy_with[out]_resize() and img_size \u003d int(X * units.Gi)\n\nI\u0027m not great at unit tests, but I bet we could use ddt or some similar to flag the different and reduce this to a single function that tests both since the functions are otherwise identical.\n\nOr maybe this is preferred as it preserves the test name and makes failures easier/quicker to parse?","commit_id":"0d71689a46e4971f17936de05d144054f2bb3345"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"fb9648c642feb2024fa001852f6b1ba2b129d61e","unresolved":false,"context_lines":[{"line_number":1658,"context_line":"                                self.assertEqual(1,"},{"line_number":1659,"context_line":"                                                 mock_temp_delete.call_count)"},{"line_number":1660,"context_line":""},{"line_number":1661,"context_line":"    @common_mocks"},{"line_number":1662,"context_line":"    def test_copy_image_no_volume_tmp(self):"},{"line_number":1663,"context_line":"        self.cfg.image_conversion_dir \u003d None"},{"line_number":1664,"context_line":"        self._copy_image()"}],"source_content_type":"text/x-python","patch_set":7,"id":"0ad03110_fdda39c3","line":1661,"in_reply_to":"ee4552e7_9fbb5e5a","updated":"2021-11-19 21:15:40.000000000","message":"You\u0027re right, I can join both","commit_id":"0d71689a46e4971f17936de05d144054f2bb3345"},{"author":{"_account_id":13671,"name":"Vladislav Belogrudov","email":"v.belogrudov@yadro.com","username":"vb"},"change_message_id":"2d88fddf78bb65a3e59d5b6325eaac4adf0c0460","unresolved":true,"context_lines":[{"line_number":1748,"context_line":""},{"line_number":1749,"context_line":"        mock_resize \u003d self.mock_object(self.driver, \u0027_resize\u0027)"},{"line_number":1750,"context_line":"        new_size \u003d 10 * units.Gi"},{"line_number":1751,"context_line":"        mock_os_stat.return_value.st_size \u003d new_size"},{"line_number":1752,"context_line":""},{"line_number":1753,"context_line":"        enc_info \u003d {\u0027encryption_key_id\u0027: key_id,"},{"line_number":1754,"context_line":"                    \u0027cipher\u0027: \u0027aes-xts-essiv\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9aafdf2f_971a4c7d","line":1751,"updated":"2022-10-20 16:00:17.000000000","message":"the test does not show how the resize logic works, e.g. whether something has been changed or not. It would be great to have mock_os_stat return two different values (via side_effect) and see that volume_a is resized to more than 10 Gi in mock_resize.assert_called_with(), i.e. 10G + mock_os_stat difference.","commit_id":"d3cf50a3a26651be03d6516096af22154907d973"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ea0411af0689bc25ea67b6a943c6d32488124029","unresolved":true,"context_lines":[{"line_number":1688,"context_line":"                    \u0027--new-format\u0027]"},{"line_number":1689,"context_line":"            args.extend(self._ceph_args())"},{"line_number":1690,"context_line":"            self._try_execute(*args)"},{"line_number":1691,"context_line":"        self._resize(volume)"},{"line_number":1692,"context_line":"        # We may need to re-enable replication because we have deleted the"},{"line_number":1693,"context_line":"        # original image and created a new one using the command line import."},{"line_number":1694,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7da5ebb0_4626e26b","side":"PARENT","line":1691,"range":{"start_line":1691,"start_character":8,"end_line":1691,"end_character":28},"updated":"2021-09-28 04:40:55.000000000","message":"my understanding of what this did was it resized the object in the rbd backend to be the size of the requested cinder volume -- I think we still need to do this?","commit_id":"51fd9aeed4d35c480edb301dcd378930cea31ae4"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"8e2d7165b8dc384a0a74c0fa3664e3fc9c22a844","unresolved":true,"context_lines":[{"line_number":1688,"context_line":"                    \u0027--new-format\u0027]"},{"line_number":1689,"context_line":"            args.extend(self._ceph_args())"},{"line_number":1690,"context_line":"            self._try_execute(*args)"},{"line_number":1691,"context_line":"        self._resize(volume)"},{"line_number":1692,"context_line":"        # We may need to re-enable replication because we have deleted the"},{"line_number":1693,"context_line":"        # original image and created a new one using the command line import."},{"line_number":1694,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"e1e5f7b0_9e3dff95","side":"PARENT","line":1691,"range":{"start_line":1691,"start_character":8,"end_line":1691,"end_character":28},"in_reply_to":"7da5ebb0_4626e26b","updated":"2021-09-29 03:37:41.000000000","message":"That\u0027s a good question because that resize call it\u0027s the source of all this bug. The source of all this trouble is RBD doesn\u0027t share the real size of their volumes to cinder when using encryption, and we have our reasons I guess. The main example is: An encrypted RBD volume in the backend is 1026M but Cinder still shows 1024M or 1G.\n\nBefore calling _copy_image_to_volume cinder creates the encrypted volume A. After that in L1670 _encrypt_image converts the source glance image to an encrypted random file in a tmp dir. Then the driver imports that data into the the volume A in L1687. Finally volume.size shows 1G but actually it takes up more space in the backend. Then resize() truncates data but doesn\u0027t fail when doing so. After that we try to create an instance from that volume but boot fails and Cinder doesn\u0027t show errors or warnings.\n\nFinally, one important thing to know is that the API checks that the image size is smaller than the requested volume size.","commit_id":"51fd9aeed4d35c480edb301dcd378930cea31ae4"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"00ea5782d93f60c7d30ed7107e588ac43bd10a81","unresolved":false,"context_lines":[{"line_number":1688,"context_line":"                    \u0027--new-format\u0027]"},{"line_number":1689,"context_line":"            args.extend(self._ceph_args())"},{"line_number":1690,"context_line":"            self._try_execute(*args)"},{"line_number":1691,"context_line":"        self._resize(volume)"},{"line_number":1692,"context_line":"        # We may need to re-enable replication because we have deleted the"},{"line_number":1693,"context_line":"        # original image and created a new one using the command line import."},{"line_number":1694,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"4cbb9743_99e34b88","side":"PARENT","line":1691,"range":{"start_line":1691,"start_character":8,"end_line":1691,"end_character":28},"in_reply_to":"e1e5f7b0_9e3dff95","updated":"2022-10-14 02:17:12.000000000","message":"Done","commit_id":"51fd9aeed4d35c480edb301dcd378930cea31ae4"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ea0411af0689bc25ea67b6a943c6d32488124029","unresolved":true,"context_lines":[{"line_number":1694,"context_line":"                    \u0027--new-format\u0027]"},{"line_number":1695,"context_line":"            args.extend(self._ceph_args())"},{"line_number":1696,"context_line":"            self._try_execute(*args)"},{"line_number":1697,"context_line":"        with RBDVolumeProxy(self, volume.name) as vol:"},{"line_number":1698,"context_line":"            image_size \u003d vol.size()"},{"line_number":1699,"context_line":"            if volume.size * units.Gi - image_size \u003e 1 * units.Gi:"},{"line_number":1700,"context_line":"                diff_size \u003d (self._round_up_to_nearest_gb(image_size) -"},{"line_number":1701,"context_line":"                             tmp_file_size)"},{"line_number":1702,"context_line":"                new_size \u003d volume.size + diff_size"},{"line_number":1703,"context_line":"                self._resize(volume, size\u003dnew_size)"},{"line_number":1704,"context_line":""},{"line_number":1705,"context_line":"        # We may need to re-enable replication because we have deleted the"},{"line_number":1706,"context_line":"        # original image and created a new one using the command line import."},{"line_number":1707,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"94b224cc_c9bda149","line":1704,"range":{"start_line":1697,"start_character":0,"end_line":1704,"end_character":0},"updated":"2021-09-28 04:40:55.000000000","message":"I\u0027m having trouble understanding this section.  My understanding of the bug is:\n- if the glance image is \u003c n GiB, a user can request that it be created as a volume of size n\n- but the image \"grows\" when it\u0027s encrypted as a luks container\n- so if the image is 100b less than n GiB, the user can ask for a size n volume ... but if the luks header is 1000b, then the resulting rbd object is n GiB + 900b which cannot fit in an n GiB volume\n\nLooking at the code:\n\n        with RBDVolumeProxy(self, volume.name) as vol:\n            # vol is the thing in the backend that we just rbd imported at\n            # lines 1690-1696\n\n            image_size \u003d vol.size()\n            # shouldn\u0027t this be the current size in bytes of the thing in\n            # the backend, that is, raw image + luks header?  So won\u0027t it\n            # equal tmp_file_size, which is the size in the filesystem of\n            # the thing we rbd imported?  (My question is why are we also\n            # keeping track of tmp_file_size, since we have vol.size().\n\n            if volume.size * units.Gi - image_size \u003e 1 * units.Gi:\n            # under the conditions of the bug, won\u0027t the expression on the\n            # left of the \u0027\u003e\u0027 be negative?  For example, volume 2 GiB, but\n            # image_size is 2 GiB + 900b?\n\n            # I think what you want to check instead is something like:\n            minimum_bytes \u003d _round_up_to_nearest_gb(image_size)\n            requested_bytes \u003d volume.size * units.Gi:\n            if  minimum_bytes \u003e requested_bytes:\n                new_size \u003d minimum_bytes\n                # ok, so now the user is getting a bigger volume than they\n                # requested.  Do we need to record this in the cinder volume?\n                # for example,\n                volume.size \u003d volume.size + 1\n            else:\n                new_size \u003d requested_bytes\n                # note that if we increment volume.size in the \u0027if\u0027 block,\n                # we don\u0027t need to keep track of new_size, we can just let\n                # _resize(volume) get the size out of the volume\n\n            # I think this needs to be outside the \u0027if\u0027 because we still need\n            # to do it for the normal case?\n            self._resize(volume, size\u003dnew_size)","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"00ea5782d93f60c7d30ed7107e588ac43bd10a81","unresolved":false,"context_lines":[{"line_number":1694,"context_line":"                    \u0027--new-format\u0027]"},{"line_number":1695,"context_line":"            args.extend(self._ceph_args())"},{"line_number":1696,"context_line":"            self._try_execute(*args)"},{"line_number":1697,"context_line":"        with RBDVolumeProxy(self, volume.name) as vol:"},{"line_number":1698,"context_line":"            image_size \u003d vol.size()"},{"line_number":1699,"context_line":"            if volume.size * units.Gi - image_size \u003e 1 * units.Gi:"},{"line_number":1700,"context_line":"                diff_size \u003d (self._round_up_to_nearest_gb(image_size) -"},{"line_number":1701,"context_line":"                             tmp_file_size)"},{"line_number":1702,"context_line":"                new_size \u003d volume.size + diff_size"},{"line_number":1703,"context_line":"                self._resize(volume, size\u003dnew_size)"},{"line_number":1704,"context_line":""},{"line_number":1705,"context_line":"        # We may need to re-enable replication because we have deleted the"},{"line_number":1706,"context_line":"        # original image and created a new one using the command line import."},{"line_number":1707,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"956dbef8_20b35364","line":1704,"range":{"start_line":1697,"start_character":0,"end_line":1704,"end_character":0},"in_reply_to":"389a4e5a_31d701f5","updated":"2022-10-14 02:17:12.000000000","message":"Done","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"8e2d7165b8dc384a0a74c0fa3664e3fc9c22a844","unresolved":true,"context_lines":[{"line_number":1694,"context_line":"                    \u0027--new-format\u0027]"},{"line_number":1695,"context_line":"            args.extend(self._ceph_args())"},{"line_number":1696,"context_line":"            self._try_execute(*args)"},{"line_number":1697,"context_line":"        with RBDVolumeProxy(self, volume.name) as vol:"},{"line_number":1698,"context_line":"            image_size \u003d vol.size()"},{"line_number":1699,"context_line":"            if volume.size * units.Gi - image_size \u003e 1 * units.Gi:"},{"line_number":1700,"context_line":"                diff_size \u003d (self._round_up_to_nearest_gb(image_size) -"},{"line_number":1701,"context_line":"                             tmp_file_size)"},{"line_number":1702,"context_line":"                new_size \u003d volume.size + diff_size"},{"line_number":1703,"context_line":"                self._resize(volume, size\u003dnew_size)"},{"line_number":1704,"context_line":""},{"line_number":1705,"context_line":"        # We may need to re-enable replication because we have deleted the"},{"line_number":1706,"context_line":"        # original image and created a new one using the command line import."},{"line_number":1707,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"389a4e5a_31d701f5","line":1704,"range":{"start_line":1697,"start_character":0,"end_line":1704,"end_character":0},"in_reply_to":"94b224cc_c9bda149","updated":"2021-09-29 03:37:41.000000000","message":"Thank you so much for your detail review.\n\n- tmp_file_size is the size of the data inside the glance image. As far as I understand it\u0027s always smaller than the volume.size. I can\u0027t remember why I need it. Probably can go away.\n\n- OK, please read the other comment before this one.\n\n- You\u0027re right, I think I haven\u0027t thought in the case of the destination volume is much bigger than the image and not only a couple of MB. This logic isn\u0027t the right way to go. \n\n\n\u003e I\u0027m having trouble understanding this section.  My understanding of the bug is:\n\u003e - if the glance image is \u003c n GiB, a user can request that it be created as a volume of size n\n\u003e - but the image \"grows\" when it\u0027s encrypted as a luks container\n\u003e - so if the image is 100b less than n GiB, the user can ask for a size n volume ... but if the luks header is 1000b, then the resulting rbd object is n GiB + 900b which cannot fit in an n GiB volume\n\u003e \n\u003e Looking at the code:\n\u003e \n\u003e         with RBDVolumeProxy(self, volume.name) as vol:\n\u003e             # vol is the thing in the backend that we just rbd imported at\n\u003e             # lines 1690-1696\n\u003e \n\u003e             image_size \u003d vol.size()\n\u003e             # shouldn\u0027t this be the current size in bytes of the thing in\n\u003e             # the backend, that is, raw image + luks header?  So won\u0027t it\n\u003e             # equal tmp_file_size, which is the size in the filesystem of\n\u003e             # the thing we rbd imported?  (My question is why are we also\n\u003e             # keeping track of tmp_file_size, since we have vol.size().\n\u003e \n\u003e             if volume.size * units.Gi - image_size \u003e 1 * units.Gi:\n\u003e             # under the conditions of the bug, won\u0027t the expression on the\n\u003e             # left of the \u0027\u003e\u0027 be negative?  For example, volume 2 GiB, but\n\u003e             # image_size is 2 GiB + 900b?\n\u003e \n\u003e             # I think what you want to check instead is something like:\n\u003e             minimum_bytes \u003d _round_up_to_nearest_gb(image_size)\n\u003e             requested_bytes \u003d volume.size * units.Gi:\n\u003e             if  minimum_bytes \u003e requested_bytes:\n\u003e                 new_size \u003d minimum_bytes\n\u003e                 # ok, so now the user is getting a bigger volume than they\n\u003e                 # requested.  Do we need to record this in the cinder volume?\n\u003e                 # for example,\n\u003e                 volume.size \u003d volume.size + 1\n\nI\u0027m not completely sure about this, what about this scenario:\nimage_size (is encypted) \u003d 1026M\nminimum_bytes \u003d _round_up_to_nearest_gb(image_size) will be 2048M\nrequested_bytes \u003d 1024M\nBut I think we don\u0027t want to update the volume.size in this case. \n\n\u003e             else:\n\u003e                 new_size \u003d requested_bytes\n\u003e                 # note that if we increment volume.size in the \u0027if\u0027 block,\n\u003e                 # we don\u0027t need to keep track of new_size, we can just let\n\u003e                 # _resize(volume) get the size out of the volume\n\u003e \n\u003e             # I think this needs to be outside the \u0027if\u0027 because we still need\n\u003e             # to do it for the normal case?\n\u003e             self._resize(volume, size\u003dnew_size)","commit_id":"4a7206907a473a9d2654b3e064bc8f7d0ac8c3ad"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"0b70147d82c15afcca417da9d9464e37c0650e49","unresolved":true,"context_lines":[{"line_number":1657,"context_line":"            finally:"},{"line_number":1658,"context_line":"                fileutils.delete_if_exists(dest_image_path)"},{"line_number":1659,"context_line":""},{"line_number":1660,"context_line":"    def _round_up_to_nearest_gb(self, size_in):"},{"line_number":1661,"context_line":"        nearest_gb \u003d math.ceil(size_in / units.Gi) * units.Gi"},{"line_number":1662,"context_line":"        return nearest_gb"},{"line_number":1663,"context_line":""},{"line_number":1664,"context_line":"    def _copy_image_to_volume(self, context, volume, image_service, image_id,"},{"line_number":1665,"context_line":"                              encrypted\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":6,"id":"21906946_1d832efd","line":1662,"range":{"start_line":1660,"start_character":1,"end_line":1662,"end_character":25},"updated":"2021-11-12 20:28:48.000000000","message":"To keep the consistency with the snapshot fix, we shouldn\u0027t round up the image but keep it the same size https://review.opendev.org/c/openstack/cinder/+/784623/16/cinder/volume/drivers/rbd.py . Sadly we can\u0027t use _calculate_new_size() because glance images don\u0027t respect the gb boundaries like snapshots.","commit_id":"58801bda0e49c335b315ee194b49dcfe72ea5f12"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"00ea5782d93f60c7d30ed7107e588ac43bd10a81","unresolved":false,"context_lines":[{"line_number":1657,"context_line":"            finally:"},{"line_number":1658,"context_line":"                fileutils.delete_if_exists(dest_image_path)"},{"line_number":1659,"context_line":""},{"line_number":1660,"context_line":"    def _round_up_to_nearest_gb(self, size_in):"},{"line_number":1661,"context_line":"        nearest_gb \u003d math.ceil(size_in / units.Gi) * units.Gi"},{"line_number":1662,"context_line":"        return nearest_gb"},{"line_number":1663,"context_line":""},{"line_number":1664,"context_line":"    def _copy_image_to_volume(self, context, volume, image_service, image_id,"},{"line_number":1665,"context_line":"                              encrypted\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":6,"id":"322b23bc_a6328b5f","line":1662,"range":{"start_line":1660,"start_character":1,"end_line":1662,"end_character":25},"in_reply_to":"21906946_1d832efd","updated":"2022-10-14 02:17:12.000000000","message":"Done","commit_id":"58801bda0e49c335b315ee194b49dcfe72ea5f12"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"d03a27aac897d8879fd0ef7d80e0001efe78325a","unresolved":true,"context_lines":[{"line_number":1696,"context_line":"            minimum_bytes \u003d image_size"},{"line_number":1697,"context_line":""},{"line_number":1698,"context_line":"            # The display size, should respect a gb boundary."},{"line_number":1699,"context_line":"            # In general can be the same size of bigger than the"},{"line_number":1700,"context_line":"            # backend_volume"},{"line_number":1701,"context_line":"            requested_bytes \u003d volume.size * units.Gi"},{"line_number":1702,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"18d0c177_af12bbc9","line":1699,"updated":"2021-11-17 18:19:43.000000000","message":"nit: size /or/ bigger\n\ncertainly not worth changing alone, IMO","commit_id":"0d71689a46e4971f17936de05d144054f2bb3345"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"fb9648c642feb2024fa001852f6b1ba2b129d61e","unresolved":false,"context_lines":[{"line_number":1696,"context_line":"            minimum_bytes \u003d image_size"},{"line_number":1697,"context_line":""},{"line_number":1698,"context_line":"            # The display size, should respect a gb boundary."},{"line_number":1699,"context_line":"            # In general can be the same size of bigger than the"},{"line_number":1700,"context_line":"            # backend_volume"},{"line_number":1701,"context_line":"            requested_bytes \u003d volume.size * units.Gi"},{"line_number":1702,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"6f70130c_76820dcc","line":1699,"in_reply_to":"18d0c177_af12bbc9","updated":"2021-11-19 21:15:40.000000000","message":"Ack","commit_id":"0d71689a46e4971f17936de05d144054f2bb3345"}]}
