Re: [PATCH] Better naming for LVM volume groups and logical volumes (#461682)

[Hans de Goede]

David Cantrell wrote:
> Try to name volume groups as vg_HOSTNAME and logical volumes as
> lv_MOUNTPOINT, if we can.  Swap partitions will be lv_swapNN where
> NN is a unique number in the instance where more than one swap
> partition in use.  The / partition will get the name lv_root.
>
> autopart.py |   26 ++++++++++++++++++++++----
>  lvm.py      |   57 +++++++++++++++++++++++++++++++++++++--------------------
>  2 files changed, 59 insertions(+), 24 deletions(-)

I've got one last comment. Above you claim that swap partitions will be called 
lv_swapNN, but ...
> diff --git a/autopart.py b/autopart.py
> index ff62f37..64237dd 100644
> --- a/autopart.py
> +++ b/autopart.py
> @@ -1463,7 +1463,7 @@ def doAutoPartition(anaconda):
>                      # FIXME: this is a hack so that autopartition'd vgs
>                      # can have a unique name
>                      if req.autoname == 1 and req.volumeGroupName == "lvm":
>                    n = lvm.createSuggestedVGName(partitions)
>                    n = lvm.createSuggestedVGName(partitions, anaconda.id.network)
>                    req.volumeGroupName = n
>
>        if (isinstance(req, partRequests.LogicalVolumeRequestSpec)):
>
> @@ -1649,6 +1649,7 @@ def autoCreateLVMPartitionRequests(autoreq):
>      requests.append(nr)
>  volnum = 0
> for (mntpt, fstype, minsize, maxsize, grow, format, asvol) in autoreq:
>        if fstype:
>            ptype = fsset.fileSystemTypeGet(fstype)
>
> @@ -1663,17 +1664,34 @@ def autoCreateLVMPartitionRequests(autoreq):
>                                                      grow = grow,
>                                                      format = format)
>          else:
>        # try to incorporate the mount point in to the logical volume name
>        if mntpt is not None and mntpt != '':
>            if mntpt == '/':
>                lvtemplate = 'lv_root'
>            else:
>                tmp = string.strip(mntpt)
>                tmp = tmp.replace('/', '_')
>                while tmp.startswith('_'):
>                    tmp = tmp[1:]
>                lvtemplate = "lv_%s" % (tmp,)
>        else:
>            if ptype == fsset.fileSystemTypeGet("swap"):
>                lvtemplate = "lv_swap"

I don't see you adding NN here, now I think that in this code path we can never 
have multiple swaps, but I'm not sure.

<snip>

Regards,

Hans