The first two patches establish a framework for using regular files as fake disks so that things that depend on parted objects (most of blivet) can function without root access. While the primary motivation was testing partition allocation in a non-destructive way as a non-root user, this also enables unit testing of several areas of the devicetree, action registration, sorting and pruning, the device factories, and determining resize constraints for existing filesystems.
The other two are the first tests to use the framework.
David Lehman (4): Add a contextmanager to create and remove sparse tempfiles. Add a DiskFile class for testing partitioning code as a non-root user. Add a couple of tests for blivet.partitioning.DiskChunk. Add some tests for blivet.partitioning.addPartition.
blivet/devices.py | 29 ++++++ blivet/util.py | 16 +++ tests/partitioning_test.py | 243 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 288 insertions(+)
with sparsetmpfile("mytest", Size("100 MiB")) as path: do_stuff(fn=path)
It creates a sparse temporary file with the specified size and yields its full path on __enter__, then unlinks the file on __exit__. --- blivet/util.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/blivet/util.py b/blivet/util.py index 1e28ddc..a15bb4a 100644 --- a/blivet/util.py +++ b/blivet/util.py @@ -9,6 +9,7 @@ import re import sys import tempfile from decimal import Decimal +from contextlib import contextmanager
import six
@@ -402,6 +403,21 @@ def create_sparse_file(path, size): os.ftruncate(fd, size) os.close(fd)
+@contextmanager +def sparsetmpfile(name, size): + """ Context manager that creates a sparse tempfile and then unlinks it. + + :param str name: suffix for filename + :param :class:`~.size.Size` size: the file size + + Yields the path to the newly created file on __enter__. + """ + path = create_sparse_tempfile(name, size) + try: + yield path + finally: + os.unlink(path) + def variable_copy(obj, memo, omit=None, shallow=None, duplicate=None): """ A configurable copy function. Any attributes not specified in omit, shallow, or duplicate are copied using copy.deepcopy().
There seems to be some special treatment from parted (first free sector is reported as 32) but the main purpose is to test partitioning code without committing the changes to disk. It may be perfectly viable to commit to these devices, but I haven't tried it yet. My intention is to write unit tests for things like partition allocation -- which require parted Device and Disk instances -- without requiring root access. --- blivet/devices.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/blivet/devices.py b/blivet/devices.py index 1028817..dfdea44 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -1234,6 +1234,35 @@ class DiskDevice(StorageDevice):
StorageDevice._preDestroy(self)
+class DiskFile(DiskDevice): + """ This is a file that we will pretend is a disk. """ + _devDir = "" + + def __init__(self, name, fmt=None, + size=None, major=None, minor=None, sysfsPath='', + parents=None, serial=None, vendor="", model="", bus="", + exists=True): + _name = os.path.basename(name) + self._devDir = os.path.dirname(name) + + super(DiskFile, self).__init__(_name, fmt=fmt, size=size, + major=major, minor=minor, sysfsPath=sysfsPath, + parents=parents, serial=serial, vendor=vendor, + model=model, bus=bus, exists=exists) + + # + # Regular files do not have sysfs entries. + # + @property + def sysfsPath(self): + return "" + + @sysfsPath.setter + def sysfsPath(self, value): + pass + + def updateSysfsPath(self): + pass
class PartitionDevice(StorageDevice): """ A disk partition.
----- Original Message -----
From: "David Lehman" dlehman@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Wednesday, July 23, 2014 5:04:26 PM Subject: [PATCH 2/4] Add a DiskFile class for testing partitioning code as a non-root user.
There seems to be some special treatment from parted (first free sector is reported as 32) but the main purpose is to test partitioning code without committing the changes to disk. It may be perfectly viable to commit to these devices, but I haven't tried it yet. My intention is to write unit tests for things like partition allocation -- which require parted Device and Disk instances -- without requiring root access.
blivet/devices.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/blivet/devices.py b/blivet/devices.py index 1028817..dfdea44 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -1234,6 +1234,35 @@ class DiskDevice(StorageDevice):
StorageDevice._preDestroy(self)+class DiskFile(DiskDevice):
- """ This is a file that we will pretend is a disk. """
- _devDir = ""
- def __init__(self, name, fmt=None,
size=None, major=None, minor=None, sysfsPath='',parents=None, serial=None, vendor="", model="", bus="",exists=True):_name = os.path.basename(name)self._devDir = os.path.dirname(name)super(DiskFile, self).__init__(_name, fmt=fmt, size=size,major=major, minor=minor, sysfsPath=sysfsPath,parents=parents, serial=serial, vendor=vendor,model=model, bus=bus, exists=exists)- #
- # Regular files do not have sysfs entries.
- #
- @property
- def sysfsPath(self):
return ""- @sysfsPath.setter
- def sysfsPath(self, value):
pass- def updateSysfsPath(self):
passclass PartitionDevice(StorageDevice): """ A disk partition. -- 1.9.3
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
It would be helpful to document that the class is not intended to represent a real device. Maybe it should find its home somewhere in testing subdirectory instead of blivet subdirectory, as more appropriate to its intended use.
- mulhern
On Thu, 2014-07-24 at 10:42 -0400, Anne Mulhern wrote:
----- Original Message -----
From: "David Lehman" dlehman@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Wednesday, July 23, 2014 5:04:26 PM Subject: [PATCH 2/4] Add a DiskFile class for testing partitioning code as a non-root user.
There seems to be some special treatment from parted (first free sector is reported as 32) but the main purpose is to test partitioning code without committing the changes to disk. It may be perfectly viable to commit to these devices, but I haven't tried it yet. My intention is to write unit tests for things like partition allocation -- which require parted Device and Disk instances -- without requiring root access.
blivet/devices.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/blivet/devices.py b/blivet/devices.py index 1028817..dfdea44 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -1234,6 +1234,35 @@ class DiskDevice(StorageDevice):
StorageDevice._preDestroy(self)+class DiskFile(DiskDevice):
- """ This is a file that we will pretend is a disk. """
- _devDir = ""
- def __init__(self, name, fmt=None,
size=None, major=None, minor=None, sysfsPath='',parents=None, serial=None, vendor="", model="", bus="",exists=True):_name = os.path.basename(name)self._devDir = os.path.dirname(name)super(DiskFile, self).__init__(_name, fmt=fmt, size=size,major=major, minor=minor, sysfsPath=sysfsPath,parents=parents, serial=serial, vendor=vendor,model=model, bus=bus, exists=exists)- #
- # Regular files do not have sysfs entries.
- #
- @property
- def sysfsPath(self):
return ""- @sysfsPath.setter
- def sysfsPath(self, value):
pass- def updateSysfsPath(self):
passclass PartitionDevice(StorageDevice): """ A disk partition. -- 1.9.3
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
It would be helpful to document that the class is not intended to represent a real device. Maybe it should find its home somewhere in testing subdirectory instead of blivet subdirectory, as more appropriate to its intended use.
I don't agree with moving the class. We should provide this class to users of Blivet for their tests. A bit clearer documentation would, of course, be useful.
----- Original Message -----
From: "Vratislav Podzimek" vpodzime@redhat.com To: "anaconda patch review" anaconda-patches@lists.fedorahosted.org Sent: Friday, July 25, 2014 2:13:47 AM Subject: Re: [PATCH 2/4] Add a DiskFile class for testing partitioning code as a non-root user.
On Thu, 2014-07-24 at 10:42 -0400, Anne Mulhern wrote:
----- Original Message -----
From: "David Lehman" dlehman@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Wednesday, July 23, 2014 5:04:26 PM Subject: [PATCH 2/4] Add a DiskFile class for testing partitioning code as a non-root user.
There seems to be some special treatment from parted (first free sector is reported as 32) but the main purpose is to test partitioning code without committing the changes to disk. It may be perfectly viable to commit to these devices, but I haven't tried it yet. My intention is to write unit tests for things like partition allocation -- which require parted Device and Disk instances -- without requiring root access.
blivet/devices.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/blivet/devices.py b/blivet/devices.py index 1028817..dfdea44 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -1234,6 +1234,35 @@ class DiskDevice(StorageDevice):
StorageDevice._preDestroy(self)+class DiskFile(DiskDevice):
- """ This is a file that we will pretend is a disk. """
- _devDir = ""
- def __init__(self, name, fmt=None,
size=None, major=None, minor=None, sysfsPath='',parents=None, serial=None, vendor="", model="", bus="",exists=True):_name = os.path.basename(name)self._devDir = os.path.dirname(name)super(DiskFile, self).__init__(_name, fmt=fmt, size=size,major=major, minor=minor,sysfsPath=sysfsPath,
parents=parents, serial=serial,vendor=vendor,
model=model, bus=bus, exists=exists)- #
- # Regular files do not have sysfs entries.
- #
- @property
- def sysfsPath(self):
return ""- @sysfsPath.setter
- def sysfsPath(self, value):
pass- def updateSysfsPath(self):
passclass PartitionDevice(StorageDevice): """ A disk partition. -- 1.9.3
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
It would be helpful to document that the class is not intended to represent a real device. Maybe it should find its home somewhere in testing subdirectory instead of blivet subdirectory, as more appropriate to its intended use.
I don't agree with moving the class. We should provide this class to users of Blivet for their tests. A bit clearer documentation would, of course, be useful.
-- Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
That's a good point. Leaving it where it is and documenting it as provided solely for testing purposes seems like the best solution.
- mulhern
--- tests/partitioning_test.py | 159 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+)
diff --git a/tests/partitioning_test.py b/tests/partitioning_test.py index 41b3731..276fe7b 100644 --- a/tests/partitioning_test.py +++ b/tests/partitioning_test.py @@ -7,16 +7,23 @@ import parted
from blivet.partitioning import getNextPartitionType from blivet.partitioning import doPartitioning +from blivet.partitioning import allocatePartitions +from blivet.partitioning import getFreeRegions from blivet.partitioning import Request from blivet.partitioning import Chunk from blivet.partitioning import LVRequest from blivet.partitioning import VGChunk +from blivet.partitioning import DiskChunk +from blivet.partitioning import PartitionRequest
from blivet.devices import StorageDevice from blivet.devices import LVMVolumeGroupDevice from blivet.devices import LVMLogicalVolumeDevice +from blivet.devices import DiskFile +from blivet.devices import PartitionDevice
from tests.imagebackedtestcase import ImageBackedTestCase +from blivet.util import sparsetmpfile from blivet.formats import getFormat from blivet.size import Size from blivet.flags import flags @@ -194,6 +201,158 @@ class PartitioningTestCase(unittest.TestCase): self.assertEqual(req2.growth, 0) self.assertEqual(req3.growth, 35)
+ def testDiskChunk1(self): + disk_size = Size("100 MiB") + with sparsetmpfile("chunktest", disk_size) as disk_file: + disk = DiskFile(disk_file) + disk.format = getFormat("disklabel", device=disk.path, exists=False) + + p1 = PartitionDevice("p1", size=Size("10 MiB"), grow=True) + p2 = PartitionDevice("p2", size=Size("30 MiB"), grow=True) + + disks = [disk] + partitions = [p1, p2] + free = getFreeRegions([disk]) + self.assertEquals(len(free), 1, + "free region count %d not expected" % len(free)) + + b = Mock() + allocatePartitions(b, disks, partitions, free) + + requests = [PartitionRequest(p) for p in partitions] + chunk = DiskChunk(free[0], requests=requests) + + # parted reports a first free sector of 32 for disk files. whatever. + length_expected = 204768 + self.assertEqual(chunk.length, length_expected) + + base_expected = sum(p.partedPartition.geometry.length for p in partitions) + self.assertEqual(chunk.base, base_expected) + + pool_expected = chunk.length - base_expected + self.assertEqual(chunk.pool, pool_expected) + + self.assertEqual(chunk.done, False) + self.assertEqual(chunk.remaining, 2) + + chunk.growRequests() + + self.assertEqual(chunk.done, True) + self.assertEqual(chunk.pool, 0) + self.assertEqual(chunk.remaining, 2) + + # + # validate the growth (everything in sectors) + # + # The chunk length is 204768. The base of p1 is 20480. The base of + # p2 is 61440. The chunk has a base of 81920 and a pool of 122848. + # + # p1 should grow by 30712 while p2 grows by 92136 since p2's base + # size is exactly three times that of p1. + self.assertEqual(requests[0].growth, 30712) + self.assertEqual(requests[1].growth, 92136) + + def testDiskChunk2(self): + disk_size = Size("100 MiB") + with sparsetmpfile("chunktest", disk_size) as disk_file: + disk = DiskFile(disk_file) + disk.format = getFormat("disklabel", device=disk.path, exists=False) + + p1 = PartitionDevice("p1", size=Size("10 MiB"), grow=True) + p2 = PartitionDevice("p2", size=Size("30 MiB"), grow=True) + + # format max size should be reflected in request max growth + fmt = getFormat("dummy") + fmt._maxSize = Size("12 MiB") + p3 = PartitionDevice("p3", size=Size("10 MiB"), grow=True, + fmt=fmt) + + p4 = PartitionDevice("p4", size=Size("7 MiB")) + + # partition max size should be reflected in request max growth + p5 = PartitionDevice("p5", size=Size("5 MiB"), grow=True, + maxsize=Size("6 MiB")) + + disks = [disk] + partitions = [p1, p2, p3, p4, p5] + free = getFreeRegions([disk]) + self.assertEquals(len(free), 1, + "free region count %d not expected" % len(free)) + + b = Mock() + allocatePartitions(b, disks, partitions, free) + + requests = [PartitionRequest(p) for p in partitions] + chunk = DiskChunk(free[0], requests=requests) + + self.assertEqual(len(chunk.requests), len(partitions)) + + # parted reports a first free sector of 32 for disk files. whatever. + length_expected = 204768 + self.assertEqual(chunk.length, length_expected) + + growable = [p for p in partitions if p.req_grow] + fixed = [p for p in partitions if not p.req_grow] + base_expected = sum(p.partedPartition.geometry.length for p in growable) + self.assertEqual(chunk.base, base_expected) + + base_fixed = sum(p.partedPartition.geometry.length for p in fixed) + pool_expected = chunk.length - base_expected - base_fixed + self.assertEqual(chunk.pool, pool_expected) + + self.assertEqual(chunk.done, False) + + # since p5 is not growable it is initially done + self.assertEqual(chunk.remaining, 4) + + chunk.growRequests() + + # + # validate the growth (in sectors) + # + # The chunk length is 204768. + # Request bases: + # p1 20480 + # p2 61440 + # p3 20480 + # p4 14336 (not included in chunk base since it isn't growable) + # p5 10240 + # + # The chunk has a base 112640 and a pool of 77792. + # + # Request max growth: + # p1 0 + # p2 0 + # p3 4096 + # p4 0 + # p5 2048 + # + # The first round should allocate to p1, p2, p3, p5 at a ratio of + # 2:6:2:1, which is 14144, 42432, 14144, 7072. Due to max growth, + # p3 and p5 will be limited and the extra (10048, 5024) will remain + # in the pool. In the second round the remaining requests will be + # p1 and p2. They will divide up the pool of 15072 at a ratio of + # 1:3, which is 3768 and 11304. At this point the pool should be + # empty. + # + # Total growth: + # p1 17912 + # p2 53736 + # p3 4096 + # p4 0 + # p5 2048 + # + self.assertEqual(chunk.done, True) + self.assertEqual(chunk.pool, 0) + self.assertEqual(chunk.remaining, 2) # p1, p2 have no max + + # chunk.requests got sorted, so use the list whose order we know + self.assertEqual(requests[0].growth, 17912) + self.assertEqual(requests[1].growth, 53736) + self.assertEqual(requests[2].growth, 4096) + self.assertEqual(requests[3].growth, 0) + self.assertEqual(requests[4].growth, 2048) + def testVGChunk(self): pv = StorageDevice("pv1", size=Size("40 GiB"), fmt=getFormat("lvmpv"))
--- tests/partitioning_test.py | 84 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+)
diff --git a/tests/partitioning_test.py b/tests/partitioning_test.py index 276fe7b..6b0d9b6 100644 --- a/tests/partitioning_test.py +++ b/tests/partitioning_test.py @@ -5,6 +5,7 @@ from mock import Mock
import parted
+from blivet.partitioning import addPartition from blivet.partitioning import getNextPartitionType from blivet.partitioning import doPartitioning from blivet.partitioning import allocatePartitions @@ -139,6 +140,89 @@ class PartitioningTestCase(unittest.TestCase): disk = self.getDisk(disk_type="mac") self.assertEqual(getNextPartitionType(disk, no_primary=True), None)
+ def testAddPartition(self): + with sparsetmpfile("addparttest", Size("50 MiB")) as disk_file: + disk = DiskFile(disk_file) + disk.format = getFormat("disklabel", device=disk.path, exists=False) + + free = disk.format.partedDisk.getFreeSpaceRegions()[0] + + # + # add a partition with an unaligned size + # + self.assertEqual(len(disk.format.partitions), 0) + part = addPartition(disk.format, free, parted.PARTITION_NORMAL, + Size("10 MiB") - Size(37)) + self.assertEqual(len(disk.format.partitions), 1) + + # an unaligned size still yields an aligned partition + alignment = disk.format.alignment + geom = part.geometry + sector_size = geom.device.sectorSize + self.assertEqual(alignment.isAligned(free, geom.start), True) + self.assertEqual(alignment.isAligned(free, geom.end + 1), True) + self.assertEqual(part.geometry.length, Size("10 MiB") / sector_size) + + disk.format.removePartition(part) + self.assertEqual(len(disk.format.partitions), 0) + + # + # add a partition with an unaligned start sector + # + start_sector = 5003 + end_sector = 15001 + part = addPartition(disk.format, free, parted.PARTITION_NORMAL, + None, start_sector, end_sector) + self.assertEqual(len(disk.format.partitions), 1) + + # start and end sectors are exactly as specified + self.assertEqual(part.geometry.start, start_sector) + self.assertEqual(part.geometry.end, end_sector) + + disk.format.removePartition(part) + self.assertEqual(len(disk.format.partitions), 0) + + # + # fail: add a logical partition to a primary free region + # + with self.assertRaisesRegexp(parted.PartitionException, + "no extended partition"): + part = addPartition(disk.format, free, parted.PARTITION_LOGICAL, + Size("10 MiB")) + + ## add an extended partition to the disk + placeholder = addPartition(disk.format, free, + parted.PARTITION_NORMAL, Size("10 MiB")) + all_free = disk.format.partedDisk.getFreeSpaceRegions() + extended = addPartition(disk.format, all_free[1], + parted.PARTITION_EXTENDED, Size("30 MiB"), + alignment.alignUp(all_free[1], + placeholder.geometry.end)) + + disk.format.removePartition(placeholder) + self.assertEqual(len(disk.format.partitions), 1) + all_free = disk.format.partedDisk.getFreeSpaceRegions() + + # + # add a logical partition to an extended free regions + # + part = addPartition(disk.format, all_free[1], + parted.PARTITION_LOGICAL, + Size("10 MiB"), all_free[1].start) + self.assertEqual(part.type, parted.PARTITION_LOGICAL) + + disk.format.removePartition(part) + self.assertEqual(len(disk.format.partitions), 1) + + # + # fail: add a primary partition to an extended free region + # + with self.assertRaisesRegexp(parted.PartitionException, "overlap"): + part = addPartition(disk.format, all_free[1], + parted.PARTITION_NORMAL, + Size("10 MiB"), all_free[1].start) + + def testChunk(self): dev1 = Mock() attrs = {"req_grow": True,
On Wed, 2014-07-23 at 16:04 -0500, David Lehman wrote:
The first two patches establish a framework for using regular files as fake disks so that things that depend on parted objects (most of blivet) can function without root access. While the primary motivation was testing partition allocation in a non-destructive way as a non-root user, this also enables unit testing of several areas of the devicetree, action registration, sorting and pruning, the device factories, and determining resize constraints for existing filesystems.
The other two are the first tests to use the framework.
David Lehman (4): Add a contextmanager to create and remove sparse tempfiles. Add a DiskFile class for testing partitioning code as a non-root user. Add a couple of tests for blivet.partitioning.DiskChunk. Add some tests for blivet.partitioning.addPartition.
blivet/devices.py | 29 ++++++ blivet/util.py | 16 +++ tests/partitioning_test.py | 243 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 288 insertions(+)
Wow, this does not at all look that bad I thought it would! These all look good to me.
----- Original Message -----
From: "David Lehman" dlehman@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Wednesday, July 23, 2014 5:04:24 PM Subject: [blivet] groundwork for better blivet unit test coverage
The first two patches establish a framework for using regular files as fake disks so that things that depend on parted objects (most of blivet) can function without root access. While the primary motivation was testing partition allocation in a non-destructive way as a non-root user, this also enables unit testing of several areas of the devicetree, action registration, sorting and pruning, the device factories, and determining resize constraints for existing filesystems.
The other two are the first tests to use the framework.
David Lehman (4): Add a contextmanager to create and remove sparse tempfiles. Add a DiskFile class for testing partitioning code as a non-root user. Add a couple of tests for blivet.partitioning.DiskChunk. Add some tests for blivet.partitioning.addPartition.
blivet/devices.py | 29 ++++++ blivet/util.py | 16 +++ tests/partitioning_test.py | 243 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 288 insertions(+)
-- 1.9.3
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
Other than the comment on patch (2) it all looks fine to me.
- mulhern
anaconda-patches@lists.fedorahosted.org
-
Anne Mulhern -
David Lehman -
Vratislav Podzimek