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

Anne Mulhern amulhern at redhat.com
Fri Mar 21 15:05:28 UTC 2014





----- 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.

Also, somewhere there is a typo: tyep.

Other than that, it looks OK to me.

- mulhern




More information about the anaconda-patches mailing list