[blivet][master/rhel7-branch][PATCH] Allow user code provide callbacks for various actions/events

David Lehman dlehman at redhat.com
Mon Sep 15 16:48:52 UTC 2014


On 08/27/2014 09:17 AM, Vratislav Podzimek wrote:
> This simplifies blivet's code, makes it cleaner and gives other user code than
> Anaconda way to react on various actions/events. It is also a building block for
> many other applications of callbacks in blivet.

I would like this more if it were a generic message bus, as that would 
make it more useful in the future.

It sounds nice in theory to register the callbacks (with DeviceTree, I 
assume -- not Blivet), but in practice that doesn't do much to reduce 
the passing around of the callback register. Given that, I think this is 
fine for its intended purpose.

David

>
> Related: rhbz#1073679
> Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> ---
>   blivet/__init__.py     |  28 +++++++----
>   blivet/callbacks.py    |  61 +++++++++++++++++++++++
>   blivet/deviceaction.py | 128 +++++++++++++++++++++++++++----------------------
>   blivet/devicetree.py   |  15 ++++--
>   4 files changed, 162 insertions(+), 70 deletions(-)
>   create mode 100644 blivet/callbacks.py
>
> diff --git a/blivet/__init__.py b/blivet/__init__.py
> index f8e8ac3..e02d7d0 100644
> --- a/blivet/__init__.py
> +++ b/blivet/__init__.py
> @@ -136,8 +136,6 @@ def enable_installer_mode():
>       flags.installer_mode = True
>
>       from . import deviceaction
> -    from pyanaconda.progress import progress_report
> -    deviceaction.progress_report = progress_report
>
>   def getSysroot():
>       """Returns the path to the target OS installation.
> @@ -198,8 +196,15 @@ def storageInitialize(storage, ksdata, protected):
>                                            if d.name not in ksdata.ignoredisk.ignoredisk]
>               log.debug("onlyuse is now: %s", ",".join(ksdata.ignoredisk.onlyuse))
>
> -def turnOnFilesystems(storage, mountOnly=False):
> -    """ Perform installer-specific activation of storage configuration. """
> +def turnOnFilesystems(storage, mountOnly=False, callbacks=None):
> +    """
> +    Perform installer-specific activation of storage configuration.
> +
> +    :param callbacks: callbacks to be invoked when actions are executed
> +    :type callbacks: return value of the :func:`~.callbacks.create_new_callbacks_register`
> +
> +    """
> +
>       if not flags.installer_mode:
>           return
>
> @@ -211,7 +216,7 @@ def turnOnFilesystems(storage, mountOnly=False):
>           storage.devicetree.teardownAll()
>
>           try:
> -            storage.doIt()
> +            storage.doIt(callbacks)
>           except FSResizeError as e:
>               if errorHandler.cb(e) == ERROR_RAISE:
>                   raise
> @@ -349,9 +354,16 @@ class Blivet(object):
>           self.roots = []
>           self.services = set()
>
> -    def doIt(self):
> -        """ Commit queued changes to disk. """
> -        self.devicetree.processActions()
> +    def doIt(self, callbacks=None):
> +        """
> +        Commit queued changes to disk.
> +
> +        :param callbacks: callbacks to be invoked when actions are executed
> +        :type callbacks: return value of the :func:`~.callbacks.create_new_callbacks_register`
> +
> +        """
> +
> +        self.devicetree.processActions(callbacks)
>           if not flags.installer_mode:
>               return
>
> diff --git a/blivet/callbacks.py b/blivet/callbacks.py
> new file mode 100644
> index 0000000..2f7a9cf
> --- /dev/null
> +++ b/blivet/callbacks.py
> @@ -0,0 +1,61 @@
> +#
> +# Copyright (C) 2014  Red Hat, Inc.
> +#
> +# This copyrighted material is made available to anyone wishing to use,
> +# modify, copy, or redistribute it subject to the terms and conditions of
> +# the GNU General Public License v.2, or (at your option) any later version.
> +# This program is distributed in the hope that it will be useful, but WITHOUT
> +# ANY WARRANTY expressed or implied, including the implied warranties of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General
> +# Public License for more details.  You should have received a copy of the
> +# GNU General Public License along with this program; if not, write to the
> +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +# 02110-1301, USA.  Any Red Hat trademarks that are incorporated in the
> +# source code or documentation are not subject to the GNU General Public
> +# License and may only be used or replicated with the express permission of
> +# Red Hat, Inc.
> +#
> +# Red Hat Author(s): Vratislav Podzimek <vpodzime at redhat.com>
> +#
> +
> +"""
> +Module providing classes defining the callbacks used by Blivet and their
> +arguments.
> +
> +"""
> +
> +from collections import namedtuple
> +
> +# A private namedtuple class with self-descriptive fields for passing callbacks
> +# to the blivet.doIt method. Each field should be populated with a function
> +# taking the matching CallbackTypeData (create_format_pre ->
> +# CreateFormatPreData, etc.)  object or None if no such callback is provided.
> +_CallbacksRegister = namedtuple("_CallbacksRegister",
> +                                ["create_format_pre",
> +                                 "create_format_post",
> +                                 "resize_format_pre",
> +                                 "resize_format_post"])
> +
> +def create_new_callbacks_register(create_format_pre=None,
> +                                  create_format_post=None,
> +                                  resize_format_pre=None,
> +                                  resize_format_post=None):
> +    """
> +    A function for creating a new opaque object holding the references to
> +    callbacks. The point of this function is to hide the implementation of such
> +    object and to provide default values for non-specified fields (e.g. newly
> +    added callbacks).
> +
> +    """
> +
> +    return _CallbacksRegister(create_format_pre, create_format_post,
> +                              resize_format_pre, resize_format_post)
> +
> +CreateFormatPreData = namedtuple("CreateFormatPreData",
> +                                 ["msg"])
> +CreateFormatPostData = namedtuple("CreateFormatPostData",
> +                                  ["msg"])
> +ResizeFormatPreData = namedtuple("ResizeFormatPreData",
> +                                 ["msg"])
> +ResizeFormatPostData = namedtuple("ResizeFormatPostData",
> +                                  ["msg"])
> diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py
> index db4c7a7..fff5f39 100644
> --- a/blivet/deviceaction.py
> +++ b/blivet/deviceaction.py
> @@ -31,19 +31,12 @@ from .formats import getFormat
>   from .storage_log import log_exception_info
>   from parted import partitionFlag, PARTITION_LBA
>   from .i18n import _, N_
> +from .callbacks import CreateFormatPreData, CreateFormatPostData
> +from .callbacks import ResizeFormatPreData, ResizeFormatPostData
>
>   import logging
>   log = logging.getLogger("blivet")
>
> -from contextlib import contextmanager
> -
> - at contextmanager
> -def progress_report_stub(message):
> -    # pylint: disable=unused-argument
> -    yield
> -
> -progress_report = progress_report_stub
> -
>   # The values are just hints as to the ordering.
>   # Eg: fsmod and devmod ordering depends on the mod (shrink -v- grow)
>   ACTION_TYPE_NONE = 0
> @@ -162,8 +155,15 @@ class DeviceAction(util.ObjectID):
>           """ apply changes related to the action to the device(s) """
>           self._applied = True
>
> -    def execute(self):
> -        """ perform the action """
> +    def execute(self, callbacks=None):
> +        """
> +        Perform the action.
> +
> +        :param callbacks: callbacks to be run when matching actions are
> +                          executed (see :meth:`~.blivet.Blivet.doIt`)
> +
> +        """
> +        # pylint: disable=unused-argument
>           if not self._applied:
>               raise RuntimeError("cannot execute unapplied action")
>
> @@ -288,7 +288,7 @@ class ActionCreateDevice(DeviceAction):
>           # FIXME: assert device.fs is None
>           DeviceAction.__init__(self, device)
>
> -    def execute(self):
> +    def execute(self, callbacks=None):
>           super(ActionCreateDevice, self).execute()
>           self.device.create()
>
> @@ -337,7 +337,7 @@ class ActionDestroyDevice(DeviceAction):
>           # XXX should we insist that device.fs be None?
>           DeviceAction.__init__(self, device)
>
> -    def execute(self):
> +    def execute(self, callbacks=None):
>           super(ActionDestroyDevice, self).execute()
>           self.device.destroy()
>
> @@ -448,7 +448,7 @@ class ActionResizeDevice(DeviceAction):
>           self.device.targetSize = self._targetSize
>           super(ActionResizeDevice, self).apply()
>
> -    def execute(self):
> +    def execute(self, callbacks=None):
>           super(ActionResizeDevice, self).execute()
>           self.device.resize()
>
> @@ -529,43 +529,49 @@ class ActionCreateFormat(DeviceAction):
>           self.device.format = self._format
>           super(ActionCreateFormat, self).apply()
>
> -    def execute(self):
> +    def execute(self, callbacks=None):
>           super(ActionCreateFormat, self).execute()
> -        msg = _("Creating %(type)s on %(device)s") % {"type": self.device.format.type, "device": self.device.path}
> -        with progress_report(msg):
> -            self.device.setup()
> -
> -            if isinstance(self.device, PartitionDevice):
> -                for flag in partitionFlag.keys():
> -                    # Keep the LBA flag on pre-existing partitions
> -                    if flag in [ PARTITION_LBA, self.format.partedFlag ]:
> -                        continue
> -                    self.device.unsetFlag(flag)
> -
> -                if self.format.partedFlag is not None:
> -                    self.device.setFlag(self.format.partedFlag)
> -
> -                if self.format.partedSystem is not None:
> -                    self.device.partedPartition.system = self.format.partedSystem
> -
> -                self.device.disk.format.commitToDisk()
> -
> -            self.device.format.create(device=self.device.path,
> -                                      options=self.device.formatArgs)
> -            # Get the UUID now that the format is created
> -            udev.settle()
> -            self.device.updateSysfsPath()
> -            info = udev.get_block_device(self.device.sysfsPath)
> -            # only do this if the format has a device known to udev
> -            # (the format might not have a normal device at all)
> -            if info:
> -                if self.device.format.type != "btrfs":
> -                    self.device.format.uuid = udev.device_get_uuid(info)
> -
> -                self.device.deviceLinks = udev.device_get_symlinks(info)
> -            elif self.device.format.type != "tmpfs":
> -                # udev lookup failing is a serious issue for anything other than tmpfs
> -                log.error("udev lookup failed for device: %s", self.device)
> +        if callbacks and callbacks.create_format_pre:
> +            msg = _("Creating %(type)s on %(device)s") % {"type": self.device.format.type, "device": self.device.path}
> +            callbacks.create_format_pre(CreateFormatPreData(msg))
> +
> +        self.device.setup()
> +
> +        if isinstance(self.device, PartitionDevice):
> +            for flag in partitionFlag.keys():
> +                # Keep the LBA flag on pre-existing partitions
> +                if flag in [ PARTITION_LBA, self.format.partedFlag ]:
> +                    continue
> +                self.device.unsetFlag(flag)
> +
> +            if self.format.partedFlag is not None:
> +                self.device.setFlag(self.format.partedFlag)
> +
> +            if self.format.partedSystem is not None:
> +                self.device.partedPartition.system = self.format.partedSystem
> +
> +            self.device.disk.format.commitToDisk()
> +
> +        self.device.format.create(device=self.device.path,
> +                                  options=self.device.formatArgs)
> +        # Get the UUID now that the format is created
> +        udev.settle()
> +        self.device.updateSysfsPath()
> +        info = udev.get_block_device(self.device.sysfsPath)
> +        # only do this if the format has a device known to udev
> +        # (the format might not have a normal device at all)
> +        if info:
> +            if self.device.format.type != "btrfs":
> +                self.device.format.uuid = udev.device_get_uuid(info)
> +
> +            self.device.deviceLinks = udev.device_get_symlinks(info)
> +        elif self.device.format.type != "tmpfs":
> +            # udev lookup failing is a serious issue for anything other than tmpfs
> +            log.error("udev lookup failed for device: %s", self.device)
> +
> +        if callbacks and callbacks.create_format_post:
> +            msg = _("Created %(type)s on %(device)s") % {"type": self.device.format.type, "device": self.device.path}
> +            callbacks.create_format_post(CreateFormatPostData(msg))
>
>       def cancel(self):
>           if not self._applied:
> @@ -626,7 +632,7 @@ class ActionDestroyFormat(DeviceAction):
>           self.device.format = None
>           super(ActionDestroyFormat, self).apply()
>
> -    def execute(self):
> +    def execute(self, callbacks=None):
>           """ wipe the filesystem signature from the device """
>           super(ActionDestroyFormat, self).execute()
>           status = self.device.status
> @@ -717,12 +723,18 @@ class ActionResizeFormat(DeviceAction):
>           self.device.format.targetSize = self._targetSize
>           super(ActionResizeFormat, self).apply()
>
> -    def execute(self):
> +    def execute(self, callbacks=None):
>           super(ActionResizeFormat, self).execute()
> -        msg = _("Resizing filesystem on %(device)s") % {"device": self.device.path}
> -        with progress_report(msg):
> -            self.device.setup(orig=True)
> -            self.device.format.doResize()
> +        if callbacks and callbacks.resize_format_pre:
> +            msg = _("Resizing filesystem on %(device)s") % {"device": self.device.path}
> +            callbacks.resize_format_pre(ResizeFormatPreData(msg))
> +
> +        self.device.setup(orig=True)
> +        self.device.format.doResize()
> +
> +        if callbacks and callbacks.resize_format_post:
> +            msg = _("Resized filesystem on %(device)s") % {"device": self.device.path}
> +            callbacks.resize_format_post(ResizeFormatPostData(msg))
>
>       def cancel(self):
>           if not self._applied:
> @@ -784,7 +796,7 @@ class ActionAddMember(DeviceAction):
>           self.container.parents.remove(self.device)
>           super(ActionAddMember, self).cancel()
>
> -    def execute(self):
> +    def execute(self, callbacks=None):
>           super(ActionAddMember, self).execute()
>           self.container.add(self.device)
>
> @@ -848,7 +860,7 @@ class ActionRemoveMember(DeviceAction):
>           self.container.parents.append(self.device)
>           super(ActionRemoveMember, self).cancel()
>
> -    def execute(self):
> +    def execute(self, callbacks=None):
>           super(ActionRemoveMember, self).execute()
>           self.container.remove(self.device)
>
> diff --git a/blivet/devicetree.py b/blivet/devicetree.py
> index bf90fb5..1a31538 100644
> --- a/blivet/devicetree.py
> +++ b/blivet/devicetree.py
> @@ -343,15 +343,22 @@ class DeviceTree(object):
>           devices = [a.name for a in active if any(d in disks for d in a.disks)]
>           return devices
>
> -    def processActions(self, dryRun=None):
> -        """ Execute all registered actions. """
> +    def processActions(self, callbacks=None, dryRun=None):
> +        """
> +        Execute all registered actions.
> +
> +        :param callbacks: callbacks to be invoked when actions are executed
> +        :type callbacks: :class:`~.callbacks.DoItCallbacks`
> +
> +        """
> +
>           self._preProcessActions()
>
>           for action in self._actions[:]:
>               log.info("executing action: %s", action)
>               if not dryRun:
>                   try:
> -                    action.execute()
> +                    action.execute(callbacks)
>                   except DiskLabelCommitError:
>                       # it's likely that a previous action
>                       # triggered setup of an lvm or md device.
> @@ -361,7 +368,7 @@ class DeviceTree(object):
>                           if dep.exists and dep.dependsOn(action.device.disk):
>                               dep.teardown(recursive=True)
>
> -                    action.execute()
> +                    action.execute(callbacks)
>
>                   udev.settle()
>                   for device in self._devices:
>



More information about the anaconda-patches mailing list