This will avoid duplicities in the resulting kernel boot argument line.

Resolves: rhbz#711002
---
booty/bootloaderInfo.py | 62 +++++++++++++++++-----------------------------
iw/zipl_gui.py | 3 +-
language.py | 4 +-
network.py | 31 ++++++++++-------------
storage/devices.py | 24 +++++++++---------
textw/zipl_text.py | 4 +-
6 files changed, 53 insertions(+), 75 deletions(-)

diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py
index d9ab62e..8886df0 100644
--- a/booty/bootloaderInfo.py
+++ b/booty/bootloaderInfo.py
@@ -87,7 +87,7 @@ def rootIsDevice(dev):
class KernelArguments:

def getDracutStorageArgs(self, devices):
- args = []
+ args = set()
types = {}
for device in devices:
for d in self.id.storage.devices:
@@ -95,62 +95,55 @@ class KernelArguments:
continue

s = d.dracutSetupString()
- types[s.split("=")[0]] = True
- if s not in args:
- args.append(s)
+ for string in s:
+ types[string.split("=")[0]] = True
+ args.update(s)

import storage
if isinstance(d, storage.devices.NetworkStorageDevice):
s = self.id.network.dracutSetupString(d)
- if s not in args:
- args.append(s)
+ args.update(s)

for i in [ [ "rd_LUKS_UUID", "rd_NO_LUKS" ],
[ "rd_LVM_LV", "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])
+ args.add(i[1])

return args

def get(self):
- args = ""
- bootArgs = []
+ bootArgs = set()
rootDev = self.id.storage.rootDevice
neededDevs = [ rootDev ] + self.id.storage.swaps

if flags.cmdline.get("fips") == "1":
bootDev = self.id.storage.mountpoints.get("/boot", rootDev)
- bootArgs = [ "boot=%s" % bootDev.fstabSpec ]
+ bootArgs.add("boot=%s" % bootDev.fstabSpec)
if bootDev is not rootDev:
neededDevs = [ rootDev, bootDev ]

if self.id.storage.fsset.swapDevices:
neededDevs.append(self.id.storage.fsset.swapDevice s[0])

- for s in bootArgs +
- self.getDracutStorageArgs(neededDevs) + [
- self.id.instLanguage.dracutSetupString(),
- self.id.keyboard.dracutSetupString(),
- self.args,
- self.appendArgs ]:
- s = s.strip()
- if not s:
- continue
- if args:
- args += " "
- args += s
+ all_args = set()
+ all_args.update(bootArgs)
+ all_args.update(self.getDracutStorageArgs(neededDe vs))
+ all_args.update(self.id.instLanguage.dracutSetupSt ring())
+ all_args.add(self.id.keyboard.dracutSetupString())
+ all_args.update(self.args)
+ all_args.update(self.appendArgs)

- return args
+ return " ".join(all_args)

def set(self, args):
self.args = args
- self.appendArgs = ""
+ self.appendArgs = set()

def getNoDracut(self):
- args = self.args.strip() + " " + self.appendArgs.strip()
- return args.strip()
+ args = " ".join(self.args) + " " + " ".join(self.appendArgs)
+ return args

def chandevget(self):
return self.cargs
@@ -158,17 +151,8 @@ class KernelArguments:
def chandevset(self, args):
self.cargs = args

- def append(self, args):
- # don't duplicate the addition of an argument (#128492)
- if self.args.find(args) != -1:
- return
- if self.appendArgs.find(args) != -1:
- return
-
- if self.appendArgs:
- self.appendArgs += " "
-
- self.appendArgs += args
+ def append(self, arg):
+ self.appendArgs.add(arg)

def __init__(self, instData):
newArgs = []
@@ -194,8 +178,8 @@ class KernelArguments:
else:
newArgs.append(arg)

- self.args = " ".join(newArgs)
- self.appendArgs = ""
+ self.args = set(newArgs)
+ self.appendArgs = set()
self.id = instData


diff --git a/iw/zipl_gui.py b/iw/zipl_gui.py
index b90eaae..9053ef3 100644
--- a/iw/zipl_gui.py
+++ b/iw/zipl_gui.py
@@ -42,8 +42,7 @@ class ZiplWindow (InstallWindow):
pass

def getNext (self):
- self.bl.args.set(self.kernelEntry.get_text())
-
+ self.bl.args.set(set([self.kernelEntry.get_text()]))

# ZiplWindow tag="zipl"
def getScreen(self, anaconda):
diff --git a/language.py b/language.py
index c0270c2..6d270ba 100644
--- a/language.py
+++ b/language.py
@@ -201,11 +201,11 @@ class Language(object):
return self.nativeLangNames.keys()

def dracutSetupString(self):
- args=""
+ args=set()

for (key, val) in self.info.iteritems():
if val != None:
- args += " %s=%s" % (key, val)
+ args.add("%s=%s" % (key, val))

return args

diff --git a/network.py b/network.py
index eaf2f7b..fa726cc 100644
--- a/network.py
+++ b/network.py
@@ -730,7 +730,7 @@ class Network:

# get a kernel cmdline string for dracut needed for access to host host
def dracutSetupString(self, networkStorageDevice):
- netargs=""
+ netargs=set()

if networkStorageDevice.nic:
# Storage bound to a specific nic (ie FCoE)
@@ -748,7 +748,7 @@ class Network:
dev = self.netdevices[nic]

if dev.get('BOOTPROTO') == 'ibft':
- netargs += "ip=ibft"
+ netargs.add("ip=ibft")
elif networkStorageDevice.host_address:
if self.hostname:
hostname = self.hostname
@@ -760,20 +760,20 @@ class Network:
if dev.get('DHCPV6C') == "yes":
# XXX combination with autoconf not yet clear,
# support for dhcpv6 is not yet implemented in NM/ifcfg-rh
- netargs += "ip=%s:dhcp6" % nic
+ netargs.add("ip=%s:dhcp6" % nic)
elif dev.get('IPV6_AUTOCONF') == "yes":
- netargs += "ip=%s:auto6" % nic
+ netargs.add("ip=%s:auto6" % nic)
elif dev.get('IPV6ADDR'):
ipaddr = "[%s]" % dev.get('IPV6ADDR')
if dev.get('IPV6_DEFAULTGW'):
gateway = "[%s]" % dev.get('IPV6_DEFAULTGW')
else:
gateway = ""
- netargs += "ip=%s::%s:%s:%s:%s:none" % (ipaddr, gateway,
- dev.get('PREFIX'), hostname, nic)
+ netargs.add("ip=%s::%s:%s:%s:%s:none" % (ipaddr, gateway,
+ dev.get('PREFIX'), hostname, nic))
else:
if dev.get('bootproto').lower() == 'dhcp':
- netargs += "ip=%s:dhcp" % nic
+ netargs.add("ip=%s:dhcp" % nic)
else:
if dev.get('GATEWAY'):
gateway = dev.get('GATEWAY')
@@ -785,28 +785,23 @@ class Network:
if not netmask and prefix:
netmask = isys.prefix2netmask(int(prefix))

- netargs += "ip=%s::%s:%s:%s:%s:none" % (dev.get('ipaddr'),
- gateway, netmask, hostname, nic)
+ netargs.add("ip=%s::%s:%s:%s:%s:none" % (dev.get('ipaddr'),
+ gateway, netmask, hostname, nic))

hwaddr = dev.get("HWADDR")
if hwaddr:
- if netargs != "":
- netargs += " "
-
- netargs += "ifname=%s:%s" % (nic, hwaddr.lower())
+ netargs.add("ifname=%s:%s" % (nic, hwaddr.lower()))

nettype = dev.get("NETTYPE")
subchannels = dev.get("SUBCHANNELS")
if iutil.isS390() and nettype and subchannels:
- if netargs != "":
- netargs += " "
-
- netargs += "rd_ZNET=%s,%s" % (nettype, subchannels)
+ znet = "rd_ZNET=%s,%s" % (nettype, subchannels)

options = dev.get("OPTIONS").strip("'"")
if options:
options = filter(lambda x: x != ', options.split(' '))
- netargs += ",%s" % (','.join(options))
+ znet += ",%s" % (','.join(options))
+ netargs.add(znet)

return netargs

diff --git a/storage/devices.py b/storage/devices.py
index a89de06..7191a45 100644
--- a/storage/devices.py
+++ b/storage/devices.py
@@ -325,7 +325,7 @@ class Device(object):
return False

def dracutSetupString(self):
- return ""
+ return set()

@property
def status(self):
@@ -1785,7 +1785,7 @@ class LUKSDevice(DMCryptDevice):
return self.parents[0]

def dracutSetupString(self):
- return "rd_LUKS_UUID=luks-%s" % self.slave.format.uuid
+ return set(["rd_LUKS_UUID=luks-%s" % self.slave.format.uuid])


class LVMVolumeGroupDevice(DMDevice):
@@ -2556,7 +2556,7 @@ class LVMLogicalVolumeDevice(DMDevice):
def dracutSetupString(self):
# Note no mapName usage here, this is a lvm cmdline name, which
# is different (ofcourse)
- return "rd_LVM_LV=%s/%s" % (self.vg.name, self._name)
+ return set(["rd_LVM_LV=%s/%s" % (self.vg.name, self._name)])

def checkSize(self):
""" Check to make sure the size of the device is allowed by the
@@ -3111,7 +3111,7 @@ class MDRaidArrayDevice(StorageDevice):
return self.type == "mdbiosraidarray"

def dracutSetupString(self):
- return "rd_MD_UUID=%s" % self.uuid
+ return set(["rd_MD_UUID=%s" % self.uuid])


class DMRaidArrayDevice(DMDevice):
@@ -3220,7 +3220,7 @@ class DMRaidArrayDevice(DMDevice):
return self.description

def dracutSetupString(self):
- return "rd_DM_UUID=%s" % self.name
+ return set(["rd_DM_UUID=%s" % self.name])

class MultipathDevice(DMDevice):
""" A multipath device """
@@ -3592,7 +3592,7 @@ class iScsiDiskDevice(DiskDevice, NetworkStorageDevice):

def dracutSetupString(self):
if self.ibft:
- return "iscsi_firmware"
+ return set(["iscsi_firmware"])

address = self.node.address
# surround ipv6 addresses with []
@@ -3609,9 +3609,9 @@ class iScsiDiskDevice(DiskDevice, NetworkStorageDevice):

netroot += "@%s::%d::%s" % (address, self.node.port, self.node.name)

- netroot += " iscsi_initiator=%s" % self.initiator
+ initiator = "iscsi_initiator=%s" % self.initiator

- return netroot
+ return set([netroot, initiator])

class FcoeDiskDevice(DiskDevice, NetworkStorageDevice):
""" An FCoE disk. """
@@ -3638,7 +3638,7 @@ class FcoeDiskDevice(DiskDevice, NetworkStorageDevice):
else:
dcbOpt = "nodcb"

- return "fcoe=edd:%s" % dcbOpt
+ return set(["fcoe=edd:%s" % dcbOpt])


class OpticalDevice(StorageDevice):
@@ -3730,7 +3730,7 @@ class ZFCPDiskDevice(DiskDevice):
'lun': self.fcp_lun}

def dracutSetupString(self):
- return "rd_ZFCP=%s,%s,%s" % (self.hba_id, self.wwpn, self.fcp_lun,)
+ return set(["rd_ZFCP=%s,%s,%s" % (self.hba_id, self.wwpn, self.fcp_lun,)])


class DASDDevice(DiskDevice):
@@ -3769,9 +3769,9 @@ class DASDDevice(DiskDevice):
opts[parts[0]] = parts

if self.busid in opts.keys():
- return "rd_DASD=%s" % ",".join(opts[self.busid])
+ return set(["rd_DASD=%s" % ",".join(opts[self.busid])])
else:
- return "rd_DASD=%s" % ",".join([self.busid] + self.getOpts())
+ return set(["rd_DASD=%s" % ",".join([self.busid] + self.getOpts())])

class NFSDevice(StorageDevice, NetworkStorageDevice):
""" An NFS device """
diff --git a/textw/zipl_text.py b/textw/zipl_text.py
index b112e0b..0576faa 100644
--- a/textw/zipl_text.py
+++ b/textw/zipl_text.py
@@ -88,9 +88,9 @@ class ZiplWindow:
return INSTALL_BACK

if kernelentry.value():
- self.bl.args.set(string.strip(kernelentry.value()) )
+ self.bl.args.set(set([string.strip(kernelentry.value())]))
else:
- self.bl.args.set("")
+ self.bl.args.set(set())

cdevs = []
if chandeventry1.value():
--
1.7.5.4

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Overall, I like it. One question though.

> @@ -865,18 +804,23 @@ class BootLoader(object):
> "setting boot args for network "
> "storage device")
>
> - setup_string = network.dracutSetupString(dep).strip()
> - self.boot_args.append(setup_string)
> - self.dracut_args.append(setup_string)
> + setup_args = network.dracutSetupArgs(dep)
> + self.boot_args.update(setup_args)
> + self.dracut_args.update(setup_args)
>
> - self.boot_args.extend(dracut_storage.values())
> - self.dracut_args.extend(dracut_storage.values())
> + self.boot_args.update(dracut_storage.values())
> + self.dracut_args.update(dracut_storage.values())
>
> # passed-in objects
> for cfg_obj in list(args) + kwargs.values():
> - setup_string = cfg_obj.dracutSetupString().strip()
> - self.boot_args.append(setup_string)
> - self.dracut_args.append(setup_string)
> + if hasattr(cfg_obj, "dracutSetupArgs"):
> + setup_args = cfg_obj.dracutSetupArgs()
> + self.boot_args.update(setup_args)
> + self.dracut_args.update(setup_args)
> + else:
> + setup_string = cfg_obj.dracutSetupString()
> + self.boot_args.add(setup_string)
> + self.dracut_args.add(setup_string)

Does this mean you're leaving some dracutSetupString methods around?
If so, why not convert them as well. If not, is this just code you had
in for testing?

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Overall, I like it. One question though.

> @@ -865,18 +804,23 @@ class BootLoader(object):
> "setting boot args for network "
> "storage device")
>
> - setup_string = network.dracutSetupString(dep).strip()
> - self.boot_args.append(setup_string)
> - self.dracut_args.append(setup_string)
> + setup_args = network.dracutSetupArgs(dep)
> + self.boot_args.update(setup_args)
> + self.dracut_args.update(setup_args)
>
> - self.boot_args.extend(dracut_storage.values())
> - self.dracut_args.extend(dracut_storage.values())
> + self.boot_args.update(dracut_storage.values())
> + self.dracut_args.update(dracut_storage.values())
>
> # passed-in objects
> for cfg_obj in list(args) + kwargs.values():
> - setup_string = cfg_obj.dracutSetupString().strip()
> - self.boot_args.append(setup_string)
> - self.dracut_args.append(setup_string)
> + if hasattr(cfg_obj, "dracutSetupArgs"):
> + setup_args = cfg_obj.dracutSetupArgs()
> + self.boot_args.update(setup_args)
> + self.dracut_args.update(setup_args)
> + else:
> + setup_string = cfg_obj.dracutSetupString()
> + self.boot_args.add(setup_string)
> + self.dracut_args.add(setup_string)

Does this mean you're leaving some dracutSetupString methods around?
If so, why not convert them as well. If not, is this just code you had
in for testing?

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

> Yes, one, the keyboard because I don't have control over it, it's in
> system-config-keybaord. What I was going to do is push this patch,
> open a bugzilla against sc-keyboard that the API has changed and
> once it is resolved I can remove the branching in bootloader.py.
> This way nothing ever gets broken.

Makes sense to me. Thanks for the clarification.

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

> Yes, one, the keyboard because I don't have control over it, it's in
> system-config-keybaord. What I was going to do is push this patch,
> open a bugzilla against sc-keyboard that the API has changed and
> once it is resolved I can remove the branching in bootloader.py.
> This way nothing ever gets broken.

Makes sense to me. Thanks for the clarification.

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list