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

Vratislav Podzimek vpodzime at redhat.com
Sun Aug 31 19:10:45 UTC 2014


On Fri, 2014-08-29 at 09:56 -0400, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Wednesday, August 27, 2014 10:17:52 AM
> > Subject: [blivet][master/rhel7-branch][PATCH] Allow user code provide	callbacks for various actions/events
> > 
> > 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.
> > 
> > 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()
> 
> Isn't it your intention to pass the callbacks arg to the
> superclass invocation of execute()? Same for the others
> above?
Yes, it is. Good catch! Fixing locally.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list