diff --git a/lib/cmdlib.py b/lib/cmdlib.py index 4cccd511ddfa2427e00847741e39ff70166cded0..ae08146fd9375d1715e13b143a07304ceed0e220 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -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: diff --git a/man/gnt-cluster.rst b/man/gnt-cluster.rst index 8082d14d94b85263fb8b696d579107ae1de32ef8..3cf77d2ec5359310e0c157e2adb11a516e3c7495 100644 --- a/man/gnt-cluster.rst +++ b/man/gnt-cluster.rst @@ -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 diff --git a/test/ganeti.cmdlib_unittest.py b/test/ganeti.cmdlib_unittest.py index 58ba4a5ca7ff92ab6f18cbba064cda43b4419ead..fe57980166cd74c852135bb1ae725e5c600139f9 100755 --- a/test/ganeti.cmdlib_unittest.py +++ b/test/ganeti.cmdlib_unittest.py @@ -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__":