It is possible for 2 disks to have the same disk dracut cmdline options, or network cmdline options (if the disks are network devices) for example 2 iscsi disks on the same host. In this case we should only include the specific option once.
This patch puts the collection of storage related setupStrings in a function, and makes get() loop over all kernel cmdline providing bits, so that the code for proper whitespace seperation of various args is not duplicated. --- booty/bootloaderInfo.py | 45 +++++++++++++++++++++++++++------------------ 1 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 067ffa7..6e6d403 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -86,26 +86,35 @@ def rootIsDevice(dev):
class KernelArguments:
- def get(self): - args = self.args + def getDracutStorageArgs(self): + args = [] root = self.id.storage.rootDevice for d in self.id.storage.devices: - if root.dependsOn(d): - dracutSetupString = d.dracutSetupString() - if len(dracutSetupString): - args += " %s" % dracutSetupString - import storage - if isinstance(d, storage.devices.NetworkStorageDevice): - args += " " - args += self.id.network.dracutSetupString(d) - - args += self.id.instLanguage.dracutSetupString() - args += self.id.keyboard.dracutSetupString() - - if args and self.appendArgs: - args += " " - - return args + self.appendArgs + if not root.dependsOn(d): + continue + + args.append(d.dracutSetupString()) + import storage + if isinstance(d, storage.devices.NetworkStorageDevice): + args.append(self.id.network.dracutSetupString(d)) + + return args + + def get(self): + args = "" + for s in self.getDracutStorageArgs() + [ + self.id.instLanguage.dracutSetupString(), + self.id.keyboard.dracutSetupString(), + self.args, + self.appendArgs ]: + s = s.strip() + if not s: + continue + if args: + args += " " + args += s + + return args
def set(self, args): self.args = args
We want to move to using dracut in such a way that the initrd only activates the minimum of storage needed to bring up the root fs. This requires us to tell it exactly what storage to activate. --- storage/devices.py | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/storage/devices.py b/storage/devices.py index 1065a68..b71b669 100644 --- a/storage/devices.py +++ b/storage/devices.py @@ -1555,6 +1555,9 @@ class LUKSDevice(DMCryptDevice): """ This device's backing device. """ return self.parents[0]
+ def dracutSetupString(self): + return "rd_LUKS_UUID=luks-%s" % self.format.uuid +
class LVMVolumeGroupDevice(DMDevice): """ An LVM Volume Group @@ -1964,6 +1967,9 @@ class LVMVolumeGroupDevice(DMDevice): """ return len(self.pvs) == self.pvCount or not self.exists
+ def dracutSetupString(self): + return "rd_LVM_VG=%s" % self.name +
class LVMLogicalVolumeDevice(DMDevice): """ An LVM Logical Volume """ @@ -2682,6 +2688,10 @@ class MDRaidArrayDevice(StorageDevice): def model(self): return "RAID%d Array" % self.level
+ def dracutSetupString(self): + return "rd_MD_UUID=%s" % self.uuid + + class DMRaidArrayDevice(DMDevice): """ A dmraid (device-mapper RAID) device """ _type = "dm-raid array" @@ -2786,6 +2796,10 @@ class DMRaidArrayDevice(DMDevice): # Even if teared down we still want to show up in storage.disks return True
+ def dracutSetupString(self): + return "rd_DM_UUID=%s" % self.name + + class _MultipathDeviceNameGenerator: def __init__(self): self.number = 0
ACK.
On 12/22/2009 09:43 PM, Hans de Goede wrote:
We want to move to using dracut in such a way that the initrd only activates the minimum of storage needed to bring up the root fs. This requires us to tell it exactly what storage to activate.
storage/devices.py | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/storage/devices.py b/storage/devices.py index 1065a68..b71b669 100644 --- a/storage/devices.py +++ b/storage/devices.py @@ -1555,6 +1555,9 @@ class LUKSDevice(DMCryptDevice): """ This device's backing device. """ return self.parents[0]
def dracutSetupString(self):
return "rd_LUKS_UUID=luks-%s" % self.format.uuid
class LVMVolumeGroupDevice(DMDevice): """ An LVM Volume Group
@@ -1964,6 +1967,9 @@ class LVMVolumeGroupDevice(DMDevice): """ return len(self.pvs) == self.pvCount or not self.exists
def dracutSetupString(self):
return "rd_LVM_VG=%s" % self.name
class LVMLogicalVolumeDevice(DMDevice): """ An LVM Logical Volume """
@@ -2682,6 +2688,10 @@ class MDRaidArrayDevice(StorageDevice): def model(self): return "RAID%d Array" % self.level
- def dracutSetupString(self):
return "rd_MD_UUID=%s" % self.uuid
- class DMRaidArrayDevice(DMDevice): """ A dmraid (device-mapper RAID) device """ _type = "dm-raid array"
@@ -2786,6 +2796,10 @@ class DMRaidArrayDevice(DMDevice): # Even if teared down we still want to show up in storage.disks return True
- def dracutSetupString(self):
return "rd_DM_UUID=%s" % self.name
- class _MultipathDeviceNameGenerator: def __init__(self): self.number = 0
On Tue, 2009-12-22 at 21:43 +0100, Hans de Goede wrote:
We want to move to using dracut in such a way that the initrd only activates the minimum of storage needed to bring up the root fs. This requires us to tell it exactly what storage to activate.
storage/devices.py | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/storage/devices.py b/storage/devices.py index 1065a68..b71b669 100644 --- a/storage/devices.py +++ b/storage/devices.py @@ -1555,6 +1555,9 @@ class LUKSDevice(DMCryptDevice): """ This device's backing device. """ return self.parents[0]
- def dracutSetupString(self):
return "rd_LUKS_UUID=luks-%s" % self.format.uuid
rd_LUKS_UUID wants something formatted like "luks-$UUID" instead of the actual UUID?
" @@ -2786,6 +2796,10 @@ class DMRaidArrayDevice(DMDevice): # Even if teared down we still want to show up in storage.disks return True
- def dracutSetupString(self):
return "rd_DM_UUID=%s" % self.name
Again, rd_DM_UUID does not want a UUID? Brilliant naming if so.
Dave
Hi,
On 12/23/2009 06:39 PM, David Lehman wrote:
On Tue, 2009-12-22 at 21:43 +0100, Hans de Goede wrote:
We want to move to using dracut in such a way that the initrd only activates the minimum of storage needed to bring up the root fs. This requires us to tell it exactly what storage to activate.
storage/devices.py | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/storage/devices.py b/storage/devices.py index 1065a68..b71b669 100644 --- a/storage/devices.py +++ b/storage/devices.py @@ -1555,6 +1555,9 @@ class LUKSDevice(DMCryptDevice): """ This device's backing device. """ return self.parents[0]
- def dracutSetupString(self):
return "rd_LUKS_UUID=luks-%s" % self.format.uuid
rd_LUKS_UUID wants something formatted like "luks-$UUID" instead of the actual UUID?
It can take both (either one), but the udev rules actually passes in "luks-$UUID" to the script which checks this cmdline option, so I thought it was best to be consistent with that.
" @@ -2786,6 +2796,10 @@ class DMRaidArrayDevice(DMDevice): # Even if teared down we still want to show up in storage.disks return True
- def dracutSetupString(self):
return "rd_DM_UUID=%s" % self.name
Again, rd_DM_UUID does not want a UUID? Brilliant naming if so.
Yes it wants the raid set name, I agree about brilliant naming, although technically dmraid sets do not have a uuid in the xxxx-xxxx-xxxx-etc name and there name is unique, so the name is a unique ID.
Regards,
Hans
When we don't specify any MD / LVM / DM / LUKS to activate dracut will activate all found by default, so when we don't specify any write RD_NO_foo to the kernel cmdline in grub. --- booty/bootloaderInfo.py | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 6e6d403..3d77254 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -88,16 +88,27 @@ class KernelArguments:
def getDracutStorageArgs(self): args = [] + types = {} root = self.id.storage.rootDevice for d in self.id.storage.devices: if not root.dependsOn(d): continue
- args.append(d.dracutSetupString()) + s = d.dracutSetupString() + types[s.split("=")[0]] = True + args.append(s) + import storage if isinstance(d, storage.devices.NetworkStorageDevice): args.append(self.id.network.dracutSetupString(d))
+ for i in [ [ "rd_LUKS_UUID", "rd_NO_LUKS" ], + [ "rd_LVM_VG", "rd_NO_LVM" ], + [ "rd_MD_UUID", "rd_NO_MD" ], + [ "rd_DM_UUID", "rd_NO_DM" ] ]: + if not types.has_key(i[0]): + args.append(i[1]) + return args
def get(self):
ACK.
On 12/22/2009 09:43 PM, Hans de Goede wrote:
When we don't specify any MD / LVM / DM / LUKS to activate dracut will activate all found by default, so when we don't specify any write RD_NO_foo to the kernel cmdline in grub.
booty/bootloaderInfo.py | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 6e6d403..3d77254 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -88,16 +88,27 @@ class KernelArguments:
def getDracutStorageArgs(self): args = []
types = {} root = self.id.storage.rootDevice for d in self.id.storage.devices: if not root.dependsOn(d): continue
args.append(d.dracutSetupString())
s = d.dracutSetupString()
types[s.split("=")[0]] = True
args.append(s)
import storage if isinstance(d, storage.devices.NetworkStorageDevice): args.append(self.id.network.dracutSetupString(d))
for i in [ [ "rd_LUKS_UUID", "rd_NO_LUKS" ],
[ "rd_LVM_VG", "rd_NO_LVM" ],
[ "rd_MD_UUID", "rd_NO_MD" ],
[ "rd_DM_UUID", "rd_NO_DM" ] ]:
if not types.has_key(i[0]):
args.append(i[1])
return args def get(self):
On Tue, 2009-12-22 at 21:43 +0100, Hans de Goede wrote:
When we don't specify any MD / LVM / DM / LUKS to activate dracut will activate all found by default, so when we don't specify any write RD_NO_foo to the kernel cmdline in grub.
The idea is fine, but...
booty/bootloaderInfo.py | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 6e6d403..3d77254 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -88,16 +88,27 @@ class KernelArguments:
def getDracutStorageArgs(self): args = []
types = {} root = self.id.storage.rootDevice for d in self.id.storage.devices: if not root.dependsOn(d): continue
args.append(d.dracutSetupString())
s = d.dracutSetupString()
types[s.split("=")[0]] = True
args.append(s)
import storage if isinstance(d, storage.devices.NetworkStorageDevice): args.append(self.id.network.dracutSetupString(d))
for i in [ [ "rd_LUKS_UUID", "rd_NO_LUKS" ],
[ "rd_LVM_VG", "rd_NO_LVM" ],
[ "rd_MD_UUID", "rd_NO_MD" ],
[ "rd_DM_UUID", "rd_NO_DM" ] ]:
if not types.has_key(i[0]):
args.append(i[1])
return args
def get(self):
It would be a bit more straightforward for types to be a list. Then you can just do:
types.append(s.split("=")[0])
and, later,
if i[0] not in types:
Dave
Hi,
On 12/23/2009 06:45 PM, David Lehman wrote:
On Tue, 2009-12-22 at 21:43 +0100, Hans de Goede wrote:
When we don't specify any MD / LVM / DM / LUKS to activate dracut will activate all found by default, so when we don't specify any write RD_NO_foo to the kernel cmdline in grub.
The idea is fine, but...
booty/bootloaderInfo.py | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 6e6d403..3d77254 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -88,16 +88,27 @@ class KernelArguments:
def getDracutStorageArgs(self): args = []
types = {} root = self.id.storage.rootDevice for d in self.id.storage.devices: if not root.dependsOn(d): continue
args.append(d.dracutSetupString())
s = d.dracutSetupString()
types[s.split("=")[0]] = True
args.append(s)
import storage if isinstance(d, storage.devices.NetworkStorageDevice): args.append(self.id.network.dracutSetupString(d))
for i in [ [ "rd_LUKS_UUID", "rd_NO_LUKS" ],
[ "rd_LVM_VG", "rd_NO_LVM" ],
[ "rd_MD_UUID", "rd_NO_MD" ],
[ "rd_DM_UUID", "rd_NO_DM" ] ]:
if not types.has_key(i[0]):
args.append(i[1])
return args def get(self):
It would be a bit more straightforward for types to be a list. Then you can just do:
types.append(s.split("=")[0])
and, later,
if i[0] not in types:
Good point, I'll respin this patch with that change (may take some time before I get around to doing that).
Regards,
Hans
Hi,
On 12/23/2009 06:45 PM, David Lehman wrote:
On Tue, 2009-12-22 at 21:43 +0100, Hans de Goede wrote:
When we don't specify any MD / LVM / DM / LUKS to activate dracut will activate all found by default, so when we don't specify any write RD_NO_foo to the kernel cmdline in grub.
The idea is fine, but...
booty/bootloaderInfo.py | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 6e6d403..3d77254 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -88,16 +88,27 @@ class KernelArguments:
def getDracutStorageArgs(self): args = []
types = {} root = self.id.storage.rootDevice for d in self.id.storage.devices: if not root.dependsOn(d): continue
args.append(d.dracutSetupString())
s = d.dracutSetupString()
types[s.split("=")[0]] = True
args.append(s)
import storage if isinstance(d, storage.devices.NetworkStorageDevice): args.append(self.id.network.dracutSetupString(d))
for i in [ [ "rd_LUKS_UUID", "rd_NO_LUKS" ],
[ "rd_LVM_VG", "rd_NO_LVM" ],
[ "rd_MD_UUID", "rd_NO_MD" ],
[ "rd_DM_UUID", "rd_NO_DM" ] ]:
if not types.has_key(i[0]):
args.append(i[1])
return args def get(self):
It would be a bit more straightforward for types to be a list. Then you can just do:
types.append(s.split("=")[0])
and, later,
if i[0] not in types:
Please scrap my previous reply about agreeing to change this to a list, I remember now why I made it a dict, as I want every type to only be in there once. I know your approach will work as well, but the idea behind the types dict, is to have a data structure which tracks which types of storage have activation commands in the dracut cmdline's, and such the struct should only have each type once (or not at all). Making a dict more appropriate for tracking this.
Regards,
Hans
Don't write the dynamically generated dracut kernel cmdline parameters to anaconda-ks.cfg. Having them there will result in having some of them twice in the resulting kickstart install, including some which refer to no longer valid UUID's. --- booty/bootloaderInfo.py | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 3d77254..9ba6a83 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -131,6 +131,10 @@ class KernelArguments: self.args = args self.appendArgs = ""
+ def getNoDracut(self): + args = self.args.strip() + " " + self.appendArgs.strip() + return args.strip() + def chandevget(self): return self.cargs
@@ -482,8 +486,8 @@ class bootloaderInfo(object): args.append("--location=%s" % (self.defaultDevice,)) args.append("--driveorder=%s" % (",".join(self.drivelist)))
- if self.args.get(): - args.append("--append="%s"" %(self.args.get())) + if self.args.getNoDracut(): + args.append("--append="%s"" %(self.args.getNoDracut()))
return args
ACK.
On 12/22/2009 09:43 PM, Hans de Goede wrote:
Don't write the dynamically generated dracut kernel cmdline parameters to anaconda-ks.cfg. Having them there will result in having some of them twice in the resulting kickstart install, including some which refer to no longer valid UUID's.
booty/bootloaderInfo.py | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 3d77254..9ba6a83 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -131,6 +131,10 @@ class KernelArguments: self.args = args self.appendArgs = ""
- def getNoDracut(self):
args = self.args.strip() + " " + self.appendArgs.strip()
return args.strip()
def chandevget(self): return self.cargs
@@ -482,8 +486,8 @@ class bootloaderInfo(object): args.append("--location=%s" % (self.defaultDevice,)) args.append("--driveorder=%s" % (",".join(self.drivelist)))
if self.args.get():
args.append("--append=\"%s\"" %(self.args.get()))
if self.args.getNoDracut():
args.append("--append=\"%s\"" %(self.args.getNoDracut())) return args
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 3d77254..9ba6a83 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -131,6 +131,10 @@ class KernelArguments: self.args = args self.appendArgs = ""
- def getNoDracut(self):
args = self.args.strip() + " " + self.appendArgs.strip()
return args.strip()
You've already stripped both parts, so there shouldn't eb a need to strip the result.
- Chris
Hi,
On 12/23/2009 05:08 PM, Chris Lumens wrote:
diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py index 3d77254..9ba6a83 100644 --- a/booty/bootloaderInfo.py +++ b/booty/bootloaderInfo.py @@ -131,6 +131,10 @@ class KernelArguments: self.args = args self.appendArgs = ""
- def getNoDracut(self):
args = self.args.strip() + " " + self.appendArgs.strip()
return args.strip()
You've already stripped both parts, so there shouldn't eb a need to strip the result.
One of them (or both) can be empty, this strip then removes the added " " between them, this is the easiest way to handle this.
Regards,
Hans
On 12/22/2009 09:43 PM, Hans de Goede wrote:
args.append(d.dracutSetupString())
import storage
if isinstance(d, storage.devices.NetworkStorageDevice):
args.append(self.id.network.dracutSetupString(d))
Is there a reason to have 'import storage' here and not at the top of the file?
Hi,
On 12/23/2009 04:42 PM, Ales Kozumplik wrote:
On 12/22/2009 09:43 PM, Hans de Goede wrote:
- args.append(d.dracutSetupString())
- import storage
- if isinstance(d, storage.devices.NetworkStorageDevice):
- args.append(self.id.network.dracutSetupString(d))
Is there a reason to have 'import storage' here and not at the top of the file?
Yes, this avoid an import loop
Regards,
Hans
anaconda-devel@lists.stg.fedoraproject.org