Commit fa8ef9d6 authored by Michael Hanselmann's avatar Michael Hanselmann

Wipe added space when growing disks

This patch adds code to wipe newly added disk space when growing disks
using “gnt-instance grow-disk”. “New disk space” is defined as the delta
between the old block device size (not necessarily equal to the amount
recorded in the configuration) and the new recorded size. Extra caution
is taken to avoid overwriting existing data. Unit tests are included.

A small typo in gnt-cluster(8) (“it's”) is also fixed.
Signed-off-by: default avatarMichael Hanselmann <hansmi@google.com>
Reviewed-by: default avatarIustin Pop <iustin@google.com>
parent 129cce69
......@@ -8972,7 +8972,7 @@ def _CalcEta(time_taken, written, total_size):
return (total_size - written) * avg_time
def _WipeDisks(lu, instance):
def _WipeDisks(lu, instance, disks=None):
"""Wipes instance disks.
@type lu: L{LogicalUnit}
......@@ -8984,13 +8984,18 @@ def _WipeDisks(lu, instance):
"""
node = instance.primary_node
for device in instance.disks:
if disks is None:
disks = [(idx, disk, 0)
for (idx, disk) in enumerate(instance.disks)]
for (_, device, _) in disks:
lu.cfg.SetDiskID(device, node)
logging.info("Pausing synchronization of disks of instance '%s'",
instance.name)
result = lu.rpc.call_blockdev_pause_resume_sync(node,
(instance.disks, instance),
(map(compat.snd, disks),
instance),
True)
result.Raise("Failed to pause disk synchronization on node '%s'" % node)
......@@ -9000,22 +9005,29 @@ def _WipeDisks(lu, instance):
" failed", idx, instance.name)
try:
for idx, device in enumerate(instance.disks):
for (idx, device, offset) in disks:
# The wipe size is MIN_WIPE_CHUNK_PERCENT % of the instance disk but
# MAX_WIPE_CHUNK at max. Truncating to integer to avoid rounding errors.
wipe_chunk_size = \
int(min(constants.MAX_WIPE_CHUNK,
device.size / 100.0 * constants.MIN_WIPE_CHUNK_PERCENT))
lu.LogInfo("* Wiping disk %d", idx)
logging.info("Wiping disk %d for instance %s on node %s using"
" chunk size %s", idx, instance.name, node, wipe_chunk_size)
offset = 0
size = device.size
last_output = 0
start_time = time.time()
if offset == 0:
info_text = ""
else:
info_text = (" (from %s to %s)" %
(utils.FormatUnit(offset, "h"),
utils.FormatUnit(size, "h")))
lu.LogInfo("* Wiping disk %s%s", idx, info_text)
logging.info("Wiping disk %d for instance %s on node %s using"
" chunk size %s", idx, instance.name, node, wipe_chunk_size)
while offset < size:
wipe_size = min(wipe_chunk_size, size - offset)
......@@ -9039,7 +9051,8 @@ def _WipeDisks(lu, instance):
instance.name)
result = lu.rpc.call_blockdev_pause_resume_sync(node,
(instance.disks, instance),
(map(compat.snd, disks),
instance),
False)
if result.fail_msg:
......@@ -11832,6 +11845,23 @@ def _LoadNodeEvacResult(lu, alloc_result, early_release, use_nodes):
for ops in jobs]
def _DiskSizeInBytesToMebibytes(lu, size):
"""Converts a disk size in bytes to mebibytes.
Warns and rounds up if the size isn't an even multiple of 1 MiB.
"""
(mib, remainder) = divmod(size, 1024 * 1024)
if remainder != 0:
lu.LogWarning("Disk size is not an even multiple of 1 MiB; rounding up"
" to not overwrite existing data (%s bytes will not be"
" wiped)", (1024 * 1024) - remainder)
mib += 1
return mib
class LUInstanceGrowDisk(LogicalUnit):
"""Grow a disk of an instance.
......@@ -11933,6 +11963,8 @@ class LUInstanceGrowDisk(LogicalUnit):
assert (self.owned_locks(locking.LEVEL_NODE) ==
self.owned_locks(locking.LEVEL_NODE_RES))
wipe_disks = self.cfg.GetClusterInfo().prealloc_wipe_disks
disks_ok, _ = _AssembleInstanceDisks(self, self.instance, disks=[disk])
if not disks_ok:
raise errors.OpExecError("Cannot activate block device to grow")
......@@ -11947,7 +11979,27 @@ class LUInstanceGrowDisk(LogicalUnit):
self.cfg.SetDiskID(disk, node)
result = self.rpc.call_blockdev_grow(node, (disk, instance), self.delta,
True, True)
result.Raise("Grow request failed to node %s" % node)
result.Raise("Dry-run grow request failed to node %s" % node)
if wipe_disks:
# Get disk size from primary node for wiping
result = self.rpc.call_blockdev_getsize(instance.primary_node, [disk])
result.Raise("Failed to retrieve disk size from node '%s'" %
instance.primary_node)
(disk_size_in_bytes, ) = result.payload
if disk_size_in_bytes is None:
raise errors.OpExecError("Failed to retrieve disk size from primary"
" node '%s'" % instance.primary_node)
old_disk_size = _DiskSizeInBytesToMebibytes(self, disk_size_in_bytes)
assert old_disk_size >= disk.size, \
("Retrieved disk size too small (got %s, should be at least %s)" %
(old_disk_size, disk.size))
else:
old_disk_size = None
# We know that (as far as we can test) operations across different
# nodes will succeed, time to run it for real on the backing storage
......@@ -11973,6 +12025,15 @@ class LUInstanceGrowDisk(LogicalUnit):
# Downgrade lock while waiting for sync
self.glm.downgrade(locking.LEVEL_INSTANCE)
assert wipe_disks ^ (old_disk_size is None)
if wipe_disks:
assert instance.disks[self.op.disk] == disk
# Wipe newly added disk space
_WipeDisks(self, instance,
disks=[(self.op.disk, disk, old_disk_size)])
if self.op.wait_for_sync:
disk_abort = not _WaitForSync(self, instance, disks=[disk])
if disk_abort:
......
......@@ -268,10 +268,10 @@ The ``--file-storage-dir`` option allows you set the directory to
use for storing the instance disk files when using file storage as
backend for instance disks.
The ``--prealloc-wipe-disks`` sets a cluster wide configuration
value for wiping disks prior to allocation. This increases security
on instance level as the instance can't access untouched data from
it's underlying storage.
The ``--prealloc-wipe-disks`` sets a cluster wide configuration value
for wiping disks prior to allocation and size changes (``gnt-instance
grow-disk``). This increases security on instance level as the instance
can't access untouched data from its underlying storage.
The ``--enabled-hypervisors`` option allows you to set the list of
hypervisors that will be enabled for this cluster. Instance
......
......@@ -1265,7 +1265,7 @@ class _DiskPauseTracker:
self.history = []
def __call__(self, (disks, instance), pause):
assert instance.disks == disks
assert not (set(disks) - set(instance.disks))
self.history.extend((i.logical_id, i.size, pause)
for i in disks)
......@@ -1336,65 +1336,117 @@ class TestWipeDisks(unittest.TestCase):
])
def testNormalWipe(self):
pt = _DiskPauseTracker()
for start_offset in [0, 280, 8895, 1563204]:
pt = _DiskPauseTracker()
progress = {}
progress = {}
def _WipeCb((disk, _), offset, size):
assert isinstance(offset, (long, int))
assert isinstance(size, (long, int))
def _WipeCb((disk, _), offset, size):
assert isinstance(offset, (long, int))
assert isinstance(size, (long, int))
max_chunk_size = (disk.size / 100.0 * constants.MIN_WIPE_CHUNK_PERCENT)
max_chunk_size = (disk.size / 100.0 * constants.MIN_WIPE_CHUNK_PERCENT)
self.assertTrue(offset >= 0)
self.assertTrue((offset + size) <= disk.size)
self.assertTrue(offset >= start_offset)
self.assertTrue((offset + size) <= disk.size)
self.assertTrue(size > 0)
self.assertTrue(size <= constants.MAX_WIPE_CHUNK)
self.assertTrue(size <= max_chunk_size)
self.assertTrue(size > 0)
self.assertTrue(size <= constants.MAX_WIPE_CHUNK)
self.assertTrue(size <= max_chunk_size)
self.assertTrue(offset == 0 or disk.logical_id in progress)
self.assertTrue(offset == start_offset or disk.logical_id in progress)
# Keep track of progress
cur_progress = progress.setdefault(disk.logical_id, 0)
self.assertEqual(cur_progress, offset)
# Keep track of progress
cur_progress = progress.setdefault(disk.logical_id, start_offset)
self.assertEqual(cur_progress, offset)
progress[disk.logical_id] += size
progress[disk.logical_id] += size
return (True, None)
return (True, None)
lu = _FakeLU(rpc=_RpcForDiskWipe(pt, _WipeCb),
cfg=_ConfigForDiskWipe())
lu = _FakeLU(rpc=_RpcForDiskWipe(pt, _WipeCb),
cfg=_ConfigForDiskWipe())
disks = [
objects.Disk(dev_type=constants.LD_LV, logical_id="disk0", size=1024),
objects.Disk(dev_type=constants.LD_LV, logical_id="disk1",
size=500 * 1024),
objects.Disk(dev_type=constants.LD_LV, logical_id="disk2", size=128),
objects.Disk(dev_type=constants.LD_LV, logical_id="disk3",
size=constants.MAX_WIPE_CHUNK),
]
instance = objects.Instance(name="inst3560",
primary_node="node1.example.com",
disk_template=constants.DT_PLAIN,
disks=disks)
cmdlib._WipeDisks(lu, instance)
self.assertEqual(pt.history, [
("disk0", 1024, True),
("disk1", 500 * 1024, True),
("disk2", 128, True),
("disk3", constants.MAX_WIPE_CHUNK, True),
("disk0", 1024, False),
("disk1", 500 * 1024, False),
("disk2", 128, False),
("disk3", constants.MAX_WIPE_CHUNK, False),
])
# Ensure the complete disk has been wiped
self.assertEqual(progress, dict((i.logical_id, i.size) for i in disks))
if start_offset > 0:
disks = [
objects.Disk(dev_type=constants.LD_LV, logical_id="disk0",
size=128),
objects.Disk(dev_type=constants.LD_LV, logical_id="disk1",
size=start_offset + (100 * 1024)),
]
else:
disks = [
objects.Disk(dev_type=constants.LD_LV, logical_id="disk0", size=1024),
objects.Disk(dev_type=constants.LD_LV, logical_id="disk1",
size=500 * 1024),
objects.Disk(dev_type=constants.LD_LV, logical_id="disk2", size=128),
objects.Disk(dev_type=constants.LD_LV, logical_id="disk3",
size=constants.MAX_WIPE_CHUNK),
]
instance = objects.Instance(name="inst3560",
primary_node="node1.example.com",
disk_template=constants.DT_PLAIN,
disks=disks)
if start_offset > 0:
# Test start offset with only one disk
cmdlib._WipeDisks(lu, instance,
disks=[(1, disks[1], start_offset)])
self.assertEqual(pt.history, [
("disk1", start_offset + (100 * 1024), True),
("disk1", start_offset + (100 * 1024), False),
])
self.assertEqual(progress, {
"disk1": disks[1].size,
})
else:
cmdlib._WipeDisks(lu, instance)
self.assertEqual(pt.history, [
("disk0", 1024, True),
("disk1", 500 * 1024, True),
("disk2", 128, True),
("disk3", constants.MAX_WIPE_CHUNK, True),
("disk0", 1024, False),
("disk1", 500 * 1024, False),
("disk2", 128, False),
("disk3", constants.MAX_WIPE_CHUNK, False),
])
# Ensure the complete disk has been wiped
self.assertEqual(progress, dict((i.logical_id, i.size) for i in disks))
class TestDiskSizeInBytesToMebibytes(unittest.TestCase):
def testLessThanOneMebibyte(self):
for i in [1, 2, 7, 512, 1000, 1023]:
lu = _FakeLU()
result = cmdlib._DiskSizeInBytesToMebibytes(lu, i)
self.assertEqual(result, 1)
self.assertEqual(len(lu.warning_log), 1)
self.assertEqual(len(lu.warning_log[0]), 2)
(_, (warnsize, )) = lu.warning_log[0]
self.assertEqual(warnsize, (1024 * 1024) - i)
def testEven(self):
for i in [1, 2, 7, 512, 1000, 1023]:
lu = _FakeLU()
result = cmdlib._DiskSizeInBytesToMebibytes(lu, i * 1024 * 1024)
self.assertEqual(result, i)
self.assertFalse(lu.warning_log)
def testLargeNumber(self):
for i in [1, 2, 7, 512, 1000, 1023, 2724, 12420]:
for j in [1, 2, 486, 326, 986, 1023]:
lu = _FakeLU()
size = (1024 * 1024 * i) + j
result = cmdlib._DiskSizeInBytesToMebibytes(lu, size)
self.assertEqual(result, i + 1, msg="Amount was not rounded up")
self.assertEqual(len(lu.warning_log), 1)
self.assertEqual(len(lu.warning_log[0]), 2)
(_, (warnsize, )) = lu.warning_log[0]
self.assertEqual(warnsize, (1024 * 1024) - j)
if __name__ == "__main__":
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment