From 33435bd48bfb7631eb298df097e41be36e6b1222 Mon Sep 17 00:00:00 2001
From: David Lehman <dlehman@redhat.com>
Date: Mon, 21 Nov 2011 09:34:44 -0600
Subject: [PATCH 05/12] Handle device name generation and checking in a more generic way.

---
 pyanaconda/storage/__init__.py       |  143 ++++++++++++++++++++--------------
 pyanaconda/storage/devicelibs/lvm.py |   15 ----
 pyanaconda/storage/devicetree.py     |   21 +++++-
 3 files changed, 103 insertions(+), 76 deletions(-)

diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py
index 6c51c63..eb7014e 100644
--- a/pyanaconda/storage/__init__.py
+++ b/pyanaconda/storage/__init__.py
@@ -44,7 +44,6 @@ from deviceaction import *
 from formats import getFormat
 from formats import get_device_format_class
 from formats import get_default_filesystem_type
-from devicelibs.lvm import safeLvmName
 from devicelibs.dm import name_from_dm_node
 from devicelibs.crypto import generateBackupPassphrase
 from devicelibs.mpath import MultipathConfigWriter
@@ -697,6 +696,10 @@ class Storage(object):
             _platform = getattr(self.anaconda, "platform", None)
         return _platform
 
+    @property
+    def names(self):
+        return self.devicetree.names
+
     def exceptionDisks(self):
         """ Return a list of removable devices to save exceptions to.
 
@@ -862,9 +865,9 @@ class Storage(object):
             hostname = ""
             if hasattr(self.anaconda, "network"):
                 hostname = self.anaconda.network.hostname
-            name = self.createSuggestedVGName(hostname=hostname)
+            name = self.suggestContainerName(hostname=hostname, prefix="vg")
 
-        if name in [d.name for d in self.devices]:
+        if name in self.names:
             raise ValueError("name already in use")
 
         return LVMVolumeGroupDevice(name, pvs, *args, **kwargs)
@@ -886,11 +889,12 @@ class Storage(object):
                 swap = True
             else:
                 swap = False
-            name = self.createSuggestedLVName(vg,
-                                              swap=swap,
-                                              mountpoint=mountpoint)
+            name = self.suggestDeviceName(prefix="lv",
+                                          parent=vg,
+                                          swap=swap,
+                                          mountpoint=mountpoint)
 
-        if name in [d.name for d in self.devices]:
+        if name in self.names:
             raise ValueError("name already in use")
 
         return LVMLogicalVolumeDevice(name, vg, *args, **kwargs)
@@ -964,71 +968,90 @@ class Storage(object):
                 return True
         return False
 
-    def createSuggestedVGName(self, hostname=None):
-        """ Return a reasonable, unused VG name. """
-        # try to create a volume group name incorporating the hostname
-        if hostname not in (None, "", 'localhost', 'localhost.localdomain'):
-            template = "vg_%s" % (hostname.split('.')[0].lower(),)
-            vgtemplate = safeLvmName(template)
-        elif flags.imageInstall:
-            vgtemplate = "vg_image"
-        else:
-            vgtemplate = "VolGroup"
+    def safeDeviceName(self, name):
+        """ Convert a device name to something safe and return that.
 
-        vgnames = [vg.name for vg in self.vgs]
-        if vgtemplate not in vgnames and \
-                vgtemplate not in lvm.lvm_vg_blacklist:
-            return vgtemplate
+            LVM limits lv names to 128 characters. I don't know the limits for
+            the other various device types, so I'm going to pick a number so
+            that we don't have to have an entire fucking library to determine
+            device name limits.
+        """
+        max_len = 96    # No, you don't need longer names than this. Really.
+        tmp = name.strip()
+        tmp = tmp.replace("/", "_")
+        tmp = re.sub("[^0-9a-zA-Z._-]", "", tmp)
+        tmp = tmp.lstrip("_")
+
+        if len(tmp) > max_len:
+            tmp = tmp[:max_len]
+
+        return tmp
+
+    def suggestContainerName(self, hostname=None, prefix=""):
+        """ Return a reasonable, unused device name. """
+        # try to create a device name incorporating the hostname
+        if hostname not in (None, "", 'localhost', 'localhost.localdomain', "(none)"):
+            template = "%s_%s" % (prefix, hostname.split('.')[0].lower())
+            template = self.safeDeviceName(template)
+        elif flags.imageInstall:
+            template = "%s_image" % prefix
         else:
-            i = 0
-            while 1:
-                tmpname = "%s%02d" % (vgtemplate, i,)
-                if not tmpname in vgnames and \
-                        tmpname not in lvm.lvm_vg_blacklist:
+            template = prefix
+
+        names = self.names
+        name = template
+        if name in names:
+            name = None
+            for i in range(100):
+                tmpname = "%s%02d" % (template, i,)
+                if tmpname not in names:
+                    name = tmpname
                     break
 
-                i += 1
-                if i > 99:
-                    tmpname = ""
+            if not name:
+                log.error("failed to create device name based on prefix "
+                          "'%s' and hostname '%s'" % (prefix, hostname))
+                raise RuntimeError("unable to find suitable device name")
 
-            return tmpname
+        return name
 
-    def createSuggestedLVName(self, vg, swap=None, mountpoint=None):
+    def suggestDeviceName(self, parent=None, swap=None,
+                                  mountpoint=None, prefix=""):
         """ Return a suitable, unused name for a new logical volume. """
-        # FIXME: this is not at all guaranteed to work
+        body = ""
         if mountpoint:
-            # try to incorporate the mountpoint into the name
-            if mountpoint == '/':
-                lvtemplate = 'lv_root'
+            if mountpoint == "/":
+                body = "_root"
             else:
-                if mountpoint.startswith("/"):
-                    template = "lv_%s" % mountpoint[1:]
+                body = mountpoint.replace("/", "_")
+        elif swap:
+            body = "_swap"
+
+        template = self.safeDeviceName(prefix + body)
+        names = self.names
+        name = template
+        def full_name(name, parent):
+            full = ""
+            if parent:
+                full = "%s-" % parent.name
+            full += name
+            return full
+
+        if full_name(name, parent) in names:
+            for i in range(100):
+                name = "%s%02d" % (template, i)
+                if name not in names:
+                    break
                 else:
-                    template = "lv_%s" % (mountpoint,)
+                    name = ""
 
-                lvtemplate = safeLvmName(template)
-        else:
-            if swap:
-                if len([s for s in self.swaps if s in vg.lvs]):
-                    idx = len([s for s in self.swaps if s in vg.lvs])
-                    while True:
-                        lvtemplate = "lv_swap%02d" % idx
-                        if lvtemplate in [lv.lvname for lv in vg.lvs]:
-                            idx += 1
-                        else:
-                            break
-                else:
-                    lvtemplate = "lv_swap"
-            else:
-                idx = len(vg.lvs)
-                while True:
-                    lvtemplate = "LogVol%02d" % idx
-                    if lvtemplate in [l.lvname for l in vg.lvs]:
-                        idx += 1
-                    else:
-                        break
+            if not name:
+                log.error("failed to create device name based on parent '%s', "
+                          "prefix '%s', mountpoint '%s', swap '%s'"
+                          % (parent.name, prefix, mountpoint, swap))
+                raise RuntimeError("unable to find suitable device name")
 
-        return lvtemplate
+        return name
 
     def doEncryptionPassphraseRetrofits(self):
         """ Add the global passphrase to all preexisting LUKS devices.
diff --git a/pyanaconda/storage/devicelibs/lvm.py b/pyanaconda/storage/devicelibs/lvm.py
index 121c896..feb1216 100644
--- a/pyanaconda/storage/devicelibs/lvm.py
+++ b/pyanaconda/storage/devicelibs/lvm.py
@@ -122,21 +122,6 @@ def getMaxLVSize():
     else:
         return (16*1024*1024) #Max is 16TiB
 
-# LVM sources set the maximum length limit on VG and LV names at 128.  Set
-# our default to 2 below that to account for 0 through 99 entries we may
-# make with this name as a prefix.  LVM doesn't seem to impose a limit of
-# 99, but we do in anaconda.
-def safeLvmName(name, maxlen=126):
-    tmp = name.strip()
-    tmp = tmp.replace("/", "_")
-    tmp = re.sub("[^0-9a-zA-Z._]", "", tmp)
-    tmp = tmp.lstrip("_")
-
-    if len(tmp) > maxlen:
-        tmp = tmp[:maxlen]
-
-    return tmp
-
 def clampSize(size, pesize, roundup=None):
     if roundup:
         round = math.ceil
diff --git a/pyanaconda/storage/devicetree.py b/pyanaconda/storage/devicetree.py
index 7b411e6..6914c6e 100644
--- a/pyanaconda/storage/devicetree.py
+++ b/pyanaconda/storage/devicetree.py
@@ -1,7 +1,7 @@
 # devicetree.py
 # Device management for anaconda's storage configuration module.
 #
-# Copyright (C) 2009  Red Hat, Inc.
+# Copyright (C) 2009, 2010, 2011  Red Hat, Inc.
 #
 # This copyrighted material is made available to anyone wishing to use,
 # modify, copy, or redistribute it subject to the terms and conditions of
@@ -158,6 +158,9 @@ class DeviceTree(object):
         self._devices = []
         self._actions = []
 
+        # a list of all device names we encounter
+        self.names = []
+
         # indicates whether or not the tree has been fully populated
         self.populated = False
 
@@ -343,6 +346,12 @@ class DeviceTree(object):
                 raise DeviceTreeError("parent device not in tree")
 
         self._devices.append(newdev)
+
+        # don't include "req%d" partition names
+        if ((newdev.type != "partition" or
+             not newdev.name.startswith("req")) and
+            newdev.name not in self.names):
+            self.names.append(newdev.name)
         log.info("added %s %s (id %d) to device tree" % (newdev.type,
                                                           newdev.name,
                                                           newdev.id))
@@ -380,6 +389,8 @@ class DeviceTree(object):
                     device.updateName()
 
         self._devices.remove(dev)
+        if dev.name in self.names:
+            self.names.remove(dev.name)
         log.info("removed %s %s (id %d) from device tree" % (dev.type,
                                                               dev.name,
                                                               dev.id))
@@ -940,6 +951,10 @@ class DeviceTree(object):
         uuid = udev_device_get_uuid(info)
         sysfs_path = udev_device_get_sysfs_path(info)
 
+        # make sure we note the name of every device we see
+        if name not in self.names:
+            self.names.append(name)
+
         if self.isIgnored(info):
             log.info("ignoring %s (%s)" % (name, sysfs_path))
             if name not in self._ignoredDisks:
@@ -1376,6 +1391,10 @@ class DeviceTree(object):
             vg_device.lv_sizes.append(lv_sizes[i])
             vg_device.lv_attr.append(lv_attr[i])
 
+            name = "%s-%s" % (vg_name, lv_names[i])
+            if name not in self.names:
+                self.names.append(name)
+
     def handleUdevMDMemberFormat(self, info, device):
         log_method_call(self, name=device.name, type=device.format.type)
         # either look up or create the array device
-- 
1.7.3.4

