[PATCH blivet/master 2/3] devicelibs.lvm: Add 'autobackup' attribute

Anne Mulhern amulhern at redhat.com
Mon Sep 22 12:43:13 UTC 2014





----- Original Message -----
> From: "Will Woods" <wwoods at redhat.com>
> To: anaconda-patches at lists.fedorahosted.org
> Sent: Friday, September 19, 2014 6:18:37 PM
> Subject: [PATCH blivet/master 2/3] devicelibs.lvm: Add 'autobackup' attribute
> 
> This adds the 'autobackup' attribute, which - if set to False - will
> suppress the automatic backup of LVM metadata by adding the appropriate
> configuration options (backup.backup=0 and backup.archive=0).
> 
> Also, extend the test case so it tests this functionality.
> ---
>  blivet/devicelibs/lvm.py          |  6 ++++++
>  tests/devicelibs_test/lvm_test.py | 21 +++++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py
> index 949532c..df4746a 100644
> --- a/blivet/devicelibs/lvm.py
> +++ b/blivet/devicelibs/lvm.py
> @@ -50,6 +50,10 @@ RAID_levels = raid.RAIDLevels(["raid0", "raid1",
> "linear"])
>  # These LVM commands are read-only, so we can run them with read-only
>  locking
>  READONLY_COMMANDS = ('lvs', 'pvs', 'vgs')
>  
> +# This defines whether we want automatic metadata backup or not.
> +# You probably want this on unless you're doing image builds.
> +autobackup = True

Probably belongs in flags.py.

> +
>  def has_lvm():
>      if util.find_program_in_path("lvm"):
>          for line in open("/proc/devices").readlines():
> @@ -91,6 +95,8 @@ def _getConfigArgs(args):
>      config_string = " devices { %s } " % (devices_string) # strings can be
>      added
>      if cmd in READONLY_COMMANDS:
>          config_string += "global {locking_type=4} "
> +    if not autobackup:
> +        config_string += "backup {backup=0 archive=0} "
>      if config_string:
>          config_args = ["--config", config_string]
>      return config_args
> diff --git a/tests/devicelibs_test/lvm_test.py
> b/tests/devicelibs_test/lvm_test.py
> index b7000ea..33f0185 100755
> --- a/tests/devicelibs_test/lvm_test.py
> +++ b/tests/devicelibs_test/lvm_test.py
> @@ -7,6 +7,8 @@ from blivet.errors import LVMError
>  
>  from tests import loopbackedtestcase
>  
> +import glob
> +

Would prefer this import with other Python imports, i.e., unittest import.

>  class LVMTestCase(unittest.TestCase):
>  
>      def testGetPossiblePhysicalExtents(self):
> @@ -38,6 +40,9 @@ class
> LVMAsRootTestCase(loopbackedtestcase.LoopBackedTestCase):
>          self._vg_name = "test-vg"
>          self._lv_name = "test-lv"
>  
> +    def _list_backups(self):
> +        return set(glob.glob("/etc/lvm/archive/%s_*" % self._vg_name))
> +
>      def tearDown(self):
>          """Destroy volume group."""
>          try:
> @@ -68,6 +73,7 @@ class
> LVMAsRootTestCase(loopbackedtestcase.LoopBackedTestCase):
>      def testLVM(self):
>          _LOOP_DEV0 = self.loopDevices[0]
>          _LOOP_DEV1 = self.loopDevices[1]
> +        _old_backups = self._list_backups()
>  
>          ##
>          ## pvcreate
> @@ -109,6 +115,16 @@ class
> LVMAsRootTestCase(loopbackedtestcase.LoopBackedTestCase):
>              lvm.vgcreate("another-vg", [_LOOP_DEV0], Size("5MiB"))
>  
>          ##
> +        ## check for autobackup - should be some new stuff here
> +        ## FIXME: skip if backups have been disabled by host
> +        ##
> +        _current_backups = self._list_backups()
> +        self.assertNotEqual(_old_backups, _current_backups,
> +                            "lvm.autobackup enabled but no backups created")
> +        _old_backups = _current_backups
> +        lvm.autobackup = False
> +
> +        ##
>          ## vgdeactivate
>          ##
>          # pass
> @@ -279,5 +295,10 @@ class
> LVMAsRootTestCase(loopbackedtestcase.LoopBackedTestCase):
>          # pv already removed
>          self.assertEqual(lvm.pvremove(_LOOP_DEV0), None)
>  
> +        ##
> +        ## check if autobackup=False worked
> +        ##
> +        self.assertEqual(_old_backups, self._list_backups())
> +

This test should be in a separate method. The existing method is already
200 lines long and you're testing something quite specific here.

>  if __name__ == "__main__":
>      unittest.main()
> --
> 1.9.3
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 

- mulhern


More information about the anaconda-patches mailing list