[blivet][master/rhel7-branch] Re-write the DASD storage code. (#1001070)

Samantha N. Bueno sbueno+anaconda at redhat.com
Wed Feb 12 16:01:46 UTC 2014


On Wed, Feb 12, 2014 at 10:04:30AM +0100, Vratislav Podzimek wrote:
> On Tue, 2014-02-11 at 18:01 -0500, Samantha N. Bueno wrote:
> > This gets rid of the DASD class and instead introduces a small number of
> > functions to interface with and manage DASDs. This is completely
> > divorced from the anaconda code and much more lightweight.
> > 
> > (The corresponding Fedora bug for this is 859997.)
> > 
> > Resolves:rhbz#1001070
> > ---
> >  blivet/__init__.py        |  13 ++-
> >  blivet/dasd.py            | 222 ----------------------------------------------
> >  blivet/deviceaction.py    |   1 +
> >  blivet/devicelibs/dasd.py | 134 ++++++++++++++++++++++++++++
> >  blivet/devicetree.py      |   6 +-
> >  5 files changed, 144 insertions(+), 232 deletions(-)
> >  delete mode 100644 blivet/dasd.py
> >  create mode 100644 blivet/devicelibs/dasd.py
> 
> > diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py
> > index a53fdb5..f90208d 100644
> > --- a/blivet/deviceaction.py
> > +++ b/blivet/deviceaction.py
> > @@ -315,6 +315,7 @@ class ActionDestroyDevice(DeviceAction):
> >              device.teardown()
> >  
> >      def execute(self):
> > +        #import pdb; pdb.set_trace()
> Debugging leftover.

Gone.
 
> >          self.device.destroy()
> >  
> >          # Make sure libparted does not keep cached info for this device
> > diff --git a/blivet/devicelibs/dasd.py b/blivet/devicelibs/dasd.py
> > new file mode 100644
> > index 0000000..8486bee
> > --- /dev/null
> > +++ b/blivet/devicelibs/dasd.py
> > @@ -0,0 +1,134 @@
> > +#
> > +# dasd.py - DASD functions
> > +#
> > +# Copyright (C) 2013 Red Hat, Inc.  All rights reserved.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty 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, see <http://www.gnu.org/licenses/>.
> > +#
> > +# Red Hat Author(s): Samantha N. Bueno
> > +#
> > +
> > +import os
> > +from blivet.errors import DasdFormatError
> > +from blivet.devices import deviceNameToDiskByPath
> > +from blivet import util
> > +from blivet import arch
> > +from blivet.udev import udev_trigger
> > +
> > +import logging
> > +log = logging.getLogger("blivet")
> > +
> > +import gettext
> > +_ = lambda x: gettext.ldgettext("blivet", x)
> > +P_ = lambda x, y, z: gettext.ldngettext("blivet", x, y, z)
> > +
> > +def get_dasd_ports():
> > +    """ Return comma delimited string of valid DASD ports. """
> > +    ports = []
> > +
> > +    f = open("/proc/dasd/devices", "r")
> You can use
> 
>     with open("/proc/dasd/devices", "r") as f:
> 
> block instead

Fixed locally.
 
> > +    lines = map(lambda x: x.strip(), f.readlines())
>     lines = (line.strip() for line in f.xreadlines())
> 
> would avoid storing the whole file in memory (I know it's tiny).

Tiny but nice.:) Fixed locally.
 
> > +    f.close()
> > +
> > +    for line in lines:
> > +        if "unknown" in line:
> > +            continue
> > +
> > +        if "(FBA )" in line or "(ECKD)" in line:
> > +            ports.append(line.split('(')[0])
> > +
> > +    return ','.join(ports)
> > +
> > +def format_dasd(dasd):
> > +    """ Run dasdfmt on a DASD. Aside from one type of device noted below, this
> > +        function _does not_ check if a DASD needs to be formatted, but rather,
> > +        assumes the list passed needs formatting.
> > +
> > +        We don't need to show or update any progress bars, since disk actions
> > +        will be taking place all in the progress hub, which is just one big
> > +        progress bar.
> > +    """
> > +    try:
> > +        rc = util.run_program(["/sbin/dasdfmt", "-y", "-d", "cdl", "-b", "4096", dasd.path])
> > +    except Exception as err:
> > +        raise DasdFormatError(err)
> > +
> > +    if rc:
> > +        raise DasdFormatError("dasdfmt failed: %s" % rc)
> > +
> > +def clear_dasd_list(dasds):
> > +    """ Zero out the list of DASDs. """
> > +    dasds = []
> > +    return dasds
> These two lines are the same as 'return []'. I guess the intention here
> is to clear the list?

Yes.
 
> > +
> > +def make_dasd_list(dasds, devicetree):
> > +    """ Create a list of DASDs recognized by the system. """
> > +    if not arch.isS390():
> > +        return
> > +
> > +    # Trigger udev data about the dasd devices on the system
> > +    udev_trigger(action="change", name="dasd*")
> > +    log.info("Generating DASD list...")
> > +    for dev in devicetree.getDevicesByType("dasd"):
> > +        if dev not in dasds:
> > +            dasds.append(dev)
> > +
> > +    return dasds
> > +
> > +def make_unformatted_dasd_list(dasds, exclusiveDisks):
> > +    """ Return a list of DASDS which are not formatted. """
> > +    unformatted = []
> > +    
> > +    for dasd in dasds:
> > +        if dasd_needs_format(dasd, exclusiveDisks):
> > +            unformatted.append(dasd)
> > +
> > +    return unformatted
> > +
> > +def dasd_needs_format(dasd, exclusiveDisks):
> > +    """ Check if a DASD needs to have dasdfmt run against it or not.
> > +        Return True if we do need dasdfmt, False if not.
> > +    """
> > +    statusfile = "/sys/block/%s/device/status" % (dasd.name,)
> > +    if not os.path.isfile(statusfile):
> > +        return False
> > +
> > +    f = open(statusfile, "r")
> > +    status = f.read().strip()
> > +    f.close()
> You can use the with block here too and in another place down below.

Fixed locally in both places.
 
> > +
> > +    if status in ["unformatted"] and dasd not in exclusiveDisks:
> > +        bypath = deviceNameToDiskByPath(dasd.name)
> > +        if not bypath:
> > +            bypath = "/dev/" + dasd
> > +
> > +        log.info("  %s (%s) status is %s, needs dasdfmt" % (dasd.name, bypath,
> > +                                                            status,))
> > +        return True
> > +
> > +    return False
> > +
> > +
> > +def write_dasd_conf(disks, ROOT_PATH):
> > +    """ Write /etc/dasd.conf to target system for all DASD devices
> > +        configured during installation.
> > +    """
> > +    if disks == {}:
> > +        return
> > +
> > +    f = open(os.path.realpath(ROOT_PATH + "/etc/dasd.conf"), "w")
> > +    for dasd in sorted(disks, key=lambda d: d.name):
> > +        fields = [dasd.busid] + dasd.getOpts()
> > +        f.write("%s\n" % (" ".join(fields),))
> > +    f.close()
> 
> -- 
> Vratislav Podzimek
> 
> Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches


More information about the anaconda-patches mailing list