[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