[PATCH] Make distro_adder and add_entry recognize multiple kernels

[Jeroen van Meeuwen]

From: Jeroen van Meeuwen (Fedora Unity) <kanarip(a)fedoraunity.org&gt;

---
 cobbler/action_import.py |  212 +++++++++++++++++++++++++++-------------------
 1 files changed, 126 insertions(+), 86 deletions(-)

diff --git a/cobbler/action_import.py b/cobbler/action_import.py
index a0a515e..bf4818c 100644
--- a/cobbler/action_import.py
+++ b/cobbler/action_import.py
@@ -1,7 +1,7 @@
 """
 Enables the "cobbler import" command to seed cobbler
 information with available distribution from rsync mirrors
-and mounted DVDs.  
+and mounted DVDs.
 
 Copyright 2006-2008, Red Hat, Inc
 Michael DeHaan <mdehaan(a)redhat.com&gt;
@@ -45,15 +45,15 @@ class Importer:
         by mirror name.  For instance, if FC-6 is given, FC-6-xen-i386
         would be a potential distro that could be created.  For content
         available on external servers via a known nfs:// or ftp:// or
-        http:// path, we can import without doing rsync mirorring to 
-        cobbler's http directory.  This is explained in more detail 
+        http:// path, we can import without doing rsync mirorring to
+        cobbler's http directory.  This is explained in more detail
         in the manpage.  Leave network_root to None if want mirroring.
         """
         self.api = api
         self.config = config
         self.mirror = mirror
         self.mirror_name = mirror_name
-        self.network_root = network_root 
+        self.network_root = network_root
         self.distros  = config.distros()
         self.profiles = config.profiles()
         self.systems  = config.systems()
@@ -98,12 +98,12 @@ class Importer:
             raise CX(_("Something already exists at this import location (%s).  You
must specify --arch to avoid potentially overwriting existing files.") % mpath)
 
         # import takes a --kickstart for forcing selection that can't be used in all
circumstances
- 
+
         if self.kickstart_file and not self.breed:
             raise CX(_("Kickstart file can only be specified when a specific breed
is selected"))
- 
+
         # if --arch is supplied, make sure the user is not importing a path with a
different
-        # arch, which would just be silly.  
+        # arch, which would just be silly.
 
         if self.arch:
             # append the arch path to the name if the arch is not already
@@ -121,8 +121,8 @@ class Importer:
         # accessible support location already exists (this is --available-as on the
command line)
 
         if self.network_root is None:
- 
-            # we need to mirror (copy) the files 
+
+            # we need to mirror (copy) the files
 
             self.path = os.path.normpath( "%s/ks_mirror/%s" %
(self.settings.webdir, self.mirror_name) )
             self.mkdir(self.path)
@@ -131,7 +131,7 @@ class Importer:
             # if we are copying via rsync
 
             if not self.mirror.endswith("/"):
-                self.mirror = "%s/" % self.mirror 
+                self.mirror = "%s/" % self.mirror
 
             if self.mirror.startswith("http://") or
self.mirror.startswith("ftp://") or self.mirror.startswith("nfs://"):
 
@@ -143,7 +143,7 @@ class Importer:
 
             else:
 
-                # good, we're going to use rsync.. 
+                # good, we're going to use rsync..
                 # we don't use SSH for public mirrors and local files.
                 # presence of user@host syntax means use SSH
 
@@ -163,12 +163,12 @@ class Importer:
             # rather than mirroring, we're going to assume the path is available
             # over http, ftp, and nfs, perhaps on an external filer.  scanning still
requires
             # --mirror is a filesystem path, but --available-as marks the network path
-          
+
             if not os.path.exists(self.mirror):
                 raise CX(_("path does not exist: %s") % self.mirror)
 
             # find the filesystem part of the path, after the server bits, as each
distro
-            # URL needs to be calculated relative to this. 
+            # URL needs to be calculated relative to this.
 
             if not self.network_root.endswith("/"):
                 self.network_root = self.network_root + "/"
@@ -198,13 +198,13 @@ class Importer:
 
         if self.network_root is None:
             print _("---------------- (associating repos)")
-            # FIXME: this automagic is not possible (yet) without mirroring 
+            # FIXME: this automagic is not possible (yet) without mirroring
             self.repo_finder(distros_added)
 
         # find the most appropriate answer files for each profile object
 
         print _("---------------- (associating kickstarts)")
-        self.kickstart_finder(distros_added) 
+        self.kickstart_finder(distros_added)
 
         # ensure everything is nicely written out to the filesystem
         # (which is not so neccessary in newer Cobbler but we're paranoid)
@@ -251,8 +251,8 @@ class Importer:
 
         """
         For all of the profiles in the config w/o a kickstart, use the
-        given kickstart file, or look at the kernel path, from that, 
-        see if we can guess the distro, and if we can, assign a kickstart 
+        given kickstart file, or look at the kernel path, from that,
+        see if we can guess the distro, and if we can, assign a kickstart
         if one is available for it.
         """
 
@@ -263,7 +263,7 @@ class Importer:
                 continue
 
             if self.kickstart_file == None:
-                kdir = os.path.dirname(distro.kernel)    
+                kdir = os.path.dirname(distro.kernel)
                 importer = import_factory(kdir,self.path)
                 for rpm in importer.get_release_files():
                      # FIXME : This redhat specific check should go into the
importer.find_release_files method
@@ -314,7 +314,7 @@ class Importer:
             # where we assign the kickstart source is relative to our current directory
             # and the input start directory in the crawl.  We find the path segments
             # between and tack them on the network source path to find the explicit
-            # network path to the distro that Anaconda can digest.  
+            # network path to the distro that Anaconda can digest.
             tail = self.path_tail(self.path, base)
             meta["tree"] = self.network_root[:-1] + tail
 
@@ -324,7 +324,7 @@ class Importer:
     # ============================================================================
 
     def path_tail(self, apath, bpath):
-        """ 
+        """
         Given two paths (B is longer than A), find the part in B not in A
         """
         position = bpath.find(apath)
@@ -339,15 +339,15 @@ class Importer:
         return result
 
     # ======================================================================
-    
+
     def repo_finder(self,distros_added):
 
         """
-        This routine looks through all distributions and tries to find 
+        This routine looks through all distributions and tries to find
         any applicable repositories in those distributions for post-install
         usage.
         """
-        
+
         for distro in distros_added:
             print _("- traversing distro %s") % distro.name
             if distro.kernel.find("ks_mirror") != -1:
@@ -366,10 +366,10 @@ class Importer:
 
         """
         This is an os.path.walk routine that looks for potential yum repositories
-        to be added to the configuration for post-install usage. 
+        to be added to the configuration for post-install usage.
         """
-        
-        matches = {} 
+
+        matches = {}
         for x in fnames:
           if x == "base" or x == "repodata":
                 print "- processing repo at : %s" % dirname
@@ -388,7 +388,7 @@ class Importer:
 
     #
=======================================================================================
 
-                
+
 
     def process_comps_file(self, comps_path, distro):
         """
@@ -436,8 +436,8 @@ class Importer:
             fname = os.path.join(self.settings.webdir, "ks_mirror",
"config", "%s-%s.repo" % (distro.name, counter))
 
             repo_url =
"http://@@http_server@@/cobbler/ks_mirror/config/%s-%s.repo" % (distro.name,
counter)
-         
-            repo_url2 = "http://@@http_server@@/cobbler/ks_mirror/%s" %
(urlseg) 
+
+            repo_url2 = "http://@@http_server@@/cobbler/ks_mirror/%s" %
(urlseg)
 
             distro.source_repos.append([repo_url,repo_url2])
 
@@ -475,48 +475,82 @@ class Importer:
         except:
             print _("- error launching createrepo, ignoring...")
             traceback.print_exc()
-        
+
 
     # ========================================================================
 
     def distro_adder(self,foo,dirname,fnames):
-        
+
         """
         This is an os.path.walk routine that finds distributions in the directory
         to be scanned and then creates them.
         """
 
-        # FIXME: If there are more than one kernel or initrd image on the same directory,

+        # FIXME: If there are more than one kernel or initrd image on the same
directory,
         # results are unpredictable
 
-        initrd = None
-        kernel = None
-        
+        initrd = {}
+        kernel = {}
+
+        counter = 0
+
         for x in fnames:
 
             fullname = os.path.join(dirname,x)
+
             if os.path.islink(fullname) and os.path.isdir(fullname):
-              if os.path.realpath(fullname) == os.path.realpath(dirname):
-                # Prevent infinite loops importing debian media
-                continue
-              print "- following symlink: %s" % fullname
-              os.path.walk(fullname, self.distro_adder, foo)
-
-            if x.startswith("initrd"):
-                initrd = os.path.join(dirname,x)
-            if ( x.startswith("vmlinuz") or
x.startswith("kernel.img") ) and x.find("initrd") == -1:
-                kernel = os.path.join(dirname,x)
-            if initrd is not None and kernel is not None and
dirname.find("isolinux") == -1:
-                adtl = self.add_entry(dirname,kernel,initrd)
-                if adtl != None:
-                    foo.extend(adtl)
-                    # Not resetting these values causes problems importing debian media
because there are remaining items in fnames
-                    initrd = None
-                    kernel = None
-    
+                if os.path.realpath(fullname) == os.path.realpath(dirname):
+                    # Prevent infinite loops importing debian media
+                    continue
+                print "- following symlink: %s" % fullname
+                os.path.walk(fullname, self.distro_adder, foo)
+
+            if x.startswith("initrd") and not initrd.has_key(x):
+                initrd[counter] = os.path.join(dirname,x)
+                print "Found initrd for %s" % x
+                if kernel.has_key(counter): counter += 1
+
+            if ( x.startswith("vmlinuz") or
x.startswith("kernel.img") ) and x.find("initrd") == -1 and not
kernel.has_key(x):
+                kernel[counter] = os.path.join(dirname,x)
+                print "Found kernel for %s" % x
+                if initrd.has_key(counter): counter += 1
+
+            if initrd.keys() > 0 and kernel.keys() > 0 and
dirname.find("isolinux") == -1:
+                for key in initrd.keys():
+                    kernel_type = None
+                    if kernel.has_key(key):
+                        kernel_type = self.find_me_kernel_type(initrd[key], kernel[key])
+                    else:
+                        print "kernel[%s] does not exist" % key
+                        continue
+
+                    adtl =
self.add_entry(dirname,kernel[key],initrd[key],kernel_type=kernel_type)
+                    if adtl != None:
+                        foo.extend(adtl)
+                        # Not resetting these values causes problems importing debian
media because there are remaining items in fnames
+                        initrd = {}
+                        kernel = {}
+
+    # ========================================================================
+
+    def find_me_kernel_type(self,initrd, kernel):
+
+        """
+        From the initrd and kernel passed, find out a name for this kernel
+        """
+
+        print "Got initrd %s and kernel %s" % (initrd,kernel)
+
+        if initrd.find("PAE") >= 0:
+            print "Found kernel type PAE"
+            return "PAE"
+
+        print "No particular kernel type found"
+        return None
+
     # ========================================================================
 
-    def add_entry(self,dirname,kernel,initrd):
+    def add_entry(self,dirname,kernel,initrd,kernel_type=None):
 
         """
         When we find a directory with a valid kernel/initrd in it, create the
distribution objects
@@ -545,7 +579,7 @@ class Importer:
 
         if len(archs)>1:
             if importer.breed in [ "redhat" ]:
-                print _("Warning: directory %s holds multiple arches : %s") %
(dirname, archs) 
+                print _("Warning: directory %s holds multiple arches : %s") %
(dirname, archs)
                 return
             print _("- Warning : Multiple archs found : %s") % (archs)
 
@@ -553,7 +587,13 @@ class Importer:
 
         for pxe_arch in archs:
 
-            name = proposed_name + "-" + pxe_arch
+            if kernel_type:
+                proposed_name_append = "-%s" % kernel_type
+            else:
+                proposed_name_append = ""
+
+            name = proposed_name + "-" + pxe_arch + proposed_name_append
+
             existing_distro = self.distros.find(name=name)
 
             if existing_distro is not None:
@@ -563,7 +603,7 @@ class Importer:
             else:
                 print _("- creating new distro: %s") % name
                 distro = self.config.new_distro()
-            
+
             if name.find("-autoboot") != -1:
                 # this is an artifact of some EL-3 imports
                 continue
@@ -576,15 +616,15 @@ class Importer:
             distro.source_repos = []
 
             self.distros.add(distro,save=True)
-            distros_added.append(distro)        
+            distros_added.append(distro)
 
-            existing_profile = self.profiles.find(name=name) 
+            existing_profile = self.profiles.find(name=name)
 
-            # see if the profile name is already used, if so, skip it and 
+            # see if the profile name is already used, if so, skip it and
             # do not modify the existing profile
 
             if existing_profile is None:
-                print _("- creating new profile: %s") % name 
+                print _("- creating new profile: %s") % name
                 profile = self.config.new_profile()
             else:
                 print _("- skipping existing profile, name already exists: %s")
% name
@@ -652,8 +692,8 @@ class Importer:
         # we know that some kernel paths should not be in the name
 
         name = name.replace("-images","")
-        name = name.replace("-pxeboot","")  
-        name = name.replace("-install","")  
+        name = name.replace("-pxeboot","")
+        name = name.replace("-install","")
 
         # some paths above the media root may have extra path segments we want
         # to clean up
@@ -695,10 +735,10 @@ class Importer:
 def guess_breed(kerneldir,path):
 
     """
-    This tries to guess the distro. Traverses from kernel dir to imported root checking 
-    for distro signatures, which are the locations in media where the search for release

-    packages should start.  When a debian/ubuntu pool is found, the upper directory
should 
-    be checked to get the real breed. If we are on a real media, the upper directory will

+    This tries to guess the distro. Traverses from kernel dir to imported root checking
+    for distro signatures, which are the locations in media where the search for release
+    packages should start.  When a debian/ubuntu pool is found, the upper directory
should
+    be checked to get the real breed. If we are on a real media, the upper directory
will
     be at the same level, as a local '.' symlink
     The lowercase names are required for fat32/vfat filesystems
     """
@@ -724,7 +764,7 @@ def guess_breed(kerneldir,path):
             if os.path.exists( d ):
                 guess = breedguess
                 break
-        if guess: 
+        if guess:
             break
 
         kerneldir = os.path.dirname(kerneldir)
@@ -733,7 +773,7 @@ def guess_breed(kerneldir,path):
 
     if guess == "debian" :
         for suite in [ "debian" , "ubuntu" ] :
-            # NOTE : Although we break the loop after the first match, 
+            # NOTE : Although we break the loop after the first match,
             # multiple debian derived distros can actually live at the same pool -- JP
             d = os.path.join( kerneldir , suite )
             if os.path.islink(d) and os.path.isdir(d):
@@ -754,8 +794,8 @@ def import_factory(kerneldir,path):
     """
 
     breed , rootdir = guess_breed(kerneldir,path)
-    # NOTE : The guess_breed code should be included in the factory, in order to make 
-    # the real root directory available, so allowing kernels at different levels within 
+    # NOTE : The guess_breed code should be included in the factory, in order to make
+    # the real root directory available, so allowing kernels at different levels within
     # the same tree (removing the isolinux rejection from distro_adder) -- JP
 
     print _("- found content (breed=%s) at %s") % (breed,kerneldir)
@@ -777,7 +817,7 @@ class BaseImporter:
     Base class for distribution specific importer code.
     """
 
-    # FIXME : Rename learn_arch_from_tree into guess_arch and simplify. 
+    # FIXME : Rename learn_arch_from_tree into guess_arch and simplify.
     # FIXME : Drop package extension check and make a single search for all names.
     # FIXME:  Next methods to be moved here: kickstart_finder TRY_LIST loop
 
@@ -789,10 +829,10 @@ class BaseImporter:
 
         The TRY_LIST is used to speed up search, and should be dropped for default
importer
         Searched kernel names are kernel-header, linux-headers-, kernel-largesmp,
kernel-hugemem
-        
+
         This method is useful to get the archs, but also to package type and a raw guess
of the breed
         """
- 
+
         # try to find a kernel header RPM and then look at it's arch.
         for x in fnames:
             if self.match_kernelarch_file(x):
@@ -803,28 +843,28 @@ class BaseImporter:
                 for arch in [ "i686" , "amd64" ]:
                     if x.find(arch) != -1:
                         foo[arch] = 1
-    
+
     # ===================================================================
 
     def get_rootdir(self):
         return self.rootdir
-    
+
     # ===================================================================
 
     def get_pkgdir(self):
         return os.path.join(self.rootdir,self.pkgdir)
-    
+
     # ===================================================================
 
     def learn_arch_from_tree(self):
-        """ 
-        If a distribution is imported from DVD, there is a good chance the path
doesn't 
-        contain the arch and we should add it back in so that it's part of the 
-        meaningful name ... so this code helps figure out the arch name.  This is
important 
+        """
+        If a distribution is imported from DVD, there is a good chance the path
doesn't
+        contain the arch and we should add it back in so that it's part of the
+        meaningful name ... so this code helps figure out the arch name.  This is
important
         for producing predictable distro names (and profile names) from differing import
sources
         """
         result = {}
-        os.path.walk(self.get_pkgdir(), self.arch_walker, result)      
+        os.path.walk(self.get_pkgdir(), self.arch_walker, result)
         # print _("- architectures found at %s: %s") % ( self.get_pkgdir(),
result.keys() )
         if result.pop("amd64",False):
             result["x86_64"] = 1
@@ -836,7 +876,7 @@ class BaseImporter:
 
     def __init__(self,(rootdir,pkgdir)):
         raise CX(_("ERROR - BaseImporter is an abstract class"))
-    
+
 # ===================================================================
 # ===================================================================
 
@@ -878,7 +918,7 @@ class RedHatImporter ( BaseImporter ) :
         # if it looks like a RHEL RPM we'll cheat.
         # it may be slightly wrong, but it will be close enough
         # for RHEL5 we can get it exactly.
-        
+
         for x in [ "4AS", "4ES", "4WS" ]:
           if rpm.find(x) != -1:
              return ("redhat", 4, 0)
@@ -913,7 +953,7 @@ class RedHatImporter ( BaseImporter ) :
         return (flavor, major, minor)
 
     def set_variance(self, flavor, major, minor):
-  
+
         """
         find the profile kickstart and set the distro breed/os-version based on what
         we can find out from the rpm filenames and then return the kickstart
@@ -1029,7 +1069,7 @@ class UbuntuImporter ( DebianImporter ) :
         return (None, accum[0], accum[1])
 
     def set_variance(self, flavor, major, minor):
-  
+
         # Release names taken from wikipedia
         dist_names = { '4.10':"WartyWarthog",
'5.4':"HoaryHedgehog", '5.10':"BreezyBadger",
'6.4':"DapperDrake", '6.10':"EdgyEft",
'7.4':"FeistyFawn", '7.10':"GutsyGibbon",
'8.4':"HardyHeron", '8.10':"IntrepidIbex",
'9.4':"JauntyJackalope" }
         dist_vers = "%s.%s" % ( major , minor )
-- 
1.5.5.1

[Jeroen van Meeuwen]

Sorry for the trailing whitespace foo in this patch, a sane patch would 
look like this:

kanarip(a)kanarip.com wrote:
>  From: Jeroen van Meeuwen (Fedora Unity)
> <kanarip(a)fedoraunity.org&gt;
>  
>  ---
>   cobbler/action_import.py |  212 +++++++++++++++++++++++++++-------------------
>   1 files changed, 126 insertions(+), 86 deletions(-)
>  
>  diff --git a/cobbler/action_import.py b/cobbler/action_import.py
>  index a0a515e..bf4818c 100644
>  --- a/cobbler/action_import.py
>  +++ b/cobbler/action_import.py
>  @@ -475,48 +475,82 @@ class Importer:
>           except:
>               print _("- error launching createrepo, ignoring...")
>               traceback.print_exc()
>  -        
>  +
>   
>       # ========================================================================
>   
>       def distro_adder(self,foo,dirname,fnames):
>  -        
>  +
>           """
>           This is an os.path.walk routine that finds distributions in the directory
>           to be scanned and then creates them.
>           """
>   
>  -        # FIXME: If there are more than one kernel or initrd image on the same
> directory, 
>  +        # FIXME: If there are more than one kernel or initrd image on the same
> directory,
>           # results are unpredictable
>   
>  -        initrd = None
>  -        kernel = None
>  -        
>  +        initrd = {}
>  +        kernel = {}
>  +
>  +        counter = 0
>  +
>           for x in fnames:
>   
>               fullname = os.path.join(dirname,x)
>  +
>               if os.path.islink(fullname) and os.path.isdir(fullname):
>  -              if os.path.realpath(fullname) == os.path.realpath(dirname):
>  -                # Prevent infinite loops importing debian media
>  -                continue
>  -              print "- following symlink: %s" % fullname
>  -              os.path.walk(fullname, self.distro_adder, foo)
>  -
>  -            if x.startswith("initrd"):
>  -                initrd = os.path.join(dirname,x)
>  -            if ( x.startswith("vmlinuz") or
> x.startswith("kernel.img") ) and x.find("initrd") == -1:
>  -                kernel = os.path.join(dirname,x)
>  -            if initrd is not None and kernel is not None and
> dirname.find("isolinux") == -1:
>  -                adtl = self.add_entry(dirname,kernel,initrd)
>  -                if adtl != None:
>  -                    foo.extend(adtl)
>  -                    # Not resetting these values causes problems importing debian media
> because there are remaining items in fnames
>  -                    initrd = None
>  -                    kernel = None
>  -    
>  +                if os.path.realpath(fullname) == os.path.realpath(dirname):
>  +                    # Prevent infinite loops importing debian media
>  +                    continue
>  +                print "- following symlink: %s" % fullname
>  +                os.path.walk(fullname, self.distro_adder, foo)
>  +
>  +            if x.startswith("initrd") and not initrd.has_key(x):
>  +                initrd[counter] = os.path.join(dirname,x)
>  +                print "Found initrd for %s" % x
>  +                if kernel.has_key(counter): counter += 1
>  +
>  +            if ( x.startswith("vmlinuz") or
> x.startswith("kernel.img") ) and x.find("initrd") == -1 and not
> kernel.has_key(x):
>  +                kernel[counter] = os.path.join(dirname,x)
>  +                print "Found kernel for %s" % x
>  +                if initrd.has_key(counter): counter += 1
>  +
>  +            if initrd.keys() > 0 and kernel.keys() > 0 and
> dirname.find("isolinux") == -1:
>  +                for key in initrd.keys():
>  +                    kernel_type = None
>  +                    if kernel.has_key(key):
>  +                        kernel_type = self.find_me_kernel_type(initrd[key],
> kernel[key])
>  +                    else:
>  +                        print "kernel[%s] does not exist" % key
>  +                        continue
>  +
>  +                    adtl =
> self.add_entry(dirname,kernel[key],initrd[key],kernel_type=kernel_type)
>  +                    if adtl != None:
>  +                        foo.extend(adtl)
>  +                        # Not resetting these values causes problems importing debian
> media because there are remaining items in fnames
>  +                        initrd = {}
>  +                        kernel = {}
>  +
>  +    # ========================================================================
>  +
>  +    def find_me_kernel_type(self,initrd, kernel):
>  +
>  +        """
>  +        From the initrd and kernel passed, find out a name for this kernel
>  +        """
>  +
>  +        print "Got initrd %s and kernel %s" % (initrd,kernel)
>  +
>  +        if initrd.find("PAE") >= 0:
>  +            print "Found kernel type PAE"
>  +            return "PAE"
>  +
>  +        print "No particular kernel type found"
>  +        return None
>  +
>       # ========================================================================
>   
>  -    def add_entry(self,dirname,kernel,initrd):
>  +    def add_entry(self,dirname,kernel,initrd,kernel_type=None):
>   
>           """
>           When we find a directory with a valid kernel/initrd in it, create the
> distribution objects
>  @@ -553,7 +587,13 @@ class Importer:
>   
>           for pxe_arch in archs:
>   
>  -            name = proposed_name + "-" + pxe_arch
>  +            if kernel_type:
>  +                proposed_name_append = "-%s" % kernel_type
>  +            else:
>  +                proposed_name_append = ""
>  +
>  +            name = proposed_name + "-" + pxe_arch + proposed_name_append
>  +
>               existing_distro = self.distros.find(name=name)
>   
>               if existing_distro is not None:

[Michael DeHaan]

Jeroen van Meeuwen wrote:
>  Sorry for the trailing whitespace foo in this patch, a sane patch
> would 
>  look like this:
>     
Jeroen,

Can you supply more in-depth information about what this patch does, 
specifically?

Examples would be good.

Thanks,

--Michael


>  kanarip(a)kanarip.com wrote:
>    
> > From: Jeroen van Meeuwen (Fedora Unity) <kanarip(a)fedoraunity.org&gt;
> >
> > ---
> >  cobbler/action_import.py |  212 +++++++++++++++++++++++++++-------------------
> >  1 files changed, 126 insertions(+), 86 deletions(-)
> >
> > diff --git a/cobbler/action_import.py b/cobbler/action_import.py
> > index a0a515e..bf4818c 100644
> > --- a/cobbler/action_import.py
> > +++ b/cobbler/action_import.py
> > @@ -475,48 +475,82 @@ class Importer:
> >          except:
> >              print _("- error launching createrepo, ignoring...")
> >              traceback.print_exc()
> > -        
> > +
> >  
> >      # ========================================================================
> >  
> >      def distro_adder(self,foo,dirname,fnames):
> > -        
> > +
> >          """
> >          This is an os.path.walk routine that finds distributions in the directory
> >          to be scanned and then creates them.
> >          """
> >  
> > -        # FIXME: If there are more than one kernel or initrd image on the same
> directory, 
> > +        # FIXME: If there are more than one kernel or initrd image on the same
> directory,
> >          # results are unpredictable
> >  
> > -        initrd = None
> > -        kernel = None
> > -        
> > +        initrd = {}
> > +        kernel = {}
> > +
> > +        counter = 0
> > +
> >          for x in fnames:
> >  
> >              fullname = os.path.join(dirname,x)
> > +
> >              if os.path.islink(fullname) and os.path.isdir(fullname):
> > -              if os.path.realpath(fullname) == os.path.realpath(dirname):
> > -                # Prevent infinite loops importing debian media
> > -                continue
> > -              print "- following symlink: %s" % fullname
> > -              os.path.walk(fullname, self.distro_adder, foo)
> > -
> > -            if x.startswith("initrd"):
> > -                initrd = os.path.join(dirname,x)
> > -            if ( x.startswith("vmlinuz") or
> x.startswith("kernel.img") ) and x.find("initrd") == -1:
> > -                kernel = os.path.join(dirname,x)
> > -            if initrd is not None and kernel is not None and
> dirname.find("isolinux") == -1:
> > -                adtl = self.add_entry(dirname,kernel,initrd)
> > -                if adtl != None:
> > -                    foo.extend(adtl)
> > -                    # Not resetting these values causes problems importing debian
> media because there are remaining items in fnames
> > -                    initrd = None
> > -                    kernel = None
> > -    
> > +                if os.path.realpath(fullname) == os.path.realpath(dirname):
> > +                    # Prevent infinite loops importing debian media
> > +                    continue
> > +                print "- following symlink: %s" % fullname
> > +                os.path.walk(fullname, self.distro_adder, foo)
> > +
> > +            if x.startswith("initrd") and not initrd.has_key(x):
> > +                initrd[counter] = os.path.join(dirname,x)
> > +                print "Found initrd for %s" % x
> > +                if kernel.has_key(counter): counter += 1
> > +
> > +            if ( x.startswith("vmlinuz") or
> x.startswith("kernel.img") ) and x.find("initrd") == -1 and not
> kernel.has_key(x):
> > +                kernel[counter] = os.path.join(dirname,x)
> > +                print "Found kernel for %s" % x
> > +                if initrd.has_key(counter): counter += 1
> > +
> > +            if initrd.keys() > 0 and kernel.keys() > 0 and
> dirname.find("isolinux") == -1:
> > +                for key in initrd.keys():
> > +                    kernel_type = None
> > +                    if kernel.has_key(key):
> > +                        kernel_type = self.find_me_kernel_type(initrd[key],
> kernel[key])
> > +                    else:
> > +                        print "kernel[%s] does not exist" % key
> > +                        continue
> > +
> > +                    adtl =
> self.add_entry(dirname,kernel[key],initrd[key],kernel_type=kernel_type)
> > +                    if adtl != None:
> > +                        foo.extend(adtl)
> > +                        # Not resetting these values causes problems importing
> debian media because there are remaining items in fnames
> > +                        initrd = {}
> > +                        kernel = {}
> > +
> > +    # ========================================================================
> > +
> > +    def find_me_kernel_type(self,initrd, kernel):
> > +
> > +        """
> > +        From the initrd and kernel passed, find out a name for this kernel
> > +        """
> > +
> > +        print "Got initrd %s and kernel %s" % (initrd,kernel)
> > +
> > +        if initrd.find("PAE") >= 0:
> > +            print "Found kernel type PAE"
> > +            return "PAE"
> > +
> > +        print "No particular kernel type found"
> > +        return None
> > +
> >      # ========================================================================
> >  
> > -    def add_entry(self,dirname,kernel,initrd):
> > +    def add_entry(self,dirname,kernel,initrd,kernel_type=None):
> >  
> >          """
> >          When we find a directory with a valid kernel/initrd in it, create the
> distribution objects
> > @@ -553,7 +587,13 @@ class Importer:
> >  
> >          for pxe_arch in archs:
> >  
> > -            name = proposed_name + "-" + pxe_arch
> > +            if kernel_type:
> > +                proposed_name_append = "-%s" % kernel_type
> > +            else:
> > +                proposed_name_append = ""
> > +
> > +            name = proposed_name + "-" + pxe_arch + proposed_name_append
> > +
> >              existing_distro = self.distros.find(name=name)
> >  
> >              if existing_distro is not None:
> >     
>
>  _______________________________________________
>  cobbler mailing list
>  cobbler(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/cobbler
>

[Jeroen van Meeuwen]

Michael DeHaan wrote:
>  Jeroen van Meeuwen wrote:
> > Sorry for the trailing whitespace foo in this patch, a sane patch would 
> > look like this:
> >   
>  
>  Jeroen,
>  
>  Can you supply more in-depth information about what this patch does, 
>  specifically?
>  
>  Examples would be good.
>   
Sure, this is a FIXME in the devel branch's code, which concerns finding 
more then just one type of vmlinuz/initrd.img in the tree we're walking;

# FIXME: If there are more than one kernel or initrd image on the same
# directory, results are unpredictable

I found that with a layout like for F-10-Beta;

[jmeeuwen@elwood pxeboot]$ pwd
/data/os/distr/fedora/releases/test/10-Beta/Fedora/i386/os/images/pxeboot
[jmeeuwen@elwood pxeboot]$ ll
total 58244
-rw-r--r-- 1 537 537 20398080 2008-09-25 18:50 efiboot.img
-rw-r--r-- 1 537 537 16749126 2008-09-25 18:50 initrd.img
-rw-r--r-- 1 537 537 16805609 2008-09-25 18:53 initrd-PAE.img
-rw-r--r-- 1 537 537      265 2008-09-25 18:50 README
-rw-r--r-- 2 537 537  2763536 2008-09-25 18:50 vmlinuz
-rw-r--r-- 1 537 537  2814192 2008-09-25 18:53 vmlinuz-PAE

Only the PAE version of vmlinuz/initrd gets imported.

The key became to store all vmlinuz/initrd's found, and matching any 
combination of a initrd-PAE.img vs. a vmlinuz-PAE, but luckily no matter 
what is in the file name, both initrd's and vmlinuz's sort the same 
(e.g. the first initrd in the list will match with the first vmlinuz, 
the second... and so forth).

-Jeroen

[Michael DeHaan]

Jeroen van Meeuwen wrote:
>  Michael DeHaan wrote:
>    
> > Jeroen van Meeuwen wrote:
> >     
> >> Sorry for the trailing whitespace foo in this patch, a sane patch would 
> >> look like this:
> >>   
> >>       
> > Jeroen,
> >
> > Can you supply more in-depth information about what this patch does, 
> > specifically?
> >
> > Examples would be good.
> >
> >     
>
>  Sure, this is a FIXME in the devel branch's code, which concerns finding 
>  more then just one type of vmlinuz/initrd.img in the tree we're walking;
>
>  # FIXME: If there are more than one kernel or initrd image on the same
>  # directory, results are unpredictable
>
>  I found that with a layout like for F-10-Beta;
>
>  [jmeeuwen@elwood pxeboot]$ pwd
>  /data/os/distr/fedora/releases/test/10-Beta/Fedora/i386/os/images/pxeboot
>  [jmeeuwen@elwood pxeboot]$ ll
>  total 58244
>  -rw-r--r-- 1 537 537 20398080 2008-09-25 18:50 efiboot.img
>  -rw-r--r-- 1 537 537 16749126 2008-09-25 18:50 initrd.img
>  -rw-r--r-- 1 537 537 16805609 2008-09-25 18:53 initrd-PAE.img
>  -rw-r--r-- 1 537 537      265 2008-09-25 18:50 README
>  -rw-r--r-- 2 537 537  2763536 2008-09-25 18:50 vmlinuz
>  -rw-r--r-- 1 537 537  2814192 2008-09-25 18:53 vmlinuz-PAE
>
>  Only the PAE version of vmlinuz/initrd gets imported.
>
>  The key became to store all vmlinuz/initrd's found, and matching any 
>  combination of a initrd-PAE.img vs. a vmlinuz-PAE, but luckily no matter 
>  what is in the file name, both initrd's and vmlinuz's sort the same 
>  (e.g. the first initrd in the list will match with the first vmlinuz, 
>  the second... and so forth).
>
>  -Jeroen
>  _______________________________________________
>  cobbler mailing list
>  cobbler(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/cobbler
>     
Thanks for the explanation, this sounds like a good thing to have -- 
I'll review this shortly.

--Michael