[Yum-devel] fs-snapshot plugin: add included_vgs and excluded_vgs options to [lvm] section

[Mike Snitzer]

On Wed, Feb 29 2012 at  1:45pm -0500,
Alfredo Moralejo <amoralej at redhat.com> wrote:

> Hi,
> 
> I've been testing yum-plugin-fs-snapshot with LVM snapshots. All
> test worked find but I missed a functionality. Tipically, OS and
> application data are located in separated Volume Groups and most
> times, when I update or install software with yum, I'll only want to
> get snapshots of the OS logical volumes, not data ones. In this case
> the only available option "exclude" can be difficult to manage (I
> may have many data filesystems) and a "include only a specific VG"
> option would be convenient. I've created following patch to
> implement "included_vgs" and "excluded_vgs" options in the lvm
> section of fs-snapshot.conf file:
> 
> -If included_vgs option exists, only the logical volumes included in
> those volume groups are snapshoted.
> -If excluded_vgs option exists, all logical volumes but those in the
> specified volume groups are snapshoted.
> 

Except for the few nits below, looks fine by me.  

> diff --git a/plugins/fs-snapshot/fs-snapshot.py
> b/plugins/fs-snapshot/fs-snapshot.py
> index 5094881..71caa3c 100644
> --- a/plugins/fs-snapshot/fs-snapshot.py
> +++ b/plugins/fs-snapshot/fs-snapshot.py
> @@ -102,6 +102,25 @@ def inspect_volume_lvm(conduit, volume):
>                            "kernel doesn't support snapshot-merge" %
> device)
>              return 0
>          volume[lvm_key] = 1
> +
> +    # Check if include_vgs is defined in [lvm] section.
> +    # If so, we compare VG name with include_vgs list
>
> +    included_vgs = conduit.confString('lvm', 'included_vgs',
> default="").split()

No need for the emtpy newlines.

> +    if len(included_vgs) > 0:
> +       vg = device.split("/")[2]

Looks like indent isn't quite right on the previous line?

> +        if vg not in included_vgs :
> +           return 0
> +
> +    # Check if excluded_vgs is defined in [lvm] section.
> +    # If so, we compare VG name with excluded_vgs list
> +
> +    excluded_vgs = conduit.confString('lvm', 'excluded_vgs',
> default="").split()

Again.. the above empty newline isn't needed.