[PATCH 1/9] Add backend functions for container member set management.

David Lehman dlehman at redhat.com
Fri Mar 21 15:19:59 UTC 2014


On Fri, 2014-03-21 at 11:05 -0400, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "David Lehman" <dlehman at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Thursday, March 20, 2014 1:14:05 PM
> > Subject: [PATCH 1/9] Add backend functions for container member set management.
> > 
> > ---
> >  blivet/devicelibs/btrfs.py  | 14 ++++++++---
> >  blivet/devicelibs/lvm.py    | 57 ++++++++++++++++++++++++++++++++++++------
> >  blivet/devicelibs/mdraid.py | 61
> >  ++++++++++++++++++++++++++++++++++++++++++---
> >  3 files changed, 118 insertions(+), 14 deletions(-)
> > 
> > diff --git a/blivet/devicelibs/btrfs.py b/blivet/devicelibs/btrfs.py
> > index bca1242..e50112a 100644
> > --- a/blivet/devicelibs/btrfs.py
> > +++ b/blivet/devicelibs/btrfs.py
> > @@ -2,7 +2,7 @@
> >  # btrfs.py
> >  # btrfs functions
> >  #
> > -# Copyright (C) 2011  Red Hat, Inc.  All rights reserved.
> > +# Copyright (C) 2011-2014  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
> > @@ -78,9 +78,17 @@ def create_volume(devices, label=None, data=None,
> > metadata=None):
> >  
> >  # destroy is handled using wipefs
> >  
> > -# add device
> > +def add(mountpoint, device):
> > +    if not os.path.ismount(mountpoint):
> > +        raise ValueError("volume not mounted")
> > +
> > +    return btrfs(["device", "add", device, mountpoint])
> > +
> > +def remove(mountpoint, device):
> > +    if not os.path.ismount(mountpoint):
> > +        raise ValueError("volume not mounted")
> >  
> > -# remove device
> > +    return btrfs(["device", "delete", device, mountpoint])
> >  
> >  def create_subvolume(mountpoint, name):
> >      """Create a subvolume named name below mountpoint mountpoint."""
> > diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py
> > index 67f4c5f..e6c0509 100644
> > --- a/blivet/devicelibs/lvm.py
> > +++ b/blivet/devicelibs/lvm.py
> > @@ -2,7 +2,7 @@
> >  # lvm.py
> >  # lvm functions
> >  #
> > -# Copyright (C) 2009  Red Hat, Inc.  All rights reserved.
> > +# Copyright (C) 2009-2014  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
> > @@ -258,6 +258,23 @@ def pvscan(device):
> >      except LVMError as msg:
> >          raise LVMError("pvscan failed for %s: %s" % (device, msg))
> >  
> > +def pvmove(source, dest=None):
> > +    """ Move physical extents from one PV to another.
> > +
> > +        :param source: pv device path to move extents off of
> > +        :type source: str
> > +        :keyword dest: pv device path to move the extents onto
> > +        :type dest: str
> > +    """
> > +    args = ["pvmove"] + _getConfigArgs() + [source]
> > +    if dest:
> > +        args.extend(dest)
> > +
> > +    try:
> > +        lvm(args)
> > +    except LVMError as msg:
> > +        raise LVMError("pvmove failed for %s->%s: %s" % (source, dest, msg))
> > +
> >  def pvinfo(device):
> >      """
> >          If the PV was created with '--metadacopies 0', lvm will do some
> > @@ -334,25 +351,49 @@ def vgdeactivate(vg_name):
> >      except LVMError as msg:
> >          raise LVMError("vgdeactivate failed for %s: %s" % (vg_name, msg))
> >  
> > -def vgreduce(vg_name, pv_list, rm=False):
> > -    """ Reduce a VG.
> > +def vgreduce(vg_name, pv, missing=False):
> > +    """ Remove PVs from a VG.
> > +
> > +        :param pv: PV device path to remove
> > +        :type pv: str
> > +        :keyword missing: whether to remove missing PVs
> > +        :type missing: bool
> > +
> > +        When missing is True any specified PV is ignored and vgreduce is
> > +        instead called with the --removemissing option.
> >  
> > -    rm -> with RemoveMissing option.
> > -    Use pv_list when rm=False, otherwise ignore pv_list and call vgreduce
> > with
> > -    the --removemissing option.
> > +        .. note::
> > +
> > +            This function does not move extents off of the PV before
> > removing
> > +            it from the VG. You must do that first by calling
> > :func:`.pvmove`.
> >      """
> >      args = ["vgreduce"]
> >      args.extend(_getConfigArgs())
> > -    if rm:
> > +    if missing:
> >          args.extend(["--removemissing", "--force", vg_name])
> >      else:
> > -        args.extend([vg_name] + pv_list)
> > +        args.extend([vg_name, pv])
> >  
> >      try:
> >          lvm(args)
> >      except LVMError as msg:
> >          raise LVMError("vgreduce failed for %s: %s" % (vg_name, msg))
> >  
> > +def vgextend(vg_name, pv):
> > +    """ Add a PV to a VG.
> > +
> > +        :param vg_name: the name of the VG
> > +        :type vg_name: str
> > +        :param pv: device path of PV to add
> > +        :tyep pv: str
> > +    """
> > +    args = ["vgextend"] + _getConfigArgs() + [vg_name, pv]
> > +
> > +    try:
> > +        lvm(args)
> > +    except LVMError as msg:
> > +        raise LVMError("vgextend failed for %s: %s" % (vg_name, msg))
> > +
> >  def vginfo(vg_name):
> >      args = ["vgs", "--noheadings", "--nosuffix", "--nameprefixes",
> >      "--unquoted",
> >              "--units", "m",
> > diff --git a/blivet/devicelibs/mdraid.py b/blivet/devicelibs/mdraid.py
> > index b5bf688..a7511ac 100644
> > --- a/blivet/devicelibs/mdraid.py
> > +++ b/blivet/devicelibs/mdraid.py
> > @@ -2,7 +2,7 @@
> >  # mdraid.py
> >  # mdraid functions
> >  #
> > -# Copyright (C) 2009  Red Hat, Inc.  All rights reserved.
> > +# Copyright (C) 2009-2014  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
> > @@ -134,8 +134,37 @@ def mddestroy(device):
> >      except MDRaidError as msg:
> >          raise MDRaidError("mddestroy failed for %s: %s" % (device, msg))
> >  
> > -def mdadd(device):
> > -    args = ["--incremental", "--quiet"]
> > +def mdadd(array, device, incremental=False, raid_devices=None):
> > +    """ Add a device to an array.
> > +
> > +        :param array: path to the array to add the device to
> > +        :type array: str
> > +        :param device: path to the device to add to the array
> > +        :type device: str
> > +        :keyword incremental: add the device incrementally (see note below)
> > +        :type incremental: bool
> > +        :keyword raid_devices: the number of active member devices
> > +        :type raid_devices: int
> > +
> > +        The raid_devices parameter is used when adding devices to a raid0
> > array
> > +        since raid0 does not allow spares.
> > +
> > +        Whether the new device will be added as a spare or an active member
> > is
> > +        decided by mdadm.
> > +
> > +        .. note::
> > +
> > +            Incremental add is used during block device discovery and is a
> > +            different operation than changing the member set of an array.
> > +
> > +    """
> > +    if incremental:
> > +        args = ["--incremental", "--quiet"]
> > +    elif raid_devices is None:
> > +        args = [array, "--add"]
> > +    else:
> > +        args = ["--grow", array, "--raid-devices", str(raid_devices),
> > "--add"]
> > +
> >      args.append(device)
> >  
> >      try:
> > @@ -143,6 +172,32 @@ def mdadd(device):
> >      except MDRaidError as msg:
> >          raise MDRaidError("mdadd failed for %s: %s" % (device, msg))
> >  
> > +def mdremove(array, device, fail=False):
> > +    """ Remove a device from an array.
> > +
> > +        :param array: path to the array to remove the device from
> > +        :type array: str
> > +        :param device: path to the device to remove
> > +        :type device: str
> > +        :keyword fail: mark the device as failed before removing it
> > +        :type fail: bool
> > +
> > +        .. note::
> > +
> > +            Only spares and failed devices can be removed. To remove an
> > active
> > +            member device you must specify fail=True.
> > +    """
> > +    args = [array]
> > +    if fail:
> > +        args.extend(["--fail", device])
> > +
> > +    args.extend(["--remove", device])
> > +
> > +    try:
> > +        mdadm(args)
> > +    except MDRaidError as msg:
> > +        raise MDRaidError("mdremove failed for %s: %s" % (device, msg))
> > +
> >  def mdactivate(device, members=[], uuid=None):
> >      """Assemble devices given by members into a single device.
> >  
> > --
> > 1.8.5.3
> > 
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > 
> 
> For sphinx doc, if the parameter type is something simple like
> str you can do just 
> 
> :param str pv: ...
> 
> instead of a two liner like:
> 
> :param pv: ...
> :type pv: str
> 
> I think the one liner is preferable.

That is better. Fixed.

> 
> Also, somewhere there is a typo: tyep.

Also fixed.

> 
> Other than that, it looks OK to me.
> 
> - mulhern
> 
> 
> _______________________________________________
> 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