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

[David Cantrell]

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 |   23 +++++++++++++++--
 lvm.py      |   76 +++++++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 74 insertions(+), 25 deletions(-)

diff --git a/autopart.py b/autopart.py
index f335602..544c403 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)):
@@ -1663,17 +1663,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 not tmp.startswith('_'):
+                        tmp = tmp[1:]
+
+                    lvtemplate = "lv_%s" % (tmp,)
+            else:
+                if ptype == fsset.fileSystemTypeGet("swap"):
+                    lvtemplate = "lv_swap"
+                else:
+                    lvtemplate = "LogVol"
+
             newrequest = partRequests.LogicalVolumeRequestSpec(ptype,
                                                                mountpoint = mntpt,
                                                                size = minsize,
                                                                maxSizeMB = maxsize,
                                                                grow = grow,
                                                                format = format,
-                                                               lvname = "LogVol%02d" %(volnum,),
+                                                               lvname = "%s" %(lvtemplate,),
                                                                volgroup = "lvm")
             volnum += 1

-        
         requests.append(newrequest)

return requests
diff --git a/lvm.py b/lvm.py
index 70d5108..ed75fb8 100644
--- a/lvm.py
+++ b/lvm.py
@@ -515,45 +515,77 @@ def getMaxLVSize(pe):
         else:
             return (16*1024*1024) #Max is 16TiB

-def createSuggestedVGName(partitions):
+def createSuggestedVGName(partitions, network):
     """Given list of partition requests, come up with a reasonable VG name

partitions - list of requests
     """
-    i = 0
-    while 1:
-	tmpname = "VolGroup%02d" % (i,)
-	if not partitions.isVolumeGroupNameInUse(tmpname):
-	    break

-	i = i + 1
-	if i>99:
-	    tmpname = ""
+    # try to create a volume group name incorporating the hostname
+    if network.hostname is not None and network.hostname != '':
+        if network.hostname.find('.') != -1:
+            vgtemplate = "vg_%s" % (network.hostname.split('.')[0].lower(),)
+        else:
+            vgtemplate = "vg_%s" % (network.hostname.lower(),)
+    else:
+        vgtemplate = "VolGroup"
+
+    if not partitions.isVolumeGroupNameInUse(vgtemplate):
+        return vgtemplate
+    else:
+        i = 0
+        while 1:
+            tmpname = "%s%02d" % (vgtemplate, i,)
+            if not partitions.isVolumeGroupNameInUse(tmpname):
+                break
+
+            i += 1
+            if i > 99:
+                tmpname = ""
+
+        return tmpname

-    return tmpname
-	    
 def createSuggestedLVName(logreqs):
     """Given list of LV requests, come up with a reasonable LV name

partitions - list of LV requests for this VG
     """
+
+    # try to incorporate the mount point in to the logical volume name
+    if lv.mountpoint is not None and lv.mountpoint != '':
+        if lv.mountpoint == '/':
+            lvtemplate = 'lv_root'
+        else:
+            tmp = string.strip(lv.mountpoint)
+            tmp = tmp.replace('/', '_')
+
+            while not tmp.startswith('_'):
+                tmp = tmp[1:]
+
+            lvtemplate = "lv_%s" % (tmp,)
+    else:
+        lvtemplate = "LogVol"
+
     i = 0

lnames = []
     for lv in logreqs:
-	lnames.append(lv.logicalVolumeName)
-    
-    while 1:
-	tmpname = "LogVol%02d" % (i,)
-	if (logreqs is None) or (tmpname not in lnames):
-	    break
+        lnames.append(lv.logicalVolumeName)
+
+    if (logreqs is None) or (lvtemplate not in lnames):
+        return lvtemplate
+    else:
+        while 1:
+            tmpname = "%s%02d" % (lvtemplate, i,)
+            if (logreqs is None) or (tmpname not in lnames):
+                break
+
+            i += 1
+            if i > 99:
+                tmpname = ""

-	i = i + 1
-	if i>99:
-	    tmpname = ""
+        return tmpname

-    return tmpname
-	    
 def getVGUsedSpace(vgreq, requests, diskset):
     vgused = 0
     for request in requests.requests:

[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 |   23 +++++++++++++++--
>  lvm.py      |   76 +++++++++++++++++++++++++++++++++++++++++-----------------
>  2 files changed, 74 insertions(+), 25 deletions(-)

See my comments below.
> diff --git a/autopart.py b/autopart.py
> index f335602..544c403 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)):
>
> @@ -1663,17 +1663,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 not tmp.startswith('_'):
>                    tmp = tmp[1:]

Shouldn't that be:
                     while tmp.startswith('_'):
                         tmp = tmp[1:]

I assume the purpose is to strip any prefixed / replaced now by _ ?
>                lvtemplate = "lv_%s" % (tmp,)
>        else:
>            if ptype == fsset.fileSystemTypeGet("swap"):
>                lvtemplate = "lv_swap"
>            else:
>                lvtemplate = "LogVol"

Ok, so if we don't know what todo we fallback to the old behavior.
>          newrequest = partRequests.LogicalVolumeRequestSpec(ptype,
>                                                             mountpoint = mntpt,
>                                                             size = minsize,
>                                                             maxSizeMB = maxsize,
>                                                             grow = grow,
>                                                             format = format,
>                                                           lvname = "LogVol%02d" %(volnum,),
>                                                           lvname = "%s" %(lvtemplate,),
>                                                           volgroup = "lvm")
>        volnum += 1

I notice that the appending of 00 01, etc. is gone here, what then if we
fallback to the old behavior twice, then we get 2 identical names.

<snip>
> def createSuggestedLVName(logreqs):
>      """Given list of LV requests, come up with a reasonable LV name
>  partitions - list of LV requests for this VG
>  """
> # try to incorporate the mount point in to the logical volume name
> if lv.mountpoint is not None and lv.mountpoint != '':
>    if lv.mountpoint == '/':
>        lvtemplate = 'lv_root'
>    else:
>        tmp = string.strip(lv.mountpoint)
>        tmp = tmp.replace('/', '_')
>        while not tmp.startswith('_'):
>            tmp = tmp[1:]

Same as above, also this seems a copy and paste job, shouldn't this be put in a 
shared function somewhere then ?

<snip>

Regards,

Hans

[Jeremy Katz]

On Mon, 2008-12-01 at 12:03 -1000, David Cantrell wrote:
> # try to create a volume group name incorporating the hostname
> if network.hostname is not None and network.hostname != '':
>    if network.hostname.find('.') != -1:
>        vgtemplate = "vg_%s" % (network.hostname.split('.')[0].lower(),)
>    else:
>        vgtemplate = "vg_%s" % (network.hostname.lower(),)
> else:
>    vgtemplate = "VolGroup"

The other comment I'd have beyond Hans's is if the hostname is localhost
or localhost.localdomain, we should probably just fall back to the
VolGroup case as well.

Jeremy