[blivet] Remove unused code using pyblock

[Vratislav Podzimek]

The pyblock module is quite a pain if we want to make blivet python3 compatible,
it kills tracing if we want to turn it on for anaconda and is in general quite a
crazy old CPython code. That's why I'd like to drop it as blivet's dependency by
implementing the required stuff in the libblockdev library. However, I'm not
much a fan of adding some cryptic C code only for something that is actually
never called and exists as a piece of code only for documentation purposes.

Vratislav Podzimek (1):
  Remove two methods that are never called

blivet/devices.py | 38 --------------------------------------
 1 file changed, 38 deletions(-)

[PATCH] Remove two methods that are never called

[Vratislav Podzimek]

These two methods are never called and including them just for documentation
doesn't make much sense. They should live in the documentation if they are just
for documentation purposes.

Signed-off-by: Vratislav Podzimek 
---
 blivet/devices.py | 38 --------------------------------------
 1 file changed, 38 deletions(-)

diff --git a/blivet/devices.py b/blivet/devices.py
index a0f8f56..dd9236d 100644
--- a/blivet/devices.py
+++ b/blivet/devices.py
@@ -1818,21 +1818,6 @@ class PartitionDevice(StorageDevice):
                 except (errors.DMError, OSError):
                     pass

-    def deactivate(self):
-        """
-        This is never called. For instructional purposes only.
-
-        We do not want multipath partitions disappearing upon their teardown().
-        """
-        if self.parents[0].type == 'dm-multipath':
-            devmap = block.getMap(major=self.major, minor=self.minor)
-            if devmap:
-                try:
-                    block.removeDeviceMap(devmap)
-                except Exception as e:
-                    raise errors.DeviceTeardownError("failed to tear down device-mapper partition %s: %s" % (self.name, e))
-            udev.settle()
-
     def _getSize(self):
         """ Get the device's size. """
         size = self._size
@@ -4252,29 +4237,6 @@ class MultipathDevice(DMDevice):
         else:
             self.parents.append(parent)

-    def deactivate(self):
-        """
-        This is never called, included just for documentation.
-
-        If we called this during teardown(), we wouldn't be able to get parted
-        object because /dev/mapper/mpathX wouldn't exist.
-        """
-        if self.exists and os.path.exists(self.path):
-            #self.teardownPartitions()
-            #rc = util.run_program(["multipath", '-f', self.name])
-            #if rc:
-            #    raise MPathError("multipath deactivation failed for '%s'" %
-            #                    self.name)
-            bdev = block.getDevice(self.name)
-            devmap = block.getMap(major=bdev[0], minor=bdev[1])
-            if devmap.open_count:
-                return
-            try:
-                block.removeDeviceMap(devmap)
-            except Exception as e:
-                raise errors.MPathError("failed to tear down multipath device %s: %s"
-                                % (self.name, e))
-
     def _setup(self, orig=False):
         """ Open, or set up, a device. """
         log_method_call(self, self.name, orig=orig, status=self.status,

[PATCH2] Remove an unused closure function

[Vratislav Podzimek]

Signed-off-by: Vratislav Podzimek 
---
 blivet/devicetree.py | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/blivet/devicetree.py b/blivet/devicetree.py
index bf90fb5..c4cb1b7 100644
--- a/blivet/devicetree.py
+++ b/blivet/devicetree.py
@@ -1659,14 +1659,6 @@ class DeviceTree(object):
         major = udev.device_get_major(info)
         minor = udev.device_get_minor(info)

-        def _all_ignored(rss):
-            retval = True
-            for rs in rss:
-                if rs.name not in self.ignoredDisks:
-                    retval = False
-                    break
-            return retval
-
         # Have we already created the DMRaidArrayDevice?
         rss = block.getRaidSetFromRelatedMem(uuid=uuid, name=name,
                                             major=major, minor=minor)

[PATCH2] Remove an unused closure function

[Anne Mulhern]

----- Original Message -----
> From: "Vratislav Podzimek" 
> To: [e-mail]
> Sent: Friday, July 18, 2014 5:13:24 AM
> Subject: [PATCH2] Remove an unused closure function
> Signed-off-by: Vratislav Podzimek
>
> blivet/devicetree.py | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/blivet/devicetree.py b/blivet/devicetree.py
> index bf90fb5..c4cb1b7 100644
> --- a/blivet/devicetree.py
> +++ b/blivet/devicetree.py
> @@ -1659,14 +1659,6 @@ class DeviceTree(object):
>          major = udev.device_get_major(info)
>          minor = udev.device_get_minor(info)
>    def _all_ignored(rss):
>        retval = True
>        for rs in rss:
>            if rs.name not in self.ignoredDisks:
>                retval = False
>                break
>        return retval
>    # Have we already created the DMRaidArrayDevice?
>    rss = block.getRaidSetFromRelatedMem(uuid=uuid, name=name,
>                                        major=major, minor=minor)
>
> --
> 1.9.3
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

Ack.

[PATCH] Remove two methods that are never called

[Anne Mulhern]

----- Original Message -----
> From: "Vratislav Podzimek" 
> To: [e-mail]
> Sent: Friday, July 18, 2014 4:15:35 AM
> Subject: [PATCH] Remove two methods that are never called
>
> These two methods are never called and including them just for documentation
> doesn't make much sense. They should live in the documentation if they are
> just
> for documentation purposes.
> Signed-off-by: Vratislav Podzimek
>
> blivet/devices.py | 38 --------------------------------------
>  1 file changed, 38 deletions(-)
>
> diff --git a/blivet/devices.py b/blivet/devices.py
> index a0f8f56..dd9236d 100644
> --- a/blivet/devices.py
> +++ b/blivet/devices.py
> @@ -1818,21 +1818,6 @@ class PartitionDevice(StorageDevice):
>                  except (errors.DMError, OSError):
>                      pass
> def deactivate(self):
>    """
>    This is never called. For instructional purposes only.
>    We do not want multipath partitions disappearing upon their
>
> teardown().
>    """
>    if self.parents[0].type == 'dm-multipath':
>        devmap = block.getMap(major=self.major, minor=self.minor)
>        if devmap:
>            try:
>                block.removeDeviceMap(devmap)
>            except Exception as e:
>                raise errors.DeviceTeardownError("failed to tear down
>
> device-mapper partition %s: %s" % (self.name, e))
>        udev.settle()
> def _getSize(self):
>        """ Get the device's size. """
>        size = self._size
>
> @@ -4252,29 +4237,6 @@ class MultipathDevice(DMDevice):
>          else:
>              self.parents.append(parent)
> def deactivate(self):
>    """
>    This is never called, included just for documentation.
>    If we called this during teardown(), we wouldn't be able to get
>
> parted
>    object because /dev/mapper/mpathX wouldn't exist.
>    """
>    if self.exists and os.path.exists(self.path):
>        #self.teardownPartitions()
>        #rc = util.run_program(["multipath", '-f', self.name])
>        #if rc:
>        #    raise MPathError("multipath deactivation failed for '%s'" %
>        #                    self.name)
>        bdev = block.getDevice(self.name)
>        devmap = block.getMap(major=bdev[0], minor=bdev[1])
>        if devmap.open_count:
>            return
>        try:
>            block.removeDeviceMap(devmap)
>        except Exception as e:
>            raise errors.MPathError("failed to tear down multipath
>
> device %s: %s"
>                            % (self.name, e))
> def _setup(self, orig=False):
>        """ Open, or set up, a device. """
>        log_method_call(self, self.name, orig=orig, status=self.status,
>
> --
> 1.9.3
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

There are four deactivate methods defined in devices.py and one
is actually invoked elsewhere.

Is there any broader action that can be taken here, like losing them
all?

- mulhern

[PATCH] Remove two methods that are never called

[Vratislav Podzimek]

On Fri, 2014-07-18 at 07:44 -0400, Anne Mulhern wrote:
> ----- Original Message -----
> > From: "Vratislav Podzimek" 
> > To: [e-mail]
> > Sent: Friday, July 18, 2014 4:15:35 AM
> > Subject: [PATCH] Remove two methods that are never called
> >
> > These two methods are never called and including them just for documentation
> > doesn't make much sense. They should live in the documentation if they are
> > just
> > for documentation purposes.
> > Signed-off-by: Vratislav Podzimek
> >
> > blivet/devices.py | 38 --------------------------------------
> >  1 file changed, 38 deletions(-)
> >
> > diff --git a/blivet/devices.py b/blivet/devices.py
> > index a0f8f56..dd9236d 100644
> > --- a/blivet/devices.py
> > +++ b/blivet/devices.py
> > @@ -1818,21 +1818,6 @@ class PartitionDevice(StorageDevice):
> >                  except (errors.DMError, OSError):
> >                      pass
> > def deactivate(self):
> >    """
> >    This is never called. For instructional purposes only.
> >    We do not want multipath partitions disappearing upon their
> >
> > teardown().
> >    """
> >    if self.parents[0].type == 'dm-multipath':
> >        devmap = block.getMap(major=self.major, minor=self.minor)
> >        if devmap:
> >            try:
> >                block.removeDeviceMap(devmap)
> >            except Exception as e:
> >                raise errors.DeviceTeardownError("failed to tear down
> >
> > device-mapper partition %s: %s" % (self.name, e))
> >        udev.settle()
> > def _getSize(self):
> >        """ Get the device's size. """
> >        size = self._size
> >
> > @@ -4252,29 +4237,6 @@ class MultipathDevice(DMDevice):
> >          else:
> >              self.parents.append(parent)
> > def deactivate(self):
> >    """
> >    This is never called, included just for documentation.
> >    If we called this during teardown(), we wouldn't be able to get
> >
> > parted
> >    object because /dev/mapper/mpathX wouldn't exist.
> >    """
> >    if self.exists and os.path.exists(self.path):
> >        #self.teardownPartitions()
> >        #rc = util.run_program(["multipath", '-f', self.name])
> >        #if rc:
> >        #    raise MPathError("multipath deactivation failed for '%s'" %
> >        #                    self.name)
> >        bdev = block.getDevice(self.name)
> >        devmap = block.getMap(major=bdev[0], minor=bdev[1])
> >        if devmap.open_count:
> >            return
> >        try:
> >            block.removeDeviceMap(devmap)
> >        except Exception as e:
> >            raise errors.MPathError("failed to tear down multipath
> >
> > device %s: %s"
> >                            % (self.name, e))
> > def _setup(self, orig=False):
> >        """ Open, or set up, a device. """
> >        log_method_call(self, self.name, orig=orig, status=self.status,
> >
> > --
> > 1.9.3
> >
> > anaconda-patches mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
> There are four deactivate methods defined in devices.py and one
> is actually invoked elsewhere.
>
> Is there any broader action that can be taken here, like losing them
> all?

I've just pushed the two patches as a first step to the right change. I
don't quite understand the difference between deactivate() and
teardown() and looking at the code they seem to be really closely
related. Maybe we should drop all deactivate() methods in devices.py and
just use teardown() methods?

[PATCH] Remove two methods that are never called

[David Lehman]

On 07/28/2014 12:18 PM, Vratislav Podzimek wrote:
> On Fri, 2014-07-18 at 07:44 -0400, Anne Mulhern wrote:
> > ----- Original Message -----
> > > From: "Vratislav Podzimek" 
> > > To: [e-mail]
> > > Sent: Friday, July 18, 2014 4:15:35 AM
> > > Subject: [PATCH] Remove two methods that are never called
> > >
> > > These two methods are never called and including them just for documentation
> > > doesn't make much sense. They should live in the documentation if they are
> > > just
> > > for documentation purposes.
> > > Signed-off-by: Vratislav Podzimek
> > >
> > > blivet/devices.py | 38 --------------------------------------
> > >   1 file changed, 38 deletions(-)
> > >
> > > diff --git a/blivet/devices.py b/blivet/devices.py
> > > index a0f8f56..dd9236d 100644
> > > --- a/blivet/devices.py
> > > +++ b/blivet/devices.py
> > > @@ -1818,21 +1818,6 @@ class PartitionDevice(StorageDevice):
> > >                   except (errors.DMError, OSError):
> > >                       pass
> > > def deactivate(self):
> > >    """
> > >    This is never called. For instructional purposes only.
> > >    We do not want multipath partitions disappearing upon their
> > >
> > > teardown().
> > >    """
> > >    if self.parents[0].type == 'dm-multipath':
> > >        devmap = block.getMap(major=self.major, minor=self.minor)
> > >        if devmap:
> > >            try:
> > >                block.removeDeviceMap(devmap)
> > >            except Exception as e:
> > >                raise errors.DeviceTeardownError("failed to tear down
> > >
> > > device-mapper partition %s: %s" % (self.name, e))
> > >        udev.settle()
> > > def _getSize(self):
> > >     """ Get the device's size. """
> > >     size = self._size
> > >
> > > @@ -4252,29 +4237,6 @@ class MultipathDevice(DMDevice):
> > >           else:
> > >               self.parents.append(parent)
> > > def deactivate(self):
> > >    """
> > >    This is never called, included just for documentation.
> > >    If we called this during teardown(), we wouldn't be able to get
> > >
> > > parted
> > >    object because /dev/mapper/mpathX wouldn't exist.
> > >    """
> > >    if self.exists and os.path.exists(self.path):
> > >        #self.teardownPartitions()
> > >        #rc = util.run_program(["multipath", '-f', self.name])
> > >        #if rc:
> > >        #    raise MPathError("multipath deactivation failed for '%s'" %
> > >        #                    self.name)
> > >        bdev = block.getDevice(self.name)
> > >        devmap = block.getMap(major=bdev[0], minor=bdev[1])
> > >        if devmap.open_count:
> > >            return
> > >        try:
> > >            block.removeDeviceMap(devmap)
> > >        except Exception as e:
> > >            raise errors.MPathError("failed to tear down multipath
> > >
> > > device %s: %s"
> > >                            % (self.name, e))
> > > def _setup(self, orig=False):
> > >     """ Open, or set up, a device. """
> > >     log_method_call(self, self.name, orig=orig, status=self.status,
> > >
> > > --
> > > 1.9.3
> > >
> > > anaconda-patches mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> >
> > There are four deactivate methods defined in devices.py and one
> > is actually invoked elsewhere.
> >
> > Is there any broader action that can be taken here, like losing them
> > all?
>
> I've just pushed the two patches as a first step to the right change. I
> don't quite understand the difference between deactivate() and
> teardown() and looking at the code they seem to be really closely
> related. Maybe we should drop all deactivate() methods in devices.py and
> just use teardown() methods?

The original idea was that dmraid and mpath should never be deactivated 
by the installer, so we kept the code but put it in a method that would 
not be called as a normal part of installation. One thing you could do 
would be to rename the methods to match the other classes and set the 
controllable attribute to False, which should have the same effect.

David