diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index 872e4fe1912..433251e2d60 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -356,7 +356,27 @@ def update_volume_group_info(self): if lv['name'] == self.vg_thin_pool: self.vg_thin_pool_size = lv['size'] - def create_thin_pool(self, name=None, size_str=0): + def _calculate_thin_pool_size(self): + """Calculates the correct size for a thin pool. + + Ideally we would use 100% of the containing volume group and be done. + But the 100%VG notation to lvcreate is not implemented and thus cannot + be used. See https://bugzilla.redhat.com/show_bug.cgi?id=998347 + + Further, some amount of free space must remain in the volume group for + metadata for the contained logical volumes. The exact amount depends + on how much volume sharing you expect. + + :returns: An lvcreate-ready string for the number of calculated bytes. + """ + + # make sure volume group information is current + self.update_volume_group_info() + + # leave 5% free for metadata + return "%sg" % (float(self.vg_free_space) * 0.95) + + def create_thin_pool(self, name=None, size_str=None): """Creates a thin provisioning pool for this VG. The syntax here is slightly different than the default @@ -365,6 +385,7 @@ def create_thin_pool(self, name=None, size_str=0): :param name: Name to use for pool, default is "-pool" :param size_str: Size to allocate for pool, default is entire VG + :returns: The size string passed to the lvcreate command """ @@ -377,20 +398,19 @@ def create_thin_pool(self, name=None, size_str=0): if name is None: name = '%s-pool' % self.vg_name - if size_str == 0: - self.update_volume_group_info() - size_str = self.vg_size + self.vg_pool_name = '%s/%s' % (self.vg_name, name) - # NOTE(jdg): lvcreate will round up extents - # to avoid issues, let's chop the size off to an int - size_str = re.sub(r'\.\d*', '', size_str) - pool_path = '%s/%s' % (self.vg_name, name) - cmd = ['lvcreate', '-T', '-L', size_str, pool_path] + if not size_str: + size_str = self._calculate_thin_pool_size() + + cmd = ['lvcreate', '-T', '-L', size_str, self.vg_pool_name] self._execute(*cmd, root_helper=self._root_helper, run_as_root=True) + self.vg_thin_pool = name + return size_str def create_volume(self, name, size_str, lv_type='default', mirror_count=0): """Creates a logical volume on the object's VG. diff --git a/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py index 6802e908aee..beb60ecd93c 100644 --- a/cinder/tests/brick/test_brick_lvm.py +++ b/cinder/tests/brick/test_brick_lvm.py @@ -214,6 +214,17 @@ def lvchange_ign_no(obj, *args, **kwargs): self.vg._supports_lvchange_ignoreskipactivation = None + def test_thin_pool_creation(self): + + # The size of fake-volumes volume group is 10g, so the calculated thin + # pool size should be 9.5g (95% of 10g). + self.assertEqual("9.5g", self.vg.create_thin_pool()) + + # Passing a size parameter should result in a thin pool of that exact + # size. + for size in ("1g", "1.2g", "1.75g"): + self.assertEqual(size, self.vg.create_thin_pool(size_str=size)) + def test_volume_create_after_thin_creation(self): """Test self.vg.vg_thin_pool is set to pool_name