From eb1bcac7180be9407bfc1b242db3ac3dfa5aba57 Mon Sep 17 00:00:00 2001
From: David Lehman <dlehman@redhat.com>
Date: Wed, 5 Nov 2014 16:42:37 -0600
Subject: [PATCH 07/26] Don't use parted.Device to obtain size info.

In order to see updated size info when a device is resize from outside of
blivet, it is necessary to delete the parted.Device and make another one.
This is happening from a change uevent handler. Deleting the parted.Device
causes parted to close its rw fd for the device, which triggers another
change uevent. We would have to ignore some of these change events to
avoid an infinite loop.

Something similar happens when resetting a Blivet instance. All devices
are deleted, including their parted.Device instances. This triggers change
uevents, which causes mdadm's udev rules to activate all arrays on those
devices, which is obviously not something we want to be triggering on a
reset. This one seems impossible to avoid as long as we're using the
parted.Device.

Related: rhbz#1069597
---
 blivet/deviceaction.py      |  8 ----
 blivet/devices/device.py    |  2 +-
 blivet/devices/disk.py      | 13 +-----
 blivet/devices/dm.py        |  3 +-
 blivet/devices/file.py      | 10 +++++
 blivet/devices/lib.py       |  3 ++
 blivet/devices/luks.py      |  6 +--
 blivet/devices/md.py        |  6 +--
 blivet/devices/partition.py | 30 +++++++-------
 blivet/devices/storage.py   | 98 ++++++++++++++++++++-------------------------
 blivet/devicetree.py        |  7 +++-
 blivet/platform.py          | 10 +++--
 tests/devices_test.py       |  2 +-
 tests/storagetestcase.py    |  7 +---
 14 files changed, 95 insertions(+), 110 deletions(-)

diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py
index 9755f0f..dc3508d 100644
--- a/blivet/deviceaction.py
+++ b/blivet/deviceaction.py
@@ -345,14 +345,6 @@ class ActionDestroyDevice(DeviceAction):
         super(ActionDestroyDevice, self).execute(callbacks=None)
         self.device.destroy()
 
-        # Make sure libparted does not keep cached info for this device
-        # and returns it when we create a new device with the same name
-        if self.device.partedDevice:
-            try:
-                self.device.partedDevice.removeFromCache()
-            except Exception: # pylint: disable=broad-except
-                log_exception_info(fmt_str="failed to remove info for device %s from libparted cache", fmt_args=[self.device])
-
     def requires(self, action):
         """ Return True if self requires action.
 
diff --git a/blivet/devices/device.py b/blivet/devices/device.py
index 3931eda..58dad62 100644
--- a/blivet/devices/device.py
+++ b/blivet/devices/device.py
@@ -97,7 +97,7 @@ class Device(util.ObjectID):
         """
         return util.variable_copy(self, memo,
            omit=('_raidSet', 'node'),
-           shallow=('_partedDevice', '_partedPartition'))
+           shallow=('_partedPartition'))
 
     def __repr__(self):
         s = ("%(type)s instance (%(id)s) --\n"
diff --git a/blivet/devices/disk.py b/blivet/devices/disk.py
index f14638d..48bb899 100644
--- a/blivet/devices/disk.py
+++ b/blivet/devices/disk.py
@@ -89,8 +89,7 @@ class DiskDevice(StorageDevice):
 
     def __repr__(self):
         s = StorageDevice.__repr__(self)
-        s += ("  removable = %(removable)s  partedDevice = %(partedDevice)r" %
-              {"removable": self.removable, "partedDevice": self.partedDevice})
+        s += ("  removable = %(removable)s" % {"removable": self.removable})
         return s
 
     @property
@@ -98,23 +97,15 @@ class DiskDevice(StorageDevice):
         if flags.testing:
             return True
 
-        if not self.partedDevice:
-            return False
-
         # Some drivers (cpqarray <blegh>) make block device nodes for
         # controllers with no disks attached and then report a 0 size,
         # treat this as no media present
-        return Size(self.partedDevice.getLength(unit="B")) != Size(0)
+        return self.exists and self.currentSize > Size(0)
 
     @property
     def description(self):
         return self.model
 
-    @property
-    def size(self):
-        """ The disk's size """
-        return super(DiskDevice, self).size
-
     def _preDestroy(self):
         """ Destroy the device. """
         log_method_call(self, self.name, status=self.status)
diff --git a/blivet/devices/dm.py b/blivet/devices/dm.py
index 9234c6e..978425d 100644
--- a/blivet/devices/dm.py
+++ b/blivet/devices/dm.py
@@ -34,6 +34,7 @@ import logging
 log = logging.getLogger("blivet")
 
 from .storage import StorageDevice
+from .lib import LINUX_SECTOR_SIZE
 
 class DMDevice(StorageDevice):
     """ A device-mapper device """
@@ -182,7 +183,7 @@ class DMLinearDevice(DMDevice):
         """ Open, or set up, a device. """
         log_method_call(self, self.name, orig=orig, status=self.status,
                         controllable=self.controllable)
-        slave_length = self.slave.partedDevice.length
+        slave_length = self.slave.currentSize / LINUX_SECTOR_SIZE
         dm.dm_create_linear(self.name, self.slave.path, slave_length,
                             self.dmUuid)
 
diff --git a/blivet/devices/file.py b/blivet/devices/file.py
index 0997035..2db7908 100644
--- a/blivet/devices/file.py
+++ b/blivet/devices/file.py
@@ -21,9 +21,11 @@
 #
 
 import os
+import stat
 
 from .. import util
 from ..storage_log import log_method_call
+from ..size import Size
 
 import logging
 log = logging.getLogger("blivet")
@@ -79,6 +81,14 @@ class FileDevice(StorageDevice):
 
         return os.path.normpath("%s%s" % (root, self.name))
 
+    def _getSize(self):
+        size = self._size
+        if self.exists:
+            st = os.stat(self.path)
+            size = Size(st[stat.ST_SIZE])
+
+        return size
+
     def _preSetup(self, orig=False):
         if self.format and self.format.exists and not self.format.status:
             self.format.device = self.path
diff --git a/blivet/devices/lib.py b/blivet/devices/lib.py
index 369b64d..14e9501 100644
--- a/blivet/devices/lib.py
+++ b/blivet/devices/lib.py
@@ -22,6 +22,9 @@ import os
 
 from .. import errors
 from .. import udev
+from ..size import Size
+
+LINUX_SECTOR_SIZE = Size(512)
 
 def get_device_majors():
     majors = {}
diff --git a/blivet/devices/luks.py b/blivet/devices/luks.py
index f02cd28..197926f 100644
--- a/blivet/devices/luks.py
+++ b/blivet/devices/luks.py
@@ -22,8 +22,6 @@
 # device backend modules
 from ..devicelibs import crypto
 
-from ..size import Size
-
 import logging
 log = logging.getLogger("blivet")
 
@@ -63,10 +61,10 @@ class LUKSDevice(DMCryptDevice):
 
     @property
     def size(self):
-        if not self.exists or not self.partedDevice:
+        if not self.exists:
             size = self.slave.size - crypto.LUKS_METADATA_SIZE
         else:
-            size = Size(self.partedDevice.getLength(unit="B"))
+            size = self.currentSize
         return size
 
     def _postCreate(self):
diff --git a/blivet/devices/md.py b/blivet/devices/md.py
index 105ee1e..d7f5167 100644
--- a/blivet/devices/md.py
+++ b/blivet/devices/md.py
@@ -200,7 +200,7 @@ class MDRaidArrayDevice(ContainerDevice, RaidDevice):
         """Returns the actual or estimated size depending on whether or
            not the array exists.
         """
-        if not self.exists or not self.partedDevice:
+        if not self.exists or not self.mediaPresent:
             try:
                 size = self.level.get_size([d.size for d in self.devices],
                     self.memberDevices,
@@ -211,7 +211,7 @@ class MDRaidArrayDevice(ContainerDevice, RaidDevice):
                 size = Size(0)
             log.debug("non-existent RAID %s size == %s", self.level, size)
         else:
-            size = Size(self.partedDevice.getLength(unit="B"))
+            size = self.currentSize
             log.debug("existing RAID %s size == %s", self.level, size)
 
         return size
@@ -531,7 +531,7 @@ class MDRaidArrayDevice(ContainerDevice, RaidDevice):
         if flags.testing:
             return True
         else:
-            return self.partedDevice is not None
+            return super(MDRaidArrayDevice, self).mediaPresent
 
     @property
     def model(self):
diff --git a/blivet/devices/partition.py b/blivet/devices/partition.py
index 2973e59..0bac6b6 100644
--- a/blivet/devices/partition.py
+++ b/blivet/devices/partition.py
@@ -139,7 +139,6 @@ class PartitionDevice(StorageDevice):
         self._partType = None
         self._partedPartition = None
         self._origPath = None
-        self._currentSize = Size(0)
 
         # FIXME: Validate size, but only if this is a new partition.
         #        For existing partitions we will get the size from
@@ -520,7 +519,6 @@ class PartitionDevice(StorageDevice):
             return
 
         self._size = Size(self.partedPartition.getLength(unit="B"))
-        self._currentSize = self._size
         self.targetSize = self._size
 
         self._partType = self.partedPartition.type
@@ -582,7 +580,6 @@ class PartitionDevice(StorageDevice):
             DeviceFormat(device=self.path, exists=True).destroy()
 
         StorageDevice._postCreate(self)
-        self._currentSize = Size(self.partedPartition.getLength(unit="B"))
 
     def _computeResize(self, partition, newsize=None):
         """ Return a new constraint and end-aligned geometry for new size.
@@ -638,7 +635,6 @@ class PartitionDevice(StorageDevice):
                                         end=geometry.end)
 
         self.disk.format.commit()
-        self._currentSize = Size(partition.getLength(unit="B"))
 
     def _preDestroy(self):
         StorageDevice._preDestroy(self)
@@ -691,25 +687,36 @@ class PartitionDevice(StorageDevice):
         return size
 
     def _setSize(self, newsize):
-        """ Set the device's size (for resize, not creation).
+        """ Set the device's size.
 
-            Arguments:
+            
+            Most devices have two scenarios for setting a size:
 
-                newsize -- the new size
+                1) set actual/current size
+                2) set target for resize
 
+            Partitions have a third scenario:
+
+                3) update size of an allocated-but-non-existent partition
         """
         log_method_call(self, self.name,
                         status=self.status, size=self._size, newsize=newsize)
         if not isinstance(newsize, Size):
             raise ValueError("new size must of type Size")
 
-        if not self.exists:
+        if not self.exists and not self.partedPartition:
             # device does not exist (a partition request), just set basic value
             self._size = newsize
             self.req_size = newsize
             self.req_base_size = newsize
             return
 
+        if self.exists:
+            super(PartitionDevice, self)._setSize(newsize)
+            return
+
+        # the rest is for changing the size of an allocated-but-not-existing
+        # partition, which I'm not sure is advisable
         if newsize > self.disk.size:
             raise ValueError("partition size would exceed disk size")
 
@@ -794,13 +801,6 @@ class PartitionDevice(StorageDevice):
         return self.alignTargetSize(unalignedMax)
 
     @property
-    def currentSize(self):
-        if self.exists:
-            return self._currentSize
-        else:
-            return Size(0)
-
-    @property
     def resizable(self):
         return super(PartitionDevice, self).resizable and \
                self.disk.type != 'dasd'
diff --git a/blivet/devices/storage.py b/blivet/devices/storage.py
index 963f998..fbd0c67 100644
--- a/blivet/devices/storage.py
+++ b/blivet/devices/storage.py
@@ -22,8 +22,6 @@
 
 import os
 import copy
-import parted
-import _ped
 import pyudev
 
 from .. import errors
@@ -39,6 +37,7 @@ log = logging.getLogger("blivet")
 
 from .device import Device
 from .network import NetworkStorageDevice
+from .lib import LINUX_SECTOR_SIZE
 
 class StorageDevice(Device):
     """ A generic storage device.
@@ -106,7 +105,11 @@ class StorageDevice(Device):
         super(StorageDevice, self).__init__(name, parents=parents)
 
         self._format = None
+
+        # XXX Should we trust passed in size or always consult /sys?
+        #     LVM snapshots probably need us to trust passed-in value.
         self._size = Size(util.numeric_type(size))
+        self._currentSize = self._size if self.exists else Size(0)
         self.major = util.numeric_type(major)
         self.minor = util.numeric_type(minor)
         self._serial = serial
@@ -125,17 +128,6 @@ class StorageDevice(Device):
 
         self.deviceLinks = []
 
-        if self.exists and flags.testing and not self._size:
-            def read_int_from_sys(path):
-                return int(open(path).readline().strip())
-
-            device_root = "/sys/class/block/%s" % self.name
-            if os.path.exists("%s/queue" % device_root):
-                sector_size = read_int_from_sys("%s/queue/logical_block_size"
-                                                % device_root)
-                size = read_int_from_sys("%s/size" % device_root)
-                self._size = Size(size * sector_size)
-
     def __str__(self):
         exist = "existing"
         if not self.exists:
@@ -199,26 +191,6 @@ class StorageDevice(Device):
         """ True if this device, or any it requires, is encrypted. """
         return self._encrypted or any(p.encrypted for p in self.parents)
 
-    def _getPartedDevicePath(self):
-        return self.path
-
-    @property
-    def partedDevice(self):
-        devicePath = self._getPartedDevicePath()
-        if self.exists and self.status and not self._partedDevice:
-            log.debug("looking up parted Device: %s", devicePath)
-
-            # We aren't guaranteed to be able to get a device.  In
-            # particular, built-in USB flash readers show up as devices but
-            # do not always have any media present, so parted won't be able
-            # to find a device.
-            try:
-                self._partedDevice = parted.Device(path=devicePath)
-            except (_ped.IOException, _ped.DeviceException):
-                pass
-
-        return self._partedDevice
-
     @property
     def raw_device(self):
         """ The device itself, or when encrypted, the backing device. """
@@ -280,12 +252,12 @@ class StorageDevice(Device):
               "  format = %(format)s\n"
               "  major = %(major)s  minor = %(minor)s  exists = %(exists)s"
               "  protected = %(protected)s\n"
-              "  sysfs path = %(sysfs)s  partedDevice = %(partedDevice)s\n"
+              "  sysfs path = %(sysfs)s\n"
               "  target size = %(targetSize)s  path = %(path)s\n"
               "  format args = %(formatArgs)s  originalFormat = %(origFmt)s" %
               {"uuid": self.uuid, "format": self.format, "size": self.size,
                "major": self.major, "minor": self.minor, "exists": self.exists,
-               "sysfs": self.sysfsPath, "partedDevice": self.partedDevice,
+               "sysfs": self.sysfsPath,
                "targetSize": self.targetSize, "path": self.path,
                "protected": self.protected,
                "formatArgs": self.formatArgs, "origFmt": self.originalFormat.type})
@@ -505,7 +477,6 @@ class StorageDevice(Device):
 
         # make sure that targetSize is updated to reflect the actual size
         if self.resizable:
-            self._partedDevice = None
             self._targetSize = self.currentSize
 
         self._updateNetDevMountOption()
@@ -554,32 +525,48 @@ class StorageDevice(Device):
 
     def _getSize(self):
         """ Get the device's size, accounting for pending changes. """
-        if self.exists and not self.mediaPresent:
-            return Size(0)
-
-        if self.exists and self.partedDevice:
-            self._size = self.currentSize
-
         size = self._size
-        if self.exists and self.resizable:
+        if self.exists and self.resizable and self.targetSize:
             size = self.targetSize
 
         return size
 
     def _setSize(self, newsize):
-        """ Set the device's size to a new value. """
+        """ Set the device's size to a new value.
+
+            This is not adequate to set up a resize as it does not set a new
+            target size for the device.
+        """
         if not isinstance(newsize, Size):
             raise ValueError("new size must of type Size")
 
-        if self.maxSize and newsize > self.maxSize:
+        # only calculate these once
+        max_size = self.maxSize
+        min_size = self.minSize
+        if max_size and newsize > max_size:
             raise errors.DeviceError("device cannot be larger than %s" %
-                              (self.maxSize,), self.name)
+                                     max_size, self.name)
+        elif min_size and newsize < min_size:
+            raise errors.DeviceError("device cannot be smaller than %s" %
+                                     min_size, self.name)
+
         self._size = newsize
 
     size = property(lambda x: x._getSize(),
                     lambda x, y: x._setSize(y),
                     doc="The device's size, accounting for pending changes")
 
+    def readCurrentSize(self):
+        log_method_call(self, exists=self.exists, path=self.path,
+                        sysfsPath=self.sysfsPath, node_exists=os.path.exists(self.path))
+        size = Size(0)
+        if self.exists and os.path.exists(self.path) and \
+           os.path.isdir(self.sysfsPath):
+            blocks = int(util.get_sysfs_attr(self.sysfsPath, "size"))
+            size = Size(blocks * LINUX_SECTOR_SIZE)
+
+        return size
+
     @property
     def currentSize(self):
         """ The device's actual size, generally the size discovered by using
@@ -588,12 +575,17 @@ class StorageDevice(Device):
 
             If the device does not exist, then the actual size is 0.
         """
-        size = Size(0)
-        if self.exists and self.partedDevice:
-            size = Size(self.partedDevice.getLength(unit="B"))
-        elif self.exists:
-            size = self._size
-        return size
+        if self._currentSize == Size(0):
+            self._currentSize = self.readCurrentSize()
+        return self._currentSize
+
+    def updateSize(self):
+        """ Update size, currentSize, and targetSize to actual size. """
+        self._currentSize = Size(0)
+        new_size = self.currentSize
+        self._size = new_size
+        self._targetSize = new_size # bypass setter checks
+        log.debug("updated %s size to %s (%s)", self.name, self.size, new_size)
 
     @property
     def minSize(self):
@@ -695,8 +687,6 @@ class StorageDevice(Device):
 
     @property
     def model(self):
-        if not self._model:
-            self._model = getattr(self.partedDevice, "model", "")
         return self._model
 
     @property
diff --git a/blivet/devicetree.py b/blivet/devicetree.py
index 442ddce..1841262 100644
--- a/blivet/devicetree.py
+++ b/blivet/devicetree.py
@@ -1067,6 +1067,7 @@ class DeviceTree(object):
         device = diskType(name,
                           major=udev.device_get_major(info),
                           minor=udev.device_get_minor(info),
+                          model=udev.device_get_model(info),
                           sysfsPath=sysfs_path, **kwargs)
 
         if diskType == DASDDevice:
@@ -1186,8 +1187,7 @@ class DeviceTree(object):
         #
         if device:
             # we successfully looked up the device. skip to format handling.
-            # first, grab the parted.Device while it's active
-            _unused = device.partedDevice
+            pass
         elif udev.device_is_loop(info):
             log.info("%s is a loop device", name)
             device = self.addUdevLoopDevice(info)
@@ -1227,6 +1227,9 @@ class DeviceTree(object):
             log.debug("no device obtained for %s", name)
             return
 
+        # first, update the size since the device is active now
+        device.updateSize()
+
         # If this device is read-only, mark it as such now.
         if self.udevDeviceIsDisk(info) and \
                 util.get_sysfs_attr(udev.device_get_sysfs_path(info), 'ro') == '1':
diff --git a/blivet/platform.py b/blivet/platform.py
index 491aa42..6d38382 100644
--- a/blivet/platform.py
+++ b/blivet/platform.py
@@ -125,10 +125,12 @@ class Platform(object):
         if flags.testing:
             return self.defaultDiskLabelType
 
+        parted_device = parted.Device(path=device.path)
+
         # if there's a required type for this device type, use that
-        labelType = self.requiredDiskLabelType(device.partedDevice.type)
+        labelType = self.requiredDiskLabelType(parted_device.type)
         log.debug("required disklabel type for %s (%s) is %s",
-                  device.name, device.partedDevice.type, labelType)
+                  device.name, parted_device.type, labelType)
         if not labelType:
             # otherwise, use the first supported type for this platform
             # that is large enough to address the whole device
@@ -136,8 +138,8 @@ class Platform(object):
             log.debug("default disklabel type for %s is %s", device.name,
                                                              labelType)
             for lt in self.diskLabelTypes:
-                l = parted.freshDisk(device=device.partedDevice, ty=lt)
-                if l.maxPartitionStartSector > device.partedDevice.length:
+                l = parted.freshDisk(device=parted_device, ty=lt)
+                if l.maxPartitionStartSector > parted_device.length:
                     labelType = lt
                     log.debug("selecting %s disklabel for %s based on size",
                               labelType, device.name)
diff --git a/tests/devices_test.py b/tests/devices_test.py
index c526c74..84cbe6c 100644
--- a/tests/devices_test.py
+++ b/tests/devices_test.py
@@ -967,7 +967,7 @@ class PartitionDeviceTestCase(unittest.TestCase):
             free = disk.format.partedDisk.getFreeSpaceRegions()[-1]
             raw_start = int(Size("9 MiB") / sector_size)
             start = disk.format.alignment.alignUp(free, raw_start) + 3
-            disk.format.addPartition(start, disk.partedDevice.length - 1)
+            disk.format.addPartition(start, disk.format.partedDevice.length - 1)
 
             # Verify the end of the free region immediately following the first
             # partition is unaligned.
diff --git a/tests/storagetestcase.py b/tests/storagetestcase.py
index 0b3f26d..ad1216f 100644
--- a/tests/storagetestcase.py
+++ b/tests/storagetestcase.py
@@ -12,8 +12,6 @@ from blivet.formats import getFormat
 from blivet.devices import StorageDevice
 from blivet.devices import PartitionDevice
 
-from blivet.size import B
-
 class StorageTestCase(unittest.TestCase):
     """ StorageTestCase
 
@@ -94,10 +92,7 @@ class StorageTestCase(unittest.TestCase):
         device = device_class(*args, **kwargs)
 
         if exists:
-            # set up mock parted.Device w/ correct size
-            device._partedDevice = Mock()
-            device._partedDevice.getLength = Mock(return_value=int(device._size.convertTo(B)))
-            device._partedDevice.sectorSize = 512
+            device._currentSize = kwargs.get("size")
 
         if isinstance(device, blivet.devices.PartitionDevice):
             #if exists:
-- 
1.9.3

