From 62044431bd55e3dbbb6c97b0ec0bf591b3c9ef6f Mon Sep 17 00:00:00 2001 From: Andrew Bogott Date: Thu, 8 Jun 2023 07:54:16 -0500 Subject: [PATCH] rbd: compute appropriate resize amount before resizing image Resolves a bug introduced in https://opendev.org/openstack/glance_store/commit/c43f19e8456b9e20f03709773fb2ffdb94807a0a This issue is only in evidence when glance is behind a proxy where the client buffer size can be lower (for haproxy: bufsize = 16384) which can cause unaligned reads (https://github.com/openstack/glance/blob/master/glance/common/wsgi.py#L1028). The response length can be bigger than the store_chunk_size for the first time, so at the end the RBD write will fail because it wants to write more data than the actual RBD image size after the first resize. Thanks to Robert Varjasi for investigating this issue! Fixes-Bug: 1916482 Change-Id: Ie03693c2cb8b096978fb156231c3b1cab695470f --- glance_store/_drivers/rbd.py | 6 ++-- glance_store/tests/unit/test_rbd_store.py | 38 ++++++++++++++--------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/glance_store/_drivers/rbd.py b/glance_store/_drivers/rbd.py index ba2defa3..e53baefb 100644 --- a/glance_store/_drivers/rbd.py +++ b/glance_store/_drivers/rbd.py @@ -535,12 +535,12 @@ class Store(driver.Store): """Handle the rbd resize when needed.""" if image_size != 0 or self.size >= bytes_written + chunk_length: return self.size - new_size = self.size + self.resize_amount - LOG.debug("resizing image to %s KiB" % (new_size / units.Ki)) - image.resize(new_size) # Note(jokke): We double how much we grow the image each time # up to 8gigs to avoid resizing for each write on bigger images self.resize_amount = min(self.resize_amount * 2, 8 * units.Gi) + new_size = self.size + self.resize_amount + LOG.debug("resizing image to %s KiB" % (new_size / units.Ki)) + image.resize(new_size) return new_size @driver.back_compat_add diff --git a/glance_store/tests/unit/test_rbd_store.py b/glance_store/tests/unit/test_rbd_store.py index 4f24c266..fb6522a8 100644 --- a/glance_store/tests/unit/test_rbd_store.py +++ b/glance_store/tests/unit/test_rbd_store.py @@ -213,10 +213,10 @@ class TestReSize(base.StoreBaseTest, data_len_temp = data_len resize_amount = self.store.WRITE_CHUNKSIZE while data_len_temp > 0: + resize_amount *= 2 expected_calls.append(resize_amount + (data_len - data_len_temp)) data_len_temp -= resize_amount - resize_amount *= 2 expected += 1 self.assertEqual(expected, resize.call_count) resize.assert_has_calls([mock.call(call) for call in @@ -244,7 +244,7 @@ class TestReSize(base.StoreBaseTest, # Current size is smaller than we need self.store.size = 8 ret = self.store._resize_on_write(image, 0, 16, 16) - self.assertEqual(8 + self.store.WRITE_CHUNKSIZE, ret) + self.assertEqual(8 + self.store.WRITE_CHUNKSIZE * 2, ret) self.assertEqual(self.store.WRITE_CHUNKSIZE * 2, self.store.resize_amount) image.resize.assert_called_once_with(ret) @@ -253,47 +253,55 @@ class TestReSize(base.StoreBaseTest, image.resize.reset_mock() self.store.size = ret ret = self.store._resize_on_write(image, 0, 64, 16) - self.assertEqual(8 + self.store.WRITE_CHUNKSIZE, ret) + self.assertEqual(8 + self.store.WRITE_CHUNKSIZE * 2, ret) image.resize.assert_not_called() # Read past the limit triggers another resize ret = self.store._resize_on_write(image, 0, ret + 1, 16) - self.assertEqual(8 + self.store.WRITE_CHUNKSIZE * 3, ret) + self.assertEqual(8 + self.store.WRITE_CHUNKSIZE * 6, ret) image.resize.assert_called_once_with(ret) self.assertEqual(self.store.WRITE_CHUNKSIZE * 4, self.store.resize_amount) # Check that we do not resize past the 8G ceiling. - # Start with resize_amount at 4G, 1G read so far + # Start with resize_amount at 2G, 1G read so far image.resize.reset_mock() - self.store.resize_amount = 4 * units.Gi + self.store.resize_amount = 2 * units.Gi self.store.size = 1 * units.Gi - # First resize happens and we get the 4G, - # resize_amount goes to limit of 8G + # First resize happens and we get to 5G, + # resize_amount goes to limit of 4G ret = self.store._resize_on_write(image, 0, 4097 * units.Mi, 16) - self.assertEqual(5 * units.Gi, ret) - self.assertEqual(8 * units.Gi, self.store.resize_amount) + self.assertEqual(4 * units.Gi, self.store.resize_amount) + self.assertEqual((1 + 4) * units.Gi, ret) self.store.size = ret - # Second resize happens and we get to 13G, + # Second resize happens and we stay at 13, no resize # resize amount stays at limit of 8G ret = self.store._resize_on_write(image, 0, 6144 * units.Mi, 16) - self.assertEqual((5 + 8) * units.Gi, ret) self.assertEqual(8 * units.Gi, self.store.resize_amount) + self.assertEqual((1 + 4 + 8) * units.Gi, ret) self.store.size = ret - # Third resize happens and we get to 21G, + # Third resize happens and we get to 21, # resize amount stays at limit of 8G ret = self.store._resize_on_write(image, 0, 14336 * units.Mi, 16) - self.assertEqual((5 + 8 + 8) * units.Gi, ret) self.assertEqual(8 * units.Gi, self.store.resize_amount) + self.assertEqual((1 + 4 + 8 + 8) * units.Gi, ret) + self.store.size = ret + + # Fourth resize happens and we get to 29, + # resize amount stays at limit of 8G + ret = self.store._resize_on_write(image, 0, 22528 * units.Mi, 16) + self.assertEqual(8 * units.Gi, self.store.resize_amount) + self.assertEqual((1 + 4 + 8 + 8 + 8) * units.Gi, ret) image.resize.assert_has_calls([ mock.call(5 * units.Gi), mock.call(13 * units.Gi), - mock.call(21 * units.Gi)]) + mock.call(21 * units.Gi), + mock.call(29 * units.Gi)]) class TestStore(base.StoreBaseTest,