Re: [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