[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH 3/3] Carry rdloaddriver= parameters through to the boot loader config.

On 08/10/2011 03:48 PM, Chris Lumens wrote:
Handle multiple rdloaddriver= parameters in the cmdlineDict.  In cases
where we already have a key, convert the value to a set and add the new
value to it.  Start new keys with just the value.

In the boot loader code, take cmdlineDict values that are sets, convert
them to lists, and join them to a single string separated by commas
before writing them to the argument list.

This set looks fine, except for comments below.

@@ -174,7 +174,10 @@ class KernelArguments:

              val = flags.cmdline.get(arg, "")
              if val:
-                newArgs.append("%s=%s" % (arg, val))
+                if type(val) == type(set()):
+                    newArgs.append("%s=%s" % (arg, ','.join(list(val)),))
+                else:
+                    newArgs.append("%s=%s" % (arg, val,))

You don't need the trailing commas in your tuples.  The only time you
need to use a trailing comma is if you're creating a singleton tuple
because python syntax is a little weird there.  Makes it a little easier
to read, I think.

Changed.  I won't post a new patch since it's removing two commas.

diff --git a/flags.py b/flags.py
index e01ec63..afb56e2 100644
--- a/flags.py
+++ b/flags.py
@@ -61,7 +61,19 @@ class Flags:
                  key = i
                  val = None

-            cmdlineDict[key] = val
+            if key.lower() == "rdloaddriver":
+                key = key.lower()
+            if cmdlineDict.has_key(key):
+                if type(cmdlineDict[key]) == type(set()):
+                    cmdlineDict[key].add(val)
+                else:
+                    tmpset = set()
+                    tmpset.add(cmdlineDict[key])
+                    tmpset.add(val)
+                    cmdlineDict[key] = tmpset
+            else:
+                cmdlineDict[key] = val

          return cmdlineDict

I don't really like this, but I think it will be okay.  We don't appear
to allow for multiple instances of a command line option anywhere else
in anaconda, so you shouldn't be breaking existing behavior by using a
set when that comes up.

I'll wait to hear from the rest of the team too. I'm not too worried about it, but tend to be thinking about this change the same way you are.

David Cantrell <dcantrell redhat com>
Supervisor, Installer Engineering Team
Red Hat, Inc. | Westford, MA | EST5EDT

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]