Commits that add support for creation of cached LVs plus two minor changes that make things a bit nicer. Please note that I'm intentionally leaving the changes in the last commit as a standalone commit as it needs some attention.
From: Vratislav Podzimek vpodzime@redhat.com
--- blivet/devices/lvm.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 094f216..f4e5a7b 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -492,10 +492,6 @@ def __init__(self, name, parents=None, size=None, uuid=None, segType=None,
For existing LVs only:
- :keyword copies: number of copies in the vg (>1 for mirrored lvs) - :type copies: int - :keyword logSize: size of log volume (for mirrored lvs) - :type logSize: :class:`~.size.Size` :keyword segType: segment type (eg: "linear", "raid1") :type segType: str
@@ -505,7 +501,7 @@ def __init__(self, name, parents=None, size=None, uuid=None, segType=None, :type grow: bool :keyword maxsize: maximum size for growable LV :type maxsize: :class:`~.size.Size` - :keyword percent -- percent of VG space to take + :keyword percent: percent of VG space to take :type percent: int
"""
From: Vratislav Podzimek vpodzime@redhat.com
Instead of passing cache parameters as separate numbers, strings, etc. we may better put them into objects that hold them as a single logical unit. --- blivet/devices/cache.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/blivet/devices/cache.py b/blivet/devices/cache.py index f036466..17da72c 100644 --- a/blivet/devices/cache.py +++ b/blivet/devices/cache.py @@ -114,3 +114,24 @@ def hits(self): def misses(self): """number of misses""" pass + +@add_metaclass(abc.ABCMeta) +class CacheRequest(object): + """Abstract base class for cache requests specifying cache parameters for a + cached device + + """ + @abc.abstractproperty + def size(self): + """Requested size""" + pass + + @abc.abstractproperty + def fast_devs_names(self): + """Names of the devices (type-specific) to allocate/create the cache on""" + pass + + @abc.abstractproperty + def mode(self): + """Mode the cache should use""" + pass
From: Vratislav Podzimek vpodzime@redhat.com
These are the basic building blocks for LVM cache creation as all high-level calls end up doing these things. --- blivet/devices/lvm.py | 78 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 4 deletions(-)
diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index f4e5a7b..5d72b1a 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -51,7 +51,7 @@ from .container import ContainerDevice from .dm import DMDevice from .md import MDRaidArrayDevice -from .cache import Cache, CacheStats +from .cache import Cache, CacheStats, CacheRequest
_INTERNAL_LV_CLASSES = []
@@ -473,7 +473,7 @@ class LVMLogicalVolumeDevice(DMDevice):
def __init__(self, name, parents=None, size=None, uuid=None, segType=None, fmt=None, exists=False, sysfsPath='', grow=None, maxsize=None, - percent=None): + percent=None, cacheRequest=None): """ :param name: the device name (generally a device node's basename) :type name: str @@ -503,6 +503,8 @@ def __init__(self, name, parents=None, size=None, uuid=None, segType=None, :type maxsize: :class:`~.size.Size` :keyword percent: percent of VG space to take :type percent: int + :keyword cacheRequest: parameters of requested cache (if any) + :type cacheRequest: :class:`~.devices.lvm.LVMCacheRequest`
"""
@@ -536,6 +538,10 @@ def __init__(self, name, parents=None, size=None, uuid=None, segType=None, self._internal_lvs = [] self._cache = None
+ if cacheRequest and not self.exists: + self._cache = LVMCache(self, size=cacheRequest.size, exists=False, + fast_pv_names=cacheRequest.fast_devs_names, mode=cacheRequest.mode) + def _check_parents(self): """Check that this device has parents as expected"""
@@ -735,7 +741,36 @@ def _create(self): """ Create the device. """ log_method_call(self, self.name, status=self.status) # should we use --zero for safety's sake? - blockdev.lvm.lvcreate(self.vg.name, self._name, self.size) + if not self.cache: + # just a plain LV + blockdev.lvm.lvcreate(self.vg.name, self._name, self.size) + else: + mode = blockdev.lvm.cache_get_mode_from_str(self.cache.mode) + # prepare the list of fast PV devices + fast_pvs = [] + for pv in self.cache.fast_pv_names: + # make sure we have the full device paths + if not pv.startswith("/dev/"): + fast_pvs.append("/dev/%s" % pv) + else: + fast_pvs.append(pv) + + # get the list of all fast PV devices used in the VG so that we can + # consider the rest to be slow PVs and generate a list of them + all_fast_pvs_names = set() + for lv in self.vg.lvs: + if lv.cached and lv.cache.fast_pv_names: + all_fast_pvs_names |= set(lv.cache.fast_pv_names) + slow_pvs = [pv.path for pv in self.vg.pvs if pv.name not in all_fast_pvs_names] + + # TODO: allow specification of metadata size + # for now, we just make the metadata+data parts take the requested cache space together + md_size = blockdev.lvm.cache_get_default_md_size(self.cache.size) + data_size = self.cache.size - md_size + + # VG name, LV name, data size, cache size, metadata size, mode, flags, slow PVs, fast PVs + blockdev.lvm.cache_create_cached_lv(self.vg.name, self._name, self.size, data_size, md_size, + mode, 0, slow_pvs, fast_pvs)
def _preDestroy(self): StorageDevice._preDestroy(self) @@ -1615,7 +1650,7 @@ def dependsOn(self, dep): class LVMCache(Cache): """Class providing the cache-related functionality of a cached LV"""
- def __init__(self, cached_lv, size=None, exists=False, mode=None): + def __init__(self, cached_lv, size=None, exists=False, fast_pv_names=None, mode=None): """ :param cached_lv: the LV the cache functionality of which to provide :type cached_lv: :class:`LVMLogicalVolumeDevice` @@ -1623,6 +1658,8 @@ def __init__(self, cached_lv, size=None, exists=False, mode=None): that cannot determine their size dynamically) :type size: :class:`~.size.Size` :param bool exists: whether the cache exists or not + :param fast_pv_names: PVs to allocate the cache on/from (ignored for existing) + :type fast_pv_names: list of str :param str mode: desired mode for non-existing cache (ignored for existing)
""" @@ -1631,8 +1668,10 @@ def __init__(self, cached_lv, size=None, exists=False, mode=None): self._exists = exists if not exists: self._mode = mode or "writethrough" + self._fast_pvs = fast_pv_names else: self._mode = None + self._fast_pvs = None
@property def size(self): @@ -1674,6 +1713,10 @@ def cache_device_name(self): else: return None
+ @property + def fast_pv_names(self): + return self._fast_pvs + def detach(self): vg_name = self._cached_lv.vg.name ret = blockdev.lvm.cache_pool_name(vg_name, self._cached_lv.lvname) @@ -1747,3 +1790,30 @@ def write_hits(self): @property def write_misses(self): return self._write_misses + +class LVMCacheRequest(CacheRequest): + """Class representing the LVM cache creation request""" + def __init__(self, size, fast_pv_names, mode=None): + """ + :param size: requested size of the cache + :type size: :class:`~.size.Size` + :param fast_pv_names: PVs to allocate the cache on/from + :type fast_pv_names: list of str + :param str mode: requested mode for the cache (``None`` means the default is used) + + """ + self._size = size + self._fast_pvs = fast_pv_names + self._mode = mode or "writethrough" + + @property + def size(self): + return self._size + + @property + def fast_devs_names(self): + return self._fast_pvs + + @property + def mode(self): + return self._mode
Why pass fast pv list as names instead of `StorageDevice` instances? It seems like you're open to problems with things in `/dev/md/` or `/dev/mapper/`, or is that not possible?
That's a very good point. I think I spent too much time in the low-level stuff libblockdev does and forgot to draw the line between that and blivet. ``StorageDevice`` instances should be used all the way from processing kickstart data down to the ``blockdev.lvm.cache_create_cached_lv()`` call. I'll rework this part today. Thanks for the catching this soon enough!
From: Vratislav Podzimek vpodzime@redhat.com
In order to minimize the risk of non-cached LVs taking space on fast PVs that should be used by caches. --- blivet/deviceaction.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py index 4fd6df2..41b8289 100644 --- a/blivet/deviceaction.py +++ b/blivet/deviceaction.py @@ -26,7 +26,7 @@ from . import udev from .util import get_current_entropy from .devices import StorageDevice -from .devices import PartitionDevice +from .devices import PartitionDevice, LVMLogicalVolumeDevice from .formats import getFormat, luks from parted import partitionFlag, PARTITION_LBA from .i18n import _, N_ @@ -334,6 +334,14 @@ def requires(self, action): otherNum = action.device.partedPartition.number if selfNum > otherNum: rc = True + elif (action.isCreate and action.isDevice and + isinstance(self.device, LVMLogicalVolumeDevice) and + isinstance(action.device, LVMLogicalVolumeDevice) and + self.device.vg == action.device.vg): + # create cached LVs before non-cached LVs so that fast cache space + # is not taken by non-cached LVs + if not self.device.cached and action.device.cached: + rc = True elif (action.isAdd and action.container == self.container): rc = True
From: Vratislav Podzimek vpodzime@redhat.com
LVM cache takes space from the VG, but it is not included in the cached LV's size. --- blivet/partitioning.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/blivet/partitioning.py b/blivet/partitioning.py index 69e79e5..ea13004 100644 --- a/blivet/partitioning.py +++ b/blivet/partitioning.py @@ -1396,6 +1396,10 @@ def addRequest(self, req): if max_raid_disks > 1: self.pool -= 5 * max_raid_disks
+ if req.device.cached: + # cached LV -> reserve space for the cache + self.pool -= int(self.vg.align(req.device.cache.size, roundup=True) / self.vg.peSize) + super(VGChunk, self).addRequest(req)
def lengthToSize(self, length):
From: Vratislav Podzimek vpodzime@redhat.com
The new LV's parent may be a VG or a thin pool depending on the type of the new LV. Let's call this object 'parent' instead of 'vg' which is way less confusing. --- blivet/blivet.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/blivet/blivet.py b/blivet/blivet.py index d27e072..bde644b 100644 --- a/blivet/blivet.py +++ b/blivet/blivet.py @@ -917,10 +917,12 @@ def newLV(self, *args, **kwargs): """ thin_volume = kwargs.pop("thin_volume", False) thin_pool = kwargs.pop("thin_pool", False) - vg = kwargs.get("parents", [None])[0] - if thin_volume and vg: + parent = kwargs.get("parents", [None])[0] + if thin_volume and parent: # kwargs["parents"] will contain the pool device, so... - vg = vg.vg + vg = parent.vg + else: + vg = parent
mountpoint = kwargs.pop("mountpoint", None) if 'fmt_type' in kwargs:
From: Vratislav Podzimek vpodzime@redhat.com
The requested size for the LV's non-cache part may be bigger than the free space on the slow PVs while it can safely fit in the set of slow and fast PVs together with the cache.
An example of this is when a kickstart is used with two PVs in a VG and LV(s) with --cachesize and --grow. In that case the LV is grown to the maximum size filling up the VG together with the cache and other LVs (and their caches). --- blivet/devices/lvm.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 5d72b1a..39a12e9 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -769,8 +769,11 @@ def _create(self): data_size = self.cache.size - md_size
# VG name, LV name, data size, cache size, metadata size, mode, flags, slow PVs, fast PVs + # XXX: we need to pass slow_pvs+fast_pvs as slow PVs because parts + # of the fast PVs may be required for allocation of the LV (it may + # span over the slow PVs and parts of fast PVs) blockdev.lvm.cache_create_cached_lv(self.vg.name, self._name, self.size, data_size, md_size, - mode, 0, slow_pvs, fast_pvs) + mode, 0, slow_pvs+fast_pvs, fast_pvs)
def _preDestroy(self): StorageDevice._preDestroy(self)
Fixed a pylint warning.
anaconda-patches@lists.fedorahosted.org
-
dashea